From 869bc63d6276792ccb92ec8ffe8da5e967c294ca Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 29 Apr 2024 11:20:52 -0400 Subject: [PATCH 1/2] Platform 0.13: look for build Dockerfiles in /generated//Dockerfile.build Newer platforms don't copy the Dockerfile from where extensions output them Signed-off-by: Natalie Arellano --- internal/build/lifecycle_execution.go | 13 ++++++- internal/build/lifecycle_execution_test.go | 45 ++++++++++++++++++---- internal/build/lifecycle_executor.go | 3 +- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 0f5c9a49a4..50d170cdb2 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -892,10 +892,21 @@ func (l *LifecycleExecution) hasExtensionsForBuild() bool { } // the directory is /generated/build inside the build container, but `CopyOutTo` only copies the directory fis, err := os.ReadDir(filepath.Join(l.tmpDir, "build")) + if err == nil && len(fis) > 0 { + return true + } + // on newer platforms, we need to find a file such as /generated//Dockerfile.build + fis, err = os.ReadDir(l.tmpDir) if err != nil { + l.logger.Warnf("failed to read generated directory, assuming no build image extensions: %s", err) return false } - return len(fis) > 0 + for _, fi := range fis { + if _, err := os.Stat(filepath.Join(l.tmpDir, fi.Name(), "Dockerfile.build")); err == nil { + return true + } + } + return false } func (l *LifecycleExecution) hasExtensionsForRun() bool { diff --git a/internal/build/lifecycle_execution_test.go b/internal/build/lifecycle_execution_test.go index ed12a2e5b1..afd9569d05 100644 --- a/internal/build/lifecycle_execution_test.go +++ b/internal/build/lifecycle_execution_test.go @@ -136,11 +136,17 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { // construct fixtures for extensions if extensionsForBuild { - // the directory is /generated/build inside the build container, but `CopyOutTo` only copies the directory - err = os.MkdirAll(filepath.Join(tmpDir, "build"), 0755) - h.AssertNil(t, err) - _, err = os.Create(filepath.Join(tmpDir, "build", "some-dockerfile")) - h.AssertNil(t, err) + if platformAPI.LessThan("0.13") { + // the directory is /generated/build inside the build container, but `CopyOutTo` only copies the directory + err = os.MkdirAll(filepath.Join(tmpDir, "build", "some-buildpack-id"), 0755) + h.AssertNil(t, err) + } else { + // the directory is /generated/some-buildpack-id inside the build container, but `CopyOutTo` only copies the directory + err = os.MkdirAll(filepath.Join(tmpDir, "some-buildpack-id"), 0755) + h.AssertNil(t, err) + _, err = os.Create(filepath.Join(tmpDir, "some-buildpack-id", "Dockerfile.build")) + h.AssertNil(t, err) + } } amd := files.Analyzed{RunImage: &files.RunImage{ Extend: false, @@ -579,7 +585,32 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { providedOrderExt = dist.Order{dist.OrderEntry{Group: []dist.ModuleRef{ /* don't care */ }}} when("for build", func() { - when("present /generated/build", func() { + when("present in /generated/", func() { + extensionsForBuild = true + + when("platform >= 0.13", func() { + platformAPI = api.MustParse("0.13") + + it("runs the extender (build)", func() { + err := lifecycle.Run(context.Background(), func(execution *build.LifecycleExecution) build.PhaseFactory { + return fakePhaseFactory + }) + h.AssertNil(t, err) + + h.AssertEq(t, len(fakePhaseFactory.NewCalledWithProvider), 5) + + var found bool + for _, entry := range fakePhaseFactory.NewCalledWithProvider { + if entry.Name() == "extender" { + found = true + } + } + h.AssertEq(t, found, true) + }) + }) + }) + + when("present in /generated/build", func() { extensionsForBuild = true when("platform < 0.10", func() { @@ -603,7 +634,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { }) }) - when("platform >= 0.10", func() { + when("platform 0.10 to 0.12", func() { platformAPI = api.MustParse("0.10") it("runs the extender (build)", func() { diff --git a/internal/build/lifecycle_executor.go b/internal/build/lifecycle_executor.go index 09d9011f0c..6394f89472 100644 --- a/internal/build/lifecycle_executor.go +++ b/internal/build/lifecycle_executor.go @@ -32,6 +32,7 @@ var ( api.MustParse("0.10"), api.MustParse("0.11"), api.MustParse("0.12"), + api.MustParse("0.13"), } ) @@ -71,7 +72,7 @@ type LifecycleOptions struct { Builder Builder BuilderImage string // differs from Builder.Name() and Builder.Image().Name() in that it includes the registry context LifecycleImage string - LifecycleApis []string // optional - populated only if custom lifecycle image is downloaded, from that lifecycle's container's Labels. + LifecycleApis []string // optional - populated only if custom lifecycle image is downloaded, from that lifecycle image's labels. RunImage string FetchRunImageWithLifecycleLayer func(name string) (string, error) ProjectMetadata files.ProjectMetadata From c1ad91c93472ba76adc37088b869c1c5adca409d Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 3 May 2024 14:14:09 -0400 Subject: [PATCH 2/2] Fix acceptance Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 2 +- .../testdata/pack_fixtures/report_output.txt | 2 +- internal/build/lifecycle_execution.go | 14 ++++++++------ internal/build/lifecycle_execution_test.go | 8 +++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 6b030cb04c..d103b55f86 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -834,7 +834,7 @@ func testAcceptance( launchCacheVolume.Clear(context.TODO()) }) - when("builder is untrusted", func() { + when("there are build image extensions", func() { it("uses the 5 phases, and runs the extender (build)", func() { output := pack.RunSuccessfully( "build", repoName, diff --git a/acceptance/testdata/pack_fixtures/report_output.txt b/acceptance/testdata/pack_fixtures/report_output.txt index 4239830438..6161c9216f 100644 --- a/acceptance/testdata/pack_fixtures/report_output.txt +++ b/acceptance/testdata/pack_fixtures/report_output.txt @@ -4,7 +4,7 @@ Pack: Default Lifecycle Version: 0.19.3 -Supported Platform APIs: 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12 +Supported Platform APIs: 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12, 0.13 Config: default-builder-image = "{{ .DefaultBuilder }}" diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 50d170cdb2..8150ead355 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -444,7 +444,7 @@ func (l *LifecycleExecution) Detect(ctx context.Context, phaseFactory PhaseFacto If(l.hasExtensions(), WithPostContainerRunOperations( CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "analyzed.toml"), l.tmpDir))), If(l.hasExtensions(), WithPostContainerRunOperations( - CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "generated", "build"), l.tmpDir))), + CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "generated"), l.tmpDir))), envOp, ) @@ -890,19 +890,21 @@ func (l *LifecycleExecution) hasExtensionsForBuild() bool { if !l.hasExtensions() { return false } - // the directory is /generated/build inside the build container, but `CopyOutTo` only copies the directory - fis, err := os.ReadDir(filepath.Join(l.tmpDir, "build")) + generatedDir := filepath.Join(l.tmpDir, "generated") + fis, err := os.ReadDir(filepath.Join(generatedDir, "build")) if err == nil && len(fis) > 0 { + // on older platforms, we need to find a file such as /generated/build//Dockerfile + // on newer platforms, /generated/build doesn't exist return true } - // on newer platforms, we need to find a file such as /generated//Dockerfile.build - fis, err = os.ReadDir(l.tmpDir) + // on newer platforms, we need to find a file such as /generated//build.Dockerfile + fis, err = os.ReadDir(generatedDir) if err != nil { l.logger.Warnf("failed to read generated directory, assuming no build image extensions: %s", err) return false } for _, fi := range fis { - if _, err := os.Stat(filepath.Join(l.tmpDir, fi.Name(), "Dockerfile.build")); err == nil { + if _, err := os.Stat(filepath.Join(generatedDir, fi.Name(), "build.Dockerfile")); err == nil { return true } } diff --git a/internal/build/lifecycle_execution_test.go b/internal/build/lifecycle_execution_test.go index afd9569d05..eee480aae8 100644 --- a/internal/build/lifecycle_execution_test.go +++ b/internal/build/lifecycle_execution_test.go @@ -137,14 +137,12 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { // construct fixtures for extensions if extensionsForBuild { if platformAPI.LessThan("0.13") { - // the directory is /generated/build inside the build container, but `CopyOutTo` only copies the directory - err = os.MkdirAll(filepath.Join(tmpDir, "build", "some-buildpack-id"), 0755) + err = os.MkdirAll(filepath.Join(tmpDir, "generated", "build", "some-buildpack-id"), 0755) h.AssertNil(t, err) } else { - // the directory is /generated/some-buildpack-id inside the build container, but `CopyOutTo` only copies the directory - err = os.MkdirAll(filepath.Join(tmpDir, "some-buildpack-id"), 0755) + err = os.MkdirAll(filepath.Join(tmpDir, "generated", "some-buildpack-id"), 0755) h.AssertNil(t, err) - _, err = os.Create(filepath.Join(tmpDir, "some-buildpack-id", "Dockerfile.build")) + _, err = os.Create(filepath.Join(tmpDir, "generated", "some-buildpack-id", "build.Dockerfile")) h.AssertNil(t, err) } }