From 3976f0357859cbd44985cd68c60fc1cde08df036 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 10 May 2024 10:11:10 -0400 Subject: [PATCH 1/3] Reorder functions in file Signed-off-by: Natalie Arellano --- platform/run_image_test.go | 54 ---------------- platform/target_data.go | 82 +++++++++++------------ platform/target_data_test.go | 122 +++++++++++++++++++++++++---------- 3 files changed, 128 insertions(+), 130 deletions(-) diff --git a/platform/run_image_test.go b/platform/run_image_test.go index 29cc1a54f..566b00cc2 100644 --- a/platform/run_image_test.go +++ b/platform/run_image_test.go @@ -4,8 +4,6 @@ import ( "path/filepath" "testing" - "github.com/apex/log" - "github.com/apex/log/handlers/memory" "github.com/google/go-containerregistry/pkg/authn" "github.com/buildpacks/lifecycle/platform" @@ -203,58 +201,6 @@ 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"}} - d := &mockDetector{ - contents: "this is just test contents really", - t: t, - HasFile: false, - } - observed := platform.EnvVarsFor(d, tm, &log.Logger{Handler: memory.New()}) - 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) - h.AssertContains(t, observed, "CNB_TARGET_DISTRO_VERSION="+tm.Distro.Version) - h.AssertContains(t, observed, "CNB_TARGET_OS="+tm.OS) - h.AssertEq(t, len(observed), 5) - }) - - it("returns the right thing from /etc/os-release", func() { - d := &mockDetector{ - contents: "this is just test contents really", - t: t, - HasFile: true, - } - tm := files.TargetMetadata{Arch: "pentium", ArchVariant: "mmx", ID: "my-id", OS: "linux", Distro: nil} - observed := platform.EnvVarsFor(d, tm, &log.Logger{Handler: memory.New()}) - 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=opensesame") - h.AssertContains(t, observed, "CNB_TARGET_DISTRO_VERSION=3.14") - h.AssertContains(t, observed, "CNB_TARGET_OS="+tm.OS) - h.AssertEq(t, len(observed), 5) - }) - - it("does not return the wrong thing", func() { - tm := files.TargetMetadata{Arch: "pentium", OS: "linux"} - d := &mockDetector{ - contents: "this is just test contents really", - t: t, - HasFile: false, - } - observed := platform.EnvVarsFor(d, tm, &log.Logger{Handler: memory.New()}) - 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 - // the empty ones: - h.AssertContains(t, observed, "CNB_TARGET_ARCH_VARIANT=") - h.AssertContains(t, observed, "CNB_TARGET_DISTRO_NAME=") - h.AssertContains(t, observed, "CNB_TARGET_DISTRO_VERSION=") - h.AssertEq(t, len(observed), 5) - }) - }) - when(".BestRunImageMirrorFor", func() { var ( stackMD *files.Stack diff --git a/platform/target_data.go b/platform/target_data.go index 82fba40c0..dc6dd49b1 100644 --- a/platform/target_data.go +++ b/platform/target_data.go @@ -9,30 +9,6 @@ import ( "github.com/buildpacks/lifecycle/platform/files" ) -// 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(d fsutil.Detector, 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 == "" || tm.Distro == nil { - logger.Info("target distro name/version labels not found, reading /etc/os-release file") - GetTargetOSFromFileSystem(d, &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 - distVersion = tm.Distro.Version - } - ret = append(ret, "CNB_TARGET_DISTRO_NAME="+distName) - ret = append(ret, "CNB_TARGET_DISTRO_VERSION="+distVersion) - return ret -} - func GetTargetMetadata(fromImage imgutil.Image) (*files.TargetMetadata, error) { tm := files.TargetMetadata{} var err error @@ -64,23 +40,6 @@ func GetTargetMetadata(fromImage imgutil.Image) (*files.TargetMetadata, error) { return &tm, nil } -// GetTargetOSFromFileSystem populates the target metadata you pass in if the information is available -// returns a boolean indicating whether it populated any data. -func GetTargetOSFromFileSystem(d fsutil.Detector, tm *files.TargetMetadata, logger log.Logger) { - if d.HasSystemdFile() { - contents, err := d.ReadSystemdFile() - if err != nil { - logger.Warnf("Encountered error trying to read /etc/os-release file: %s", err.Error()) - return - } - info := d.GetInfo(contents) - if info.Version != "" || info.Name != "" { - tm.OS = "linux" - tm.Distro = &files.OSDistro{Name: info.Name, Version: info.Version} - } - } -} - // TargetSatisfiedForBuild treats empty fields as wildcards and returns true if all populated fields match. func TargetSatisfiedForBuild(base files.TargetMetadata, module buildpack.TargetMetadata) bool { if !matches(base.OS, module.OS) { @@ -115,6 +74,47 @@ func matches(target1, target2 string) bool { return target1 == target2 } +// GetTargetOSFromFileSystem populates the target metadata you pass in if the information is available +// returns a boolean indicating whether it populated any data. +func GetTargetOSFromFileSystem(d fsutil.Detector, tm *files.TargetMetadata, logger log.Logger) { + if d.HasSystemdFile() { + contents, err := d.ReadSystemdFile() + if err != nil { + logger.Warnf("Encountered error trying to read /etc/os-release file: %s", err.Error()) + return + } + info := d.GetInfo(contents) + if info.Version != "" || info.Name != "" { + tm.OS = "linux" + tm.Distro = &files.OSDistro{Name: info.Name, Version: info.Version} + } + } +} + +// 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(d fsutil.Detector, 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 == "" || tm.Distro == nil { + logger.Info("target distro name/version labels not found, reading /etc/os-release file") + GetTargetOSFromFileSystem(d, &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 + distVersion = tm.Distro.Version + } + ret = append(ret, "CNB_TARGET_DISTRO_NAME="+distName) + ret = append(ret, "CNB_TARGET_DISTRO_VERSION="+distVersion) + return ret +} + // TargetSatisfiedForRebase treats optional fields (ArchVariant and Distribution fields) as wildcards if empty, returns true if all populated fields match func TargetSatisfiedForRebase(t files.TargetMetadata, appTargetMetadata files.TargetMetadata) bool { if t.OS != appTargetMetadata.OS || t.Arch != appTargetMetadata.Arch { diff --git a/platform/target_data_test.go b/platform/target_data_test.go index 2742f8f46..6c0c32265 100644 --- a/platform/target_data_test.go +++ b/platform/target_data_test.go @@ -94,6 +94,93 @@ func testTargetData(t *testing.T, when spec.G, it spec.S) { }) }) + when(".GetTargetOSFromFileSystem", func() { + it("populates appropriately", func() { + logr := &log.Logger{Handler: memory.New()} + tm := files.TargetMetadata{} + d := mockDetector{contents: "this is just test contents really", + t: t, + HasFile: true} + platform.GetTargetOSFromFileSystem(&d, &tm, logr) + h.AssertEq(t, "opensesame", tm.Distro.Name) + h.AssertEq(t, "3.14", tm.Distro.Version) + }) + + it("doesn't populate if there's no file", func() { + logr := &log.Logger{Handler: memory.New()} + tm := files.TargetMetadata{} + d := mockDetector{contents: "in unit tests 2.0 the users will generate the content but we'll serve them ads", + t: t, + HasFile: false} + platform.GetTargetOSFromFileSystem(&d, &tm, logr) + h.AssertNil(t, tm.Distro) + }) + + it("doesn't populate if there's an error reading the file", func() { + logr := &log.Logger{Handler: memory.New()} + tm := files.TargetMetadata{} + d := mockDetector{contents: "contentment is the greatest wealth", + t: t, + HasFile: true, + ReadFileErr: fmt.Errorf("I'm sorry Dave, I don't even remember exactly what HAL says"), + } + platform.GetTargetOSFromFileSystem(&d, &tm, logr) + h.AssertNil(t, tm.Distro) + }) + }) + + 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"}} + d := &mockDetector{ + contents: "this is just test contents really", + t: t, + HasFile: false, + } + observed := platform.EnvVarsFor(d, tm, &log.Logger{Handler: memory.New()}) + 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) + h.AssertContains(t, observed, "CNB_TARGET_DISTRO_VERSION="+tm.Distro.Version) + h.AssertContains(t, observed, "CNB_TARGET_OS="+tm.OS) + h.AssertEq(t, len(observed), 5) + }) + + it("returns the right thing from /etc/os-release", func() { + d := &mockDetector{ + contents: "this is just test contents really", + t: t, + HasFile: true, + } + tm := files.TargetMetadata{Arch: "pentium", ArchVariant: "mmx", ID: "my-id", OS: "linux", Distro: nil} + observed := platform.EnvVarsFor(d, tm, &log.Logger{Handler: memory.New()}) + 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=opensesame") + h.AssertContains(t, observed, "CNB_TARGET_DISTRO_VERSION=3.14") + h.AssertContains(t, observed, "CNB_TARGET_OS="+tm.OS) + h.AssertEq(t, len(observed), 5) + }) + + it("does not return the wrong thing", func() { + tm := files.TargetMetadata{Arch: "pentium", OS: "linux"} + d := &mockDetector{ + contents: "this is just test contents really", + t: t, + HasFile: false, + } + observed := platform.EnvVarsFor(d, tm, &log.Logger{Handler: memory.New()}) + 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 + // the empty ones: + h.AssertContains(t, observed, "CNB_TARGET_ARCH_VARIANT=") + h.AssertContains(t, observed, "CNB_TARGET_DISTRO_NAME=") + h.AssertContains(t, observed, "CNB_TARGET_DISTRO_VERSION=") + h.AssertEq(t, len(observed), 5) + }) + }) + when(".TargetSatisfiedForRebase", func() { var baseTarget files.TargetMetadata when("orig image data", func() { @@ -167,41 +254,6 @@ func testTargetData(t *testing.T, when spec.G, it spec.S) { }) }) }) - - when(".GetTargetOSFromFileSystem", func() { - it("populates appropriately", func() { - logr := &log.Logger{Handler: memory.New()} - tm := files.TargetMetadata{} - d := mockDetector{contents: "this is just test contents really", - t: t, - HasFile: true} - platform.GetTargetOSFromFileSystem(&d, &tm, logr) - h.AssertEq(t, "opensesame", tm.Distro.Name) - h.AssertEq(t, "3.14", tm.Distro.Version) - }) - - it("doesn't populate if there's no file", func() { - logr := &log.Logger{Handler: memory.New()} - tm := files.TargetMetadata{} - d := mockDetector{contents: "in unit tests 2.0 the users will generate the content but we'll serve them ads", - t: t, - HasFile: false} - platform.GetTargetOSFromFileSystem(&d, &tm, logr) - h.AssertNil(t, tm.Distro) - }) - - it("doesn't populate if there's an error reading the file", func() { - logr := &log.Logger{Handler: memory.New()} - tm := files.TargetMetadata{} - d := mockDetector{contents: "contentment is the greatest wealth", - t: t, - HasFile: true, - ReadFileErr: fmt.Errorf("I'm sorry Dave, I don't even remember exactly what HAL says"), - } - platform.GetTargetOSFromFileSystem(&d, &tm, logr) - h.AssertNil(t, tm.Distro) - }) - }) } type mockDetector struct { From f46e91d50d4ad5ac054effbc6095f36d3ea9529b Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 10 May 2024 10:41:36 -0400 Subject: [PATCH 2/3] Also read distro information from /etc/os-release when checking target compat https://github.com/buildpacks/lifecycle/pull/1347 reads the file when providing target env vars to buildpacks during detect, but we also need to consider this info when deciding whether or not to run detect for the buildpack Signed-off-by: Natalie Arellano --- phase/analyzer.go | 4 --- phase/detector.go | 2 +- platform/target_data.go | 13 ++++++-- platform/target_data_test.go | 59 ++++++++++++++++++++++++++---------- 4 files changed, 55 insertions(+), 23 deletions(-) diff --git a/phase/analyzer.go b/phase/analyzer.go index 0217411bf..37ecc8341 100644 --- a/phase/analyzer.go +++ b/phase/analyzer.go @@ -6,7 +6,6 @@ import ( "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/image" - "github.com/buildpacks/lifecycle/internal/fsutil" "github.com/buildpacks/lifecycle/internal/layer" "github.com/buildpacks/lifecycle/log" "github.com/buildpacks/lifecycle/platform" @@ -87,9 +86,6 @@ func (a *Analyzer) Analyze() (files.Analyzed, error) { if err != nil { return files.Analyzed{}, errors.Wrap(err, "unpacking metadata from image") } - if atm.OS == "" { - platform.GetTargetOSFromFileSystem(&fsutil.Detect{}, atm, a.Logger) - } } } diff --git a/phase/detector.go b/phase/detector.go index b370251ff..216c96321 100644 --- a/phase/detector.go +++ b/phase/detector.go @@ -206,7 +206,7 @@ func (d *Detector) detectGroup(group buildpack.Group, done []buildpack.GroupElem } else { for i := range descriptor.TargetsList() { d.Logger.Debugf("Checking for match against descriptor: %s", descriptor.TargetsList()[i]) - if platform.TargetSatisfiedForBuild(*d.AnalyzeMD.RunImage.TargetMetadata, descriptor.TargetsList()[i]) { + if platform.TargetSatisfiedForBuild(&fsutil.Detect{}, *d.AnalyzeMD.RunImage.TargetMetadata, descriptor.TargetsList()[i], d.Logger) { targetMatch = true break } diff --git a/platform/target_data.go b/platform/target_data.go index dc6dd49b1..64b8e5997 100644 --- a/platform/target_data.go +++ b/platform/target_data.go @@ -41,7 +41,13 @@ func GetTargetMetadata(fromImage imgutil.Image) (*files.TargetMetadata, error) { } // TargetSatisfiedForBuild treats empty fields as wildcards and returns true if all populated fields match. -func TargetSatisfiedForBuild(base files.TargetMetadata, module buildpack.TargetMetadata) bool { +func TargetSatisfiedForBuild(d fsutil.Detector, base files.TargetMetadata, module buildpack.TargetMetadata, logger log.Logger) bool { + // ensure we have all available data + if base.OS == "" || base.Distro == nil { + logger.Info("target distro name/version labels not found, reading /etc/os-release file") + GetTargetOSFromFileSystem(d, &base, logger) + } + // check matches if !matches(base.OS, module.OS) { return false } @@ -51,6 +57,7 @@ func TargetSatisfiedForBuild(base files.TargetMetadata, module buildpack.TargetM if !matches(base.ArchVariant, module.ArchVariant) { return false } + // check distro if len(module.Distros) == 0 { return true } @@ -84,8 +91,10 @@ func GetTargetOSFromFileSystem(d fsutil.Detector, tm *files.TargetMetadata, logg return } info := d.GetInfo(contents) - if info.Version != "" || info.Name != "" { + if tm.OS == "" { tm.OS = "linux" + } + if info.Version != "" || info.Name != "" { tm.Distro = &files.OSDistro{Name: info.Name, Version: info.Version} } } diff --git a/platform/target_data_test.go b/platform/target_data_test.go index 6c0c32265..421e0bdd3 100644 --- a/platform/target_data_test.go +++ b/platform/target_data_test.go @@ -21,34 +21,61 @@ func TestTargetData(t *testing.T) { func testTargetData(t *testing.T, when spec.G, it spec.S) { when(".TargetSatisfiedForBuild", func() { - var baseTarget files.TargetMetadata + baseTarget := files.TargetMetadata{OS: "Win95", Arch: "Pentium"} + d := mockDetector{ + contents: "this is just test contents really", + t: t, + HasFile: false, // by default, don't use info from /etc/os-release for these tests + } + when("base image data", func() { when("has os and arch", func() { - baseTarget = files.TargetMetadata{OS: "Win95", Arch: "Pentium"} - when("buildpack data", func() { when("has os and arch", func() { it("must match", func() { - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch}), true) - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: "Win98", Arch: baseTarget.Arch}), false) - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: "Pentium MMX"}), false) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch}, &log.Logger{Handler: memory.New()}), true) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: "Win98", Arch: baseTarget.Arch}, &log.Logger{Handler: memory.New()}), false) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: "Pentium MMX"}, &log.Logger{Handler: memory.New()}), false) }) }) when("missing os and arch", func() { it("matches", func() { - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: "", Arch: ""}), true) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: "", Arch: ""}, &log.Logger{Handler: memory.New()}), true) }) }) when("has distro information", func() { it("does not match", func() { - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch, ArchVariant: "MMX"}), true) - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{ + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{ OS: baseTarget.OS, Arch: baseTarget.Arch, Distros: []buildpack.OSDistro{{Name: "a", Version: "2"}}, - }), false) + }, &log.Logger{Handler: memory.New()}), false) + }) + + when("/etc/os-release has information", func() { + it("must match", func() { + d := mockDetector{ + contents: "this is just test contents really", + t: t, + HasFile: true, + } + h.AssertEq( + t, + platform.TargetSatisfiedForBuild( + &d, + baseTarget, + buildpack.TargetMetadata{ + OS: baseTarget.OS, + Arch: baseTarget.Arch, + Distros: []buildpack.OSDistro{ + {Name: "opensesame", Version: "3.14"}, + }, + }, + &log.Logger{Handler: memory.New()}, + ), true) + }) }) }) }) @@ -59,14 +86,14 @@ func testTargetData(t *testing.T, when spec.G, it spec.S) { when("buildpack data", func() { when("has arch variant", func() { it("must match", func() { - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch, ArchVariant: "some-arch-variant"}), true) - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch, ArchVariant: "some-other-arch-variant"}), false) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch, ArchVariant: "some-arch-variant"}, &log.Logger{Handler: memory.New()}), true) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch, ArchVariant: "some-other-arch-variant"}, &log.Logger{Handler: memory.New()}), false) }) }) when("missing arch variant", func() { it("matches", func() { - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch}), true) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch}, &log.Logger{Handler: memory.New()}), true) }) }) }) @@ -78,14 +105,14 @@ func testTargetData(t *testing.T, when spec.G, it spec.S) { when("buildpack data", func() { when("has distro information", func() { it("must match", func() { - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch, Distros: []buildpack.OSDistro{{Name: "B", Version: "2"}, {Name: "A", Version: "1"}}}), true) - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch, Distros: []buildpack.OSDistro{{Name: "g", Version: "2"}, {Name: "B", Version: "2"}}}), false) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch, Distros: []buildpack.OSDistro{{Name: "B", Version: "2"}, {Name: "A", Version: "1"}}}, &log.Logger{Handler: memory.New()}), true) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch, Distros: []buildpack.OSDistro{{Name: "g", Version: "2"}, {Name: "B", Version: "2"}}}, &log.Logger{Handler: memory.New()}), false) }) }) when("missing distro information", func() { it("matches", func() { - h.AssertEq(t, platform.TargetSatisfiedForBuild(baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch}), true) + h.AssertEq(t, platform.TargetSatisfiedForBuild(&d, baseTarget, buildpack.TargetMetadata{OS: baseTarget.OS, Arch: baseTarget.Arch}, &log.Logger{Handler: memory.New()}), true) }) }) }) From 609e287c875673c39b0dee18c41d1a33afeaae20 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 10 May 2024 11:11:27 -0400 Subject: [PATCH 3/3] Error if we don't find run image OS during analyze And remove checks for missing OS later in the build, as it should always be there Signed-off-by: Natalie Arellano --- phase/analyzer.go | 3 +++ phase/analyzer_test.go | 13 +++++++++++++ platform/target_data.go | 7 ++----- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/phase/analyzer.go b/phase/analyzer.go index 37ecc8341..946d5ba4c 100644 --- a/phase/analyzer.go +++ b/phase/analyzer.go @@ -86,6 +86,9 @@ func (a *Analyzer) Analyze() (files.Analyzed, error) { if err != nil { return files.Analyzed{}, errors.Wrap(err, "unpacking metadata from image") } + if atm.OS == "" { + return files.Analyzed{}, errors.New("failed to find OS in run image config") + } } } diff --git a/phase/analyzer_test.go b/phase/analyzer_test.go index 71f965f0a..0424f3f60 100644 --- a/phase/analyzer_test.go +++ b/phase/analyzer_test.go @@ -484,6 +484,7 @@ func testAnalyzer(platformAPI string) func(t *testing.T, when spec.G, it spec.S) h.AssertEq(t, md.RunImage.Reference, "s0m3D1g3sT") }) + it("populates target metadata from the run image", func() { h.AssertNil(t, previousImage.SetLabel("io.buildpacks.base.id", "id software")) h.AssertNil(t, previousImage.SetOS("windows")) @@ -508,6 +509,18 @@ func testAnalyzer(platformAPI string) func(t *testing.T, when spec.G, it spec.S) h.AssertEq(t, md.RunImage.TargetMetadata.Distro.Version, "Helpful Holstein") } }) + + when("run image is missing OS", func() { + it("errors", func() { + h.AssertNil(t, previousImage.SetOS("")) + _, err := analyzer.Analyze() + if api.MustParse(platformAPI).LessThan("0.12") { + h.AssertNil(t, err) + } else { + h.AssertError(t, err, "failed to find OS") + } + }) + }) }) }) } diff --git a/platform/target_data.go b/platform/target_data.go index 64b8e5997..cfb8c4746 100644 --- a/platform/target_data.go +++ b/platform/target_data.go @@ -43,7 +43,7 @@ func GetTargetMetadata(fromImage imgutil.Image) (*files.TargetMetadata, error) { // TargetSatisfiedForBuild treats empty fields as wildcards and returns true if all populated fields match. func TargetSatisfiedForBuild(d fsutil.Detector, base files.TargetMetadata, module buildpack.TargetMetadata, logger log.Logger) bool { // ensure we have all available data - if base.OS == "" || base.Distro == nil { + if base.Distro == nil { logger.Info("target distro name/version labels not found, reading /etc/os-release file") GetTargetOSFromFileSystem(d, &base, logger) } @@ -91,9 +91,6 @@ func GetTargetOSFromFileSystem(d fsutil.Detector, tm *files.TargetMetadata, logg return } info := d.GetInfo(contents) - if tm.OS == "" { - tm.OS = "linux" - } if info.Version != "" || info.Name != "" { tm.Distro = &files.OSDistro{Name: info.Name, Version: info.Version} } @@ -105,7 +102,7 @@ func GetTargetOSFromFileSystem(d fsutil.Detector, tm *files.TargetMetadata, logg func EnvVarsFor(d fsutil.Detector, 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 == "" || tm.Distro == nil { + if tm.Distro == nil { logger.Info("target distro name/version labels not found, reading /etc/os-release file") GetTargetOSFromFileSystem(d, &tm, logger) }