From c332148dd559d3268060171e14f932a02696157d Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 10 Feb 2022 00:14:04 +0000 Subject: [PATCH] push: always skip foreign layers Foreign layers are only kept as foreign at this point if the user requested it to be. Since foreign layers are not meant to be pushed, automatically skip those layers. Signed-off-by: Brian Goff --- client/client_test.go | 98 +++++++++++++++++++++++++++---------------- util/push/push.go | 15 ++++++- 2 files changed, 76 insertions(+), 37 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 4e8e9786..fd79fe59 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -28,6 +28,8 @@ import ( ctderrdefs "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" "github.com/containerd/containerd/namespaces" + "github.com/containerd/containerd/platforms" + "github.com/containerd/containerd/remotes/docker" "github.com/containerd/containerd/snapshots" "github.com/containerd/continuity/fs/fstest" "github.com/moby/buildkit/client/llb" @@ -2228,14 +2230,14 @@ func testBuildExportWithForeignLayer(t *testing.T, sb integration.Sandbox) { def, err := st.Marshal(sb.Context()) require.NoError(t, err) - registry, err := sb.NewRegistry() - if errors.Is(err, integration.ErrRequirements) { - t.Skip(err.Error()) - } - require.NoError(t, err) - t.Run("propagate=1", func(t *testing.T) { - target := registry + "/buildkit/build/exporter:withforeignlayer" + registry, err := sb.NewRegistry() + if errors.Is(err, integration.ErrRequirements) { + t.Skip(err.Error()) + } + require.NoError(t, err) + + target := registry + "/buildkit/build/exporter/foreign:latest" _, err = c.Solve(sb.Context(), def, SolveOpt{ Exports: []ExportEntry{ { @@ -2251,24 +2253,38 @@ func testBuildExportWithForeignLayer(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) ctx := namespaces.WithNamespace(sb.Context(), "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, 2, len(mfst.Layers)) - require.Equal(t, images.MediaTypeDockerSchema2LayerForeign, mfst.Layers[0].MediaType) - require.Equal(t, images.MediaTypeDockerSchema2Layer, mfst.Layers[1].MediaType) - } + resolver := docker.NewResolver(docker.ResolverOptions{PlainHTTP: true}) + name, desc, err := resolver.Resolve(ctx, target) + require.NoError(t, err) + + fetcher, err := resolver.Fetcher(ctx, name) + require.NoError(t, err) + mfst, err := images.Manifest(ctx, contentutil.FromFetcher(fetcher), desc, platforms.Any()) + require.NoError(t, err) + + require.Equal(t, 2, len(mfst.Layers)) + require.Equal(t, images.MediaTypeDockerSchema2LayerForeign, mfst.Layers[0].MediaType) + require.Len(t, mfst.Layers[0].URLs, 1) + require.Equal(t, images.MediaTypeDockerSchema2Layer, mfst.Layers[1].MediaType) + + rc, err := fetcher.Fetch(ctx, ocispecs.Descriptor{Digest: mfst.Layers[0].Digest, Size: mfst.Layers[0].Size}) + require.NoError(t, err) + defer rc.Close() + + // `Fetch` doesn't error (in the docker resolver), it just returns a reader immediately and does not make a request. + // The request is only made when we attempt to read from the reader. + buf := make([]byte, 1) + _, err = rc.Read(buf) + require.Truef(t, ctderrdefs.IsNotFound(err), "expected error for blob that should not be in registry: %s, %v", mfst.Layers[0].Digest, err) }) t.Run("propagate=0", func(t *testing.T) { - target := registry + "/buildkit/build/exporter:noforeignlayer" + registry, err := sb.NewRegistry() + if errors.Is(err, integration.ErrRequirements) { + t.Skip(err.Error()) + } + require.NoError(t, err) + target := registry + "/buildkit/build/exporter/noforeign:latest" _, err = c.Solve(sb.Context(), def, SolveOpt{ Exports: []ExportEntry{ { @@ -2283,21 +2299,31 @@ func testBuildExportWithForeignLayer(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) ctx := namespaces.WithNamespace(sb.Context(), "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, 2, len(mfst.Layers)) - require.Equal(t, images.MediaTypeDockerSchema2Layer, mfst.Layers[0].MediaType) - require.Equal(t, images.MediaTypeDockerSchema2Layer, mfst.Layers[1].MediaType) - } + resolver := docker.NewResolver(docker.ResolverOptions{PlainHTTP: true}) + name, desc, err := resolver.Resolve(ctx, target) + require.NoError(t, err) + + fetcher, err := resolver.Fetcher(ctx, name) + require.NoError(t, err) + + mfst, err := images.Manifest(ctx, contentutil.FromFetcher(fetcher), desc, platforms.Any()) + require.NoError(t, err) + + require.Equal(t, 2, len(mfst.Layers)) + require.Equal(t, images.MediaTypeDockerSchema2Layer, mfst.Layers[0].MediaType) + require.Len(t, mfst.Layers[0].URLs, 0) + require.Equal(t, images.MediaTypeDockerSchema2Layer, mfst.Layers[1].MediaType) + + rc, err := fetcher.Fetch(ctx, ocispecs.Descriptor{Digest: mfst.Layers[0].Digest, Size: mfst.Layers[0].Size}) + require.NoError(t, err) + defer rc.Close() + + // `Fetch` doesn't error (in the docker resolver), it just returns a reader immediately and does not make a request. + // The request is only made when we attempt to read from the reader. + buf := make([]byte, 1) + _, err = rc.Read(buf) + require.NoError(t, err) }) } diff --git a/util/push/push.go b/util/push/push.go index e59e5b4a..ffa3d35f 100644 --- a/util/push/push.go +++ b/util/push/push.go @@ -11,6 +11,7 @@ import ( "github.com/containerd/containerd/content" "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" + "github.com/containerd/containerd/log" "github.com/containerd/containerd/remotes" "github.com/containerd/containerd/remotes/docker" "github.com/docker/distribution/reference" @@ -126,7 +127,7 @@ func Push(ctx context.Context, sm *session.Manager, sid string, provider content } layersDone := oneOffProgress(ctx, "pushing layers") - err = images.Dispatch(ctx, images.Handlers(handlers...), nil, ocispecs.Descriptor{ + err = images.Dispatch(ctx, skipNonDistributableBlobs(images.Handlers(handlers...)), nil, ocispecs.Descriptor{ Digest: dgst, Size: ra.Size(), MediaType: mtype, @@ -144,6 +145,18 @@ func Push(ctx context.Context, sm *session.Manager, sid string, provider content return mfstDone(nil) } +// TODO: the containerd function for this is filtering too much, that needs to be fixed. +// For now we just carry this. +func skipNonDistributableBlobs(f images.HandlerFunc) images.HandlerFunc { + return func(ctx context.Context, desc ocispecs.Descriptor) ([]ocispecs.Descriptor, error) { + if images.IsNonDistributable(desc.MediaType) { + log.G(ctx).WithField("digest", desc.Digest).WithField("mediatype", desc.MediaType).Debug("Skipping non-distributable blob") + return nil, images.ErrSkipDesc + } + return f(ctx, desc) + } +} + func annotateDistributionSourceHandler(manager content.Manager, annotations map[digest.Digest]map[string]string, f images.HandlerFunc) func(ctx context.Context, desc ocispecs.Descriptor) ([]ocispecs.Descriptor, error) { return func(ctx context.Context, desc ocispecs.Descriptor) ([]ocispecs.Descriptor, error) { children, err := f(ctx, desc)