From 1fd2d66df89890ca7d830a862d66e1275337ef65 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Thu, 28 Jun 2018 17:41:47 -0700 Subject: [PATCH] Fix manifest lists to always use correct size Stores complete OCI descriptor instead of digest and platform fields. This includes the size which was getting lost by not storing the original manifest bytes. Attempt to support existing cached files, if not output the filename with the incorrect content. Signed-off-by: Derek McGowan --- cli/command/manifest/annotate.go | 14 ++-- cli/command/manifest/inspect.go | 3 +- cli/command/manifest/inspect_test.go | 17 ++++- cli/command/manifest/push.go | 30 ++++++--- .../manifest/testdata/inspect-annotate.golden | 22 +++--- .../testdata/inspect-manifest-list.golden | 8 +-- cli/manifest/store/store.go | 37 +++++++++- cli/manifest/types/types.go | 67 ++++++++++--------- cli/registry/client/fetcher.go | 45 +++++++------ 9 files changed, 161 insertions(+), 82 deletions(-) diff --git a/cli/command/manifest/annotate.go b/cli/command/manifest/annotate.go index a94fb57e2a..e6c47394e0 100644 --- a/cli/command/manifest/annotate.go +++ b/cli/command/manifest/annotate.go @@ -6,6 +6,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/manifest/store" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -64,20 +65,23 @@ func runManifestAnnotate(dockerCli command.Cli, opts annotateOptions) error { } // Update the mf + if imageManifest.Descriptor.Platform == nil { + imageManifest.Descriptor.Platform = new(ocispec.Platform) + } if opts.os != "" { - imageManifest.Platform.OS = opts.os + imageManifest.Descriptor.Platform.OS = opts.os } if opts.arch != "" { - imageManifest.Platform.Architecture = opts.arch + imageManifest.Descriptor.Platform.Architecture = opts.arch } for _, osFeature := range opts.osFeatures { - imageManifest.Platform.OSFeatures = appendIfUnique(imageManifest.Platform.OSFeatures, osFeature) + imageManifest.Descriptor.Platform.OSFeatures = appendIfUnique(imageManifest.Descriptor.Platform.OSFeatures, osFeature) } if opts.variant != "" { - imageManifest.Platform.Variant = opts.variant + imageManifest.Descriptor.Platform.Variant = opts.variant } - if !isValidOSArch(imageManifest.Platform.OS, imageManifest.Platform.Architecture) { + if !isValidOSArch(imageManifest.Descriptor.Platform.OS, imageManifest.Descriptor.Platform.Architecture) { return errors.Errorf("manifest entry for image has unsupported os/arch combination: %s/%s", opts.os, opts.arch) } return manifestStore.Save(targetRef, imgRef, imageManifest) diff --git a/cli/command/manifest/inspect.go b/cli/command/manifest/inspect.go index efb528ecca..c270ee53be 100644 --- a/cli/command/manifest/inspect.go +++ b/cli/command/manifest/inspect.go @@ -12,6 +12,7 @@ import ( "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/reference" "github.com/docker/docker/registry" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -123,7 +124,7 @@ func printManifestList(dockerCli command.Cli, namedRef reference.Named, list []t for _, img := range list { mfd, err := buildManifestDescriptor(targetRepo, img) if err != nil { - return fmt.Errorf("error assembling ManifestDescriptor") + return errors.Wrap(err, "failed to assemble ManifestDescriptor") } manifests = append(manifests, mfd) } diff --git a/cli/command/manifest/inspect_test.go b/cli/command/manifest/inspect_test.go index 4c43796808..7abe06d215 100644 --- a/cli/command/manifest/inspect_test.go +++ b/cli/command/manifest/inspect_test.go @@ -14,6 +14,7 @@ import ( "github.com/docker/distribution/manifest/schema2" "github.com/docker/distribution/reference" digest "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "gotest.tools/assert" is "gotest.tools/assert/cmp" @@ -50,8 +51,22 @@ func fullImageManifest(t *testing.T, ref reference.Named) types.ImageManifest { }, }) assert.NilError(t, err) + // TODO: include image data for verbose inspect - return types.NewImageManifest(ref, digest.Digest("sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd"), types.Image{OS: "linux", Architecture: "amd64"}, man) + mt, raw, err := man.Payload() + assert.NilError(t, err) + + desc := ocispec.Descriptor{ + Digest: digest.FromBytes(raw), + Size: int64(len(raw)), + MediaType: mt, + Platform: &ocispec.Platform{ + Architecture: "amd64", + OS: "linux", + }, + } + + return types.NewImageManifest(ref, desc, man) } func TestInspectCommandLocalManifestNotFound(t *testing.T) { diff --git a/cli/command/manifest/push.go b/cli/command/manifest/push.go index 0d752371c5..fa734afa2e 100644 --- a/cli/command/manifest/push.go +++ b/cli/command/manifest/push.go @@ -10,6 +10,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/manifest/types" registryclient "github.com/docker/cli/cli/registry/client" + "github.com/docker/distribution" "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/schema2" "github.com/docker/distribution/reference" @@ -141,7 +142,9 @@ func buildManifestList(manifests []types.ImageManifest, targetRef reference.Name descriptors := []manifestlist.ManifestDescriptor{} for _, imageManifest := range manifests { - if imageManifest.Platform.Architecture == "" || imageManifest.Platform.OS == "" { + if imageManifest.Descriptor.Platform == nil || + imageManifest.Descriptor.Platform.Architecture == "" || + imageManifest.Descriptor.Platform.OS == "" { return nil, errors.Errorf( "manifest %s must have an OS and Architecture to be pushed to a registry", imageManifest.Ref) } @@ -167,17 +170,18 @@ func buildManifestDescriptor(targetRepo *registry.RepositoryInfo, imageManifest return manifestlist.ManifestDescriptor{}, errors.Errorf("cannot use source images from a different registry than the target image: %s != %s", manifestRepoHostname, targetRepoHostname) } - mediaType, raw, err := imageManifest.Payload() - if err != nil { - return manifestlist.ManifestDescriptor{}, err + manifest := manifestlist.ManifestDescriptor{ + Descriptor: distribution.Descriptor{ + Digest: imageManifest.Descriptor.Digest, + Size: imageManifest.Descriptor.Size, + MediaType: imageManifest.Descriptor.MediaType, + }, } - manifest := manifestlist.ManifestDescriptor{ - Platform: imageManifest.Platform, + platform := types.PlatformSpecFromOCI(imageManifest.Descriptor.Platform) + if platform != nil { + manifest.Platform = *platform } - manifest.Descriptor.Digest = imageManifest.Digest - manifest.Size = int64(len(raw)) - manifest.MediaType = mediaType if err = manifest.Descriptor.Digest.Validate(); err != nil { return manifestlist.ManifestDescriptor{}, errors.Wrapf(err, @@ -195,7 +199,11 @@ func buildBlobRequestList(imageManifest types.ImageManifest, repoName reference. if err != nil { return nil, err } - blobReqs = append(blobReqs, manifestBlob{canonical: canonical, os: imageManifest.Platform.OS}) + var os string + if imageManifest.Descriptor.Platform != nil { + os = imageManifest.Descriptor.Platform.OS + } + blobReqs = append(blobReqs, manifestBlob{canonical: canonical, os: os}) } return blobReqs, nil } @@ -206,7 +214,7 @@ func buildPutManifestRequest(imageManifest types.ImageManifest, targetRef refere if err != nil { return mountRequest{}, err } - mountRef, err := reference.WithDigest(refWithoutTag, imageManifest.Digest) + mountRef, err := reference.WithDigest(refWithoutTag, imageManifest.Descriptor.Digest) if err != nil { return mountRequest{}, err } diff --git a/cli/command/manifest/testdata/inspect-annotate.golden b/cli/command/manifest/testdata/inspect-annotate.golden index d39438c447..4d65b729f1 100644 --- a/cli/command/manifest/testdata/inspect-annotate.golden +++ b/cli/command/manifest/testdata/inspect-annotate.golden @@ -1,6 +1,18 @@ { "Ref": "example.com/alpine:3.0", - "Digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd", + "Descriptor": { + "mediaType": "application/vnd.docker.distribution.manifest.v2+json", + "digest": "sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe", + "size": 528, + "platform": { + "architecture": "arm", + "os": "freebsd", + "os.features": [ + "feature1" + ], + "variant": "v7" + } + }, "SchemaV2Manifest": { "schemaVersion": 2, "mediaType": "application/vnd.docker.distribution.manifest.v2+json", @@ -16,13 +28,5 @@ "digest": "sha256:88286f41530e93dffd4b964e1db22ce4939fffa4a4c665dab8591fbab03d4926" } ] - }, - "Platform": { - "architecture": "arm", - "os": "freebsd", - "os.features": [ - "feature1" - ], - "variant": "v7" } } diff --git a/cli/command/manifest/testdata/inspect-manifest-list.golden b/cli/command/manifest/testdata/inspect-manifest-list.golden index 95f8c46722..a0c2673e97 100644 --- a/cli/command/manifest/testdata/inspect-manifest-list.golden +++ b/cli/command/manifest/testdata/inspect-manifest-list.golden @@ -4,8 +4,8 @@ "manifests": [ { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", - "size": 428, - "digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd", + "size": 528, + "digest": "sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe", "platform": { "architecture": "amd64", "os": "linux" @@ -13,8 +13,8 @@ }, { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", - "size": 428, - "digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd", + "size": 528, + "digest": "sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe", "platform": { "architecture": "amd64", "os": "linux" diff --git a/cli/manifest/store/store.go b/cli/manifest/store/store.go index 5fb57468b2..1fd0207b34 100644 --- a/cli/manifest/store/store.go +++ b/cli/manifest/store/store.go @@ -9,7 +9,11 @@ import ( "strings" "github.com/docker/cli/cli/manifest/types" + "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/reference" + digest "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" ) // Store manages local storage of image distribution manifests @@ -50,8 +54,37 @@ func (s *fsStore) getFromFilename(ref reference.Reference, filename string) (typ case err != nil: return types.ImageManifest{}, err } - var manifestInfo types.ImageManifest - return manifestInfo, json.Unmarshal(bytes, &manifestInfo) + var manifestInfo struct { + types.ImageManifest + + // Deprecated Fields, replaced by Descriptor + Digest digest.Digest + Platform *manifestlist.PlatformSpec + } + + if err := json.Unmarshal(bytes, &manifestInfo); err != nil { + return types.ImageManifest{}, err + } + + // Compatibility with image manifests created before + // descriptor, newer versions omit Digest and Platform + if manifestInfo.Digest != "" { + mediaType, raw, err := manifestInfo.Payload() + if err != nil { + return types.ImageManifest{}, err + } + if dgst := digest.FromBytes(raw); dgst != manifestInfo.Digest { + return types.ImageManifest{}, errors.Errorf("invalid manifest file %v: image manifest digest mismatch (%v != %v)", filename, manifestInfo.Digest, dgst) + } + manifestInfo.ImageManifest.Descriptor = ocispec.Descriptor{ + Digest: manifestInfo.Digest, + Size: int64(len(raw)), + MediaType: mediaType, + Platform: types.OCIPlatform(manifestInfo.Platform), + } + } + + return manifestInfo.ImageManifest, nil } // GetList returns all the local manifests for a transaction diff --git a/cli/manifest/types/types.go b/cli/manifest/types/types.go index f618fd2b03..62f0d6cc03 100644 --- a/cli/manifest/types/types.go +++ b/cli/manifest/types/types.go @@ -8,15 +8,46 @@ import ( "github.com/docker/distribution/manifest/schema2" "github.com/docker/distribution/reference" "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" ) // ImageManifest contains info to output for a manifest object. type ImageManifest struct { - Ref *SerializableNamed - Digest digest.Digest + Ref *SerializableNamed + Descriptor ocispec.Descriptor + + // SchemaV2Manifest is used for inspection + // TODO: Deprecate this and store manifest blobs SchemaV2Manifest *schema2.DeserializedManifest `json:",omitempty"` - Platform manifestlist.PlatformSpec +} + +// OCIPlatform creates an OCI platform from a manifest list platform spec +func OCIPlatform(ps *manifestlist.PlatformSpec) *ocispec.Platform { + if ps == nil { + return nil + } + return &ocispec.Platform{ + Architecture: ps.Architecture, + OS: ps.OS, + OSVersion: ps.OSVersion, + OSFeatures: ps.OSFeatures, + Variant: ps.Variant, + } +} + +// PlatformSpecFromOCI creates a platform spec from OCI platform +func PlatformSpecFromOCI(p *ocispec.Platform) *manifestlist.PlatformSpec { + if p == nil { + return nil + } + return &manifestlist.PlatformSpec{ + Architecture: p.Architecture, + OS: p.OS, + OSVersion: p.OSVersion, + OSFeatures: p.OSFeatures, + Variant: p.Variant, + } } // Blobs returns the digests for all the blobs referenced by this manifest @@ -30,6 +61,7 @@ func (i ImageManifest) Blobs() []digest.Digest { // Payload returns the media type and bytes for the manifest func (i ImageManifest) Payload() (string, []byte, error) { + // TODO: If available, read content from a content store by digest switch { case i.SchemaV2Manifest != nil: return i.SchemaV2Manifest.Payload() @@ -51,18 +83,11 @@ func (i ImageManifest) References() []distribution.Descriptor { // NewImageManifest returns a new ImageManifest object. The values for Platform // are initialized from those in the image -func NewImageManifest(ref reference.Named, digest digest.Digest, img Image, manifest *schema2.DeserializedManifest) ImageManifest { - platform := manifestlist.PlatformSpec{ - OS: img.OS, - Architecture: img.Architecture, - OSVersion: img.OSVersion, - OSFeatures: img.OSFeatures, - } +func NewImageManifest(ref reference.Named, desc ocispec.Descriptor, manifest *schema2.DeserializedManifest) ImageManifest { return ImageManifest{ Ref: &SerializableNamed{Named: ref}, - Digest: digest, + Descriptor: desc, SchemaV2Manifest: manifest, - Platform: platform, } } @@ -87,21 +112,3 @@ func (s *SerializableNamed) UnmarshalJSON(b []byte) error { func (s *SerializableNamed) MarshalJSON() ([]byte, error) { return json.Marshal(s.String()) } - -// Image is the minimal set of fields required to set default platform settings -// on a manifest. -type Image struct { - Architecture string `json:"architecture,omitempty"` - OS string `json:"os,omitempty"` - OSVersion string `json:"os.version,omitempty"` - OSFeatures []string `json:"os.features,omitempty"` -} - -// NewImageFromJSON creates an Image configuration from json. -func NewImageFromJSON(src []byte) (*Image, error) { - img := &Image{} - if err := json.Unmarshal(src, img); err != nil { - return nil, err - } - return img, nil -} diff --git a/cli/registry/client/fetcher.go b/cli/registry/client/fetcher.go index aea50a31fe..66c11ce220 100644 --- a/cli/registry/client/fetcher.go +++ b/cli/registry/client/fetcher.go @@ -2,6 +2,7 @@ package client import ( "context" + "encoding/json" "fmt" "github.com/docker/cli/cli/manifest/types" @@ -14,6 +15,7 @@ import ( distclient "github.com/docker/distribution/registry/client" "github.com/docker/docker/registry" digest "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -72,7 +74,7 @@ func getManifest(ctx context.Context, repo distribution.Repository, ref referenc } func pullManifestSchemaV2(ctx context.Context, ref reference.Named, repo distribution.Repository, mfst schema2.DeserializedManifest) (types.ImageManifest, error) { - manifestDigest, err := validateManifestDigest(ref, mfst) + manifestDesc, err := validateManifestDigest(ref, mfst) if err != nil { return types.ImageManifest{}, err } @@ -81,11 +83,16 @@ func pullManifestSchemaV2(ctx context.Context, ref reference.Named, repo distrib return types.ImageManifest{}, err } - img, err := types.NewImageFromJSON(configJSON) - if err != nil { + if manifestDesc.Platform == nil { + manifestDesc.Platform = &ocispec.Platform{} + } + + // Fill in os and architecture fields from config JSON + if err := json.Unmarshal(configJSON, manifestDesc.Platform); err != nil { return types.ImageManifest{}, err } - return types.NewImageManifest(ref, manifestDigest, *img, &mfst), nil + + return types.NewImageManifest(ref, manifestDesc, &mfst), nil } func pullManifestSchemaV2ImageConfig(ctx context.Context, dgst digest.Digest, repo distribution.Repository) ([]byte, error) { @@ -110,29 +117,26 @@ func pullManifestSchemaV2ImageConfig(ctx context.Context, dgst digest.Digest, re // validateManifestDigest computes the manifest digest, and, if pulling by // digest, ensures that it matches the requested digest. -func validateManifestDigest(ref reference.Named, mfst distribution.Manifest) (digest.Digest, error) { - _, canonical, err := mfst.Payload() +func validateManifestDigest(ref reference.Named, mfst distribution.Manifest) (ocispec.Descriptor, error) { + mediaType, canonical, err := mfst.Payload() if err != nil { - return "", err + return ocispec.Descriptor{}, err + } + desc := ocispec.Descriptor{ + Digest: digest.FromBytes(canonical), + Size: int64(len(canonical)), + MediaType: mediaType, } // If pull by digest, then verify the manifest digest. if digested, isDigested := ref.(reference.Canonical); isDigested { - verifier := digested.Digest().Verifier() - if err != nil { - return "", err - } - if _, err := verifier.Write(canonical); err != nil { - return "", err - } - if !verifier.Verified() { + if digested.Digest() != desc.Digest { err := fmt.Errorf("manifest verification failed for digest %s", digested.Digest()) - return "", err + return ocispec.Descriptor{}, err } - return digested.Digest(), nil } - return digest.FromBytes(canonical), nil + return desc, nil } // pullManifestList handles "manifest lists" which point to various @@ -166,7 +170,10 @@ func pullManifestList(ctx context.Context, ref reference.Named, repo distributio if err != nil { return nil, err } - imageManifest.Platform = manifestDescriptor.Platform + + // Replace platform from config + imageManifest.Descriptor.Platform = types.OCIPlatform(&manifestDescriptor.Platform) + infos = append(infos, imageManifest) } return infos, nil