Merge pull request #1156 from dmcgowan/fix-manifest-list-size

Fix manifest lists to always use correct size

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