Skip to content

Commit f5a333e

Browse files
mtrmacmtrmac
authored
Merge pull request #866 from mtrmac/updates-s2s1
Fix conversions from/to schema1 with layer updates
2 parents 69d6e9c + a5a3165 commit f5a333e

13 files changed

+557
-45
lines changed

‎go.sum‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ github.com/mtrmac/gpgme v0.1.2/go.mod h1:GYYHnGSuS7HK3zVS2n3y73y0okK/BeKzwnn5jgi
109109
github.com/opencontainers/go-digest v0.0.0-20180430190053-c9281466c8b2/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=
110110
github.com/opencontainers/go-digest v1.0.0-rc1 h1:WzifXhOVOEOuFYOJAW6aQqW0TooG2iki3E3Ii+WN7gQ=
111111
github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=
112+
github.com/opencontainers/image-spec v1.0.1 h1:JMemWkRwHx4Zj+fVxWoMCFm/8sYGGrUVojFA6h/TRcI=
112113
github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
113114
github.com/opencontainers/image-spec v1.0.2-0.20190823105129-775207bd45b6 h1:yN8BPXVwMBAm3Cuvh1L5XE8XpvYRMdsVLd82ILprhUU=
114115
github.com/opencontainers/image-spec v1.0.2-0.20190823105129-775207bd45b6/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=

‎image/docker_schema1.go‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (m *manifestSchema1) ConfigBlob(context.Context) ([]byte, error) {
5656
// layers in the resulting configuration isn't guaranteed to be returned to due how
5757
// old image manifests work (docker v2s1 especially).
5858
func (m *manifestSchema1) OCIConfig(ctx context.Context) (*imgspecv1.Image, error) {
59-
v2s2, err := m.convertToManifestSchema2(ctx, types.ManifestUpdateInformation{})
59+
v2s2, err := m.convertToManifestSchema2(ctx, &types.ManifestUpdateOptions{})
6060
if err != nil {
6161
return nil, err
6262
}
@@ -113,7 +113,7 @@ func (m *manifestSchema1) UpdatedImage(ctx context.Context, options types.Manife
113113
if options.ManifestMIMEType != manifest.DockerV2Schema1MediaType && options.ManifestMIMEType != manifest.DockerV2Schema1SignedMediaType {
114114
converted, err := convertManifestIfRequiredWithUpdate(ctx, options, map[string]manifestConvertFn{
115115
imgspecv1.MediaTypeImageManifest: copy.convertToManifestOCI1,
116-
manifest.DockerV2Schema2MediaType: copy.convertToManifestSchema2,
116+
manifest.DockerV2Schema2MediaType: copy.convertToManifestSchema2Generic,
117117
})
118118
if err != nil {
119119
return nil, err
@@ -142,10 +142,26 @@ func (m *manifestSchema1) UpdatedImage(ctx context.Context, options types.Manife
142142
return memoryImageFromManifest(&copy), nil
143143
}
144144

145+
// convertToManifestSchema2Generic returns a genericManifest implementation converted to manifest.DockerV2Schema2MediaType.
146+
// It may use options.InformationOnly and also adjust *options to be appropriate for editing the returned
147+
// value.
148+
// This does not change the state of the original manifestSchema1 object.
149+
//
150+
// We need this function just because a function returning an implementation of the genericManifest
151+
// interface is not automatically assignable to a function type returning the genericManifest interface
152+
func (m *manifestSchema1) convertToManifestSchema2Generic(ctx context.Context, options *types.ManifestUpdateOptions) (genericManifest, error) {
153+
return m.convertToManifestSchema2(ctx, options)
154+
}
155+
156+
// convertToManifestSchema2 returns a genericManifest implementation converted to manifest.DockerV2Schema2MediaType.
157+
// It may use options.InformationOnly and also adjust *options to be appropriate for editing the returned
158+
// value.
159+
// This does not change the state of the original manifestSchema1 object.
160+
//
145161
// Based on github.com/docker/docker/distribution/pull_v2.go
146-
func (m *manifestSchema1) convertToManifestSchema2(_ context.Context, updateInfo types.ManifestUpdateInformation) (types.Image, error) {
147-
uploadedLayerInfos := updateInfo.LayerInfos
148-
layerDiffIDs := updateInfo.LayerDiffIDs
162+
func (m *manifestSchema1) convertToManifestSchema2(_ context.Context, options *types.ManifestUpdateOptions) (*manifestSchema2, error) {
163+
uploadedLayerInfos := options.InformationOnly.LayerInfos
164+
layerDiffIDs := options.InformationOnly.LayerDiffIDs
149165

150166
if len(m.m.ExtractedV1Compatibility) == 0 {
151167
// What would this even mean?! Anyhow, the rest of the code depends on FSLayers[0] and ExtractedV1Compatibility[0] existing.
@@ -161,6 +177,15 @@ func (m *manifestSchema1) convertToManifestSchema2(_ context.Context, updateInfo
161177
return nil, errors.Errorf("Internal error: collected %d DiffID values, but schema1 manifest has %d fsLayers", len(layerDiffIDs), len(m.m.FSLayers))
162178
}
163179

180+
var convertedLayerUpdates []types.BlobInfo // Only used if options.LayerInfos != nil
181+
if options.LayerInfos != nil {
182+
if len(options.LayerInfos) != len(m.m.FSLayers) {
183+
return nil, errors.Errorf("Error converting image: layer edits for %d layers vs %d existing layers",
184+
len(options.LayerInfos), len(m.m.FSLayers))
185+
}
186+
convertedLayerUpdates = []types.BlobInfo{}
187+
}
188+
164189
// Build a list of the diffIDs for the non-empty layers.
165190
diffIDs := []digest.Digest{}
166191
var layers []manifest.Schema2Descriptor
@@ -181,6 +206,9 @@ func (m *manifestSchema1) convertToManifestSchema2(_ context.Context, updateInfo
181206
Size: size,
182207
Digest: m.m.FSLayers[v1Index].BlobSum,
183208
})
209+
if options.LayerInfos != nil {
210+
convertedLayerUpdates = append(convertedLayerUpdates, options.LayerInfos[v2Index])
211+
}
184212
diffIDs = append(diffIDs, d)
185213
}
186214
}
@@ -194,21 +222,24 @@ func (m *manifestSchema1) convertToManifestSchema2(_ context.Context, updateInfo
194222
Digest: digest.FromBytes(configJSON),
195223
}
196224

197-
m1 := manifestSchema2FromComponents(configDescriptor, nil, configJSON, layers)
198-
return memoryImageFromManifest(m1), nil
225+
if options.LayerInfos != nil {
226+
options.LayerInfos = convertedLayerUpdates
227+
}
228+
return manifestSchema2FromComponents(configDescriptor, nil, configJSON, layers), nil
199229
}
200230

201-
func (m *manifestSchema1) convertToManifestOCI1(ctx context.Context, updateInfo types.ManifestUpdateInformation) (types.Image, error) {
231+
// convertToManifestOCI1 returns a genericManifest implementation converted to imgspecv1.MediaTypeImageManifest.
232+
// It may use options.InformationOnly and also adjust *options to be appropriate for editing the returned
233+
// value.
234+
// This does not change the state of the original manifestSchema1 object.
235+
func (m *manifestSchema1) convertToManifestOCI1(ctx context.Context, options *types.ManifestUpdateOptions) (genericManifest, error) {
202236
// We can't directly convert to OCI, but we can transitively convert via a Docker V2.2 Distribution manifest
203-
m2, err := m.convertToManifestSchema2(ctx, updateInfo)
237+
m2, err := m.convertToManifestSchema2(ctx, options)
204238
if err != nil {
205239
return nil, err
206240
}
207241

208-
return m2.UpdatedImage(ctx, types.ManifestUpdateOptions{
209-
ManifestMIMEType: imgspecv1.MediaTypeImageManifest,
210-
InformationOnly: updateInfo,
211-
})
242+
return m2.convertToManifestOCI1(ctx, options)
212243
}
213244

214245
// SupportsEncryption returns if encryption is supported for the manifest type

‎image/docker_schema1_test.go‎

Lines changed: 190 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,42 @@ var schema1FixtureLayerDiffIDs = []digest.Digest{
5959
"sha256:9bd63850e406167b4751f5050f6dc0ebd789bb5ef5e5c6c31ed062bda8c063e8",
6060
}
6161

62+
var schema1WithThrowawaysFixtureLayerInfos = []types.BlobInfo{
63+
{Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb", Size: 51354364},
64+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
65+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
66+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
67+
{Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c", Size: 150},
68+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
69+
{Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", Size: 11739507},
70+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
71+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
72+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
73+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
74+
{Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", Size: 8841833},
75+
{Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa", Size: 291},
76+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
77+
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
78+
}
79+
80+
var schema1WithThrowawaysFixtureLayerDiffIDs = []digest.Digest{
81+
"sha256:142a601d97936307e75220c35dde0348971a9584c21e7cb42e1f7004005432ab",
82+
GzippedEmptyLayerDigest,
83+
GzippedEmptyLayerDigest,
84+
GzippedEmptyLayerDigest,
85+
"sha256:90fcc66ad3be9f1757f954b750deb37032f208428aa12599fcb02182b9065a9c",
86+
GzippedEmptyLayerDigest,
87+
"sha256:5a8624bb7e76d1e6829f9c64c43185e02bc07f97a2189eb048609a8914e72c56",
88+
GzippedEmptyLayerDigest,
89+
GzippedEmptyLayerDigest,
90+
GzippedEmptyLayerDigest,
91+
GzippedEmptyLayerDigest,
92+
"sha256:d349ff6b3afc6a2800054768c82bfbf4289c9aa5da55c1290f802943dcd4d1e9",
93+
"sha256:8c064bb1f60e84fa8cc6079b6d2e76e0423389fd6aeb7e497dfdae5e05b2b25b",
94+
GzippedEmptyLayerDigest,
95+
GzippedEmptyLayerDigest,
96+
}
97+
6298
func manifestSchema1FromFixture(t *testing.T, fixture string) genericManifest {
6399
manifest, err := ioutil.ReadFile(filepath.Join("fixtures", fixture))
64100
require.NoError(t, err)
@@ -167,7 +203,7 @@ func TestManifestSchema1ConfigBlob(t *testing.T) {
167203
}
168204

169205
func TestManifestSchema1OCIConfig(t *testing.T) {
170-
m := manifestSchema1FromFixture(t, "schema1-to-oci-config.json")
206+
m := manifestSchema1FromFixture(t, "schema1-for-oci-config.json")
171207
configOCI, err := m.OCIConfig(context.Background())
172208
require.NoError(t, err)
173209
// FIXME: A more comprehensive test?
@@ -398,7 +434,7 @@ func TestManifestSchema1ConvertToSchema2(t *testing.T) {
398434
err = json.Unmarshal(byHandJSON, &byHand)
399435
require.NoError(t, err)
400436
err = json.Unmarshal(convertedJSON, &converted)
401-
delete(converted, "config")
437+
delete(converted, "config") // We don’t want to hard-code a specific digest and size of the marshaled config here
402438
delete(byHand, "config")
403439
require.NoError(t, err)
404440
assert.Equal(t, byHand, converted)
@@ -416,6 +452,150 @@ func TestManifestSchema1ConvertToSchema2(t *testing.T) {
416452
require.NoError(t, err)
417453
assert.Equal(t, byHand, converted)
418454

455+
// Conversion to schema2 together with changing LayerInfos works as expected (which requires
456+
// handling schema1 throwaway layers):
457+
// Use the recorded result of converting the schema2 fixture to schema1, because that one
458+
// (unlike schem1.json) contains throwaway layers.
459+
original = manifestSchema1FromFixture(t, "schema2-to-schema1-by-docker.json")
460+
updatedLayers, updatedLayersCopy := modifiedLayerInfos(t, schema1WithThrowawaysFixtureLayerInfos)
461+
res, err = original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
462+
LayerInfos: updatedLayers,
463+
ManifestMIMEType: manifest.DockerV2Schema2MediaType,
464+
InformationOnly: types.ManifestUpdateInformation{
465+
LayerInfos: updatedLayers,
466+
LayerDiffIDs: schema1WithThrowawaysFixtureLayerDiffIDs,
467+
},
468+
})
469+
require.NoError(t, err)
470+
assert.Equal(t, updatedLayersCopy, updatedLayers) // updatedLayers have not been modified in place
471+
convertedJSON, mt, err = res.Manifest(context.Background())
472+
require.NoError(t, err)
473+
assert.Equal(t, manifest.DockerV2Schema2MediaType, mt)
474+
// Layers have been updated as expected
475+
originalSrc := newSchema2ImageSource(t, "httpd:latest")
476+
s2Manifest, err := manifestSchema2FromManifest(originalSrc, convertedJSON)
477+
require.NoError(t, err)
478+
assert.Equal(t, []types.BlobInfo{
479+
{
480+
Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5ba",
481+
Size: 51354365,
482+
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
483+
},
484+
{
485+
Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680d",
486+
Size: 151,
487+
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
488+
},
489+
{
490+
Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a8",
491+
Size: 11739506,
492+
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
493+
},
494+
{
495+
Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25908",
496+
Size: 8841832,
497+
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
498+
},
499+
{
500+
Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fb",
501+
Size: 290,
502+
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
503+
},
504+
}, s2Manifest.LayerInfos())
505+
506+
// FIXME? Test also the various failure cases, if only to see that we don't crash?
507+
}
508+
509+
func TestManifestSchema1ConvertToManifestOCI1(t *testing.T) {
510+
original := manifestSchema1FromFixture(t, "schema1.json")
511+
res, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
512+
ManifestMIMEType: imgspecv1.MediaTypeImageManifest,
513+
InformationOnly: types.ManifestUpdateInformation{
514+
LayerInfos: schema1FixtureLayerInfos,
515+
LayerDiffIDs: schema1FixtureLayerDiffIDs,
516+
},
517+
})
518+
require.NoError(t, err)
519+
520+
convertedJSON, mt, err := res.Manifest(context.Background())
521+
require.NoError(t, err)
522+
assert.Equal(t, imgspecv1.MediaTypeImageManifest, mt)
523+
524+
byHandJSON, err := ioutil.ReadFile("fixtures/schema1-to-oci1.json")
525+
require.NoError(t, err)
526+
var converted, byHand map[string]interface{}
527+
err = json.Unmarshal(byHandJSON, &byHand)
528+
require.NoError(t, err)
529+
err = json.Unmarshal(convertedJSON, &converted)
530+
delete(converted, "config") // We don’t want to hard-code a specific digest and size of the marshaled config here
531+
delete(byHand, "config")
532+
require.NoError(t, err)
533+
assert.Equal(t, byHand, converted)
534+
535+
convertedConfig, err := res.ConfigBlob(context.Background())
536+
require.NoError(t, err)
537+
538+
byHandConfig, err := ioutil.ReadFile("fixtures/schema1-to-oci1-config.json")
539+
require.NoError(t, err)
540+
converted = map[string]interface{}{}
541+
byHand = map[string]interface{}{}
542+
err = json.Unmarshal(byHandConfig, &byHand)
543+
require.NoError(t, err)
544+
err = json.Unmarshal(convertedConfig, &converted)
545+
require.NoError(t, err)
546+
assert.Equal(t, byHand, converted)
547+
548+
// Conversion to OCI together with changing LayerInfos works as expected (which requires
549+
// handling schema1 throwaway layers):
550+
// Use the recorded result of converting the schema2 fixture to schema1, because that one
551+
// (unlike schem1.json) contains throwaway layers.
552+
original = manifestSchema1FromFixture(t, "schema2-to-schema1-by-docker.json")
553+
updatedLayers, updatedLayersCopy := modifiedLayerInfos(t, schema1WithThrowawaysFixtureLayerInfos)
554+
res, err = original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
555+
LayerInfos: updatedLayers,
556+
ManifestMIMEType: imgspecv1.MediaTypeImageManifest,
557+
InformationOnly: types.ManifestUpdateInformation{ // FIXME: deduplicate this data
558+
LayerInfos: updatedLayers,
559+
LayerDiffIDs: schema1WithThrowawaysFixtureLayerDiffIDs,
560+
},
561+
})
562+
require.NoError(t, err)
563+
assert.Equal(t, updatedLayersCopy, updatedLayers) // updatedLayers have not been modified in place
564+
convertedJSON, mt, err = res.Manifest(context.Background())
565+
require.NoError(t, err)
566+
assert.Equal(t, imgspecv1.MediaTypeImageManifest, mt)
567+
// Layers have been updated as expected
568+
originalSrc := newSchema2ImageSource(t, "httpd:latest")
569+
ociManifest, err := manifestOCI1FromManifest(originalSrc, convertedJSON)
570+
require.NoError(t, err)
571+
assert.Equal(t, []types.BlobInfo{
572+
{
573+
Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5ba",
574+
Size: 51354365,
575+
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
576+
},
577+
{
578+
Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680d",
579+
Size: 151,
580+
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
581+
},
582+
{
583+
Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a8",
584+
Size: 11739506,
585+
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
586+
},
587+
{
588+
Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25908",
589+
Size: 8841832,
590+
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
591+
},
592+
{
593+
Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fb",
594+
Size: 290,
595+
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
596+
},
597+
}, ociManifest.LayerInfos())
598+
419599
// FIXME? Test also the various failure cases, if only to see that we don't crash?
420600
}
421601

@@ -466,6 +646,10 @@ func TestConvertSchema1ToManifestOCIWithAnnotations(t *testing.T) {
466646
res, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
467647
ManifestMIMEType: imgspecv1.MediaTypeImageManifest,
468648
LayerInfos: layerInfoOverwrites,
649+
InformationOnly: types.ManifestUpdateInformation{
650+
LayerInfos: schema1FixtureLayerInfos,
651+
LayerDiffIDs: schema1FixtureLayerDiffIDs,
652+
},
469653
})
470654
require.NoError(t, err)
471655
assert.Equal(t, res.LayerInfos(), layerInfoOverwrites)
@@ -475,9 +659,11 @@ func TestConvertSchema1ToManifestOCIWithAnnotations(t *testing.T) {
475659
res, err = original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
476660
ManifestMIMEType: manifest.DockerV2Schema2MediaType,
477661
LayerInfos: layerInfoOverwrites,
662+
InformationOnly: types.ManifestUpdateInformation{
663+
LayerInfos: schema1FixtureLayerInfos,
664+
LayerDiffIDs: schema1FixtureLayerDiffIDs,
665+
},
478666
})
479667
require.NoError(t, err)
480668
assert.NotEqual(t, res.LayerInfos(), layerInfoOverwrites)
481669
}
482-
483-
// FIXME: Schema1→OCI conversion untested

0 commit comments

Comments
 (0)