From c53600bdec0f3f2df99534d1295d3371d6c57cb4 Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Sat, 2 Apr 2022 00:13:24 +0100 Subject: [PATCH] Fix issue with wildcard stacks and meta buildpacks Signed-off-by: Sambhav Kothari --- internal/stack/merge.go | 43 ++++++++++++++++++++++-- internal/stack/merge_test.go | 54 ++++++++++++++++++++++++++++++ pkg/buildpack/builder_test.go | 62 +++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 2 deletions(-) diff --git a/internal/stack/merge.go b/internal/stack/merge.go index 854e96362..7c9975126 100644 --- a/internal/stack/merge.go +++ b/internal/stack/merge.go @@ -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. // // --- // @@ -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) diff --git a/internal/stack/merge_test.go b/internal/stack/merge_test.go index 17e185097..e3381ef8b 100644 --- a/internal/stack/merge_test.go +++ b/internal/stack/merge_test.go @@ -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: "*"}, + }) + }) + }) }) } diff --git a/pkg/buildpack/builder_test.go b/pkg/buildpack/builder_test.go index 4064e98af..b81fe82af 100644 --- a/pkg/buildpack/builder_test.go +++ b/pkg/buildpack/builder_test.go @@ -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{