Merge pull request #1111 from buildpacks/fix/1081-1108-buildpack-dupl…

…icates

Fix duplicate detection and content collision
Signed-off-by: David Freilich <[email protected]>

acceptance/acceptance_test.go

@@ -704,6 +704,53 @@ func testAcceptance(
 				builderName = value
 			})
 
+			when("complex builder", func() {
+				it.Before(func() {
+					// create our nested builder
+					h.SkipIf(t, imageManager.HostOS() == "windows", "These tests are not yet compatible with Windows-based containers")
+					h.SkipIf(t, !createBuilderPack.SupportsFeature(invoke.BuilderNoDuplicateLayers), "bug fixed in 0.18.0")
+
+					// create a task, handled by a 'task manager' which executes our pack commands during tests.
+					// looks like this is used to de-dup tasks
+					key := taskKey(
+						"create-complex-builder",
+						append(
+							[]string{runImageMirror, createBuilderPackConfig.Path(), lifecycle.Identifier()},
+							createBuilderPackConfig.FixturePaths()...,
+						)...,
+					)
+
+					value, err := suiteManager.RunTaskOnceString(key, func() (string, error) {
+						return createComplexBuilder(
+							t,
+							assert,
+							createBuilderPack,
+							lifecycle,
+							buildpackManager,
+							runImageMirror,
+						)
+					})
+					assert.Nil(err)
+
+					// register task to be run to 'clean up' a task
+					suiteManager.RegisterCleanUp("clean-"+key, func() error {
+						imageManager.CleanupImages(value)
+						return nil
+					})
+					builderName = value
+
+					output := pack.RunSuccessfully(
+						"config", "run-image-mirrors", "add", "pack-test/run", "--mirror", "some-registry.com/pack-test/run1")
+					assertOutput := assertions.NewOutputAssertionManager(t, output)
+					assertOutput.ReportsSuccesfulRunImageMirrorsAdd("pack-test/run", "some-registry.com/pack-test/run1")
+				})
+				when("builder has duplicate buildpacks", func() {
+					it("buildpack layers have no duplication", func() {
+						assertImage.DoesNotHaveDuplicateLayers(builderName)
+					})
+				})
+			})
+
 			when("builder.toml is invalid", func() {
 				it("displays an error", func() {
 					builderConfigPath := createBuilderPack.FixtureManager().FixtureLocation("invalid_builder.toml")
@@ -1941,6 +1988,7 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ]
 					it.Before(func() {
 						// create our nested builder
 						h.SkipIf(t, imageManager.HostOS() == "windows", "These tests are not yet compatible with Windows-based containers")
+						h.SkipIf(t, !pack.SupportsFeature(invoke.BuilderNoDuplicateLayers), "bug fixed in 0.18.0")
 
 						// create a task, handled by a 'task manager' which executes our pack commands during tests.
 						// looks like this is used to de-dup tasks
@@ -2457,12 +2505,14 @@ func createComplexBuilder(t *testing.T,
 	packageImageName := registryConfig.RepoName("nested-level-1-buildpack-" + h.RandString(8))
 	nestedLevelTwoBuildpackName := registryConfig.RepoName("nested-level-2-buildpack-" + h.RandString(8))
 	simpleLayersBuildpackName := registryConfig.RepoName("simple-layers-buildpack-" + h.RandString(8))
+	simpleLayersBuildpackDifferentShaName := registryConfig.RepoName("simple-layers-buildpack-different-name-" + h.RandString(8))
 
 	templateMapping["package_id"] = "simple/nested-level-1"
 	templateMapping["package_image_name"] = packageImageName
 	templateMapping["nested_level_1_buildpack"] = packageImageName
 	templateMapping["nested_level_2_buildpack"] = nestedLevelTwoBuildpackName
 	templateMapping["simple_layers_buildpack"] = simpleLayersBuildpackName
+	templateMapping["simple_layers_buildpack_different_sha"] = simpleLayersBuildpackDifferentShaName
 
 	fixtureManager := pack.FixtureManager()
 
@@ -2483,6 +2533,7 @@ func createComplexBuilder(t *testing.T,
 		nestedLevelTwoConfigFile,
 		templateMapping,
 	)
+
 	err = nestedLevelTwoConfigFile.Close()
 	assert.Nil(err)
 
@@ -2512,11 +2563,20 @@ func createComplexBuilder(t *testing.T,
 		),
 	)
 
-	defer imageManager.CleanupImages(packageImageName, nestedLevelTwoBuildpackName, simpleLayersBuildpackName)
+	simpleLayersDifferentShaBuildpack := buildpacks.NewPackageImage(
+		t,
+		pack,
+		simpleLayersBuildpackDifferentShaName,
+		fixtureManager.FixtureLocation("simple-layers-buildpack-different-sha_package.toml"),
+		buildpacks.WithRequiredBuildpacks(buildpacks.SimpleLayersDifferentSha),
+	)
+
+	defer imageManager.CleanupImages(packageImageName, nestedLevelTwoBuildpackName, simpleLayersBuildpackName, simpleLayersBuildpackDifferentShaName)
 
 	builderBuildpacks = append(
 		builderBuildpacks,
 		packageImageBuildpack,
+		simpleLayersDifferentShaBuildpack,
 	)
 
 	buildpackManager.PrepareBuildpacks(tmpDir, builderBuildpacks...)

acceptance/assertions/image.go

@@ -100,3 +100,19 @@ func (a ImageAssertionManager) NotExistsInRegistry(name string) {
 		"Didn't expect to see image %s in the registry",
 	)
 }
+
+func (a ImageAssertionManager) DoesNotHaveDuplicateLayers(name string) {
+	a.testObject.Helper()
+
+	out, err := a.imageManager.InspectLocal(name)
+	a.assert.Nil(err)
+
+	layerSet := map[string]interface{}{}
+	for _, layer := range out.RootFS.Layers {
+		_, ok := layerSet[layer]
+		if ok {
+			a.testObject.Fatalf("duplicate layer found in builder %s", layer)
+		}
+		layerSet[layer] = true
+	}
+}

acceptance/buildpacks/archive_buildpack.go

@@ -84,16 +84,17 @@ func (a archiveBuildpack) createTgz(sourceDir string) (string, error) {
 }
 
 var (
-	SimpleLayersParent  = &archiveBuildpack{name: "simple-layers-parent-buildpack"}
-	SimpleLayers        = &archiveBuildpack{name: "simple-layers-buildpack"}
-	InternetCapable     = &archiveBuildpack{name: "internet-capable-buildpack"}
-	ReadVolume          = &archiveBuildpack{name: "read-volume-buildpack"}
-	ReadWriteVolume     = &archiveBuildpack{name: "read-write-volume-buildpack"}
-	ArchiveNotInBuilder = &archiveBuildpack{name: "not-in-builder-buildpack"}
-	Noop                = &archiveBuildpack{name: "noop-buildpack"}
-	Noop2               = &archiveBuildpack{name: "noop-buildpack-2"}
-	OtherStack          = &archiveBuildpack{name: "other-stack-buildpack"}
-	ReadEnv             = &archiveBuildpack{name: "read-env-buildpack"}
-	NestedLevelOne      = &archiveBuildpack{name: "nested-level-1-buildpack"}
-	NestedLevelTwo      = &archiveBuildpack{name: "nested-level-2-buildpack"}
+	SimpleLayersParent       = &archiveBuildpack{name: "simple-layers-parent-buildpack"}
+	SimpleLayers             = &archiveBuildpack{name: "simple-layers-buildpack"}
+	SimpleLayersDifferentSha = &archiveBuildpack{name: "simple-layers-buildpack-different-sha"}
+	InternetCapable          = &archiveBuildpack{name: "internet-capable-buildpack"}
+	ReadVolume               = &archiveBuildpack{name: "read-volume-buildpack"}
+	ReadWriteVolume          = &archiveBuildpack{name: "read-write-volume-buildpack"}
+	ArchiveNotInBuilder      = &archiveBuildpack{name: "not-in-builder-buildpack"}
+	Noop                     = &archiveBuildpack{name: "noop-buildpack"}
+	Noop2                    = &archiveBuildpack{name: "noop-buildpack-2"}
+	OtherStack               = &archiveBuildpack{name: "other-stack-buildpack"}
+	ReadEnv                  = &archiveBuildpack{name: "read-env-buildpack"}
+	NestedLevelOne           = &archiveBuildpack{name: "nested-level-1-buildpack"}
+	NestedLevelTwo           = &archiveBuildpack{name: "nested-level-2-buildpack"}
 )

acceptance/buildpacks/package_image_buildpack.go

@@ -83,6 +83,7 @@ func (p PackageImage) Prepare(sourceDir, _ string) error {
 		packArgs = append(packArgs, "--publish")
 	}
 
+	p.testObject.Log("packaging image: ", p.name)
 	output := p.pack.RunSuccessfully("buildpack", append([]string{"package"}, packArgs...)...)
 	assertOutput := assertions.NewOutputAssertionManager(p.testObject, output)
 	if p.publish {

acceptance/invoke/pack.go

@@ -219,12 +219,16 @@ type Feature int
 
 const (
 	InspectRemoteImage = iota
+	BuilderNoDuplicateLayers
 )
 
 var featureTests = map[Feature]func(i *PackInvoker) bool{
 	InspectRemoteImage: func(i *PackInvoker) bool {
 		return i.laterThan("0.17.0")
 	},
+	BuilderNoDuplicateLayers: func(i *PackInvoker) bool {
+		return i.laterThan("0.18.0")
+	},
 }
 
 func (i *PackInvoker) SupportsFeature(f Feature) bool {

acceptance/testdata/mock_buildpacks/0.2/simple-layers-buildpack-different-sha/bin/build

@@ -0,0 +1,49 @@
+#!/usr/bin/env bash
+
+echo "---> Build: Simple Layers Different Sha Buildpack"
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+launch_dir=$1
+
+## makes a launch layer
+echo "making launch layer"
+
+# Add color line, to test for --no-color
+echo "Color: Styled"
+
+mkdir "$launch_dir/launch-layer"
+echo "Launch Dep Contents" > "$launch_dir/launch-layer/launch-dep"
+ln -snf "$launch_dir/launch-layer" launch-deps
+echo "launch = true" > "$launch_dir/launch-layer.toml"
+
+## makes a cached launch layer
+if [[ ! -f "$launch_dir/cached-launch-layer.toml" ]]; then
+    echo "making cached launch layer"
+    mkdir "$launch_dir/cached-launch-layer"
+    echo "Cached Dep Contents" > "$launch_dir/cached-launch-layer/cached-dep"
+    ln -snf "$launch_dir/cached-launch-layer" cached-deps
+    echo "launch = true" > "$launch_dir/cached-launch-layer.toml"
+    echo "cache = true" >> "$launch_dir/cached-launch-layer.toml"
+else
+  echo "reusing cached launch layer"
+  ln -snf "$launch_dir/cached-launch-layer" cached-deps
+fi
+
+## adds a process
+cat <<EOF > "$launch_dir/launch.toml"
+[[processes]]
+  type = "web"
+  command = "./run"
+  args = ["8080"]
+
+[[processes]]
+  type = "hello"
+  command = "echo"
+  args = ["hello", "world"]
+  direct = true
+EOF
+
+echo "---> Done"

acceptance/testdata/mock_buildpacks/0.2/simple-layers-buildpack-different-sha/bin/build.bat

@@ -0,0 +1,40 @@
+@echo off
+echo --- Build: Simple Layers Different Sha Buildpack
+
+set launch_dir=%1
+
+:: makes a launch layer
+echo making launch layer %launch_dir%\launch-layer
+mkdir %launch_dir%\launch-layer
+echo Launch Dep Contents > "%launch_dir%\launch-layer\launch-dep
+mklink /j launch-deps %launch_dir%\launch-layer
+echo launch = true > %launch_dir%\launch-layer.toml
+
+:: makes a cached launch layer
+if not exist %launch_dir%\cached-launch-layer.toml (
+    echo making cached launch layer %launch_dir%\cached-launch-layer
+    mkdir %launch_dir%\cached-launch-layer
+    echo Cached Dep Contents > %launch_dir%\cached-launch-layer\cached-dep
+    mklink /j cached-deps %launch_dir%\cached-launch-layer
+    echo launch = true > %launch_dir%\cached-launch-layer.toml
+    echo cache = true >> %launch_dir%\cached-launch-layer.toml
+) else (
+    echo reusing cached launch layer %launch_dir%\cached-launch-layer
+    mklink /j cached-deps %launch_dir%\cached-launch-layer
+)
+
+:: adds a process
+(
+echo [[processes]]
+echo   type = "web"
+echo   command = '.\run'
+echo   args = ["8080"]
+echo.
+echo [[processes]]
+echo   type = "hello"
+echo   command = "cmd"
+echo   args = ["/c", "echo hello world"]
+echo   direct = true
+) > %launch_dir%\launch.toml
+
+echo --- Done

acceptance/testdata/mock_buildpacks/0.2/simple-layers-buildpack-different-sha/bin/detect

@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+## always detect

acceptance/testdata/mock_buildpacks/0.2/simple-layers-buildpack-different-sha/bin/detect.bat

@@ -0,0 +1,2 @@
+@echo off
+:: always detect

acceptance/testdata/mock_buildpacks/0.2/simple-layers-buildpack-different-sha/bin/extra_file.txt

@@ -0,0 +1 @@
+Just some extra content to change the sha256 of this buildpack :)

acceptance/testdata/mock_buildpacks/0.2/simple-layers-buildpack-different-sha/buildpack.toml

@@ -0,0 +1,9 @@
+api = "0.2"
+
+[buildpack]
+  id = "simple/layers"
+  version = "simple-layers-version"
+  name = "Simple Layers Buildpack"
+
+[[stacks]]
+  id = "pack.test.stack"

acceptance/testdata/pack_fixtures/nested_builder.toml

@@ -11,10 +11,9 @@
   # noop-buildpack-2 has the same id but a different version compared to noop-buildpack
   uri = "noop-buildpack-2.tgz"
 
-
-{{- if .simple_layers_buildpack}}
+{{- if .simple_layers_buildpack_different_sha}}
 [[buildpacks]]
-  image = "{{.simple_layers_buildpack}}"
+  image = "{{.simple_layers_buildpack_different_sha}}"
   version = "simple-layers-version"
 {{- end}}
 

acceptance/testdata/pack_fixtures/simple-layers-buildpack-different-sha_package.toml

@@ -0,0 +1,2 @@
+[buildpack]
+uri = "simple-layers-buildpack-different-sha.tgz"

acceptance/testdata/pack_previous_fixtures_overrides/simple-layers-buildpack-different-sha_package.toml

@@ -0,0 +1,2 @@
+[buildpack]
+uri = "simple-layers-buildpack-different-sha.tgz"