Also read distro information from /etc/os-release when checking targe…

…t compat (#1352) * Reorder functions in file Signed-off-by: Natalie Arellano <narellano@vmware.com> * Also read distro information from /etc/os-release when checking target compat #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 <narellano@vmware.com> * 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 <narellano@vmware.com> --------- Signed-off-by: Natalie Arellano <narellano@vmware.com>
buildpacks · May 10, 2024 · 83efa75 · 83efa75
1 parent c0590cd
commit 83efa75
Show file tree

Hide file tree

Showing 6 changed files with 193 additions and 150 deletions.
diff --git 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"
@@ -88,7 +87,7 @@ func (a *Analyzer) Analyze() (files.Analyzed, error) {
 				return files.Analyzed{}, errors.Wrap(err, "unpacking metadata from image")
 			}
 			if atm.OS == "" {
-				platform.GetTargetOSFromFileSystem(&fsutil.Detect{}, atm, a.Logger)
+				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
@@ -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/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/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
@@ -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,25 +40,14 @@ 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 {
+func TargetSatisfiedForBuild(d fsutil.Detector, base files.TargetMetadata, module buildpack.TargetMetadata, logger log.Logger) bool {
+	// ensure we have all available data
+	if 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
 	}
@@ -92,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
 	}
@@ -115,6 +81,46 @@ 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.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.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 {