Skip to content

Commit

Permalink
Merge pull request #866 from mtrmac/updates-s2s1
Browse files Browse the repository at this point in the history 
Fix conversions from/to schema1 with layer updates
  • Loading branch information
@mtrmac
mtrmac authored Mar 27, 2020
2 parents 69d6e9c + a5a3165 commit f5a333e
Show file tree
Hide file tree
Showing 13 changed files with 557 additions and 45 deletions.
1 change: 1 addition & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ github.com/mtrmac/gpgme v0.1.2/go.mod h1:GYYHnGSuS7HK3zVS2n3y73y0okK/BeKzwnn5jgi
github.com/opencontainers/go-digest v0.0.0-20180430190053-c9281466c8b2/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=
github.com/opencontainers/go-digest v1.0.0-rc1 h1:WzifXhOVOEOuFYOJAW6aQqW0TooG2iki3E3Ii+WN7gQ=
github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=
github.com/opencontainers/image-spec v1.0.1 h1:JMemWkRwHx4Zj+fVxWoMCFm/8sYGGrUVojFA6h/TRcI=
github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
github.com/opencontainers/image-spec v1.0.2-0.20190823105129-775207bd45b6 h1:yN8BPXVwMBAm3Cuvh1L5XE8XpvYRMdsVLd82ILprhUU=
github.com/opencontainers/image-spec v1.0.2-0.20190823105129-775207bd45b6/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
Expand Down
57 changes: 44 additions & 13 deletions image/docker_schema1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (m *manifestSchema1) ConfigBlob(context.Context) ([]byte, error) {
// layers in the resulting configuration isn't guaranteed to be returned to due how
// old image manifests work (docker v2s1 especially).
func (m *manifestSchema1) OCIConfig(ctx context.Context) (*imgspecv1.Image, error) {
v2s2, err := m.convertToManifestSchema2(ctx, types.ManifestUpdateInformation{})
v2s2, err := m.convertToManifestSchema2(ctx, &types.ManifestUpdateOptions{})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -113,7 +113,7 @@ func (m *manifestSchema1) UpdatedImage(ctx context.Context, options types.Manife
if options.ManifestMIMEType != manifest.DockerV2Schema1MediaType && options.ManifestMIMEType != manifest.DockerV2Schema1SignedMediaType {
converted, err := convertManifestIfRequiredWithUpdate(ctx, options, map[string]manifestConvertFn{
imgspecv1.MediaTypeImageManifest: copy.convertToManifestOCI1,
manifest.DockerV2Schema2MediaType: copy.convertToManifestSchema2,
manifest.DockerV2Schema2MediaType: copy.convertToManifestSchema2Generic,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -142,10 +142,26 @@ func (m *manifestSchema1) UpdatedImage(ctx context.Context, options types.Manife
return memoryImageFromManifest(&copy), nil
}

// convertToManifestSchema2Generic returns a genericManifest implementation converted to manifest.DockerV2Schema2MediaType.
// It may use options.InformationOnly and also adjust *options to be appropriate for editing the returned
// value.
// This does not change the state of the original manifestSchema1 object.
//
// We need this function just because a function returning an implementation of the genericManifest
// interface is not automatically assignable to a function type returning the genericManifest interface
func (m *manifestSchema1) convertToManifestSchema2Generic(ctx context.Context, options *types.ManifestUpdateOptions) (genericManifest, error) {
return m.convertToManifestSchema2(ctx, options)
}

// convertToManifestSchema2 returns a genericManifest implementation converted to manifest.DockerV2Schema2MediaType.
// It may use options.InformationOnly and also adjust *options to be appropriate for editing the returned
// value.
// This does not change the state of the original manifestSchema1 object.
//
// Based on github.com/docker/docker/distribution/pull_v2.go
func (m *manifestSchema1) convertToManifestSchema2(_ context.Context, updateInfo types.ManifestUpdateInformation) (types.Image, error) {
uploadedLayerInfos := updateInfo.LayerInfos
layerDiffIDs := updateInfo.LayerDiffIDs
func (m *manifestSchema1) convertToManifestSchema2(_ context.Context, options *types.ManifestUpdateOptions) (*manifestSchema2, error) {
uploadedLayerInfos := options.InformationOnly.LayerInfos
layerDiffIDs := options.InformationOnly.LayerDiffIDs

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

var convertedLayerUpdates []types.BlobInfo // Only used if options.LayerInfos != nil
if options.LayerInfos != nil {
if len(options.LayerInfos) != len(m.m.FSLayers) {
return nil, errors.Errorf("Error converting image: layer edits for %d layers vs %d existing layers",
len(options.LayerInfos), len(m.m.FSLayers))
}
convertedLayerUpdates = []types.BlobInfo{}
}

// Build a list of the diffIDs for the non-empty layers.
diffIDs := []digest.Digest{}
var layers []manifest.Schema2Descriptor
Expand All @@ -181,6 +206,9 @@ func (m *manifestSchema1) convertToManifestSchema2(_ context.Context, updateInfo
Size: size,
Digest: m.m.FSLayers[v1Index].BlobSum,
})
if options.LayerInfos != nil {
convertedLayerUpdates = append(convertedLayerUpdates, options.LayerInfos[v2Index])
}
diffIDs = append(diffIDs, d)
}
}
Expand All @@ -194,21 +222,24 @@ func (m *manifestSchema1) convertToManifestSchema2(_ context.Context, updateInfo
Digest: digest.FromBytes(configJSON),
}

m1 := manifestSchema2FromComponents(configDescriptor, nil, configJSON, layers)
return memoryImageFromManifest(m1), nil
if options.LayerInfos != nil {
options.LayerInfos = convertedLayerUpdates
}
return manifestSchema2FromComponents(configDescriptor, nil, configJSON, layers), nil
}

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

return m2.UpdatedImage(ctx, types.ManifestUpdateOptions{
ManifestMIMEType: imgspecv1.MediaTypeImageManifest,
InformationOnly: updateInfo,
})
return m2.convertToManifestOCI1(ctx, options)
}

// SupportsEncryption returns if encryption is supported for the manifest type
Expand Down
194 changes: 190 additions & 4 deletions image/docker_schema1_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,42 @@ var schema1FixtureLayerDiffIDs = []digest.Digest{
"sha256:9bd63850e406167b4751f5050f6dc0ebd789bb5ef5e5c6c31ed062bda8c063e8",
}

var schema1WithThrowawaysFixtureLayerInfos = []types.BlobInfo{
{Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb", Size: 51354364},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
{Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c", Size: 150},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
{Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", Size: 11739507},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
{Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", Size: 8841833},
{Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa", Size: 291},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))},
}

var schema1WithThrowawaysFixtureLayerDiffIDs = []digest.Digest{
"sha256:142a601d97936307e75220c35dde0348971a9584c21e7cb42e1f7004005432ab",
GzippedEmptyLayerDigest,
GzippedEmptyLayerDigest,
GzippedEmptyLayerDigest,
"sha256:90fcc66ad3be9f1757f954b750deb37032f208428aa12599fcb02182b9065a9c",
GzippedEmptyLayerDigest,
"sha256:5a8624bb7e76d1e6829f9c64c43185e02bc07f97a2189eb048609a8914e72c56",
GzippedEmptyLayerDigest,
GzippedEmptyLayerDigest,
GzippedEmptyLayerDigest,
GzippedEmptyLayerDigest,
"sha256:d349ff6b3afc6a2800054768c82bfbf4289c9aa5da55c1290f802943dcd4d1e9",
"sha256:8c064bb1f60e84fa8cc6079b6d2e76e0423389fd6aeb7e497dfdae5e05b2b25b",
GzippedEmptyLayerDigest,
GzippedEmptyLayerDigest,
}

func manifestSchema1FromFixture(t *testing.T, fixture string) genericManifest {
manifest, err := ioutil.ReadFile(filepath.Join("fixtures", fixture))
require.NoError(t, err)
Expand Down Expand Up @@ -167,7 +203,7 @@ func TestManifestSchema1ConfigBlob(t *testing.T) {
}

func TestManifestSchema1OCIConfig(t *testing.T) {
m := manifestSchema1FromFixture(t, "schema1-to-oci-config.json")
m := manifestSchema1FromFixture(t, "schema1-for-oci-config.json")
configOCI, err := m.OCIConfig(context.Background())
require.NoError(t, err)
// FIXME: A more comprehensive test?
Expand Down Expand Up @@ -398,7 +434,7 @@ func TestManifestSchema1ConvertToSchema2(t *testing.T) {
err = json.Unmarshal(byHandJSON, &byHand)
require.NoError(t, err)
err = json.Unmarshal(convertedJSON, &converted)
delete(converted, "config")
delete(converted, "config") // We don’t want to hard-code a specific digest and size of the marshaled config here
delete(byHand, "config")
require.NoError(t, err)
assert.Equal(t, byHand, converted)
Expand All @@ -416,6 +452,150 @@ func TestManifestSchema1ConvertToSchema2(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, byHand, converted)

// Conversion to schema2 together with changing LayerInfos works as expected (which requires
// handling schema1 throwaway layers):
// Use the recorded result of converting the schema2 fixture to schema1, because that one
// (unlike schem1.json) contains throwaway layers.
original = manifestSchema1FromFixture(t, "schema2-to-schema1-by-docker.json")
updatedLayers, updatedLayersCopy := modifiedLayerInfos(t, schema1WithThrowawaysFixtureLayerInfos)
res, err = original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
LayerInfos: updatedLayers,
ManifestMIMEType: manifest.DockerV2Schema2MediaType,
InformationOnly: types.ManifestUpdateInformation{
LayerInfos: updatedLayers,
LayerDiffIDs: schema1WithThrowawaysFixtureLayerDiffIDs,
},
})
require.NoError(t, err)
assert.Equal(t, updatedLayersCopy, updatedLayers) // updatedLayers have not been modified in place
convertedJSON, mt, err = res.Manifest(context.Background())
require.NoError(t, err)
assert.Equal(t, manifest.DockerV2Schema2MediaType, mt)
// Layers have been updated as expected
originalSrc := newSchema2ImageSource(t, "httpd:latest")
s2Manifest, err := manifestSchema2FromManifest(originalSrc, convertedJSON)
require.NoError(t, err)
assert.Equal(t, []types.BlobInfo{
{
Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5ba",
Size: 51354365,
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
},
{
Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680d",
Size: 151,
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
},
{
Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a8",
Size: 11739506,
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
},
{
Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25908",
Size: 8841832,
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
},
{
Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fb",
Size: 290,
MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
},
}, s2Manifest.LayerInfos())

// FIXME? Test also the various failure cases, if only to see that we don't crash?
}

func TestManifestSchema1ConvertToManifestOCI1(t *testing.T) {
original := manifestSchema1FromFixture(t, "schema1.json")
res, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
ManifestMIMEType: imgspecv1.MediaTypeImageManifest,
InformationOnly: types.ManifestUpdateInformation{
LayerInfos: schema1FixtureLayerInfos,
LayerDiffIDs: schema1FixtureLayerDiffIDs,
},
})
require.NoError(t, err)

convertedJSON, mt, err := res.Manifest(context.Background())
require.NoError(t, err)
assert.Equal(t, imgspecv1.MediaTypeImageManifest, mt)

byHandJSON, err := ioutil.ReadFile("fixtures/schema1-to-oci1.json")
require.NoError(t, err)
var converted, byHand map[string]interface{}
err = json.Unmarshal(byHandJSON, &byHand)
require.NoError(t, err)
err = json.Unmarshal(convertedJSON, &converted)
delete(converted, "config") // We don’t want to hard-code a specific digest and size of the marshaled config here
delete(byHand, "config")
require.NoError(t, err)
assert.Equal(t, byHand, converted)

convertedConfig, err := res.ConfigBlob(context.Background())
require.NoError(t, err)

byHandConfig, err := ioutil.ReadFile("fixtures/schema1-to-oci1-config.json")
require.NoError(t, err)
converted = map[string]interface{}{}
byHand = map[string]interface{}{}
err = json.Unmarshal(byHandConfig, &byHand)
require.NoError(t, err)
err = json.Unmarshal(convertedConfig, &converted)
require.NoError(t, err)
assert.Equal(t, byHand, converted)

// Conversion to OCI together with changing LayerInfos works as expected (which requires
// handling schema1 throwaway layers):
// Use the recorded result of converting the schema2 fixture to schema1, because that one
// (unlike schem1.json) contains throwaway layers.
original = manifestSchema1FromFixture(t, "schema2-to-schema1-by-docker.json")
updatedLayers, updatedLayersCopy := modifiedLayerInfos(t, schema1WithThrowawaysFixtureLayerInfos)
res, err = original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
LayerInfos: updatedLayers,
ManifestMIMEType: imgspecv1.MediaTypeImageManifest,
InformationOnly: types.ManifestUpdateInformation{ // FIXME: deduplicate this data
LayerInfos: updatedLayers,
LayerDiffIDs: schema1WithThrowawaysFixtureLayerDiffIDs,
},
})
require.NoError(t, err)
assert.Equal(t, updatedLayersCopy, updatedLayers) // updatedLayers have not been modified in place
convertedJSON, mt, err = res.Manifest(context.Background())
require.NoError(t, err)
assert.Equal(t, imgspecv1.MediaTypeImageManifest, mt)
// Layers have been updated as expected
originalSrc := newSchema2ImageSource(t, "httpd:latest")
ociManifest, err := manifestOCI1FromManifest(originalSrc, convertedJSON)
require.NoError(t, err)
assert.Equal(t, []types.BlobInfo{
{
Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5ba",
Size: 51354365,
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
},
{
Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680d",
Size: 151,
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
},
{
Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a8",
Size: 11739506,
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
},
{
Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25908",
Size: 8841832,
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
},
{
Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fb",
Size: 290,
MediaType: "application/vnd.oci.image.layer.v1.tar+gzip",
},
}, ociManifest.LayerInfos())

// FIXME? Test also the various failure cases, if only to see that we don't crash?
}

Expand Down Expand Up @@ -466,6 +646,10 @@ func TestConvertSchema1ToManifestOCIWithAnnotations(t *testing.T) {
res, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
ManifestMIMEType: imgspecv1.MediaTypeImageManifest,
LayerInfos: layerInfoOverwrites,
InformationOnly: types.ManifestUpdateInformation{
LayerInfos: schema1FixtureLayerInfos,
LayerDiffIDs: schema1FixtureLayerDiffIDs,
},
})
require.NoError(t, err)
assert.Equal(t, res.LayerInfos(), layerInfoOverwrites)
Expand All @@ -475,9 +659,11 @@ func TestConvertSchema1ToManifestOCIWithAnnotations(t *testing.T) {
res, err = original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
ManifestMIMEType: manifest.DockerV2Schema2MediaType,
LayerInfos: layerInfoOverwrites,
InformationOnly: types.ManifestUpdateInformation{
LayerInfos: schema1FixtureLayerInfos,
LayerDiffIDs: schema1FixtureLayerDiffIDs,
},
})
require.NoError(t, err)
assert.NotEqual(t, res.LayerInfos(), layerInfoOverwrites)
}

// FIXME: Schema1→OCI conversion untested
Loading

0 comments on commit f5a333e

Please sign in to comment.