Skip to content

Commit

Permalink
Do not re-tag non-distributable blob descriptors
Browse files Browse the repository at this point in the history
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 distributable and stores the urls so we can
keep those values when building the new manifest.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
  • Loading branch information
cpuguy83 committed Jan 14, 2022
1 parent 2dc3e74 commit 1aa279e
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 6 deletions.
1 change: 1 addition & 0 deletions cache/blobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ func (sr *immutableRef) setBlob(ctx context.Context, compressionType compression
sr.queueBlob(desc.Digest)
sr.queueMediaType(desc.MediaType)
sr.queueBlobSize(desc.Size)
sr.queueURLs(desc.URLs)
if err := sr.commitMetadata(); err != nil {
return err
}
Expand Down
1 change: 1 addition & 0 deletions cache/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.queueURLs(desc.URLs)
rec.queueCommitted(true)

if err := rec.commitMetadata(); err != nil {
Expand Down
59 changes: 59 additions & 0 deletions cache/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, solver.CompressionOpt{}, 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
Expand Down
24 changes: 24 additions & 0 deletions cache/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:"
Expand Down Expand Up @@ -281,6 +282,17 @@ func (md *cacheMetadata) queueBlob(str digest.Digest) error {
return md.queueValue(keyBlob, str, "")
}

func (md *cacheMetadata) queueURLs(urls []string) error {
if len(urls) == 0 {
return nil
}
return md.queueValue(keyURLs, urls, "")
}

func (md *cacheMetadata) getURLs() []string {
return md.GetStringSlice(keyURLs)
}

func (md *cacheMetadata) getBlob() digest.Digest {
return digest.Digest(md.GetString(keyBlob))
}
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions cache/refs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions cache/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,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]
Expand Down
13 changes: 7 additions & 6 deletions util/compression/compression.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,11 @@ func (ct Type) IsMediaType(mt string) bool {

func FromMediaType(mediaType string) Type {
switch toOCILayerType[mediaType] {
case ocispecs.MediaTypeImageLayer:
case ocispecs.MediaTypeImageLayer, ocispecs.MediaTypeImageLayerNonDistributable:
return Uncompressed
case ocispecs.MediaTypeImageLayerGzip:
case ocispecs.MediaTypeImageLayerGzip, ocispecs.MediaTypeImageLayerNonDistributableGzip:
return Gzip
case mediaTypeImageLayerZstd:
case mediaTypeImageLayerZstd, ocispecs.MediaTypeImageLayerNonDistributableZstd:
return Zstd
default:
return UnknownCompression
Expand Down Expand Up @@ -176,18 +176,19 @@ var toDockerLayerType = map[string]string{
ocispecs.MediaTypeImageLayerGzip: images.MediaTypeDockerSchema2LayerGzip,
images.MediaTypeDockerSchema2LayerGzip: images.MediaTypeDockerSchema2LayerGzip,
images.MediaTypeDockerSchema2LayerForeign: images.MediaTypeDockerSchema2Layer,
images.MediaTypeDockerSchema2LayerForeignGzip: images.MediaTypeDockerSchema2LayerGzip,
images.MediaTypeDockerSchema2LayerForeignGzip: images.MediaTypeDockerSchema2LayerForeignGzip,
mediaTypeImageLayerZstd: mediaTypeDockerSchema2LayerZstd,
mediaTypeDockerSchema2LayerZstd: mediaTypeDockerSchema2LayerZstd,
}

var toOCILayerType = map[string]string{
ocispecs.MediaTypeImageLayer: ocispecs.MediaTypeImageLayer,
ocispecs.MediaTypeImageLayerNonDistributable: ocispecs.MediaTypeImageLayerNonDistributable,
images.MediaTypeDockerSchema2Layer: ocispecs.MediaTypeImageLayer,
ocispecs.MediaTypeImageLayerGzip: ocispecs.MediaTypeImageLayerGzip,
images.MediaTypeDockerSchema2LayerGzip: ocispecs.MediaTypeImageLayerGzip,
images.MediaTypeDockerSchema2LayerForeign: ocispecs.MediaTypeImageLayer,
images.MediaTypeDockerSchema2LayerForeignGzip: ocispecs.MediaTypeImageLayerGzip,
images.MediaTypeDockerSchema2LayerForeign: ocispecs.MediaTypeImageLayerNonDistributable,
images.MediaTypeDockerSchema2LayerForeignGzip: ocispecs.MediaTypeImageLayerNonDistributableGzip,
mediaTypeImageLayerZstd: mediaTypeImageLayerZstd,
mediaTypeDockerSchema2LayerZstd: mediaTypeImageLayerZstd,
}
Expand Down

0 comments on commit 1aa279e

Please sign in to comment.