Skip to content

Commit

Permalink
Fixes for pack acceptance in the current-current-current configuration (
Browse files Browse the repository at this point in the history 
#1075)

* Fixes for pack acceptance in the current-current-current configuration

Signed-off-by: Joe Kimmel <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>

* Add unit tests for fixes

Signed-off-by: Natalie Arellano <[email protected]>

---------

Signed-off-by: Joe Kimmel <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
  • Loading branch information
@natalieparellano
natalieparellano authored Apr 28, 2023
1 parent 710d938 commit 68b4ce9
Show file tree
Hide file tree
Showing 20 changed files with 375 additions and 109 deletions.
13 changes: 13 additions & 0 deletions buildpack/bp_descriptor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package buildpack

import (
"fmt"
"os"
"path/filepath"

Expand Down Expand Up @@ -30,11 +31,23 @@ type TargetMetadata struct {
Distributions []OSDistribution `json:"distributions" toml:"distributions"`
}

func (t *TargetMetadata) String() string {
s := fmt.Sprintf("OS: %s, Arch: %s, ArchVariant: %s", t.OS, t.Arch, t.ArchVariant)
if len(t.Distributions) > 0 {
s += fmt.Sprintf(", Distributions: %s", t.Distributions)
}
return s
}

type OSDistribution struct {
Name string `json:"name" toml:"name"`
Version string `json:"version" toml:"version"`
}

func (d OSDistribution) String() string {
return fmt.Sprintf("Distribution: (Name: %s, Version: %s)", d.Name, d.Version)
}

type BpInfo struct {
BaseInfo
SBOM []string `toml:"sbom-formats,omitempty" json:"sbom-formats,omitempty"`
Expand Down
35 changes: 35 additions & 0 deletions buildpack/bp_descriptor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,38 @@ func TestBpDescriptor(t *testing.T) {
}

func testBpDescriptor(t *testing.T, when spec.G, it spec.S) {
when("TargetMetadata", func() {
when("#String()", func() {
when("there is a distribution", func() {
it("prints the target", func() {
tm := &buildpack.TargetMetadata{
OS: "some-os",
Arch: "some-arch",
ArchVariant: "some-arch-variant",
Distributions: []buildpack.OSDistribution{
{
Name: "some-os-dist",
Version: "some-os-dist-version",
},
},
}
h.AssertEq(t, tm.String(), "OS: some-os, Arch: some-arch, ArchVariant: some-arch-variant, Distributions: [Distribution: (Name: some-os-dist, Version: some-os-dist-version)]")
})
})

when("there is no distribution", func() {
it("prints the target", func() {
tm := &buildpack.TargetMetadata{
OS: "some-os",
Arch: "some-arch",
ArchVariant: "some-arch-variant",
}
h.AssertEq(t, tm.String(), "OS: some-os, Arch: some-arch, ArchVariant: some-arch-variant")
})
})
})
})

when("#ReadBpDescriptor", func() {
it("returns a buildpack descriptor", func() {
path := filepath.Join("testdata", "buildpack", "by-id", "A", "v1", "buildpack.toml")
Expand Down Expand Up @@ -88,6 +120,7 @@ func testBpDescriptor(t *testing.T, when spec.G, it spec.S) {
h.AssertEq(t, descriptor.Targets[0].Distributions[0].Name, "ubuntu")
h.AssertEq(t, descriptor.Targets[0].Distributions[0].Version, "18.04")
})

it("does not translate non-special stack values", func() {
path := filepath.Join("testdata", "buildpack", "by-id", "C", "v1", "buildpack.toml")
descriptor, err := buildpack.ReadBpDescriptor(path)
Expand All @@ -103,6 +136,7 @@ func testBpDescriptor(t *testing.T, when spec.G, it spec.S) {
h.AssertEq(t, descriptor.Stacks[0].ID, "some.non-magic.value")
h.AssertEq(t, len(descriptor.Targets), 0)
})

it("does autodetect linux buildpacks from the bin dir contents", func() {
path := filepath.Join("testdata", "buildpack", "by-id", "C", "v2", "buildpack.toml")
descriptor, err := buildpack.ReadBpDescriptor(path)
Expand All @@ -122,6 +156,7 @@ func testBpDescriptor(t *testing.T, when spec.G, it spec.S) {
h.AssertEq(t, descriptor.Targets[0].OS, "linux")
h.AssertEq(t, len(descriptor.Targets[0].Distributions), 0)
})

it("detects windows/* if batch files are present and ignores linux", func() {
path := filepath.Join("testdata", "buildpack", "by-id", "A", "v1", "buildpack.toml")
descriptor, err := buildpack.ReadBpDescriptor(path)
Expand Down
29 changes: 1 addition & 28 deletions cmd/lifecycle/exporter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (e *exportCmd) export(group buildpack.Group, cacheStore lifecycle.Cache, an
return err
}

runImageForExport, err := e.getRunImageForExport()
runImageForExport, err := platform.GetRunImageForExport(*e.LifecycleInputs)
if err != nil {
return err
}
Expand Down Expand Up @@ -508,33 +508,6 @@ func (e *exportCmd) getExtendedConfig(runImage *platform.RunImage) (*v1.Config,
return &extendedConfig.Config, nil
}

func (e *exportCmd) getRunImageForExport() (platform.RunImageForExport, error) {
if e.PlatformAPI.LessThan("0.12") {
stackMD, err := platform.ReadStack(e.StackPath, cmd.DefaultLogger)
if err != nil {
return platform.RunImageForExport{}, err
}
return stackMD.RunImage, nil
}
runMD, err := platform.ReadRun(e.RunPath, cmd.DefaultLogger)
if err != nil {
return platform.RunImageForExport{}, err
}
if len(runMD.Images) == 0 {
return platform.RunImageForExport{Image: e.RunImageRef}, nil
}
runRef, err := name.ParseReference(e.RunImageRef)
if err != nil {
return platform.RunImageForExport{}, err
}
for _, runImage := range runMD.Images {
if runImage.Image == runRef.Context().Name() {
return runImage, nil
}
}
return platform.RunImageForExport{Image: e.RunImageRef}, nil
}

func (e *exportCmd) hasExtendedLayers() bool {
if e.ExtendedDir == "" {
return false
Expand Down
2 changes: 1 addition & 1 deletion cmd/lifecycle/rebaser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (r *rebaseCmd) setAppImage() error {

// for backwards compatibility, we need to fallback to the stack metadata
// fail if there is no run image metadata available from either location
if md.Stack.RunImage.Image == "" {
if md.Stack == nil || md.Stack.RunImage.Image == "" {
return cmd.FailErrCode(errors.New("-run-image is required when there is no run image metadata available"), cmd.CodeForInvalidArgs, "parse arguments")
}

Expand Down
15 changes: 8 additions & 7 deletions detector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,13 @@ func (d *Detector) detectGroup(group buildpack.Group, done []buildpack.GroupElem
return nil, nil, err
}

// Resolve order if element is a composite buildpack.
if order := bpDescriptor.Order; len(order) > 0 {
// FIXME: double-check slice safety here
// FIXME: cyclical references lead to infinite recursion
return d.detectOrder(order, done, group.Group[i+1:], groupEl.Optional, wg)
}

if d.PlatformAPI.AtLeast("0.12") {
targetMatch := false
if isWildcard(d.AnalyzeMD.RunImageTarget()) || hasWildcard(bpDescriptor.Targets) {
Expand All @@ -249,18 +256,12 @@ func (d *Detector) detectGroup(group buildpack.Group, done []buildpack.GroupElem
keyFor(groupEl),
buildpack.DetectOutputs{
Code: -1,
Err: fmt.Errorf("unable to satisfy Target OS/Arch constraints: %v", d.AnalyzeMD.RunImage.TargetMetadata),
Err: fmt.Errorf("unable to satisfy Target OS/Arch constraints; run image: %v, buildpack: %v", d.AnalyzeMD.RunImage.TargetMetadata, bpDescriptor.Targets),
})
continue
}
}

// Resolve order if element is a composite buildpack.
if order := bpDescriptor.Order; len(order) > 0 {
// FIXME: double-check slice safety here
// FIXME: cyclical references lead to infinite recursion
return d.detectOrder(order, done, group.Group[i+1:], groupEl.Optional, wg)
}
descriptor = bpDescriptor // standardize the type so below we don't have to care whether it was an extension
} else {
descriptor, err = d.DirStore.LookupExt(groupEl.ID, groupEl.Version)
Expand Down
163 changes: 108 additions & 55 deletions detector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -794,48 +794,79 @@ func testDetector(t *testing.T, when spec.G, it spec.S) {
}
})
})
})

when("A Target is provided from AnalyzeMD", func() {
it("errors if the buildpacks don't share that target arch/os", func() {
detector.AnalyzeMD.RunImage = &platform.RunImage{
TargetMetadata: &platform.TargetMetadata{
OS: "MacOS",
Arch: "ARM64",
Distribution: &platform.OSDistribution{Name: "MacOS", Version: "some kind of big cat"},
},
}
when("target resolution", func() {
it("totally works if the constraints are met", func() {
detector.AnalyzeMD.RunImage = &platform.RunImage{
TargetMetadata: &platform.TargetMetadata{
OS: "MacOS",
Arch: "ARM64",
Distribution: &platform.OSDistribution{Name: "MacOS", Version: "snow cheetah"},
},
}

bpA1 := &buildpack.BpDescriptor{
WithAPI: "0.12",
Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "A", Version: "v1"}},
Targets: []buildpack.TargetMetadata{
{Arch: "P6", ArchVariant: "Pentium Pro", OS: "Win95",
Distributions: []buildpack.OSDistribution{
{Name: "Windows 95", Version: "OSR1"}, {Name: "Windows 95", Version: "OSR2.5"}}},
{Arch: "Pentium M", OS: "Win98",
Distributions: []buildpack.OSDistribution{{Name: "Windows 2000", Version: "Server"}}},
},
}
dirStore.EXPECT().LookupBp("A", "v1").Return(bpA1, nil).AnyTimes()
bpA1 := &buildpack.BpDescriptor{
WithAPI: "0.12",
Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "A", Version: "v1"}},
Targets: []buildpack.TargetMetadata{
{Arch: "P6", ArchVariant: "Pentium Pro", OS: "Win95",
Distributions: []buildpack.OSDistribution{
{Name: "Windows 95", Version: "OSR1"}, {Name: "Windows 95", Version: "OSR2.5"}}},
{Arch: "ARM64", OS: "MacOS", Distributions: []buildpack.OSDistribution{{Name: "MacOS", Version: "snow cheetah"}}}},
}
dirStore.EXPECT().LookupBp("A", "v1").Return(bpA1, nil).AnyTimes()
executor.EXPECT().Detect(bpA1, gomock.Any(), gomock.Any())

resolver.EXPECT().Resolve(gomock.Any(), gomock.Any()).DoAndReturn(
func(done []buildpack.GroupElement, detectRuns *sync.Map) ([]buildpack.GroupElement, []platform.BuildPlanEntry, error) {
h.AssertEq(t, len(done), 1)
val, ok := detectRuns.Load("Buildpack A@v1")
h.AssertEq(t, ok, true)
outs := val.(buildpack.DetectOutputs)
h.AssertEq(t, outs.Code, -1)
h.AssertStringContains(t, outs.Err.Error(), "unable to satisfy Target OS/Arch constraints")
return []buildpack.GroupElement{}, []platform.BuildPlanEntry{}, nil
})
group := []buildpack.GroupElement{
{ID: "A", Version: "v1", API: "0.12"},
}
// the most meaningful assertion in this test is that `group` is the first argument to Resolve, meaning that the buildpack matched.
resolver.EXPECT().Resolve(group, detector.Runs).Return(
[]buildpack.GroupElement{},
[]platform.BuildPlanEntry{},
nil,
)

group := []buildpack.GroupElement{
{ID: "A", Version: "v1", API: "0.3"},
}
detector.Order = buildpack.Order{{Group: group}}
_, _, err := detector.Detect() // even though the returns from this are directly from the mock above, if we don't check the returns the linter declares we've done it wrong and fails on the lack of assertions.
h.AssertNil(t, err)
})
detector.Order = buildpack.Order{{Group: group}}
_, _, err := detector.Detect()
h.AssertNil(t, err)
})

it("was born to be wildcard compliant", func() {
detector.AnalyzeMD.RunImage = &platform.RunImage{
TargetMetadata: &platform.TargetMetadata{
OS: "MacOS",
Arch: "ARM64",
Distribution: &platform.OSDistribution{Name: "MacOS", Version: "snow cheetah"},
},
}

bpA1 := &buildpack.BpDescriptor{
WithAPI: "0.12",
Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "A", Version: "v1"}},
Targets: []buildpack.TargetMetadata{
{Arch: "*", OS: "*"}},
}
dirStore.EXPECT().LookupBp("A", "v1").Return(bpA1, nil).AnyTimes()
executor.EXPECT().Detect(bpA1, gomock.Any(), gomock.Any())

group := []buildpack.GroupElement{
{ID: "A", Version: "v1", API: "0.12"},
}
// the most meaningful assertion in this test is that `group` is the first argument to Resolve, meaning that the buildpack matched.
resolver.EXPECT().Resolve(group, detector.Runs).Return(
[]buildpack.GroupElement{},
[]platform.BuildPlanEntry{},
nil,
)

detector.Order = buildpack.Order{{Group: group}}
_, _, err := detector.Detect()
h.AssertNil(t, err)
})

when("there is a composite buildpack", func() {
it("totally works if the constraints are met", func() {
detector.AnalyzeMD.RunImage = &platform.RunImage{
TargetMetadata: &platform.TargetMetadata{
Expand All @@ -845,6 +876,16 @@ func testDetector(t *testing.T, when spec.G, it spec.S) {
},
}

bpF1 := &buildpack.BpDescriptor{
WithAPI: "0.2",
Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "F", Version: "v1"}},
Order: []buildpack.Group{
{Group: []buildpack.GroupElement{
{ID: "A", Version: "v1"},
}},
},
}
dirStore.EXPECT().LookupBp("F", "v1").Return(bpF1, nil).AnyTimes()
bpA1 := &buildpack.BpDescriptor{
WithAPI: "0.12",
Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "A", Version: "v1"}},
Expand All @@ -855,53 +896,65 @@ func testDetector(t *testing.T, when spec.G, it spec.S) {
{Arch: "ARM64", OS: "MacOS", Distributions: []buildpack.OSDistribution{{Name: "MacOS", Version: "snow cheetah"}}}},
}
dirStore.EXPECT().LookupBp("A", "v1").Return(bpA1, nil).AnyTimes()

executor.EXPECT().Detect(bpA1, gomock.Any(), gomock.Any())

group := []buildpack.GroupElement{
expectedGroup := []buildpack.GroupElement{
{ID: "A", Version: "v1", API: "0.12"},
}
// the most meaningful assertion in this test is that `group` is the first argument to Resolve, meaning that the buildpack matched.
resolver.EXPECT().Resolve(group, detector.Runs).Return(
// the most meaningful assertion in this test is that `expectedGroup` is the first argument to Resolve, meaning that the buildpack matched.
resolver.EXPECT().Resolve(expectedGroup, detector.Runs).Return(
[]buildpack.GroupElement{},
[]platform.BuildPlanEntry{},
nil,
)

detector.Order = buildpack.Order{{Group: group}}
detector.Order = buildpack.Order{{Group: []buildpack.GroupElement{
{ID: "F", Version: "v1", API: "0.12"},
}}}
_, _, err := detector.Detect()
h.AssertNil(t, err)
})
})
it("was born to be wildcard compliant", func() {

it("errors if the buildpacks don't share that target arch/os", func() {
detector.AnalyzeMD.RunImage = &platform.RunImage{
TargetMetadata: &platform.TargetMetadata{
OS: "MacOS",
Arch: "ARM64",
Distribution: &platform.OSDistribution{Name: "MacOS", Version: "snow cheetah"},
Distribution: &platform.OSDistribution{Name: "MacOS", Version: "some kind of big cat"},
},
}

bpA1 := &buildpack.BpDescriptor{
WithAPI: "0.12",
Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "A", Version: "v1"}},
Targets: []buildpack.TargetMetadata{
{Arch: "*", OS: "*"}},
{Arch: "P6", ArchVariant: "Pentium Pro", OS: "Win95",
Distributions: []buildpack.OSDistribution{
{Name: "Windows 95", Version: "OSR1"}, {Name: "Windows 95", Version: "OSR2.5"}}},
{Arch: "Pentium M", OS: "Win98",
Distributions: []buildpack.OSDistribution{{Name: "Windows 2000", Version: "Server"}}},
},
}
dirStore.EXPECT().LookupBp("A", "v1").Return(bpA1, nil).AnyTimes()
executor.EXPECT().Detect(bpA1, gomock.Any(), gomock.Any())

resolver.EXPECT().Resolve(gomock.Any(), gomock.Any()).DoAndReturn(
func(done []buildpack.GroupElement, detectRuns *sync.Map) ([]buildpack.GroupElement, []platform.BuildPlanEntry, error) {
h.AssertEq(t, len(done), 1)
val, ok := detectRuns.Load("Buildpack A@v1")
h.AssertEq(t, ok, true)
outs := val.(buildpack.DetectOutputs)
h.AssertEq(t, outs.Code, -1)
h.AssertStringContains(t, outs.Err.Error(), "unable to satisfy Target OS/Arch constraints")
return []buildpack.GroupElement{}, []platform.BuildPlanEntry{}, nil
})

group := []buildpack.GroupElement{
{ID: "A", Version: "v1", API: "0.12"},
{ID: "A", Version: "v1", API: "0.3"},
}
// the most meaningful assertion in this test is that `group` is the first argument to Resolve, meaning that the buildpack matched.
resolver.EXPECT().Resolve(group, detector.Runs).Return(
[]buildpack.GroupElement{},
[]platform.BuildPlanEntry{},
nil,
)

detector.Order = buildpack.Order{{Group: group}}
_, _, err := detector.Detect()
_, _, err := detector.Detect() // even though the returns from this are directly from the mock above, if we don't check the returns the linter declares we've done it wrong and fails on the lack of assertions.
h.AssertNil(t, err)
})
})
Expand Down
Loading

0 comments on commit 68b4ce9

Please sign in to comment.