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 <derek@mcgstyle.net>

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)

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)
 		}

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) {

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()
+	manifest := manifestlist.ManifestDescriptor{
-	if err != nil {
+		Descriptor: distribution.Descriptor{
-		return manifestlist.ManifestDescriptor{}, err
+			Digest:    imageManifest.Descriptor.Digest,
+			Size:      imageManifest.Descriptor.Size,
+			MediaType: imageManifest.Descriptor.MediaType,
+		},
 	}
 
-	manifest := manifestlist.ManifestDescriptor{
+	platform := types.PlatformSpecFromOCI(imageManifest.Descriptor.Platform)
-		Platform: imageManifest.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
 	}

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"
 	}
 }

cli/command/manifest/testdata/inspect-manifest-list.golden

@@ -4,8 +4,8 @@
    "manifests": [
       {
          "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
-         "size": 428,
+         "size": 528,
-         "digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd",
+         "digest": "sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe",
          "platform": {
             "architecture": "amd64",
             "os": "linux"
@@ -13,8 +13,8 @@
       },
       {
          "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
-         "size": 428,
+         "size": 528,
-         "digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd",
+         "digest": "sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe",
          "platform": {
             "architecture": "amd64",
             "os": "linux"

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
+	var manifestInfo struct {
-	return manifestInfo, json.Unmarshal(bytes, &manifestInfo)
+		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

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
+	Ref        *SerializableNamed
-	Digest           digest.Digest
+	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 {
+func NewImageManifest(ref reference.Named, desc ocispec.Descriptor, manifest *schema2.DeserializedManifest) ImageManifest {
-	platform := manifestlist.PlatformSpec{
-		OS:           img.OS,
-		Architecture: img.Architecture,
-		OSVersion:    img.OSVersion,
-		OSFeatures:   img.OSFeatures,
-	}
 	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
-}

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 manifestDesc.Platform == nil {
-	if err != 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) {
+func validateManifestDigest(ref reference.Named, mfst distribution.Manifest) (ocispec.Descriptor, error) {
-	_, canonical, err := mfst.Payload()
+	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 digested.Digest() != desc.Digest {
-		if err != nil {
-			return "", err
-		}
-		if _, err := verifier.Write(canonical); err != nil {
-			return "", err
-		}
-		if !verifier.Verified() {
 			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