From 1148b713e08cd00122db1839c79a211ab92f0a9d Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 4 Mar 2024 12:33:20 -0500 Subject: [PATCH] Always set CNB_TARGET_* variables during detect, build, and generate (#1309) * Always set CNB_TARGET_* variables during detect, build, and generate when the Buildpack API version is at least 0.10. Previously, we only set these variables when the Platform API version was at least 0.12. But, newer Buildpack APIs expect these variables regardless of the Platform API version. If we are on an older platform, derive the target variables from the base image OS. Fixes https://github.com/buildpacks/lifecycle/issues/1308 Signed-off-by: Natalie Arellano * Fix unit Signed-off-by: Natalie Arellano * Fix unit again Signed-off-by: Natalie Arellano --------- Signed-off-by: Natalie Arellano --- buildpack/build.go | 4 ++++ buildpack/detect.go | 4 ++++ buildpack/generate.go | 5 +++++ env/build.go | 5 ----- env/build_test.go | 10 ---------- phase/builder.go | 7 ++----- phase/builder_test.go | 20 ++++++++++---------- phase/detector.go | 7 ++----- phase/detector_test.go | 10 +++++----- phase/generator.go | 4 +--- phase/generator_test.go | 4 ++-- platform/run_image_test.go | 4 ++-- platform/target_data.go | 16 ++++++++++++---- 13 files changed, 49 insertions(+), 51 deletions(-) diff --git a/buildpack/build.go b/buildpack/build.go index 0e757e330..354e724c4 100644 --- a/buildpack/build.go +++ b/buildpack/build.go @@ -35,6 +35,7 @@ type BuildInputs struct { LayersDir string PlatformDir string Env BuildEnv + TargetEnv []string Out, Err io.Writer Plan Plan } @@ -151,6 +152,9 @@ func runBuildCmd(d BpDescriptor, bpLayersDir, planPath string, inputs BuildInput EnvLayersDir+"="+bpLayersDir, ) } + if api.MustParse(d.API()).AtLeast("0.10") { + cmd.Env = append(cmd.Env, inputs.TargetEnv...) + } if err = cmd.Run(); err != nil { return NewError(err, ErrTypeBuildpack) diff --git a/buildpack/detect.go b/buildpack/detect.go index 80716d5dd..d5bcb2a54 100644 --- a/buildpack/detect.go +++ b/buildpack/detect.go @@ -30,6 +30,7 @@ type DetectInputs struct { BuildConfigDir string PlatformDir string Env BuildEnv + TargetEnv []string } type DetectOutputs struct { @@ -178,6 +179,9 @@ func runDetect(d detectable, inputs DetectInputs, planPath string, envRootDirKey EnvBuildPlanPath+"="+planPath, ) } + if api.MustParse(d.API()).AtLeast("0.10") { + cmd.Env = append(cmd.Env, inputs.TargetEnv...) + } if err := cmd.Run(); err != nil { if err, ok := err.(*exec.ExitError); ok { diff --git a/buildpack/generate.go b/buildpack/generate.go index a3aacd6ac..3d0e2b954 100644 --- a/buildpack/generate.go +++ b/buildpack/generate.go @@ -7,6 +7,7 @@ import ( "os/exec" "path/filepath" + "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/internal/extend" "github.com/buildpacks/lifecycle/launch" "github.com/buildpacks/lifecycle/log" @@ -25,6 +26,7 @@ type GenerateInputs struct { OutputDir string // a temp directory provided by the lifecycle to capture extensions output PlatformDir string Env BuildEnv + TargetEnv []string Out, Err io.Writer Plan Plan } @@ -103,6 +105,9 @@ func runGenerateCmd(d ExtDescriptor, extOutputDir, planPath string, inputs Gener EnvOutputDir+"="+extOutputDir, EnvPlatformDir+"="+inputs.PlatformDir, ) + if api.MustParse(d.API()).AtLeast("0.10") { + cmd.Env = append(cmd.Env, inputs.TargetEnv...) + } if err := cmd.Run(); err != nil { return NewError(err, ErrTypeBuildpack) diff --git a/env/build.go b/env/build.go index 1843595cd..ee2e7f25a 100644 --- a/env/build.go +++ b/env/build.go @@ -8,11 +8,6 @@ import ( // BuildEnvIncludelist are env vars that, if set in the lifecycle's execution environment - either in a builder or by the platform, are passed-through to buildpack executables var BuildEnvIncludelist = []string{ "CNB_STACK_ID", // deprecated as of api 0.12.0 - "CNB_TARGET_OS", - "CNB_TARGET_ARCH", - "CNB_TARGET_ARCH_VARIANT", - "CNB_TARGET_DISTRO_NAME", - "CNB_TARGET_DISTRO_VERSION", "HOSTNAME", "HOME", "HTTPS_PROXY", diff --git a/env/build_test.go b/env/build_test.go index e5f51404a..07db7e6cd 100644 --- a/env/build_test.go +++ b/env/build_test.go @@ -35,11 +35,6 @@ func testBuildEnv(t *testing.T, when spec.G, it spec.S) { it("includes expected vars", func() { benv := env.NewBuildEnv([]string{ "CNB_STACK_ID=some-stack-id", - "CNB_TARGET_ARCH=st-louis", - "CNB_TARGET_ARCH_VARIANT=suburban", - "CNB_TARGET_OS=BeOS", - "CNB_TARGET_DISTRO_NAME=web", - "CNB_TARGET_DISTRO_VERSION=3.0", "HOSTNAME=some-hostname", "HOME=some-home", "HTTPS_PROXY=some-https-proxy", @@ -59,11 +54,6 @@ func testBuildEnv(t *testing.T, when spec.G, it spec.S) { sort.Strings(out) expectedVars := []string{ "CNB_STACK_ID=some-stack-id", - "CNB_TARGET_ARCH=st-louis", - "CNB_TARGET_ARCH_VARIANT=suburban", - "CNB_TARGET_DISTRO_NAME=web", - "CNB_TARGET_DISTRO_VERSION=3.0", - "CNB_TARGET_OS=BeOS", "CPATH=some-cpath", "HOME=some-home", "HOSTNAME=some-hostname", diff --git a/phase/builder.go b/phase/builder.go index 6f9ae736f..eae12decd 100644 --- a/phase/builder.go +++ b/phase/builder.go @@ -67,11 +67,6 @@ func (b *Builder) Build() (*files.BuildMetadata, error) { ) processMap := newProcessMap() inputs := b.getBuildInputs() - if b.AnalyzeMD.RunImage != nil && b.AnalyzeMD.RunImage.TargetMetadata != nil && b.PlatformAPI.AtLeast("0.12") { - inputs.Env = env.NewBuildEnv(append(os.Environ(), platform.EnvVarsFor(*b.AnalyzeMD.RunImage.TargetMetadata)...)) - } else { - inputs.Env = env.NewBuildEnv(os.Environ()) - } filteredPlan := b.Plan @@ -153,6 +148,8 @@ func (b *Builder) getBuildInputs() buildpack.BuildInputs { BuildConfigDir: b.BuildConfigDir, LayersDir: b.LayersDir, PlatformDir: b.PlatformDir, + Env: env.NewBuildEnv(os.Environ()), + TargetEnv: platform.EnvVarsFor(b.AnalyzeMD.RunImageTarget(), b.Logger), Out: b.Out, Err: b.Err, } diff --git a/phase/builder_test.go b/phase/builder_test.go index f47787619..045073076 100644 --- a/phase/builder_test.go +++ b/phase/builder_test.go @@ -188,11 +188,11 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { executor.EXPECT().Build(*bpA, gomock.Any(), gomock.Any()).DoAndReturn( func(_ buildpack.BpDescriptor, inputs buildpack.BuildInputs, logger llog.Logger) (buildpack.BuildOutputs, error) { - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_ARCH=amd64") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_ARCH_VARIANT=") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_OS=linux") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_DISTRO_NAME=") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_DISTRO_VERSION=") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_ARCH=amd64") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_ARCH_VARIANT=") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_OS=linux") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_DISTRO_NAME=") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_DISTRO_VERSION=") return buildpack.BuildOutputs{}, nil }, ) @@ -202,11 +202,11 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { dirStore.EXPECT().LookupBp("B", "v2").Return(bpB, nil) executor.EXPECT().Build(*bpB, gomock.Any(), gomock.Any()).Do( func(_ buildpack.BpDescriptor, inputs buildpack.BuildInputs, _ llog.Logger) (buildpack.BuildOutputs, error) { - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_ARCH=amd64") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_ARCH_VARIANT=") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_OS=linux") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_DISTRO_NAME=") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_DISTRO_VERSION=") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_ARCH=amd64") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_ARCH_VARIANT=") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_OS=linux") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_DISTRO_NAME=") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_DISTRO_VERSION=") return buildpack.BuildOutputs{}, nil }) diff --git a/phase/detector.go b/phase/detector.go index f230a878a..4324fdc67 100644 --- a/phase/detector.go +++ b/phase/detector.go @@ -238,11 +238,8 @@ func (d *Detector) detectGroup(group buildpack.Group, done []buildpack.GroupElem AppDir: d.AppDir, BuildConfigDir: d.BuildConfigDir, PlatformDir: d.PlatformDir, - } - if d.AnalyzeMD.RunImage != nil && d.AnalyzeMD.RunImage.TargetMetadata != nil && d.PlatformAPI.AtLeast("0.12") { - inputs.Env = env.NewBuildEnv(append(os.Environ(), platform.EnvVarsFor(*d.AnalyzeMD.RunImage.TargetMetadata)...)) - } else { - inputs.Env = env.NewBuildEnv(os.Environ()) + Env: env.NewBuildEnv(os.Environ()), + TargetEnv: platform.EnvVarsFor(d.AnalyzeMD.RunImageTarget(), d.Logger), } d.Runs.Store(key, d.Executor.Detect(descriptor, inputs, d.Logger)) // this is where we finally invoke bin/detect } diff --git a/phase/detector_test.go b/phase/detector_test.go index 3663c2d1f..6820ef37a 100644 --- a/phase/detector_test.go +++ b/phase/detector_test.go @@ -220,11 +220,11 @@ func testDetector(t *testing.T, when spec.G, it spec.S) { dirStore.EXPECT().LookupBp("A", "v1").Return(bpA1, nil).AnyTimes() executor.EXPECT().Detect(bpA1, gomock.Any(), gomock.Any()).Do( func(_ buildpack.Descriptor, inputs buildpack.DetectInputs, _ log.Logger) buildpack.DetectOutputs { - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_ARCH=amd64") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_ARCH_VARIANT=") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_OS=linux") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_DISTRO_NAME=") - h.AssertContains(t, inputs.Env.List(), "CNB_TARGET_DISTRO_VERSION=") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_ARCH=amd64") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_ARCH_VARIANT=") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_OS=linux") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_DISTRO_NAME=") + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_DISTRO_VERSION=") return buildpack.DetectOutputs{} }) diff --git a/phase/generator.go b/phase/generator.go index ac80ba010..7f6e54105 100644 --- a/phase/generator.go +++ b/phase/generator.go @@ -100,9 +100,6 @@ func (g *Generator) Generate() (GenerateResult, error) { g.Logger.Debug("Finding plan") inputs.Plan = filteredPlan.Find(buildpack.KindExtension, ext.ID) - if g.AnalyzedMD.RunImage != nil && g.AnalyzedMD.RunImage.TargetMetadata != nil && g.PlatformAPI.AtLeast("0.12") { - inputs.Env = env.NewBuildEnv(append(inputs.Env.List(), platform.EnvVarsFor(*g.AnalyzedMD.RunImage.TargetMetadata)...)) - } g.Logger.Debug("Invoking command") result, err := g.Executor.Generate(*descriptor, inputs, g.Logger) if err != nil { @@ -154,6 +151,7 @@ func (g *Generator) getGenerateInputs() buildpack.GenerateInputs { BuildConfigDir: g.BuildConfigDir, PlatformDir: g.PlatformDir, Env: env.NewBuildEnv(os.Environ()), + TargetEnv: platform.EnvVarsFor(g.AnalyzedMD.RunImageTarget(), g.Logger), Out: g.Out, Err: g.Err, } diff --git a/phase/generator_test.go b/phase/generator_test.go index 607ff6b6a..cd6895bf3 100644 --- a/phase/generator_test.go +++ b/phase/generator_test.go @@ -281,7 +281,7 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { h.Mkfile(t, "some-build.Dockerfile-content-A", buildDockerfilePathA) executor.EXPECT().Generate(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( func(d buildpack.ExtDescriptor, inputs buildpack.GenerateInputs, _ *log.Logger) (buildpack.GenerateOutputs, error) { - h.AssertContains(t, inputs.Env.List(), + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_ARCH=amd64", "CNB_TARGET_ARCH_VARIANT=", "CNB_TARGET_DISTRO_NAME=", @@ -293,7 +293,7 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { }) executor.EXPECT().Generate(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( func(d buildpack.ExtDescriptor, inputs buildpack.GenerateInputs, _ *log.Logger) (buildpack.GenerateOutputs, error) { - h.AssertContains(t, inputs.Env.List(), + h.AssertContains(t, inputs.TargetEnv, "CNB_TARGET_ARCH=amd64", "CNB_TARGET_ARCH_VARIANT=", "CNB_TARGET_DISTRO_NAME=", diff --git a/platform/run_image_test.go b/platform/run_image_test.go index deb9a05e9..7a14e6849 100644 --- a/platform/run_image_test.go +++ b/platform/run_image_test.go @@ -204,7 +204,7 @@ func testRunImage(t *testing.T, when spec.G, it spec.S) { when(".EnvVarsFor", func() { it("returns the right thing", func() { tm := files.TargetMetadata{Arch: "pentium", ArchVariant: "mmx", ID: "my-id", OS: "linux", Distro: &files.OSDistro{Name: "nix", Version: "22.11"}} - observed := platform.EnvVarsFor(tm) + observed := platform.EnvVarsFor(tm, nil) h.AssertContains(t, observed, "CNB_TARGET_ARCH="+tm.Arch) h.AssertContains(t, observed, "CNB_TARGET_ARCH_VARIANT="+tm.ArchVariant) h.AssertContains(t, observed, "CNB_TARGET_DISTRO_NAME="+tm.Distro.Name) @@ -215,7 +215,7 @@ func testRunImage(t *testing.T, when spec.G, it spec.S) { it("does not return the wrong thing", func() { tm := files.TargetMetadata{Arch: "pentium", OS: "linux"} - observed := platform.EnvVarsFor(tm) + observed := platform.EnvVarsFor(tm, nil) h.AssertContains(t, observed, "CNB_TARGET_ARCH="+tm.Arch) h.AssertContains(t, observed, "CNB_TARGET_OS="+tm.OS) // note: per the spec only the ID field is optional, so I guess the others should always be set: https://github.com/buildpacks/rfcs/blob/main/text/0096-remove-stacks-mixins.md#runtime-metadata diff --git a/platform/target_data.go b/platform/target_data.go index 299af83c3..26977d676 100644 --- a/platform/target_data.go +++ b/platform/target_data.go @@ -9,11 +9,19 @@ import ( "github.com/buildpacks/lifecycle/platform/files" ) -// Fulfills the prophecy set forth in https://github.com/buildpacks/rfcs/blob/b8abe33f2bdc58792acf0bd094dc4ce3c8a54dbb/text/0096-remove-stacks-mixins.md?plain=1#L97 +// EnvVarsFor fulfills the prophecy set forth in https://github.com/buildpacks/rfcs/blob/b8abe33f2bdc58792acf0bd094dc4ce3c8a54dbb/text/0096-remove-stacks-mixins.md?plain=1#L97 // by returning an array of "VARIABLE=value" strings suitable for inclusion in your environment or complete breakfast. -func EnvVarsFor(tm files.TargetMetadata) []string { - ret := []string{"CNB_TARGET_OS=" + tm.OS, "CNB_TARGET_ARCH=" + tm.Arch} - ret = append(ret, "CNB_TARGET_ARCH_VARIANT="+tm.ArchVariant) +func EnvVarsFor(tm files.TargetMetadata, logger log.Logger) []string { + // we should always have os & arch, + // if they are not populated try to get target information from the build-time base image + if tm.OS == "" { + GetTargetOSFromFileSystem(&fsutil.Detect{}, &tm, logger) + } + ret := []string{ + "CNB_TARGET_OS=" + tm.OS, + "CNB_TARGET_ARCH=" + tm.Arch, + "CNB_TARGET_ARCH_VARIANT=" + tm.ArchVariant, + } var distName, distVersion string if tm.Distro != nil { distName = tm.Distro.Name