Merge pull request #1541 from sipsma/fix-media-type

image export: Use correct media type when creating new layer blobs.

cache/blobs/compression.go

@@ -11,6 +11,7 @@ import (
 	digest "github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // CompressionType represents compression type for blob data.
@@ -101,22 +102,64 @@ func detectCompressionType(cr io.Reader) (CompressionType, error) {
 }
 
 // GetMediaTypeForLayers retrieves media type for layer from ref information.
+// If there is a mismatch in diff IDs or blobsums between the diffPairs and
+// corresponding ref, the returned slice will have an empty media type for
+// that layer and all parents.
 func GetMediaTypeForLayers(diffPairs []DiffPair, ref cache.ImmutableRef) []string {
-	tref := ref
+	layerTypes := make([]string, len(diffPairs))
+	if ref == nil {
+		return layerTypes
+	}
 
-	layerTypes := make([]string, 0, len(diffPairs))
-	for _, dp := range diffPairs {
-		if tref == nil {
-			return nil
-		}
+	tref := ref.Clone()
+	// diffPairs is ordered parent->child, but we iterate over refs from child->parent,
+	// so iterate over diffPairs in reverse
+	for i := range diffPairs {
+		dp := diffPairs[len(diffPairs)-1-i]
 
 		info := tref.Info()
 		if !(info.DiffID == dp.DiffID && info.Blob == dp.Blobsum) {
-			return nil
+			break
 		}
+		layerTypes[len(diffPairs)-1-i] = info.MediaType
 
-		layerTypes = append(layerTypes, info.MediaType)
-		tref = tref.Parent()
+		parent := tref.Parent()
+		tref.Release(context.TODO())
+		tref = parent
+		if tref == nil {
+			break
+		}
+	}
+	if tref != nil {
+		tref.Release(context.TODO())
 	}
 	return layerTypes
 }
+
+var toDockerLayerType = map[string]string{
+	ocispec.MediaTypeImageLayer:            images.MediaTypeDockerSchema2Layer,
+	images.MediaTypeDockerSchema2Layer:     images.MediaTypeDockerSchema2Layer,
+	ocispec.MediaTypeImageLayerGzip:        images.MediaTypeDockerSchema2LayerGzip,
+	images.MediaTypeDockerSchema2LayerGzip: images.MediaTypeDockerSchema2LayerGzip,
+}
+
+var toOCILayerType = map[string]string{
+	ocispec.MediaTypeImageLayer:            ocispec.MediaTypeImageLayer,
+	images.MediaTypeDockerSchema2Layer:     ocispec.MediaTypeImageLayer,
+	ocispec.MediaTypeImageLayerGzip:        ocispec.MediaTypeImageLayerGzip,
+	images.MediaTypeDockerSchema2LayerGzip: ocispec.MediaTypeImageLayerGzip,
+}
+
+func ConvertLayerMediaType(mediaType string, oci bool) string {
+	var converted string
+	if oci {
+		converted = toOCILayerType[mediaType]
+	} else {
+		converted = toDockerLayerType[mediaType]
+	}
+	if converted == "" {
+		logrus.Warnf("unhandled conversion for mediatype %q", mediaType)
+		return mediaType
+	}
+	return converted
+}

client/client_test.go

@@ -1737,6 +1737,22 @@ func testBuildExportWithUncompressed(t *testing.T, sb integration.Sandbox) {
 	}, nil)
 	require.NoError(t, err)
 
+	ctx := namespaces.WithNamespace(context.Background(), "buildkit")
+	cdAddress := sb.ContainerdAddress()
+	var client *containerd.Client
+	if cdAddress != "" {
+		client, err = newContainerd(cdAddress)
+		require.NoError(t, err)
+		defer client.Close()
+
+		img, err := client.GetImage(ctx, target)
+		require.NoError(t, err)
+		mfst, err := images.Manifest(ctx, client.ContentStore(), img.Target(), nil)
+		require.NoError(t, err)
+		require.Equal(t, 1, len(mfst.Layers))
+		require.Equal(t, images.MediaTypeDockerSchema2Layer, mfst.Layers[0].MediaType)
+	}
+
 	// new layer with gzip compression
 	targetImg := llb.Image(target)
 	cmd = `sh -e -c "echo -n gzip > data"`
@@ -1759,16 +1775,10 @@ func testBuildExportWithUncompressed(t *testing.T, sb integration.Sandbox) {
 	}, nil)
 	require.NoError(t, err)
 
-	cdAddress := sb.ContainerdAddress()
 	if cdAddress == "" {
 		t.Skip("rest of test requires containerd worker")
 	}
 
-	client, err := newContainerd(cdAddress)
-	require.NoError(t, err)
-	defer client.Close()
-
-	ctx := namespaces.WithNamespace(context.Background(), "buildkit")
 	err = client.ImageService().Delete(ctx, target, images.SynchronousDelete())
 	require.NoError(t, err)
 	err = client.ImageService().Delete(ctx, compressedTarget, images.SynchronousDelete())
@@ -2911,6 +2921,7 @@ loop0:
 	// examine contents of exported tars (requires containerd)
 	cdAddress := sb.ContainerdAddress()
 	if cdAddress == "" {
+		t.Logf("checkAllReleasable: skipping check for exported tars in non-containerd test")
 		return
 	}
 

exporter/containerimage/writer.go

@@ -167,7 +167,7 @@ func (ic *ImageWriter) exportLayers(ctx context.Context, compression blobs.Compr
 	return out, nil
 }
 
-func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, ref cache.ImmutableRef, config []byte, layers []blobs.DiffPair, oci bool, cache []byte) (*ocispec.Descriptor, error) {
+func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, ref cache.ImmutableRef, config []byte, layers []blobs.DiffPair, oci bool, inlineCache []byte) (*ocispec.Descriptor, error) {
 	if len(config) == 0 {
 		var err error
 		config, err = emptyImageConfig()
@@ -183,7 +183,7 @@ func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, ref cache
 
 	diffPairs, history := normalizeLayersAndHistory(layers, history, ref)
 
-	config, err = patchImageConfig(config, diffPairs, history, cache)
+	config, err = patchImageConfig(config, diffPairs, history, inlineCache)
 	if err != nil {
 		return nil, err
 	}
@@ -232,11 +232,7 @@ func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, ref cache
 			return nil, errors.Wrapf(err, "could not find blob %s from contentstore", dp.Blobsum)
 		}
 
-		var layerType string
-		if len(layerMediaTypes) > i {
-			layerType = layerMediaTypes[i]
-		}
-
+		layerType := blobs.ConvertLayerMediaType(layerMediaTypes[i], oci)
 		// NOTE: The media type might be missing for some migrated ones
 		// from before lease based storage. If so, we should detect
 		// the media type from blob data.