Repository: pivotal/kpack (go)

Issue:
Builder images contain duplicate layers
I noticed [this issue with pack](https://github.com/buildpacks/pack/issues/1081) where `pack builder create` produces builder images with multiple copies of an identical layer. This happens when a builder contains metabuildpacks that include some overlapping implementation buildpacks. (A common example is the Paketo Procfile buildpack, which is included in the Paketo Go, Dotnet Core, and Ruby buildpacks, to name a few.)

Out of curiosity, I took a look to see if kpack has the same problem. It seems to. Take a look at [this gist](https://gist.github.com/fg-j/72bc224a8770a5094678385d661cd610) for an analysis of the layers in a builder whose metabuildpacks contain overlapping buildpacks.

The layer duplication creates unnecessary builder size bloat. This can slow down image pulling. But it also poses a larger problem: builders grow rapidly with the addition of each metabuildpack. Since there is an underlying limit to the number of layers that a filesystem driver can package into an image ([for docker's backend this is about 128 layers](https://docs.docker.com/storage/storagedriver/overlayfs-driver/#how-the-overlay2-driver-works)), there will probably come a point where a builder gets too large and can't be built/pushed to the registry!


Pull request:
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

resolves #645 

Co-authored-by Matthew McNew <mmcnew@vmware.com>

Patch:
diff --git a/pkg/cnb/builder_builder.go b/pkg/cnb/builder_builder.go
index f1f10cef..43b5c40d 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
+}


Test patch:
diff --git a/pkg/cnb/create_builder_test.go b/pkg/cnb/create_builder_test.go
index f3f256eb..5fd32923 100644
--- a/pkg/cnb/create_builder_test.go
+++ b/pkg/cnb/create_builder_test.go
@@ -157,6 +157,14 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
 								Optional: true,
 							},
 						},
+						{
+							BuildpackRef: corev1alpha1.BuildpackRef{
+								BuildpackInfo: corev1alpha1.BuildpackInfo{
+									Id:      "io.buildpack.4",
+									Version: "v4",
+								},
+							},
+						},
 					},
 				},
 			},
@@ -263,9 +271,32 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
 				},
 			},
 		}
+		buildpackWithDuplicatLayer := 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",
+					},
+				},
+			},
+		}
 
 		fetcher.AddBuildpack(t, "io.buildpack.1", "v1", []buildpackLayer{buildpack1})
 		fetcher.AddBuildpack(t, "io.buildpack.2", "v2", []buildpackLayer{buildpack3, buildpack2})
+		fetcher.AddBuildpack(t, "io.buildpack.4", "v4", []buildpackLayer{buildpackWithDuplicatLayer})
 	})
 
 	registryClient.AddSaveKeychain("custom/example", builderKeychain)
@@ -318,10 +349,11 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) {
 			builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, 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)
@@ -338,6 +370,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,
+						},
 					},
 				},
 			})
@@ -466,6 +502,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"
 `}})
 
 			})
@@ -473,7 +513,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)
@@ -508,6 +548,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",
@@ -571,6 +616,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)