Merge pull request #472 from mtrmac/storage-emptylayers

Don't physically store “empty layers” in v2s1 images in containers-storage:

image/docker_schema1.go

@@ -2,7 +2,6 @@ package image
 
 import (
 	"context"
-	"encoding/json"
 
 	"github.com/containers/image/docker/reference"
 	"github.com/containers/image/manifest"
@@ -25,8 +24,12 @@ func manifestSchema1FromManifest(manifestBlob []byte) (genericManifest, error) {
 }
 
 // manifestSchema1FromComponents builds a new manifestSchema1 from the supplied data.
-func manifestSchema1FromComponents(ref reference.Named, fsLayers []manifest.Schema1FSLayers, history []manifest.Schema1History, architecture string) genericManifest {
-	return &manifestSchema1{m: manifest.Schema1FromComponents(ref, fsLayers, history, architecture)}
+func manifestSchema1FromComponents(ref reference.Named, fsLayers []manifest.Schema1FSLayers, history []manifest.Schema1History, architecture string) (genericManifest, error) {
+	m, err := manifest.Schema1FromComponents(ref, fsLayers, history, architecture)
+	if err != nil {
+		return nil, err
+	}
+	return &manifestSchema1{m: m}, nil
 }
 
 func (m *manifestSchema1) serialize() ([]byte, error) {
@@ -64,7 +67,7 @@ func (m *manifestSchema1) OCIConfig(ctx context.Context) (*imgspecv1.Image, erro
 // The Digest field is guaranteed to be provided; Size may be -1.
 // WARNING: The list may contain duplicates, and they are semantically relevant.
 func (m *manifestSchema1) LayerInfos() []types.BlobInfo {
-	return m.m.LayerInfos()
+	return manifestLayerInfosToBlobInfos(m.m.LayerInfos())
 }
 
 // EmbeddedDockerReferenceConflicts whether a Docker reference embedded in the manifest, if any, conflicts with destination ref.
@@ -148,12 +151,12 @@ func (m *manifestSchema1) UpdatedImage(ctx context.Context, options types.Manife
 
 // Based on github.com/docker/docker/distribution/pull_v2.go
 func (m *manifestSchema1) convertToManifestSchema2(uploadedLayerInfos []types.BlobInfo, layerDiffIDs []digest.Digest) (genericManifest, error) {
-	if len(m.m.History) == 0 {
-		// What would this even mean?! Anyhow, the rest of the code depends on fsLayers[0] and history[0] existing.
+	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.
 		return nil, errors.Errorf("Cannot convert an image with 0 history entries to %s", manifest.DockerV2Schema2MediaType)
 	}
-	if len(m.m.History) != len(m.m.FSLayers) {
-		return nil, errors.Errorf("Inconsistent schema 1 manifest: %d history entries, %d fsLayers entries", len(m.m.History), len(m.m.FSLayers))
+	if len(m.m.ExtractedV1Compatibility) != len(m.m.FSLayers) {
+		return nil, errors.Errorf("Inconsistent schema 1 manifest: %d history entries, %d fsLayers entries", len(m.m.ExtractedV1Compatibility), len(m.m.FSLayers))
 	}
 	if uploadedLayerInfos != nil && len(uploadedLayerInfos) != len(m.m.FSLayers) {
 		return nil, errors.Errorf("Internal error: uploaded %d blobs, but schema1 manifest has %d fsLayers", len(uploadedLayerInfos), len(m.m.FSLayers))
@@ -165,14 +168,10 @@ func (m *manifestSchema1) convertToManifestSchema2(uploadedLayerInfos []types.Bl
 	// Build a list of the diffIDs for the non-empty layers.
 	diffIDs := []digest.Digest{}
 	var layers []manifest.Schema2Descriptor
-	for v1Index := len(m.m.History) - 1; v1Index >= 0; v1Index-- {
-		v2Index := (len(m.m.History) - 1) - v1Index
+	for v1Index := len(m.m.ExtractedV1Compatibility) - 1; v1Index >= 0; v1Index-- {
+		v2Index := (len(m.m.ExtractedV1Compatibility) - 1) - v1Index
 
-		var v1compat manifest.Schema1V1Compatibility
-		if err := json.Unmarshal([]byte(m.m.History[v1Index].V1Compatibility), &v1compat); err != nil {
-			return nil, errors.Wrapf(err, "Error decoding history entry %d", v1Index)
-		}
-		if !v1compat.ThrowAway {
+		if !m.m.ExtractedV1Compatibility[v1Index].ThrowAway {
 			var size int64
 			if uploadedLayerInfos != nil {
 				size = uploadedLayerInfos[v2Index].Size

image/docker_schema1_test.go

@@ -71,7 +71,7 @@ func manifestSchema1FromFixture(t *testing.T, fixture string) genericManifest {
 func manifestSchema1FromComponentsLikeFixture(t *testing.T) genericManifest {
 	ref, err := reference.ParseNormalizedNamed("rhosp12/openstack-nova-api:latest")
 	require.NoError(t, err)
-	return manifestSchema1FromComponents(ref, []manifest.Schema1FSLayers{
+	m, err := manifestSchema1FromComponents(ref, []manifest.Schema1FSLayers{
 		{BlobSum: "sha256:e623934bca8d1a74f51014256445937714481e49343a31bda2bc5f534748184d"},
 		{BlobSum: "sha256:62e48e39dc5b30b75a97f05bccc66efbae6058b860ee20a5c9a184b9d5e25788"},
 		{BlobSum: "sha256:9e92df2aea7dc0baf5f1f8d509678d6a6306de27ad06513f8e218371938c07a6"},
@@ -86,6 +86,8 @@ func manifestSchema1FromComponentsLikeFixture(t *testing.T) genericManifest {
 		{V1Compatibility: "{\"id\":\"0e7730eccb3d014b33147b745d771bc0e38a967fd932133a6f5325a3c84282e2\",\"parent\":\"3e49094c0233214ab73f8e5c204af8a14cfc6f0403384553c17fbac2e9d38345\",\"created\":\"2017-11-21T16:49:37.292899Z\",\"container_config\":{\"Cmd\":[\"/bin/sh -c rm -f '/etc/yum.repos.d/compose-rpms-1.repo'\"]},\"author\":\"Red Hat, Inc.\"}"},
 		{V1Compatibility: "{\"id\":\"3e49094c0233214ab73f8e5c204af8a14cfc6f0403384553c17fbac2e9d38345\",\"comment\":\"Imported from -\",\"created\":\"2017-11-21T16:47:27.755341705Z\",\"container_config\":{\"Cmd\":[\"\"]}}"},
 	}, "amd64")
+	require.NoError(t, err)
+	return m
 }
 
 func TestManifestSchema1FromManifest(t *testing.T) {
@@ -102,6 +104,10 @@ func TestManifestSchema1FromComponents(t *testing.T) {
 	// This just smoke-tests that the manifest can be created; we test that the parsed
 	// values are correctly returned in tests for the individual getter methods.
 	_ = manifestSchema1FromComponentsLikeFixture(t)
+
+	// Error on invalid input
+	_, err := manifestSchema1FromComponents(nil, []manifest.Schema1FSLayers{}, []manifest.Schema1History{}, "amd64")
+	assert.Error(t, err)
 }
 
 func TestManifestSchema1Serialize(t *testing.T) {

image/docker_schema2.go

@@ -18,17 +18,17 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// gzippedEmptyLayer is a gzip-compressed version of an empty tar file (1024 NULL bytes)
+// GzippedEmptyLayer is a gzip-compressed version of an empty tar file (1024 NULL bytes)
 // This comes from github.com/docker/distribution/manifest/schema1/config_builder.go; there is
 // a non-zero embedded timestamp; we could zero that, but that would just waste storage space
 // in registries, so let’s use the same values.
-var gzippedEmptyLayer = []byte{
+var GzippedEmptyLayer = []byte{
 	31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 98, 24, 5, 163, 96, 20, 140, 88,
 	0, 8, 0, 0, 255, 255, 46, 175, 181, 239, 0, 4, 0, 0,
 }
 
-// gzippedEmptyLayerDigest is a digest of gzippedEmptyLayer
-const gzippedEmptyLayerDigest = digest.Digest("sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4")
+// GzippedEmptyLayerDigest is a digest of GzippedEmptyLayer
+const GzippedEmptyLayerDigest = digest.Digest("sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4")
 
 type manifestSchema2 struct {
 	src        types.ImageSource // May be nil if configBlob is not nil
@@ -95,11 +95,7 @@ func (m *manifestSchema2) ConfigBlob(ctx context.Context) ([]byte, error) {
 		if m.src == nil {
 			return nil, errors.Errorf("Internal error: neither src nor configBlob set in manifestSchema2")
 		}
-		stream, _, err := m.src.GetBlob(ctx, types.BlobInfo{
-			Digest: m.m.ConfigDescriptor.Digest,
-			Size:   m.m.ConfigDescriptor.Size,
-			URLs:   m.m.ConfigDescriptor.URLs,
-		})
+		stream, _, err := m.src.GetBlob(ctx, manifest.BlobInfoFromSchema2Descriptor(m.m.ConfigDescriptor))
 		if err != nil {
 			return nil, err
 		}
@@ -121,7 +117,7 @@ func (m *manifestSchema2) ConfigBlob(ctx context.Context) ([]byte, error) {
 // The Digest field is guaranteed to be provided; Size may be -1.
 // WARNING: The list may contain duplicates, and they are semantically relevant.
 func (m *manifestSchema2) LayerInfos() []types.BlobInfo {
-	return m.m.LayerInfos()
+	return manifestLayerInfosToBlobInfos(m.m.LayerInfos())
 }
 
 // EmbeddedDockerReferenceConflicts whether a Docker reference embedded in the manifest, if any, conflicts with destination ref.
@@ -253,16 +249,16 @@ func (m *manifestSchema2) convertToManifestSchema1(ctx context.Context, dest typ
 		if historyEntry.EmptyLayer {
 			if !haveGzippedEmptyLayer {
 				logrus.Debugf("Uploading empty layer during conversion to schema 1")
-				info, err := dest.PutBlob(ctx, bytes.NewReader(gzippedEmptyLayer), types.BlobInfo{Digest: gzippedEmptyLayerDigest, Size: int64(len(gzippedEmptyLayer))}, false)
+				info, err := dest.PutBlob(ctx, bytes.NewReader(GzippedEmptyLayer), types.BlobInfo{Digest: GzippedEmptyLayerDigest, Size: int64(len(GzippedEmptyLayer))}, false)
 				if err != nil {
 					return nil, errors.Wrap(err, "Error uploading empty layer")
 				}
-				if info.Digest != gzippedEmptyLayerDigest {
-					return nil, errors.Errorf("Internal error: Uploaded empty layer has digest %#v instead of %s", info.Digest, gzippedEmptyLayerDigest)
+				if info.Digest != GzippedEmptyLayerDigest {
+					return nil, errors.Errorf("Internal error: Uploaded empty layer has digest %#v instead of %s", info.Digest, GzippedEmptyLayerDigest)
 				}
 				haveGzippedEmptyLayer = true
 			}
-			blobDigest = gzippedEmptyLayerDigest
+			blobDigest = GzippedEmptyLayerDigest
 		} else {
 			if nonemptyLayerIndex >= len(m.m.LayersDescriptors) {
 				return nil, errors.Errorf("Invalid image configuration, needs more than the %d distributed layers", len(m.m.LayersDescriptors))
@@ -308,7 +304,10 @@ func (m *manifestSchema2) convertToManifestSchema1(ctx context.Context, dest typ
 	}
 	history[0].V1Compatibility = string(v1Config)
 
-	m1 := manifestSchema1FromComponents(dest.Reference().DockerReference(), fsLayers, history, imageConfig.Architecture)
+	m1, err := manifestSchema1FromComponents(dest.Reference().DockerReference(), fsLayers, history, imageConfig.Architecture)
+	if err != nil {
+		return nil, err // This should never happen, we should have created all the components correctly.
+	}
 	return memoryImageFromManifest(m1), nil
 }
 

image/docker_schema2_test.go

@@ -139,8 +139,9 @@ func TestManifestSchema2ConfigInfo(t *testing.T) {
 		manifestSchema2FromComponentsLikeFixture(nil),
 	} {
 		assert.Equal(t, types.BlobInfo{
-			Size:   5940,
-			Digest: "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f",
+			Size:      5940,
+			Digest:    "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f",
+			MediaType: "application/octet-stream",
 		}, m.ConfigInfo())
 	}
 }
@@ -220,24 +221,29 @@ func TestManifestSchema2LayerInfo(t *testing.T) {
 	} {
 		assert.Equal(t, []types.BlobInfo{
 			{
-				Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb",
-				Size:   51354364,
+				Digest:    "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb",
+				Size:      51354364,
+				MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
 			},
 			{
-				Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c",
-				Size:   150,
+				Digest:    "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c",
+				Size:      150,
+				MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
 			},
 			{
-				Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9",
-				Size:   11739507,
+				Digest:    "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9",
+				Size:      11739507,
+				MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
 			},
 			{
-				Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909",
-				Size:   8841833,
+				Digest:    "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909",
+				Size:      8841833,
+				MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
 			},
 			{
-				Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa",
-				Size:   291,
+				Digest:    "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa",
+				Size:      291,
+				MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
 			},
 		}, m.LayerInfos())
 	}
@@ -533,7 +539,7 @@ func TestConvertToManifestSchema1(t *testing.T) {
 	delete(converted, "signatures")
 	assert.Equal(t, byDocker, converted)
 
-	assert.Equal(t, gzippedEmptyLayer, memoryDest.storedBlobs[gzippedEmptyLayerDigest])
+	assert.Equal(t, GzippedEmptyLayer, memoryDest.storedBlobs[GzippedEmptyLayerDigest])
 
 	// FIXME? Test also the various failure cases, if only to see that we don't crash?
 }

image/manifest.go

@@ -62,3 +62,12 @@ func manifestInstanceFromBlob(ctx context.Context, sys *types.SystemContext, src
 		return nil, fmt.Errorf("Unimplemented manifest MIME type %s", mt)
 	}
 }
+
+// manifestLayerInfosToBlobInfos extracts a []types.BlobInfo from a []manifest.LayerInfo.
+func manifestLayerInfosToBlobInfos(layers []manifest.LayerInfo) []types.BlobInfo {
+	blobs := make([]types.BlobInfo, len(layers))
+	for i, layer := range layers {
+		blobs[i] = layer.BlobInfo
+	}
+	return blobs
+}

image/manifest_test.go

@@ -0,0 +1,71 @@
+package image
+
+import (
+	"testing"
+
+	"github.com/containers/image/manifest"
+	"github.com/containers/image/types"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestManifestLayerInfosToBlobInfos(t *testing.T) {
+	blobs := manifestLayerInfosToBlobInfos([]manifest.LayerInfo{})
+	assert.Equal(t, []types.BlobInfo{}, blobs)
+
+	blobs = manifestLayerInfosToBlobInfos([]manifest.LayerInfo{
+		{
+			BlobInfo: types.BlobInfo{
+				MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+				Digest:    "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4",
+				Size:      32,
+			},
+			EmptyLayer: true,
+		},
+		{
+			BlobInfo: types.BlobInfo{
+				MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+				Digest:    "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909",
+				Size:      8841833,
+			},
+			EmptyLayer: false,
+		},
+		{
+			BlobInfo: types.BlobInfo{
+				MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+				Digest:    "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa",
+				Size:      291,
+			},
+			EmptyLayer: false,
+		},
+		{
+			BlobInfo: types.BlobInfo{
+				MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+				Digest:    "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4",
+				Size:      32,
+			},
+			EmptyLayer: true,
+		},
+	})
+	assert.Equal(t, []types.BlobInfo{
+		{
+			MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+			Digest:    "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4",
+			Size:      32,
+		},
+		{
+			MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+			Digest:    "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909",
+			Size:      8841833,
+		},
+		{
+			MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+			Digest:    "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa",
+			Size:      291,
+		},
+		{
+			MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+			Digest:    "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4",
+			Size:      32,
+		},
+	}, blobs)
+}

image/oci.go

@@ -60,11 +60,7 @@ func (m *manifestOCI1) ConfigBlob(ctx context.Context) ([]byte, error) {
 		if m.src == nil {
 			return nil, errors.Errorf("Internal error: neither src nor configBlob set in manifestOCI1")
 		}
-		stream, _, err := m.src.GetBlob(ctx, types.BlobInfo{
-			Digest: m.m.Config.Digest,
-			Size:   m.m.Config.Size,
-			URLs:   m.m.Config.URLs,
-		})
+		stream, _, err := m.src.GetBlob(ctx, manifest.BlobInfoFromOCI1Descriptor(m.m.Config))
 		if err != nil {
 			return nil, err
 		}
@@ -101,7 +97,7 @@ func (m *manifestOCI1) OCIConfig(ctx context.Context) (*imgspecv1.Image, error)
 // The Digest field is guaranteed to be provided; Size may be -1.
 // WARNING: The list may contain duplicates, and they are semantically relevant.
 func (m *manifestOCI1) LayerInfos() []types.BlobInfo {
-	return m.m.LayerInfos()
+	return manifestLayerInfosToBlobInfos(m.m.LayerInfos())
 }
 
 // EmbeddedDockerReferenceConflicts whether a Docker reference embedded in the manifest, if any, conflicts with destination ref.

image/oci_test.go

@@ -131,6 +131,7 @@ func TestManifestOCI1ConfigInfo(t *testing.T) {
 			Annotations: map[string]string{
 				"test-annotation-1": "one",
 			},
+			MediaType: "application/vnd.oci.image.config.v1+json",
 		}, m.ConfigInfo())
 	}
 }