From b4089a35576f2e83f9a68634fd81cfe2bab0eea7 Mon Sep 17 00:00:00 2001 From: Tom Kennedy Date: Tue, 6 Jun 2023 16:44:27 -0400 Subject: [PATCH] Deduplicate builder layers by diff id - This will remove duplicate layers that result from two buildpacks in a builder that have the same layer diff id Co-authored-by Matthew McNew --- pkg/cnb/builder_builder.go | 24 +++++++++++-- pkg/cnb/create_builder_test.go | 62 ++++++++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/pkg/cnb/builder_builder.go b/pkg/cnb/builder_builder.go index f1f10cefc..43b5c40db 100644 --- a/pkg/cnb/builder_builder.go +++ b/pkg/cnb/builder_builder.go @@ -126,7 +126,7 @@ func (bb *builderBlder) WriteableImage() (v1.Image, error) { } image, err := mutate.AppendLayers(bb.baseImage, - layers( + deduplicateLayers(layers( []v1.Layer{ defaultLayer, bb.lifecycleLayer, @@ -136,7 +136,7 @@ func (bb *builderBlder) WriteableImage() (v1.Image, error) { stackLayer, orderLayer, }, - )...) + ))...) if err != nil { return nil, err } @@ -388,3 +388,23 @@ func layers(layers ...[]v1.Layer) []v1.Layer { } return appendedLayers } + +func deduplicateLayers(layers []v1.Layer) []v1.Layer { + layerMap := map[v1.Hash]struct{}{} + res := make([]v1.Layer, 0) + + for _, l := range layers { + diffId, err := l.DiffID() + if err != nil { + res = append(res, l) + continue + } + + if _, ok := layerMap[diffId]; !ok { + res = append(res, l) + layerMap[diffId] = struct{}{} + } + } + + return res +} diff --git a/pkg/cnb/create_builder_test.go b/pkg/cnb/create_builder_test.go index 55db98394..742e17372 100644 --- a/pkg/cnb/create_builder_test.go +++ b/pkg/cnb/create_builder_test.go @@ -161,6 +161,12 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }, Optional: true, }, + { + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "io.buildpack.4", + Version: "v4", + }, + }, }, }, }, @@ -251,6 +257,30 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }, }) + buildpackWithDuplicateLayer := buildpackLayer{ + v1Layer: buildpack3Layer, + BuildpackInfo: DescriptiveBuildpackInfo{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "io.buildpack.4", + Version: "v4", + }, + Homepage: "buildpack.4.com", + }, + BuildpackLayerInfo: BuildpackLayerInfo{ + API: "0.3", + LayerDiffID: buildpack3Layer.diffID, + Stacks: []corev1alpha1.BuildpackStack{ + { + ID: stackID, + }, + { + ID: "io.some.other.stack", + }, + }, + }, + } + buildpackRepository.AddBP("io.buildpack.4", "v4", []buildpackLayer{buildpackWithDuplicateLayer}) + registryClient.AddSaveKeychain("custom/example", keychain) when("CreateBuilder", func() { @@ -301,10 +331,11 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { builderRecord, err := subject.CreateBuilder(keychain, store, stack, clusterBuilderSpec) require.NoError(t, err) - assert.Len(t, builderRecord.Buildpacks, 3) + assert.Len(t, builderRecord.Buildpacks, 4) assert.Contains(t, builderRecord.Buildpacks, corev1alpha1.BuildpackMetadata{Id: "io.buildpack.1", Version: "v1", Homepage: "buildpack.1.com"}) assert.Contains(t, builderRecord.Buildpacks, corev1alpha1.BuildpackMetadata{Id: "io.buildpack.2", Version: "v2", Homepage: "buildpack.2.com"}) assert.Contains(t, builderRecord.Buildpacks, corev1alpha1.BuildpackMetadata{Id: "io.buildpack.3", Version: "v3", Homepage: "buildpack.3.com"}) + assert.Contains(t, builderRecord.Buildpacks, corev1alpha1.BuildpackMetadata{Id: "io.buildpack.4", Version: "v4", Homepage: "buildpack.4.com"}) assert.Equal(t, corev1alpha1.BuildStack{RunImage: runImage, ID: stackID}, builderRecord.Stack) assert.Equal(t, int64(10), builderRecord.ObservedStoreGeneration) assert.Equal(t, int64(11), builderRecord.ObservedStackGeneration) @@ -321,6 +352,10 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { BuildpackInfo: corev1alpha1.BuildpackInfo{Id: "io.buildpack.2", Version: "v2"}, Optional: true, }, + { + BuildpackInfo: corev1alpha1.BuildpackInfo{Id: "io.buildpack.4", Version: "v4"}, + Optional: false, + }, }, }, }) @@ -449,6 +484,10 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { id = "io.buildpack.2" version = "v2" optional = true + + [[order.group]] + id = "io.buildpack.4" + version = "v4" `}}) }) @@ -456,7 +495,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { buildpackOrder, err := imagehelpers.GetStringLabel(savedImage, buildpackOrderLabel) assert.NoError(t, err) assert.JSONEq(t, //language=json - `[{"group":[{"id":"io.buildpack.1","version":"v1"},{"id":"io.buildpack.2","version":"v2","optional":true}]}]`, buildpackOrder) + `[{"group":[{"id":"io.buildpack.1","version":"v1"},{"id":"io.buildpack.2","version":"v2","optional":true},{"id":"io.buildpack.4","version":"v4"}]}]`, buildpackOrder) buildpackMetadata, err := imagehelpers.GetStringLabel(savedImage, buildpackMetadataLabel) assert.NoError(t, err) @@ -491,6 +530,11 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { "version": "v1.2.3 (git sha: abcdefg123456)" }, "buildpacks": [ + { + "id": "io.buildpack.4", + "version": "v4", + "homepage": "buildpack.4.com" + }, { "id": "io.buildpack.3", "version": "v3", @@ -554,6 +598,20 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { } ] } + }, + "io.buildpack.4": { + "v4": { + "api": "0.3", + "layerDiffID": "sha256:3bf8899667b8d1e6b124f663faca32903b470831e5e4e992644ac5c839ab3462", + "stacks": [ + { + "id": "io.buildpacks.stacks.some-stack" + }, + { + "id": "io.some.other.stack" + } + ] + } } }`, buildpackLayers)