From d67243ed0b51ab05c6f212ada902f93f5dfb92eb Mon Sep 17 00:00:00 2001 From: "Vlad A. Ionescu" Date: Wed, 16 Feb 2022 17:15:29 -0800 Subject: [PATCH] Fix invalid layer index error Signed-off-by: Vlad A. Ionescu --- cache/remotecache/v1/utils.go | 9 --------- client/client_test.go | 8 ++++---- exporter/containerimage/exptypes/types.go | 3 --- exporter/containerimage/writer.go | 11 +++-------- frontend/dockerfile/dockerfile_test.go | 4 ++-- snapshot/imagerefchecker/checker.go | 5 +---- 6 files changed, 10 insertions(+), 30 deletions(-) diff --git a/cache/remotecache/v1/utils.go b/cache/remotecache/v1/utils.go index 5ff0c4a796be..f7139035fabb 100644 --- a/cache/remotecache/v1/utils.go +++ b/cache/remotecache/v1/utils.go @@ -5,7 +5,6 @@ import ( "fmt" "sort" - "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/solver" digest "github.com/opencontainers/go-digest" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" @@ -13,10 +12,6 @@ import ( "github.com/sirupsen/logrus" ) -// EmptyLayerRemovalSupported defines if implementation supports removal of empty layers. Buildkit image exporter -// removes empty layers, but moby layerstore based implementation does not. -var EmptyLayerRemovalSupported = true - // sortConfig sorts the config structure to make sure it is deterministic func sortConfig(cc *CacheConfig) { type indexedLayer struct { @@ -303,10 +298,6 @@ func marshalRemote(ctx context.Context, r *solver.Remote, state *marshalState) s } desc := r.Descriptors[len(r.Descriptors)-1] - if desc.Digest == exptypes.EmptyGZLayer && EmptyLayerRemovalSupported { - return parentID - } - state.descriptors[desc.Digest] = DescriptorProviderPair{ Descriptor: desc, Provider: r.Provider, diff --git a/client/client_test.go b/client/client_test.go index 28c9af68a252..891e9262c733 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -2798,7 +2798,7 @@ func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) { require.NotEqual(t, "", ociimg.Architecture) require.NotEqual(t, "", ociimg.Config.WorkingDir) require.Equal(t, "layers", ociimg.RootFS.Type) - require.Equal(t, 2, len(ociimg.RootFS.DiffIDs)) + require.Equal(t, 3, len(ociimg.RootFS.DiffIDs)) require.NotNil(t, ociimg.Created) require.True(t, time.Since(*ociimg.Created) < 2*time.Minute) require.Condition(t, func() bool { @@ -2815,7 +2815,7 @@ func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) { require.Contains(t, ociimg.History[1].CreatedBy, "true") require.Contains(t, ociimg.History[2].CreatedBy, "foo/sub/baz") require.False(t, ociimg.History[0].EmptyLayer) - require.True(t, ociimg.History[1].EmptyLayer) + require.False(t, ociimg.History[1].EmptyLayer) require.False(t, ociimg.History[2].EmptyLayer) dt, err = content.ReadBlob(ctx, img.ContentStore(), img.Target()) @@ -2830,7 +2830,7 @@ func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) require.Equal(t, images.MediaTypeDockerSchema2Manifest, mfst.MediaType) - require.Equal(t, 2, len(mfst.Layers)) + require.Equal(t, 3, len(mfst.Layers)) require.Equal(t, images.MediaTypeDockerSchema2LayerGzip, mfst.Layers[0].MediaType) require.Equal(t, images.MediaTypeDockerSchema2LayerGzip, mfst.Layers[1].MediaType) @@ -2857,7 +2857,7 @@ func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) { _, ok = m["foo/sub/baz"] require.False(t, ok) - dt, err = content.ReadBlob(ctx, img.ContentStore(), ocispecs.Descriptor{Digest: mfst.Layers[1].Digest}) + dt, err = content.ReadBlob(ctx, img.ContentStore(), ocispecs.Descriptor{Digest: mfst.Layers[2].Digest}) require.NoError(t, err) m, err = testutil.ReadTarToMap(dt, true) diff --git a/exporter/containerimage/exptypes/types.go b/exporter/containerimage/exptypes/types.go index 94f91e85d5cf..a18d660a5c4a 100644 --- a/exporter/containerimage/exptypes/types.go +++ b/exporter/containerimage/exptypes/types.go @@ -1,7 +1,6 @@ package exptypes import ( - digest "github.com/opencontainers/go-digest" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -16,8 +15,6 @@ const ( ExporterPlatformsKey = "refs.platforms" ) -const EmptyGZLayer = digest.Digest("sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1") - type Platforms struct { Platforms []Platform } diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index 88b9cd5768a0..e19ce0f204bc 100644 --- a/exporter/containerimage/writer.go +++ b/exporter/containerimage/writer.go @@ -476,15 +476,10 @@ func normalizeLayersAndHistory(ctx context.Context, remote *solver.Remote, histo var layerIndex int for i, h := range history { if !h.EmptyLayer { - if remote.Descriptors[layerIndex].Digest == exptypes.EmptyGZLayer { - h.EmptyLayer = true - remote.Descriptors = append(remote.Descriptors[:layerIndex], remote.Descriptors[layerIndex+1:]...) - } else { - if h.Created == nil { - h.Created = refMeta[layerIndex].createdAt - } - layerIndex++ + if h.Created == nil { + h.Created = refMeta[layerIndex].createdAt } + layerIndex++ } history[i] = h } diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index 051cea213866..e312793ead1c 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -2946,7 +2946,7 @@ RUN ["ls"] require.Equal(t, "layers", ociimg.RootFS.Type) // this depends on busybox. should be ok after freezing images - require.Equal(t, 3, len(ociimg.RootFS.DiffIDs)) + require.Equal(t, 4, len(ociimg.RootFS.DiffIDs)) require.Equal(t, 7, len(ociimg.History)) require.Contains(t, ociimg.History[2].CreatedBy, "lbl=val") @@ -2962,7 +2962,7 @@ RUN ["ls"] require.Equal(t, false, ociimg.History[5].EmptyLayer) require.NotNil(t, ociimg.History[5].Created) require.Contains(t, ociimg.History[6].CreatedBy, "RUN ls") - require.Equal(t, true, ociimg.History[6].EmptyLayer) + require.Equal(t, false, ociimg.History[6].EmptyLayer) require.NotNil(t, ociimg.History[6].Created) } diff --git a/snapshot/imagerefchecker/checker.go b/snapshot/imagerefchecker/checker.go index af8a7ab2f5ce..eb6cb25f3263 100644 --- a/snapshot/imagerefchecker/checker.go +++ b/snapshot/imagerefchecker/checker.go @@ -9,7 +9,6 @@ import ( "github.com/containerd/containerd/content" "github.com/containerd/containerd/images" "github.com/moby/buildkit/cache" - "github.com/moby/buildkit/exporter/containerimage/exptypes" digest "github.com/opencontainers/go-digest" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -90,9 +89,7 @@ func toDigests(layers []ocispecs.Descriptor) []digest.Digest { func layerKey(layers []digest.Digest) string { b := &strings.Builder{} for _, l := range layers { - if l != exptypes.EmptyGZLayer { - b.Write([]byte(l)) - } + b.Write([]byte(l)) } return b.String() }