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>

Author: dmcgowan

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()
-	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.PlatfromSpecFromOCI(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
 	}

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,17 +4,17 @@
    "manifests": [
       {
          "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
-         "size": 428,
-         "digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd",
+         "size": 528,
+         "digest": "sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe",
          "platform": {
             "architecture": "amd64",
             "os": "linux"
          }
       },
       {
          "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
-         "size": 428,
-         "digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd",
+         "size": 528,
+         "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
-	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

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 platfrom spec from OCI platform
+func PlatfromSpecFromOCI(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,
 	}
 }
 
@@ -88,20 +113,11 @@ 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 {
+// PlatformFromJSON returns an OCI platform from formatted JSON bytes
+func PlatformFromJSON(src []byte) (*ocispec.Platform, error) {
+	var platform ocispec.Platform
+	if err := json.Unmarshal(src, &platform); err != nil {
 		return nil, err
 	}
-	return img, nil
+	return &platform, nil
 }