Skip to content

Commit 69e1743

Browse files
authored
Merge pull request #1156 from dmcgowan/fix-manifest-list-size
Fix manifest lists to always use correct size
2 parents f5393c9 + 1fd2d66 commit 69e1743

9 files changed

Lines changed: 161 additions & 82 deletions

File tree

cli/command/manifest/annotate.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/docker/cli/cli"
77
"github.com/docker/cli/cli/command"
88
"github.com/docker/cli/cli/manifest/store"
9+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
910
"github.com/pkg/errors"
1011
"github.com/spf13/cobra"
1112
)
@@ -64,20 +65,23 @@ func runManifestAnnotate(dockerCli command.Cli, opts annotateOptions) error {
6465
}
6566

6667
// Update the mf
68+
if imageManifest.Descriptor.Platform == nil {
69+
imageManifest.Descriptor.Platform = new(ocispec.Platform)
70+
}
6771
if opts.os != "" {
68-
imageManifest.Platform.OS = opts.os
72+
imageManifest.Descriptor.Platform.OS = opts.os
6973
}
7074
if opts.arch != "" {
71-
imageManifest.Platform.Architecture = opts.arch
75+
imageManifest.Descriptor.Platform.Architecture = opts.arch
7276
}
7377
for _, osFeature := range opts.osFeatures {
74-
imageManifest.Platform.OSFeatures = appendIfUnique(imageManifest.Platform.OSFeatures, osFeature)
78+
imageManifest.Descriptor.Platform.OSFeatures = appendIfUnique(imageManifest.Descriptor.Platform.OSFeatures, osFeature)
7579
}
7680
if opts.variant != "" {
77-
imageManifest.Platform.Variant = opts.variant
81+
imageManifest.Descriptor.Platform.Variant = opts.variant
7882
}
7983

80-
if !isValidOSArch(imageManifest.Platform.OS, imageManifest.Platform.Architecture) {
84+
if !isValidOSArch(imageManifest.Descriptor.Platform.OS, imageManifest.Descriptor.Platform.Architecture) {
8185
return errors.Errorf("manifest entry for image has unsupported os/arch combination: %s/%s", opts.os, opts.arch)
8286
}
8387
return manifestStore.Save(targetRef, imgRef, imageManifest)

cli/command/manifest/inspect.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/docker/distribution/manifest/manifestlist"
1313
"github.com/docker/distribution/reference"
1414
"github.com/docker/docker/registry"
15+
"github.com/pkg/errors"
1516
"github.com/spf13/cobra"
1617
)
1718

@@ -123,7 +124,7 @@ func printManifestList(dockerCli command.Cli, namedRef reference.Named, list []t
123124
for _, img := range list {
124125
mfd, err := buildManifestDescriptor(targetRepo, img)
125126
if err != nil {
126-
return fmt.Errorf("error assembling ManifestDescriptor")
127+
return errors.Wrap(err, "failed to assemble ManifestDescriptor")
127128
}
128129
manifests = append(manifests, mfd)
129130
}

cli/command/manifest/inspect_test.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/docker/distribution/manifest/schema2"
1515
"github.com/docker/distribution/reference"
1616
digest "github.com/opencontainers/go-digest"
17+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
1718
"github.com/pkg/errors"
1819
"gotest.tools/assert"
1920
is "gotest.tools/assert/cmp"
@@ -50,8 +51,22 @@ func fullImageManifest(t *testing.T, ref reference.Named) types.ImageManifest {
5051
},
5152
})
5253
assert.NilError(t, err)
54+
5355
// TODO: include image data for verbose inspect
54-
return types.NewImageManifest(ref, digest.Digest("sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd"), types.Image{OS: "linux", Architecture: "amd64"}, man)
56+
mt, raw, err := man.Payload()
57+
assert.NilError(t, err)
58+
59+
desc := ocispec.Descriptor{
60+
Digest: digest.FromBytes(raw),
61+
Size: int64(len(raw)),
62+
MediaType: mt,
63+
Platform: &ocispec.Platform{
64+
Architecture: "amd64",
65+
OS: "linux",
66+
},
67+
}
68+
69+
return types.NewImageManifest(ref, desc, man)
5570
}
5671

5772
func TestInspectCommandLocalManifestNotFound(t *testing.T) {

cli/command/manifest/push.go

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/docker/cli/cli/command"
1111
"github.com/docker/cli/cli/manifest/types"
1212
registryclient "github.com/docker/cli/cli/registry/client"
13+
"github.com/docker/distribution"
1314
"github.com/docker/distribution/manifest/manifestlist"
1415
"github.com/docker/distribution/manifest/schema2"
1516
"github.com/docker/distribution/reference"
@@ -141,7 +142,9 @@ func buildManifestList(manifests []types.ImageManifest, targetRef reference.Name
141142

142143
descriptors := []manifestlist.ManifestDescriptor{}
143144
for _, imageManifest := range manifests {
144-
if imageManifest.Platform.Architecture == "" || imageManifest.Platform.OS == "" {
145+
if imageManifest.Descriptor.Platform == nil ||
146+
imageManifest.Descriptor.Platform.Architecture == "" ||
147+
imageManifest.Descriptor.Platform.OS == "" {
145148
return nil, errors.Errorf(
146149
"manifest %s must have an OS and Architecture to be pushed to a registry", imageManifest.Ref)
147150
}
@@ -167,17 +170,18 @@ func buildManifestDescriptor(targetRepo *registry.RepositoryInfo, imageManifest
167170
return manifestlist.ManifestDescriptor{}, errors.Errorf("cannot use source images from a different registry than the target image: %s != %s", manifestRepoHostname, targetRepoHostname)
168171
}
169172

170-
mediaType, raw, err := imageManifest.Payload()
171-
if err != nil {
172-
return manifestlist.ManifestDescriptor{}, err
173+
manifest := manifestlist.ManifestDescriptor{
174+
Descriptor: distribution.Descriptor{
175+
Digest: imageManifest.Descriptor.Digest,
176+
Size: imageManifest.Descriptor.Size,
177+
MediaType: imageManifest.Descriptor.MediaType,
178+
},
173179
}
174180

175-
manifest := manifestlist.ManifestDescriptor{
176-
Platform: imageManifest.Platform,
181+
platform := types.PlatformSpecFromOCI(imageManifest.Descriptor.Platform)
182+
if platform != nil {
183+
manifest.Platform = *platform
177184
}
178-
manifest.Descriptor.Digest = imageManifest.Digest
179-
manifest.Size = int64(len(raw))
180-
manifest.MediaType = mediaType
181185

182186
if err = manifest.Descriptor.Digest.Validate(); err != nil {
183187
return manifestlist.ManifestDescriptor{}, errors.Wrapf(err,
@@ -195,7 +199,11 @@ func buildBlobRequestList(imageManifest types.ImageManifest, repoName reference.
195199
if err != nil {
196200
return nil, err
197201
}
198-
blobReqs = append(blobReqs, manifestBlob{canonical: canonical, os: imageManifest.Platform.OS})
202+
var os string
203+
if imageManifest.Descriptor.Platform != nil {
204+
os = imageManifest.Descriptor.Platform.OS
205+
}
206+
blobReqs = append(blobReqs, manifestBlob{canonical: canonical, os: os})
199207
}
200208
return blobReqs, nil
201209
}
@@ -206,7 +214,7 @@ func buildPutManifestRequest(imageManifest types.ImageManifest, targetRef refere
206214
if err != nil {
207215
return mountRequest{}, err
208216
}
209-
mountRef, err := reference.WithDigest(refWithoutTag, imageManifest.Digest)
217+
mountRef, err := reference.WithDigest(refWithoutTag, imageManifest.Descriptor.Digest)
210218
if err != nil {
211219
return mountRequest{}, err
212220
}
Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
11
{
22
"Ref": "example.com/alpine:3.0",
3-
"Digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd",
3+
"Descriptor": {
4+
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
5+
"digest": "sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe",
6+
"size": 528,
7+
"platform": {
8+
"architecture": "arm",
9+
"os": "freebsd",
10+
"os.features": [
11+
"feature1"
12+
],
13+
"variant": "v7"
14+
}
15+
},
416
"SchemaV2Manifest": {
517
"schemaVersion": 2,
618
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
@@ -16,13 +28,5 @@
1628
"digest": "sha256:88286f41530e93dffd4b964e1db22ce4939fffa4a4c665dab8591fbab03d4926"
1729
}
1830
]
19-
},
20-
"Platform": {
21-
"architecture": "arm",
22-
"os": "freebsd",
23-
"os.features": [
24-
"feature1"
25-
],
26-
"variant": "v7"
2731
}
2832
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,17 @@
44
"manifests": [
55
{
66
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
7-
"size": 428,
8-
"digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd",
7+
"size": 528,
8+
"digest": "sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe",
99
"platform": {
1010
"architecture": "amd64",
1111
"os": "linux"
1212
}
1313
},
1414
{
1515
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
16-
"size": 428,
17-
"digest": "sha256:7328f6f8b41890597575cbaadc884e7386ae0acc53b747401ebce5cf0d62abcd",
16+
"size": 528,
17+
"digest": "sha256:1072e499f3f655a032e88542330cf75b02e7bdf673278f701d7ba61629ee3ebe",
1818
"platform": {
1919
"architecture": "amd64",
2020
"os": "linux"

cli/manifest/store/store.go

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ import (
99
"strings"
1010

1111
"github.com/docker/cli/cli/manifest/types"
12+
"github.com/docker/distribution/manifest/manifestlist"
1213
"github.com/docker/distribution/reference"
14+
digest "github.com/opencontainers/go-digest"
15+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
16+
"github.com/pkg/errors"
1317
)
1418

1519
// Store manages local storage of image distribution manifests
@@ -50,8 +54,37 @@ func (s *fsStore) getFromFilename(ref reference.Reference, filename string) (typ
5054
case err != nil:
5155
return types.ImageManifest{}, err
5256
}
53-
var manifestInfo types.ImageManifest
54-
return manifestInfo, json.Unmarshal(bytes, &manifestInfo)
57+
var manifestInfo struct {
58+
types.ImageManifest
59+
60+
// Deprecated Fields, replaced by Descriptor
61+
Digest digest.Digest
62+
Platform *manifestlist.PlatformSpec
63+
}
64+
65+
if err := json.Unmarshal(bytes, &manifestInfo); err != nil {
66+
return types.ImageManifest{}, err
67+
}
68+
69+
// Compatibility with image manifests created before
70+
// descriptor, newer versions omit Digest and Platform
71+
if manifestInfo.Digest != "" {
72+
mediaType, raw, err := manifestInfo.Payload()
73+
if err != nil {
74+
return types.ImageManifest{}, err
75+
}
76+
if dgst := digest.FromBytes(raw); dgst != manifestInfo.Digest {
77+
return types.ImageManifest{}, errors.Errorf("invalid manifest file %v: image manifest digest mismatch (%v != %v)", filename, manifestInfo.Digest, dgst)
78+
}
79+
manifestInfo.ImageManifest.Descriptor = ocispec.Descriptor{
80+
Digest: manifestInfo.Digest,
81+
Size: int64(len(raw)),
82+
MediaType: mediaType,
83+
Platform: types.OCIPlatform(manifestInfo.Platform),
84+
}
85+
}
86+
87+
return manifestInfo.ImageManifest, nil
5588
}
5689

5790
// GetList returns all the local manifests for a transaction

cli/manifest/types/types.go

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,46 @@ import (
88
"github.com/docker/distribution/manifest/schema2"
99
"github.com/docker/distribution/reference"
1010
"github.com/opencontainers/go-digest"
11+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
1112
"github.com/pkg/errors"
1213
)
1314

1415
// ImageManifest contains info to output for a manifest object.
1516
type ImageManifest struct {
16-
Ref *SerializableNamed
17-
Digest digest.Digest
17+
Ref *SerializableNamed
18+
Descriptor ocispec.Descriptor
19+
20+
// SchemaV2Manifest is used for inspection
21+
// TODO: Deprecate this and store manifest blobs
1822
SchemaV2Manifest *schema2.DeserializedManifest `json:",omitempty"`
19-
Platform manifestlist.PlatformSpec
23+
}
24+
25+
// OCIPlatform creates an OCI platform from a manifest list platform spec
26+
func OCIPlatform(ps *manifestlist.PlatformSpec) *ocispec.Platform {
27+
if ps == nil {
28+
return nil
29+
}
30+
return &ocispec.Platform{
31+
Architecture: ps.Architecture,
32+
OS: ps.OS,
33+
OSVersion: ps.OSVersion,
34+
OSFeatures: ps.OSFeatures,
35+
Variant: ps.Variant,
36+
}
37+
}
38+
39+
// PlatformSpecFromOCI creates a platform spec from OCI platform
40+
func PlatformSpecFromOCI(p *ocispec.Platform) *manifestlist.PlatformSpec {
41+
if p == nil {
42+
return nil
43+
}
44+
return &manifestlist.PlatformSpec{
45+
Architecture: p.Architecture,
46+
OS: p.OS,
47+
OSVersion: p.OSVersion,
48+
OSFeatures: p.OSFeatures,
49+
Variant: p.Variant,
50+
}
2051
}
2152

2253
// Blobs returns the digests for all the blobs referenced by this manifest
@@ -30,6 +61,7 @@ func (i ImageManifest) Blobs() []digest.Digest {
3061

3162
// Payload returns the media type and bytes for the manifest
3263
func (i ImageManifest) Payload() (string, []byte, error) {
64+
// TODO: If available, read content from a content store by digest
3365
switch {
3466
case i.SchemaV2Manifest != nil:
3567
return i.SchemaV2Manifest.Payload()
@@ -51,18 +83,11 @@ func (i ImageManifest) References() []distribution.Descriptor {
5183

5284
// NewImageManifest returns a new ImageManifest object. The values for Platform
5385
// are initialized from those in the image
54-
func NewImageManifest(ref reference.Named, digest digest.Digest, img Image, manifest *schema2.DeserializedManifest) ImageManifest {
55-
platform := manifestlist.PlatformSpec{
56-
OS: img.OS,
57-
Architecture: img.Architecture,
58-
OSVersion: img.OSVersion,
59-
OSFeatures: img.OSFeatures,
60-
}
86+
func NewImageManifest(ref reference.Named, desc ocispec.Descriptor, manifest *schema2.DeserializedManifest) ImageManifest {
6187
return ImageManifest{
6288
Ref: &SerializableNamed{Named: ref},
63-
Digest: digest,
89+
Descriptor: desc,
6490
SchemaV2Manifest: manifest,
65-
Platform: platform,
6691
}
6792
}
6893

@@ -87,21 +112,3 @@ func (s *SerializableNamed) UnmarshalJSON(b []byte) error {
87112
func (s *SerializableNamed) MarshalJSON() ([]byte, error) {
88113
return json.Marshal(s.String())
89114
}
90-
91-
// Image is the minimal set of fields required to set default platform settings
92-
// on a manifest.
93-
type Image struct {
94-
Architecture string `json:"architecture,omitempty"`
95-
OS string `json:"os,omitempty"`
96-
OSVersion string `json:"os.version,omitempty"`
97-
OSFeatures []string `json:"os.features,omitempty"`
98-
}
99-
100-
// NewImageFromJSON creates an Image configuration from json.
101-
func NewImageFromJSON(src []byte) (*Image, error) {
102-
img := &Image{}
103-
if err := json.Unmarshal(src, img); err != nil {
104-
return nil, err
105-
}
106-
return img, nil
107-
}

0 commit comments

Comments
 (0)