Skip to content

Commit

Permalink
Fix issue with wildcard stacks and meta buildpacks
Browse files Browse the repository at this point in the history
Signed-off-by: Sambhav Kothari <[email protected]>
  • Loading branch information
sambhav committed Apr 1, 2022
1 parent 79a40b7 commit c53600b
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 2 deletions.
43 changes: 41 additions & 2 deletions internal/stack/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@ import (
"github.com/buildpacks/pack/pkg/dist"
)

const WildcardStack = "*"

// MergeCompatible determines the allowable set of stacks that a combination of buildpacks may run on, given each
// buildpack's set of stacks. Compatibility between the two sets of buildpack stacks is defined by the following rules:
//
// 1. The stack must be supported by both buildpacks. That is, any resulting stack ID must appear in both input sets.
// 2. For each supported stack ID, all required mixins for all buildpacks must be provided by the result. That is,
// mixins for the stack ID in both input sets are unioned.
// 3. If there is a wildcard stack in either of the stack list, the stack list not having the wild card stack is returned.
// 4. If both the stack lists contain a wildcard stack, a list containing just the wildcard stack is returned.
//
// ---
//
Expand All @@ -34,20 +38,55 @@ import (
// stacksB = [{ID: "stack2", mixins: ["mixinA", "run:mixinB"]}}]
// result = []
//
// stacksA = [{ID: "*"}, {ID: "stack1", mixins: ["build:mixinC"]}]
// stacksB = [{ID: "stack1", mixins: ["build:mixinA"]}, {ID: "stack2", mixins: ["mixinA", "run:mixinB"]}]
// result = [{ID: "stack1", mixins: ["build:mixinA"]}, {ID: "stack2", mixins: ["mixinA", "run:mixinB"]}]
//
// stacksA = [{ID: "stack1", mixins: ["build:mixinA"]}, {ID: "stack2", mixins: ["mixinA", "run:mixinB"]}]
// stacksB = [{ID: "*"}, {ID: "stack1", mixins: ["build:mixinC"]}]
// result = [{ID: "stack1", mixins: ["build:mixinA"]}, {ID: "stack2", mixins: ["mixinA", "run:mixinB"]}]
//
// stacksA = [{ID: "*"}, {ID: "stack1", mixins: ["build:mixinA"]}, {ID: "stack2", mixins: ["mixinA", "run:mixinB"]}]
// stacksB = [{ID: "*"}, {ID: "stack1", mixins: ["build:mixinC"]}]
// result = [{ID: "*"}]
//
func MergeCompatible(stacksA []dist.Stack, stacksB []dist.Stack) []dist.Stack {
set := map[string][]string{}
AHasWildcardStack, BHasWildcardStack := false, false

for _, s := range stacksA {
set[s.ID] = s.Mixins
if s.ID == WildcardStack {
AHasWildcardStack = true
}
}

for _, s := range stacksB {
if s.ID == WildcardStack {
BHasWildcardStack = true
}
}

if AHasWildcardStack && BHasWildcardStack {
return []dist.Stack{{ID: WildcardStack}}
}

if AHasWildcardStack {
return stacksB
}

if BHasWildcardStack {
return stacksA
}

var results []dist.Stack

for _, s := range stacksB {
if stackMixins, ok := set[s.ID]; ok {
mixinsSet := stringset.FromSlice(append(stackMixins, s.Mixins...))
var mixins []string
for s := range mixinsSet {
mixins = append(mixins, s)
for m := range mixinsSet {
mixins = append(mixins, m)
}
sort.Strings(mixins)

Expand Down
54 changes: 54 additions & 0 deletions internal/stack/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,59 @@ func testMerge(t *testing.T, when spec.G, it spec.S) {
h.AssertEq(t, result, []dist.Stack{{ID: "stack1"}})
})
})

when("a set has a wildcard stack", func() {
it("returns the other set of stacks", func() {
result := stack.MergeCompatible(
[]dist.Stack{{ID: "*"}},
[]dist.Stack{
{ID: "stack1"},
{ID: "stack2"},
},
)

h.AssertEq(t, len(result), 2)
h.AssertEq(t, result, []dist.Stack{
{ID: "stack1"},
{ID: "stack2"},
})
})

it("returns the other set of stacks", func() {
result := stack.MergeCompatible(
[]dist.Stack{
{ID: "stack1"},
{ID: "stack2"},
},
[]dist.Stack{{ID: "*"}},
)

h.AssertEq(t, len(result), 2)
h.AssertEq(t, result, []dist.Stack{
{ID: "stack1"},
{ID: "stack2"},
})
})

it("returns the wildcard stack", func() {
result := stack.MergeCompatible(
[]dist.Stack{
{ID: "stack1"},
{ID: "stack2"},
{ID: "*"},
},
[]dist.Stack{
{ID: "*"},
{ID: "stack3"},
{ID: "stack1"},
},
)

h.AssertEq(t, len(result), 1)
h.AssertEq(t, result, []dist.Stack{
{ID: "*"},
})
})
})
})
}
62 changes: 62 additions & 0 deletions pkg/buildpack/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,68 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
})
})

when("dependency has wildcard stacks", func() {
it("should support all the possible stacks", func() {
bp, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{
API: api.MustParse("0.2"),
Info: dist.BuildpackInfo{
ID: "bp.1.id",
Version: "bp.1.version",
},
Order: dist.Order{{
Group: []dist.BuildpackRef{{
BuildpackInfo: dist.BuildpackInfo{ID: "bp.2.id", Version: "bp.2.version"},
Optional: false,
}, {
BuildpackInfo: dist.BuildpackInfo{ID: "bp.3.id", Version: "bp.3.version"},
Optional: false,
}},
}},
}, 0644)
h.AssertNil(t, err)

builder := buildpack.NewBuilder(mockImageFactory(expectedImageOS))
builder.SetBuildpack(bp)

dependency1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{
API: api.MustParse("0.2"),
Info: dist.BuildpackInfo{
ID: "bp.2.id",
Version: "bp.2.version",
},
Stacks: []dist.Stack{
{ID: "*", Mixins: []string{"Mixin-A"}},
},
Order: nil,
}, 0644)
h.AssertNil(t, err)
builder.AddDependency(dependency1)

dependency2, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{
API: api.MustParse("0.2"),
Info: dist.BuildpackInfo{
ID: "bp.3.id",
Version: "bp.3.version",
},
Stacks: []dist.Stack{
{ID: "stack.id.1", Mixins: []string{"Mixin-A"}},
},
Order: nil,
}, 0644)
h.AssertNil(t, err)
builder.AddDependency(dependency2)

img, err := builder.SaveAsImage("some/package", false, expectedImageOS)
h.AssertNil(t, err)

metadata := buildpack.Metadata{}
_, err = dist.GetLabel(img, "io.buildpacks.buildpackage.metadata", &metadata)
h.AssertNil(t, err)

h.AssertEq(t, metadata.Stacks, []dist.Stack{{ID: "stack.id.1", Mixins: []string{"Mixin-A"}}})
})
})

when("dependency is meta-buildpack", func() {
it("should succeed and compute common stacks", func() {
bp, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{
Expand Down

0 comments on commit c53600b

Please sign in to comment.