From 67d982a6a373a0beec6ac1036fdbe418752bb352 Mon Sep 17 00:00:00 2001 From: Javier Romero Date: Wed, 18 Dec 2019 17:49:59 -0600 Subject: [PATCH 1/2] Fix: support meta-buildpack in packages Signed-off-by: Javier Romero --- internal/buildpackage/builder.go | 24 ++++--- internal/buildpackage/builder_test.go | 92 +++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 17 deletions(-) diff --git a/internal/buildpackage/builder.go b/internal/buildpackage/builder.go index db566b2a1b..f2031b2d25 100644 --- a/internal/buildpackage/builder.go +++ b/internal/buildpackage/builder.go @@ -42,22 +42,20 @@ func (p *PackageBuilder) Save(repoName string, publish bool) (imgutil.Image, err } stacks := p.buildpack.Descriptor().Stacks - if len(stacks) == 0 { - return nil, errors.Errorf( - "buildpack %s must support at least one stack", - style.Symbol(p.buildpack.Descriptor().Info.FullName()), - ) - } - for _, bp := range p.dependencies { bpd := bp.Descriptor() - stacks = stack.MergeCompatible(stacks, bpd.Stacks) + if len(stacks) == 0 { - return nil, errors.Errorf( - "buildpack %s does not support any stacks from %s", - style.Symbol(p.buildpack.Descriptor().Info.FullName()), - style.Symbol(bpd.Info.FullName()), - ) + stacks = bpd.Stacks + } else if len(bpd.Stacks) > 0 { // skip over "meta-buildpacks" + stacks = stack.MergeCompatible(stacks, bpd.Stacks) + if len(stacks) == 0 { + return nil, errors.Errorf( + "buildpack %s does not support any stacks from %s", + style.Symbol(p.buildpack.Descriptor().Info.FullName()), + style.Symbol(bpd.Info.FullName()), + ) + } } } diff --git a/internal/buildpackage/builder_test.go b/internal/buildpackage/builder_test.go index 9bcd645292..5aabfa0a41 100644 --- a/internal/buildpackage/builder_test.go +++ b/internal/buildpackage/builder_test.go @@ -58,8 +58,8 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { }) when("validate stacks", func() { - when("buildpack doesn't have a declared stack", func() { - it("should error", func() { + when("buildpack is meta-buildpack", func() { + it("should succeed", func() { buildpack, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ @@ -67,13 +67,33 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { Version: "bp.1.version", }, Stacks: nil, - Order: nil, + Order: dist.Order{{ + Group: []dist.BuildpackRef{ + {BuildpackInfo: dist.BuildpackInfo{ID: "bp.nested.id", Version: "bp.nested.version"}}, + }, + }}, }, 0644) h.AssertNil(t, err) subject.SetBuildpack(buildpack) + + dependency, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "bp.nested.id", + Version: "bp.nested.version", + }, + Stacks: []dist.Stack{ + {ID: "stack.id.1", Mixins: []string{"Mixin-A"}}, + }, + Order: nil, + }, 0644) + h.AssertNil(t, err) + + subject.AddDependency(dependency) + _, err = subject.Save("some/package", false) - h.AssertError(t, err, "buildpack 'bp.1.id@bp.1.version' must support at least one stack") + h.AssertNil(t, err) }) }) @@ -157,6 +177,70 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, metadata.Stacks, []dist.Stack{{ID: "stack.id.1", Mixins: []string{"Mixin-A"}}}) }) }) + + when("dependency is meta-buildpack", func() { + it("should succeed with common stacks", func() { + buildpack, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "bp.1.id", + Version: "bp.1.version", + }, + Stacks: nil, + Order: dist.Order{{ + Group: []dist.BuildpackRef{ + {BuildpackInfo: dist.BuildpackInfo{ID: "bp.nested.id", Version: "bp.nested.version"}}, + }, + }}, + }, 0644) + h.AssertNil(t, err) + + subject.SetBuildpack(buildpack) + + dependencyOrder, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "bp.nested.id", + Version: "bp.nested.version", + }, + Order: dist.Order{{ + Group: []dist.BuildpackRef{ + {BuildpackInfo: dist.BuildpackInfo{ + ID: "bp.nested.nested.id", + Version: "bp.nested.nested.version", + }}, + }, + }}, + }, 0644) + h.AssertNil(t, err) + + subject.AddDependency(dependencyOrder) + + dependencyNestedNested, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "bp.nested.nested.id", + Version: "bp.nested.nested.version", + }, + Stacks: []dist.Stack{ + {ID: "stack.id.1", Mixins: []string{"Mixin-A"}}, + }, + Order: nil, + }, 0644) + h.AssertNil(t, err) + + subject.AddDependency(dependencyNestedNested) + + img, err := subject.Save("some/package", false) + h.AssertNil(t, err) + + metadata := buildpackage.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"}}}) + }) + }) }) it("sets metadata", func() { From 29b4189598944b70f8fa89c7b0d54ce7b967cf3e Mon Sep 17 00:00:00 2001 From: Andrew Meyer Date: Thu, 19 Dec 2019 10:10:52 -0600 Subject: [PATCH 2/2] Add check for missing dependencies Signed-off-by: Andrew Meyer Signed-off-by: Javier Romero --- internal/buildpackage/builder.go | 4 ++ internal/buildpackage/builder_test.go | 73 +++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/internal/buildpackage/builder.go b/internal/buildpackage/builder.go index f2031b2d25..5acc120349 100644 --- a/internal/buildpackage/builder.go +++ b/internal/buildpackage/builder.go @@ -59,6 +59,10 @@ func (p *PackageBuilder) Save(repoName string, publish bool) (imgutil.Image, err } } + if len(stacks) == 0 { + return nil, errors.Errorf("no compatible stacks among provided buildpacks") + } + image, err := p.imageFactory.NewImage(repoName, !publish) if err != nil { return nil, errors.Wrapf(err, "creating image") diff --git a/internal/buildpackage/builder_test.go b/internal/buildpackage/builder_test.go index 5aabfa0a41..338240a165 100644 --- a/internal/buildpackage/builder_test.go +++ b/internal/buildpackage/builder_test.go @@ -48,8 +48,8 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { }) when("#Save", func() { - when("validate default", func() { - when("default not set", func() { + when("validate buildpack", func() { + when("buildpack not set", func() { it("returns error", func() { _, err := subject.Save(fakePackageImage.Name(), false) h.AssertError(t, err, "buildpack must be set") @@ -97,6 +97,30 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { }) }) + when("buildpack is meta-buildpack and its dependency is missing", func() { + it("should return an error", func() { + buildpack, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "bp.1.id", + Version: "bp.1.version", + }, + Stacks: nil, + Order: dist.Order{{ + Group: []dist.BuildpackRef{ + {BuildpackInfo: dist.BuildpackInfo{ID: "bp.nested.id", Version: "bp.nested.version"}}, + }, + }}, + }, 0644) + h.AssertNil(t, err) + + subject.SetBuildpack(buildpack) + + _, err = subject.Save("some/package", false) + h.AssertError(t, err, "no compatible stacks among provided buildpacks") + }) + }) + when("dependency does not have any matching stack", func() { it("should error", func() { buildpack, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ @@ -179,7 +203,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { }) when("dependency is meta-buildpack", func() { - it("should succeed with common stacks", func() { + it("should succeed and compute common stacks", func() { buildpack, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ @@ -241,6 +265,49 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, metadata.Stacks, []dist.Stack{{ID: "stack.id.1", Mixins: []string{"Mixin-A"}}}) }) }) + + when("dependency is meta-buildpack and its dependency is missing", func() { + it("should return an error", func() { + buildpack, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "bp.1.id", + Version: "bp.1.version", + }, + Stacks: nil, + Order: dist.Order{{ + Group: []dist.BuildpackRef{ + {BuildpackInfo: dist.BuildpackInfo{ID: "bp.nested.id", Version: "bp.nested.version"}}, + }, + }}, + }, 0644) + h.AssertNil(t, err) + + subject.SetBuildpack(buildpack) + + dependencyOrder, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "bp.nested.id", + Version: "bp.nested.version", + }, + Order: dist.Order{{ + Group: []dist.BuildpackRef{ + {BuildpackInfo: dist.BuildpackInfo{ + ID: "bp.nested.nested.id", + Version: "bp.nested.nested.version", + }}, + }, + }}, + }, 0644) + h.AssertNil(t, err) + + subject.AddDependency(dependencyOrder) + + _, err = subject.Save("some/package", false) + h.AssertError(t, err, "no compatible stacks among provided buildpacks") + }) + }) }) it("sets metadata", func() {