Do not re-tag non-distributable blob descriptors

Before this change buildkit was changing the media type for
non-distributable layers to normal layers.
It was also clearing out the urls to get those blobs.

This keeps those layers as non-distributable and stores the urls so we
can keep those values when building the new manifest.

Signed-off-by: Brian Goff <[email protected]>

cache/blobs.go

@@ -306,6 +306,7 @@ func (sr *immutableRef) setBlob(ctx context.Context, compressionType compression
 	sr.queueBlob(desc.Digest)
 	sr.queueMediaType(desc.MediaType)
 	sr.queueBlobSize(desc.Size)
+	sr.appendURLs(desc.URLs)
 	if err := sr.commitMetadata(); err != nil {
 		return err
 	}

cache/manager.go

@@ -282,6 +282,7 @@ func (cm *cacheManager) GetByBlob(ctx context.Context, desc ocispecs.Descriptor,
 	rec.queueBlobOnly(blobOnly)
 	rec.queueMediaType(desc.MediaType)
 	rec.queueBlobSize(desc.Size)
+	rec.appendURLs(desc.URLs)
 	rec.queueCommitted(true)
 
 	if err := rec.commitMetadata(); err != nil {

cache/manager_test.go

@@ -1508,6 +1508,65 @@ func checkVariantsCoverage(ctx context.Context, t *testing.T, variants idxToVari
 	require.Equal(t, 0, len(got))
 }
 
+// Make sure that media type and urls are persisted for non-distributable blobs.
+func TestNondistributableBlobs(t *testing.T) {
+	t.Parallel()
+
+	ctx := namespaces.WithNamespace(context.Background(), "buildkit-test")
+
+	tmpdir, err := ioutil.TempDir("", "cachemanager")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpdir)
+
+	snapshotter, err := native.NewSnapshotter(filepath.Join(tmpdir, "snapshots"))
+	require.NoError(t, err)
+
+	co, cleanup, err := newCacheManager(ctx, cmOpt{
+		snapshotter:     snapshotter,
+		snapshotterName: "native",
+	})
+	require.NoError(t, err)
+	defer cleanup()
+
+	cm := co.manager
+
+	ctx, done, err := leaseutil.WithLease(ctx, co.lm, leaseutil.MakeTemporary)
+	require.NoError(t, err)
+	defer done(context.TODO())
+
+	contentBuffer := contentutil.NewBuffer()
+	descHandlers := DescHandlers(map[digest.Digest]*DescHandler{})
+
+	data, desc, err := mapToBlob(map[string]string{"foo": "bar"}, false)
+	require.NoError(t, err)
+
+	// Pretend like this is non-distributable
+	desc.MediaType = ocispecs.MediaTypeImageLayerNonDistributable
+	desc.URLs = []string{"https://buildkit.moby.dev/foo"}
+
+	cw, err := contentBuffer.Writer(ctx)
+	require.NoError(t, err)
+	_, err = cw.Write(data)
+	require.NoError(t, err)
+	err = cw.Commit(ctx, 0, cw.Digest())
+	require.NoError(t, err)
+
+	descHandlers[desc.Digest] = &DescHandler{
+		Provider: func(_ session.Group) content.Provider { return contentBuffer },
+	}
+
+	ref, err := cm.GetByBlob(ctx, desc, nil, descHandlers)
+	require.NoError(t, err)
+
+	remotes, err := ref.GetRemotes(ctx, true, compression.Config{}, false, nil)
+	require.NoError(t, err)
+
+	desc2 := remotes[0].Descriptors[0]
+
+	require.Equal(t, desc.MediaType, desc2.MediaType)
+	require.Equal(t, desc.URLs, desc2.URLs)
+}
+
 func checkInfo(ctx context.Context, t *testing.T, cs content.Store, info content.Info) {
 	if info.Labels == nil {
 		return

cache/metadata.go

@@ -37,6 +37,7 @@ const keyMediaType = "cache.mediatype"
 const keyImageRefs = "cache.imageRefs"
 const keyDeleted = "cache.deleted"
 const keyBlobSize = "cache.blobsize" // the packed blob size as specified in the oci descriptor
+const keyURLs = "cache.layer.urls"
 
 // Indexes
 const blobchainIndex = "blobchainid:"
@@ -281,6 +282,17 @@ func (md *cacheMetadata) queueBlob(str digest.Digest) error {
 	return md.queueValue(keyBlob, str, "")
 }
 
+func (md *cacheMetadata) appendURLs(urls []string) error {
+	if len(urls) == 0 {
+		return nil
+	}
+	return md.appendStringSlice(keyURLs, urls...)
+}
+
+func (md *cacheMetadata) getURLs() []string {
+	return md.GetStringSlice(keyURLs)
+}
+
 func (md *cacheMetadata) getBlob() digest.Digest {
 	return digest.Digest(md.GetString(keyBlob))
 }
@@ -468,6 +480,18 @@ func (md *cacheMetadata) GetString(key string) string {
 	return str
 }
 
+func (md *cacheMetadata) GetStringSlice(key string) []string {
+	v := md.si.Get(key)
+	if v == nil {
+		return nil
+	}
+	var val []string
+	if err := v.Unmarshal(&val); err != nil {
+		return nil
+	}
+	return val
+}
+
 func (md *cacheMetadata) setTime(key string, value time.Time, index string) error {
 	return md.setValue(key, value.UnixNano(), index)
 }
@@ -512,20 +536,33 @@ func (md *cacheMetadata) getInt64(key string) (int64, bool) {
 	return i, true
 }
 
-func (md *cacheMetadata) appendStringSlice(key string, value string) error {
+func (md *cacheMetadata) appendStringSlice(key string, values ...string) error {
 	return md.si.GetAndSetValue(key, func(v *metadata.Value) (*metadata.Value, error) {
 		var slice []string
 		if v != nil {
 			if err := v.Unmarshal(&slice); err != nil {
 				return nil, err
 			}
 		}
+
+		idx := make(map[string]struct{}, len(values))
+		for _, v := range values {
+			idx[v] = struct{}{}
+		}
+
 		for _, existing := range slice {
-			if existing == value {
-				return nil, metadata.ErrSkipSetValue
+			if _, ok := idx[existing]; ok {
+				delete(idx, existing)
 			}
 		}
-		slice = append(slice, value)
+
+		if len(idx) == 0 {
+			return nil, metadata.ErrSkipSetValue
+		}
+
+		for value := range idx {
+			slice = append(slice, value)
+		}
 		v, err := metadata.NewValue(slice)
 		if err != nil {
 			return nil, err

cache/refs.go

@@ -596,6 +596,7 @@ func (sr *immutableRef) ociDesc(ctx context.Context, dhs DescHandlers) (ocispecs
 		Size:        sr.getBlobSize(),
 		MediaType:   sr.getMediaType(),
 		Annotations: make(map[string]string),
+		URLs:        sr.getURLs(),
 	}
 
 	if blobDesc, err := getBlobDesc(ctx, sr.cm.ContentStore, desc.Digest); err == nil {
@@ -744,6 +745,7 @@ func getBlobDesc(ctx context.Context, cs content.Store, dgst digest.Digest) (oci
 	if !ok {
 		return ocispecs.Descriptor{}, fmt.Errorf("no media type is stored for %q", info.Digest)
 	}
+
 	desc := ocispecs.Descriptor{
 		Digest:    info.Digest,
 		Size:      info.Size,

cache/remote.go

@@ -224,6 +224,7 @@ func (sr *immutableRef) getRemote(ctx context.Context, createIfNeeded bool, comp
 				newDesc.MediaType = blobDesc.MediaType
 				newDesc.Digest = blobDesc.Digest
 				newDesc.Size = blobDesc.Size
+				newDesc.URLs = blobDesc.URLs
 				newDesc.Annotations = nil
 				for _, k := range addAnnotations {
 					newDesc.Annotations[k] = desc.Annotations[k]

exporter/containerimage/export.go

@@ -44,6 +44,12 @@ const (
 	keyCompressionLevel = "compression-level"
 	keyBuildInfo        = "buildinfo"
 	ociTypes            = "oci-mediatypes"
+	// propagateNondistLayersKey is an exporter option which can be used to mark a layer as non-distributable if the layer reference was
+	// already found to use a non-distributable media type.
+	// When this option is not set, the exporter will change the media type of the layer to a distributable one.
+	//
+	// There may be other behaviors associated with this option which are specific to the exporter implementation.
+	propagateNondistLayersKey = "propagate-nondist-layers"
 )
 
 type Opt struct {
@@ -181,6 +187,12 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp
 				return nil, err
 			}
 			i.buildInfoMode = bimode
+		case propagateNondistLayersKey:
+			b, err := strconv.ParseBool(v)
+			if err != nil {
+				return nil, errors.Wrapf(err, "non-bool value %s specified for %s", v, k)
+			}
+			i.propagateNondistLayers = b
 		default:
 			if i.meta == nil {
 				i.meta = make(map[string][]byte)
@@ -197,19 +209,20 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp
 
 type imageExporterInstance struct {
 	*imageExporter
-	targetName       string
-	push             bool
-	pushByDigest     bool
-	unpack           bool
-	insecure         bool
-	ociTypes         bool
-	nameCanonical    bool
-	danglingPrefix   string
-	layerCompression compression.Type
-	forceCompression bool
-	compressionLevel *int
-	buildInfoMode    buildinfo.ExportMode
-	meta             map[string][]byte
+	targetName             string
+	push                   bool
+	pushByDigest           bool
+	unpack                 bool
+	insecure               bool
+	ociTypes               bool
+	nameCanonical          bool
+	danglingPrefix         string
+	layerCompression       compression.Type
+	forceCompression       bool
+	compressionLevel       *int
+	buildInfoMode          buildinfo.ExportMode
+	meta                   map[string][]byte
+	propagateNondistLayers bool
 }
 
 func (e *imageExporterInstance) Name() string {
@@ -244,7 +257,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source,
 	}
 	defer done(context.TODO())
 
-	desc, err := e.opt.ImageWriter.Commit(ctx, src, e.ociTypes, e.compression(), e.buildInfoMode, sessionID)
+	desc, err := e.opt.ImageWriter.Commit(ctx, src, e.ociTypes, e.compression(), e.buildInfoMode, e.propagateNondistLayers, sessionID)
 	if err != nil {
 		return nil, err
 	}

exporter/containerimage/writer.go

@@ -21,6 +21,7 @@ import (
 	"github.com/moby/buildkit/util/bklog"
 	"github.com/moby/buildkit/util/buildinfo"
 	"github.com/moby/buildkit/util/compression"
+	"github.com/moby/buildkit/util/convert"
 	"github.com/moby/buildkit/util/progress"
 	"github.com/moby/buildkit/util/system"
 	"github.com/moby/buildkit/util/tracing"
@@ -48,14 +49,15 @@ type ImageWriter struct {
 	opt WriterOpt
 }
 
-func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, oci bool, comp compression.Config, buildInfoMode buildinfo.ExportMode, sessionID string) (*ocispecs.Descriptor, error) {
+func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, oci bool, comp compression.Config, buildInfoMode buildinfo.ExportMode, propagateNonDist bool, sessionID string) (*ocispecs.Descriptor, error) {
 	platformsBytes, ok := inp.Metadata[exptypes.ExporterPlatformsKey]
 
 	if len(inp.Refs) > 0 && !ok {
 		return nil, errors.Errorf("unable to export multiple refs, missing platforms mapping")
 	}
 
 	if len(inp.Refs) == 0 {
+		// TODO: pass through non-dist layers?
 		remotes, err := ic.exportLayers(ctx, comp, session.NewGroup(sessionID), inp.Ref)
 		if err != nil {
 			return nil, err
@@ -66,7 +68,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, oci bool
 			buildInfo = inp.Metadata[exptypes.ExporterBuildInfo]
 		}
 
-		mfstDesc, configDesc, err := ic.commitDistributionManifest(ctx, inp.Ref, inp.Metadata[exptypes.ExporterImageConfigKey], &remotes[0], oci, inp.Metadata[exptypes.ExporterInlineCache], buildInfo)
+		mfstDesc, configDesc, err := ic.commitDistributionManifest(ctx, inp.Ref, inp.Metadata[exptypes.ExporterImageConfigKey], &remotes[0], oci, inp.Metadata[exptypes.ExporterInlineCache], buildInfo, propagateNonDist)
 		if err != nil {
 			return nil, err
 		}
@@ -94,6 +96,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, oci bool
 		refs = append(refs, r)
 	}
 
+	// TODO: Pass through non-distributable layers
 	remotes, err := ic.exportLayers(ctx, comp, session.NewGroup(sessionID), refs...)
 	if err != nil {
 		return nil, err
@@ -133,7 +136,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, oci bool
 			buildInfo = inp.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, p.ID)]
 		}
 
-		desc, _, err := ic.commitDistributionManifest(ctx, r, config, &remotes[remotesMap[p.ID]], oci, inlineCache, buildInfo)
+		desc, _, err := ic.commitDistributionManifest(ctx, r, config, &remotes[remotesMap[p.ID]], oci, inlineCache, buildInfo, propagateNonDist)
 		if err != nil {
 			return nil, err
 		}
@@ -202,7 +205,7 @@ func (ic *ImageWriter) exportLayers(ctx context.Context, comp compression.Config
 	return out, err
 }
 
-func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, ref cache.ImmutableRef, config []byte, remote *solver.Remote, oci bool, inlineCache []byte, buildInfo []byte) (*ocispecs.Descriptor, *ocispecs.Descriptor, error) {
+func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, ref cache.ImmutableRef, config []byte, remote *solver.Remote, oci bool, inlineCache []byte, buildInfo []byte, propagateNonDist bool) (*ocispecs.Descriptor, *ocispecs.Descriptor, error) {
 	if len(config) == 0 {
 		var err error
 		config, err = emptyImageConfig()
@@ -278,6 +281,10 @@ func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, ref cache
 		} else {
 			desc.Annotations = nil
 		}
+		if !propagateNonDist {
+			desc.MediaType = convert.LayerToDistributable(oci, desc.MediaType)
+			desc.URLs = nil
+		}
 		mfst.Layers = append(mfst.Layers, desc)
 		labels[fmt.Sprintf("containerd.io/gc.ref.content.%d", i+1)] = desc.Digest.String()
 	}

exporter/oci/export.go

@@ -38,6 +38,12 @@ const (
 	keyForceCompression = "force-compression"
 	keyCompressionLevel = "compression-level"
 	keyBuildInfo        = "buildinfo"
+	// propagateNondistLayersKey is an exporter option which can be used to mark a layer as non-distributable if the layer reference was
+	// already found to use a non-distributable media type.
+	// When this option is not set, the exporter will change the media type of the layer to a distributable one.
+	//
+	// There may be other behaviors associated with this option which are specific to the exporter implementation.
+	propagateNondistLayersKey = "propagate-nondist-layers"
 )
 
 type Opt struct {
@@ -119,6 +125,12 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp
 				return nil, err
 			}
 			i.buildInfoMode = bimode
+		case propagateNondistLayersKey:
+			b, err := strconv.ParseBool(v)
+			if err != nil {
+				return nil, errors.Wrapf(err, "non-bool value specified for %s", k)
+			}
+			i.propagateNonDist = b
 		default:
 			if i.meta == nil {
 				i.meta = make(map[string][]byte)
@@ -147,6 +159,7 @@ type imageExporterInstance struct {
 	forceCompression bool
 	compressionLevel *int
 	buildInfoMode    buildinfo.ExportMode
+	propagateNonDist bool
 }
 
 func (e *imageExporterInstance) Name() string {
@@ -185,7 +198,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source,
 	}
 	defer done(context.TODO())
 
-	desc, err := e.opt.ImageWriter.Commit(ctx, src, e.ociTypes, e.compression(), e.buildInfoMode, sessionID)
+	desc, err := e.opt.ImageWriter.Commit(ctx, src, e.ociTypes, e.compression(), e.buildInfoMode, e.propagateNonDist, sessionID)
 	if err != nil {
 		return nil, err
 	}