Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also read distro information from /etc/os-release when checking target compat #1352

Merged
merged 3 commits into from
May 10, 2024
Merged

Also read distro information from /etc/os-release when checking target compat #1352

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3976f03
Reorder functions in file
natalieparellano May 10, 2024
f46e91d
Also read distro information from /etc/os-release when checking targe…
natalieparellano May 10, 2024
609e287
Error if we don't find run image OS during analyze
natalieparellano May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions phase/analyzer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
natalieparellano marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
2 changes: 1 addition & 1 deletion phase/detector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
54 changes: 0 additions & 54 deletions platform/run_image_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
93 changes: 51 additions & 42 deletions platform/target_data.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.OS == "" || base.Distro == nil {
logger.Info("target distro name/version labels not found, reading /etc/os-release file")
Copy link
Contributor

@edmorley edmorley May 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what cases could OS be the empty string? If the author of the run image created it in such a way that the image didn't have properly specified metadata? The OCI spec says OS is required:
https://github.com/opencontainers/image-spec/blob/main/config.md#properties

It's just at the moment, the log message (albeit whose wording exists prior to this PR) isn't consistent with the conditional. ie: The message says "target distro name/version labels", but the conditional suggests it should say "OS metadata, or target distro name/version labels" etc. (But if OS should always be present, perhaps the OS check can be moved to an earlier assertion that errors instead? then the log message wouldn't need updating)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I added an error in 609e287

GetTargetOSFromFileSystem(d, &base, logger)
}
// check matches
if !matches(base.OS, module.OS) {
return false
}
Expand All @@ -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
}
Expand All @@ -115,6 +81,49 @@ 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 tm.OS == "" {
tm.OS = "linux"
}
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.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 {
Expand Down
Loading
Loading