From 15695813a4251e5948c4877cd26d806ca0fcb1f4 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 (cherry picked from commit 1fd2d66df89890ca7d830a862d66e1275337ef65) Signed-off-by: Andrew Hsu --- 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 a94fb57e2ae6..e6c47394e08f 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 efb528eccaee..c270ee53beb7 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 4c43796808b1..7abe06d21575 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 0d752371c5b9..fa734afa2eb0 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 d39438c447e1..4d65b729f11d 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 95f8c46722e3..a0c2673e978a 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 5fb57468b272..1fd0207b3462 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 f618fd2b0311..62f0d6cc03f5 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 aea50a31fe16..66c11ce2207d 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