From b3c53c59e5601c6f7954b44644da4d39909bcb59 Mon Sep 17 00:00:00 2001 From: Javier Romero Date: Wed, 8 Jan 2020 10:07:15 -0600 Subject: [PATCH 1/3] Allow for the inclusion of buildpacks in builder when an additional buildpack is provided. Resolves #352 Resolves #304 Signed-off-by: Javier Romero --- build.go | 122 +++++++++++++--- build_test.go | 130 +++++++++++++++++- create_builder_test.go | 2 +- internal/builder/builder.go | 10 +- internal/fakes/fake_images.go | 43 +++++- testdata/buildpack/buildpack.toml | 2 +- testdata/buildpack2/buildpack.toml | 2 +- .../lifecycle-platform-0.1/lifecycle.toml | 2 +- testdata/lifecycle/lifecycle.toml | 2 +- 9 files changed, 277 insertions(+), 38 deletions(-) diff --git a/build.go b/build.go index 8dcaeff70..6adec8099 100644 --- a/build.go +++ b/build.go @@ -1,8 +1,10 @@ package pack import ( + "bytes" "context" "fmt" + "io" "math/rand" "net/url" "os" @@ -10,6 +12,7 @@ import ( "runtime" "strings" + "github.com/BurntSushi/toml" "github.com/buildpacks/imgutil" "github.com/docker/docker/api/types" "github.com/google/go-containerregistry/pkg/name" @@ -95,7 +98,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return err } - fetchedBPs, group, err := c.processBuildpacks(ctx, opts.Buildpacks) + fetchedBPs, group, err := c.processBuildpacks(ctx, bldr.Order(), opts.Buildpacks) if err != nil { return err } @@ -325,42 +328,121 @@ func (c *Client) processProxyConfig(config *ProxyConfig) ProxyConfig { } } -func (c *Client) processBuildpacks(ctx context.Context, buildpacks []string) ([]dist.Buildpack, dist.OrderEntry, error) { - group := dist.OrderEntry{Group: []dist.BuildpackRef{}} - var bps []dist.Buildpack - for _, bp := range buildpacks { - if isBuildpackID(bp) { +// processBuildpacks computes an order group based on the existing builder order and declared buildpacks. Additionally, +// it returns buildpacks that should be added to the builder. +// +// Visual examples: +// +// BUILDER ORDER +// ---------- +// - group: +// - A +// - B +// - group: +// - A +// +// WITH DECLARED: "from-builder=all", X +// ---------- +// - group: +// - (io.buildpacks.builder-buildpacks@embedded) +// - group: +// - A +// - B +// - group: +// - A +// - X +// +// WITH DECLARED: X +// ---------- +// - group: +// - X +// +// WITH DECLARED: A +// ---------- +// - group: +// - A +func (c *Client) processBuildpacks(ctx context.Context, builderOrder dist.Order, declaredBPs []string) (fetchedBPs []dist.Buildpack, group dist.OrderEntry, err error) { + for _, bp := range declaredBPs { + switch { + case bp == "from-builder=all": + builderBuildpack := createEphemeralBuilderBuildpack(builderOrder) + fetchedBPs = append(fetchedBPs, builderBuildpack) + group = appendBuildpackToOrder(group, builderBuildpack.Descriptor().Info) + case isBuildpackID(bp): id, version := c.parseBuildpack(bp) - group.Group = append(group.Group, dist.BuildpackRef{ - BuildpackInfo: dist.BuildpackInfo{ - ID: id, - Version: version, - }, + group = appendBuildpackToOrder(group, dist.BuildpackInfo{ + ID: id, + Version: version, }) - } else { + default: err := ensureBPSupport(bp) if err != nil { - return nil, dist.OrderEntry{}, errors.Wrapf(err, "checking buildpack path") + return fetchedBPs, group, errors.Wrapf(err, "checking support") } blob, err := c.downloader.Download(ctx, bp) if err != nil { - return nil, dist.OrderEntry{}, errors.Wrapf(err, "downloading buildpack from %s", style.Symbol(bp)) + return fetchedBPs, group, errors.Wrapf(err, "downloading buildpack from %s", style.Symbol(bp)) } fetchedBP, err := dist.BuildpackFromRootBlob(blob) if err != nil { - return nil, dist.OrderEntry{}, errors.Wrapf(err, "creating buildpack from %s", style.Symbol(bp)) + return fetchedBPs, group, errors.Wrapf(err, "creating buildpack from %s", style.Symbol(bp)) } - bps = append(bps, fetchedBP) + fetchedBPs = append(fetchedBPs, fetchedBP) - group.Group = append(group.Group, dist.BuildpackRef{ - BuildpackInfo: fetchedBP.Descriptor().Info, - }) + group = appendBuildpackToOrder(group, fetchedBP.Descriptor().Info) } } - return bps, group, nil + + return fetchedBPs, group, nil +} + +func createEphemeralBuilderBuildpack(builderOrder dist.Order) dist.Buildpack { + return &ephemeralBuilderBuildpack{ + descriptor: dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "io.buildpacks.builder-buildpacks", + Version: "embedded", + }, + Order: builderOrder, + }, + } +} + +type ephemeralBuilderBuildpack struct { + descriptor dist.BuildpackDescriptor +} + +func (b *ephemeralBuilderBuildpack) Descriptor() dist.BuildpackDescriptor { + return b.descriptor +} + +func (b *ephemeralBuilderBuildpack) Open() (io.ReadCloser, error) { + buf := &bytes.Buffer{} + if err := toml.NewEncoder(buf).Encode(b.descriptor); err != nil { + return nil, err + } + + tarBuilder := archive.TarBuilder{} + ts := archive.NormalizedDateTime + tarBuilder.AddDir(fmt.Sprintf("/cnb/buildpacks/%s", b.descriptor.EscapedID()), 0777, ts) + bpDir := fmt.Sprintf("/cnb/buildpacks/%s/%s", b.descriptor.EscapedID(), b.descriptor.Info.Version) + tarBuilder.AddDir(bpDir, 0777, ts) + tarBuilder.AddFile(bpDir+"/buildpack.toml", 0777, ts, buf.Bytes()) + + return tarBuilder.Reader(), nil +} + +func appendBuildpackToOrder(group dist.OrderEntry, bpInfo dist.BuildpackInfo) dist.OrderEntry { + group.Group = append(group.Group, dist.BuildpackRef{ + BuildpackInfo: bpInfo, + Optional: false, + }) + + return group } func isBuildpackID(bp string) bool { diff --git a/build_test.go b/build_test.go index 33b98e18f..a4210d951 100644 --- a/build_test.go +++ b/build_test.go @@ -68,6 +68,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { builderName = "example.com/default/builder:tag" defaultBuilderStackID = "some.stack.id" defaultBuilderImage = ifakes.NewFakeBuilderImage(t, + tmpDir, builderName, defaultBuilderStackID, "1234", @@ -95,7 +96,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }, }, API: builder.LifecycleAPI{ - BuildpackVersion: api.MustParse("0.3"), + BuildpackVersion: api.MustParse("0.2"), PlatformVersion: api.MustParse("0.2"), }, }, @@ -103,6 +104,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { dist.BuildpackLayers{ "buildpack.id": { "buildpack.version": { + API: api.MustParse("0.2"), Stacks: []dist.Stack{ { ID: defaultBuilderStackID, @@ -347,6 +349,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { it.Before(func() { customBuilderImage = ifakes.NewFakeBuilderImage(t, + tmpDir, builderName, "some.stack.id", "1234", @@ -547,23 +550,135 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }) when("Buildpacks option", func() { + var ( + additionalBuildpackPath string + additionalBuildpackInfo dist.BuildpackInfo + ) + + it.Before(func() { + additionalBuildpackInfo = dist.BuildpackInfo{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", + } + + buildpack, err := ifakes.NewFakeBuildpackBlob(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: additionalBuildpackInfo, + Stacks: []dist.Stack{{ID: defaultBuilderStackID}}, + Order: nil, + }, 0777) + h.AssertNil(t, err) + + tempFile, err := ioutil.TempFile(tmpDir, "additional-bp-*.tar") + h.AssertNil(t, err) + defer tempFile.Close() + + reader, err := buildpack.Open() + h.AssertNil(t, err) + + _, err = io.Copy(tempFile, reader) + h.AssertNil(t, err) + + additionalBuildpackPath = tempFile.Name() + }) + it("builder order is overwritten", func() { h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ Image: "some/app", Builder: builderName, ClearCache: true, - Buildpacks: []string{"buildpack.id@buildpack.version"}, + Buildpacks: []string{additionalBuildpackPath}, })) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ - {Group: []dist.BuildpackRef{{ - BuildpackInfo: dist.BuildpackInfo{ + {Group: []dist.BuildpackRef{ + {BuildpackInfo: additionalBuildpackInfo}, + }}, + }) + }) + + when("buildpack `from-builder=all` is set first", func() { + it("builder order is prepended", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: builderName, + ClearCache: true, + Buildpacks: []string{ + "from-builder=all", + additionalBuildpackPath, + }, + })) + h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) + bldr, err := builder.FromImage(defaultBuilderImage) + h.AssertNil(t, err) + h.AssertEq(t, bldr.Order(), dist.Order{{Group: []dist.BuildpackRef{ + { + BuildpackInfo: dist.BuildpackInfo{ + ID: "io.buildpacks.builder-buildpacks", + Version: "embedded", + }, + Optional: false, + }, + { + BuildpackInfo: additionalBuildpackInfo, + Optional: false, + }, + }}}) + + h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{ + {BuildpackInfo: dist.BuildpackInfo{ ID: "buildpack.id", Version: "buildpack.version", - }}, - }}, + }, Latest: true}, + {BuildpackInfo: dist.BuildpackInfo{ + ID: "io.buildpacks.builder-buildpacks", + Version: "embedded", + }, Latest: true}, + {BuildpackInfo: additionalBuildpackInfo, Latest: true}, + }) + }) + }) + + when("buildpack `from-builder=all` is set last", func() { + it("builder order is appended", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: builderName, + ClearCache: true, + Buildpacks: []string{ + additionalBuildpackPath, + "from-builder=all", + }, + })) + h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) + bldr, err := builder.FromImage(defaultBuilderImage) + h.AssertNil(t, err) + h.AssertEq(t, bldr.Order(), dist.Order{{Group: []dist.BuildpackRef{ + { + BuildpackInfo: additionalBuildpackInfo, + Optional: false, + }, { + BuildpackInfo: dist.BuildpackInfo{ + ID: "io.buildpacks.builder-buildpacks", + Version: "embedded", + }, + Optional: false, + }, + }}}) + + h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{ + {BuildpackInfo: dist.BuildpackInfo{ + ID: "buildpack.id", + Version: "buildpack.version", + }, Latest: true}, + {BuildpackInfo: additionalBuildpackInfo, Latest: true}, + {BuildpackInfo: dist.BuildpackInfo{ + ID: "io.buildpacks.builder-buildpacks", + Version: "embedded", + }, Latest: true}, + }) }) }) @@ -998,6 +1113,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { var incompatibleBuilderImage *fakes.Image it.Before(func() { incompatibleBuilderImage = ifakes.NewFakeBuilderImage(t, + tmpDir, "incompatible-"+builderName, defaultBuilderStackID, "1234", @@ -1019,7 +1135,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }, }, API: builder.LifecycleAPI{ - BuildpackVersion: api.MustParse("0.3"), + BuildpackVersion: api.MustParse("0.2"), PlatformVersion: api.MustParse("0.9"), }, }, diff --git a/create_builder_test.go b/create_builder_test.go index 8a0726cbd..35fc1a279 100644 --- a/create_builder_test.go +++ b/create_builder_test.go @@ -501,7 +501,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { mockImageFactory.EXPECT().NewImage(packageImage.Name(), false).Return(packageImage, nil) bpd := dist.BuildpackDescriptor{ - API: api.MustParse("0.3"), + API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "some.pkg.bp", Version: "2.3.4"}, Stacks: []dist.Stack{{ID: "some.stack.id"}}, } diff --git a/internal/builder/builder.go b/internal/builder/builder.go index ea869ff23..6905d539f 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -258,7 +258,7 @@ func (b *Builder) Save(logger logging.Logger) error { } } - if err := validateBuildpacks(b.StackID, b.Mixins(), b.LifecycleDescriptor(), b.additionalBuildpacks); err != nil { + if err := validateBuildpacks(b.StackID, b.Mixins(), b.LifecycleDescriptor(), b.Buildpacks(), b.additionalBuildpacks); err != nil { return errors.Wrap(err, "validating buildpacks") } @@ -408,14 +408,14 @@ func hasBuildpackWithVersion(bps []dist.BuildpackInfo, version string) bool { return false } -func validateBuildpacks(stackID string, mixins []string, lifecycleDescriptor LifecycleDescriptor, bps []dist.Buildpack) error { +func validateBuildpacks(stackID string, mixins []string, lifecycleDescriptor LifecycleDescriptor, allBuildpacks []BuildpackMetadata, bpsToValidate []dist.Buildpack) error { bpLookup := map[string]interface{}{} - for _, bp := range bps { - bpLookup[bp.Descriptor().Info.FullName()] = nil + for _, bp := range allBuildpacks { + bpLookup[bp.FullName()] = nil } - for _, bp := range bps { + for _, bp := range bpsToValidate { bpd := bp.Descriptor() if !bpd.API.SupportsVersion(lifecycleDescriptor.API.BuildpackVersion) { diff --git a/internal/fakes/fake_images.go b/internal/fakes/fake_images.go index cf5a64d92..7c22f2320 100644 --- a/internal/fakes/fake_images.go +++ b/internal/fakes/fake_images.go @@ -2,6 +2,10 @@ package fakes import ( "encoding/json" + "fmt" + "io" + "os" + "path/filepath" "testing" "github.com/buildpacks/imgutil/fakes" @@ -11,7 +15,7 @@ import ( h "github.com/buildpacks/pack/testhelpers" ) -func NewFakeBuilderImage(t *testing.T, name string, stackID, uid, gid string, metadata builder.Metadata, bpLayers dist.BuildpackLayers) *fakes.Image { +func NewFakeBuilderImage(t *testing.T, tmpDir, name string, stackID, uid, gid string, metadata builder.Metadata, bpLayers dist.BuildpackLayers) *fakes.Image { fakeBuilderImage := fakes.NewImage(name, "", nil) h.AssertNil(t, fakeBuilderImage.SetLabel("io.buildpacks.stack.id", stackID)) @@ -26,5 +30,42 @@ func NewFakeBuilderImage(t *testing.T, name string, stackID, uid, gid string, me h.AssertNil(t, err) h.AssertNil(t, fakeBuilderImage.SetLabel("io.buildpacks.buildpack.layers", string(label))) + for bpID, v := range bpLayers { + for bpVersion, bpLayerInfo := range v { + buildpack, err := NewFakeBuildpack(dist.BuildpackDescriptor{ + API: bpLayerInfo.API, + Info: dist.BuildpackInfo{ + ID: bpID, + Version: bpVersion, + }, + Stacks: bpLayerInfo.Stacks, + Order: bpLayerInfo.Order, + }, 0755) + h.AssertNil(t, err) + + buildpackTar := createBuildpackTar(t, tmpDir, buildpack) + err = fakeBuilderImage.AddLayer(buildpackTar) + h.AssertNil(t, err) + } + } + return fakeBuilderImage } + +func createBuildpackTar(t *testing.T, tmpDir string, buildpack dist.Buildpack) string { + f, err := os.Create(filepath.Join(tmpDir, fmt.Sprintf( + "%s.%s.tar", + buildpack.Descriptor().Info.ID, + buildpack.Descriptor().Info.Version), + )) + h.AssertNil(t, err) + defer f.Close() + + reader, err := buildpack.Open() + h.AssertNil(t, err) + + _, err = io.Copy(f, reader) + h.AssertNil(t, err) + + return f.Name() +} diff --git a/testdata/buildpack/buildpack.toml b/testdata/buildpack/buildpack.toml index 366fa46b3..bd340a94b 100644 --- a/testdata/buildpack/buildpack.toml +++ b/testdata/buildpack/buildpack.toml @@ -1,4 +1,4 @@ -api = "0.3" +api = "0.2" [buildpack] id = "bp.one" diff --git a/testdata/buildpack2/buildpack.toml b/testdata/buildpack2/buildpack.toml index 61a175d81..4ecc1362a 100644 --- a/testdata/buildpack2/buildpack.toml +++ b/testdata/buildpack2/buildpack.toml @@ -1,4 +1,4 @@ -api = "0.3" +api = "0.2" [buildpack] id = "some-other-buildpack-id" diff --git a/testdata/lifecycle-platform-0.1/lifecycle.toml b/testdata/lifecycle-platform-0.1/lifecycle.toml index a04d09398..6c5d87299 100644 --- a/testdata/lifecycle-platform-0.1/lifecycle.toml +++ b/testdata/lifecycle-platform-0.1/lifecycle.toml @@ -1,6 +1,6 @@ [api] platform = "0.1" - buildpack = "0.3" + buildpack = "0.2" [lifecycle] version = "3.4.5" diff --git a/testdata/lifecycle/lifecycle.toml b/testdata/lifecycle/lifecycle.toml index 2518f16ad..8dbd0b379 100644 --- a/testdata/lifecycle/lifecycle.toml +++ b/testdata/lifecycle/lifecycle.toml @@ -1,6 +1,6 @@ [api] platform = "0.2" - buildpack = "0.3" + buildpack = "0.2" [lifecycle] version = "3.4.5" \ No newline at end of file From 8b30f6b0864b22e4ad3625e7b4d6d005aaa58585 Mon Sep 17 00:00:00 2001 From: Javier Romero Date: Wed, 8 Jan 2020 20:19:09 -0600 Subject: [PATCH 2/3] Merge orders instead of ephemeral meta-buildpack Signed-off-by: Javier Romero --- build.go | 132 +++-- build_test.go | 486 +++++++++++++----- create_builder_test.go | 2 +- internal/builder/builder_test.go | 18 +- internal/buildpackage/builder_test.go | 1 + internal/fakes/fake_buildpack.go | 9 +- internal/fakes/fake_images.go | 42 +- testdata/buildpack/buildpack.toml | 2 +- testdata/buildpack2/buildpack.toml | 2 +- .../lifecycle-platform-0.1/lifecycle.toml | 2 +- testdata/lifecycle/lifecycle.toml | 2 +- 11 files changed, 466 insertions(+), 232 deletions(-) diff --git a/build.go b/build.go index 6adec8099..cd89a5794 100644 --- a/build.go +++ b/build.go @@ -1,10 +1,8 @@ package pack import ( - "bytes" "context" "fmt" - "io" "math/rand" "net/url" "os" @@ -12,7 +10,6 @@ import ( "runtime" "strings" - "github.com/BurntSushi/toml" "github.com/buildpacks/imgutil" "github.com/docker/docker/api/types" "github.com/google/go-containerregistry/pkg/name" @@ -59,6 +56,8 @@ type ContainerConfig struct { Network string } +const fromBuilderPrefix = "from=builder" + func (c *Client) Build(ctx context.Context, opts BuildOptions) error { imageRef, err := c.parseTagReference(opts.Image) if err != nil { @@ -98,7 +97,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return err } - fetchedBPs, group, err := c.processBuildpacks(ctx, bldr.Order(), opts.Buildpacks) + fetchedBPs, order, err := c.processBuildpacks(ctx, bldr.Order(), opts.Buildpacks) if err != nil { return err } @@ -107,7 +106,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return errors.Wrap(err, "validating stack mixins") } - ephemeralBuilder, err := c.createEphemeralBuilder(rawBuilderImage, opts.Env, group, fetchedBPs) + ephemeralBuilder, err := c.createEphemeralBuilder(rawBuilderImage, opts.Env, order, fetchedBPs) if err != nil { return err } @@ -341,17 +340,28 @@ func (c *Client) processProxyConfig(config *ProxyConfig) ProxyConfig { // - group: // - A // -// WITH DECLARED: "from-builder=all", X +// WITH DECLARED: "from=builder", X // ---------- // - group: -// - (io.buildpacks.builder-buildpacks@embedded) -// - group: -// - A -// - B -// - group: -// - A +// - A +// - B +// - X +// - group: +// - A // - X // +// WITH DECLARED: X, "from=builder", Y +// ---------- +// - group: +// - X +// - A +// - B +// - Y +// - group: +// - X +// - A +// - Y +// // WITH DECLARED: X // ---------- // - group: @@ -361,91 +371,75 @@ func (c *Client) processProxyConfig(config *ProxyConfig) ProxyConfig { // ---------- // - group: // - A -func (c *Client) processBuildpacks(ctx context.Context, builderOrder dist.Order, declaredBPs []string) (fetchedBPs []dist.Buildpack, group dist.OrderEntry, err error) { +func (c *Client) processBuildpacks(ctx context.Context, builderOrder dist.Order, declaredBPs []string) (fetchedBPs []dist.Buildpack, order dist.Order, err error) { + order = dist.Order{{Group: []dist.BuildpackRef{}}} for _, bp := range declaredBPs { switch { - case bp == "from-builder=all": - builderBuildpack := createEphemeralBuilderBuildpack(builderOrder) - fetchedBPs = append(fetchedBPs, builderBuildpack) - group = appendBuildpackToOrder(group, builderBuildpack.Descriptor().Info) + case bp == fromBuilderPrefix: + switch { + case len(order) == 0 || len(order[0].Group) == 0: + order = builderOrder + case len(order) > 1: + // This should only ever be possible if they are using from=builder twice which we don't allow + return nil, nil, errors.New("buildpacks from builder can only be defined once") + default: + newOrder := dist.Order{} + groupToAdd := order[0].Group + for _, bOrderEntry := range builderOrder { + newEntry := dist.OrderEntry{Group: append(groupToAdd, bOrderEntry.Group...)} + newOrder = append(newOrder, newEntry) + } + + order = newOrder + } case isBuildpackID(bp): id, version := c.parseBuildpack(bp) - group = appendBuildpackToOrder(group, dist.BuildpackInfo{ + order = appendBuildpackToOrder(order, dist.BuildpackInfo{ ID: id, Version: version, }) default: err := ensureBPSupport(bp) if err != nil { - return fetchedBPs, group, errors.Wrapf(err, "checking support") + return fetchedBPs, order, errors.Wrapf(err, "checking support") } blob, err := c.downloader.Download(ctx, bp) if err != nil { - return fetchedBPs, group, errors.Wrapf(err, "downloading buildpack from %s", style.Symbol(bp)) + return fetchedBPs, order, errors.Wrapf(err, "downloading buildpack from %s", style.Symbol(bp)) } fetchedBP, err := dist.BuildpackFromRootBlob(blob) if err != nil { - return fetchedBPs, group, errors.Wrapf(err, "creating buildpack from %s", style.Symbol(bp)) + return fetchedBPs, order, errors.Wrapf(err, "creating buildpack from %s", style.Symbol(bp)) } fetchedBPs = append(fetchedBPs, fetchedBP) - - group = appendBuildpackToOrder(group, fetchedBP.Descriptor().Info) + order = appendBuildpackToOrder(order, fetchedBP.Descriptor().Info) } } - return fetchedBPs, group, nil + return fetchedBPs, order, nil } -func createEphemeralBuilderBuildpack(builderOrder dist.Order) dist.Buildpack { - return &ephemeralBuilderBuildpack{ - descriptor: dist.BuildpackDescriptor{ - API: api.MustParse("0.2"), - Info: dist.BuildpackInfo{ - ID: "io.buildpacks.builder-buildpacks", - Version: "embedded", - }, - Order: builderOrder, - }, +func appendBuildpackToOrder(order dist.Order, bpInfo dist.BuildpackInfo) (newOrder dist.Order) { + for _, orderEntry := range order { + newEntry := orderEntry + newEntry.Group = append(newEntry.Group, dist.BuildpackRef{ + BuildpackInfo: bpInfo, + Optional: false, + }) + newOrder = append(newOrder, newEntry) } -} - -type ephemeralBuilderBuildpack struct { - descriptor dist.BuildpackDescriptor -} -func (b *ephemeralBuilderBuildpack) Descriptor() dist.BuildpackDescriptor { - return b.descriptor + return newOrder } -func (b *ephemeralBuilderBuildpack) Open() (io.ReadCloser, error) { - buf := &bytes.Buffer{} - if err := toml.NewEncoder(buf).Encode(b.descriptor); err != nil { - return nil, err +func isBuildpackID(bp string) bool { + if strings.HasPrefix(bp, fromBuilderPrefix+":") { + return true } - tarBuilder := archive.TarBuilder{} - ts := archive.NormalizedDateTime - tarBuilder.AddDir(fmt.Sprintf("/cnb/buildpacks/%s", b.descriptor.EscapedID()), 0777, ts) - bpDir := fmt.Sprintf("/cnb/buildpacks/%s/%s", b.descriptor.EscapedID(), b.descriptor.Info.Version) - tarBuilder.AddDir(bpDir, 0777, ts) - tarBuilder.AddFile(bpDir+"/buildpack.toml", 0777, ts, buf.Bytes()) - - return tarBuilder.Reader(), nil -} - -func appendBuildpackToOrder(group dist.OrderEntry, bpInfo dist.BuildpackInfo) dist.OrderEntry { - group.Group = append(group.Group, dist.BuildpackRef{ - BuildpackInfo: bpInfo, - Optional: false, - }) - - return group -} - -func isBuildpackID(bp string) bool { if !paths.IsURI(bp) { if _, err := os.Stat(bp); err != nil { return true @@ -486,7 +480,7 @@ func ensureBPSupport(bpPath string) (err error) { } func (c *Client) parseBuildpack(bp string) (string, string) { - parts := strings.Split(bp, "@") + parts := strings.Split(strings.TrimPrefix(bp, fromBuilderPrefix+":"), "@") if len(parts) == 2 { if parts[1] == "latest" { c.logger.Warn("@latest syntax is deprecated, will not work in future releases") @@ -499,7 +493,7 @@ func (c *Client) parseBuildpack(bp string) (string, string) { return parts[0], "" } -func (c *Client) createEphemeralBuilder(rawBuilderImage imgutil.Image, env map[string]string, group dist.OrderEntry, buildpacks []dist.Buildpack) (*builder.Builder, error) { +func (c *Client) createEphemeralBuilder(rawBuilderImage imgutil.Image, env map[string]string, order dist.Order, buildpacks []dist.Buildpack) (*builder.Builder, error) { origBuilderName := rawBuilderImage.Name() bldr, err := builder.New(rawBuilderImage, fmt.Sprintf("pack.local/builder/%x:latest", randString(10))) if err != nil { @@ -512,9 +506,9 @@ func (c *Client) createEphemeralBuilder(rawBuilderImage imgutil.Image, env map[s c.logger.Debugf("Adding buildpack %s version %s to builder", style.Symbol(bpInfo.ID), style.Symbol(bpInfo.Version)) bldr.AddBuildpack(bp) } - if len(group.Group) > 0 { + if len(order) > 0 && len(order[0].Group) > 0 { c.logger.Debug("Setting custom order") - bldr.SetOrder([]dist.OrderEntry{group}) + bldr.SetOrder(order) } if err := bldr.Save(c.logger); err != nil { diff --git a/build_test.go b/build_test.go index a4210d951..b380dd755 100644 --- a/build_test.go +++ b/build_test.go @@ -76,7 +76,11 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { builder.Metadata{ Buildpacks: []builder.BuildpackMetadata{ { - BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.id", Version: "buildpack.version"}, + BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"}, + Latest: true, + }, + { + BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"}, Latest: true, }, }, @@ -96,15 +100,15 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }, }, API: builder.LifecycleAPI{ - BuildpackVersion: api.MustParse("0.2"), + BuildpackVersion: api.MustParse("0.3"), PlatformVersion: api.MustParse("0.2"), }, }, }, dist.BuildpackLayers{ - "buildpack.id": { - "buildpack.version": { - API: api.MustParse("0.2"), + "buildpack.1.id": { + "buildpack.1.version": { + API: api.MustParse("0.3"), Stacks: []dist.Stack{ { ID: defaultBuilderStackID, @@ -113,7 +117,33 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }, }, }, + "buildpack.2.id": { + "buildpack.2.version": { + API: api.MustParse("0.3"), + Stacks: []dist.Stack{ + { + ID: defaultBuilderStackID, + Mixins: []string{"mixinX", "build:mixinY"}, + }, + }, + }, + }, }, + dist.Order{{ + Group: []dist.BuildpackRef{{ + BuildpackInfo: dist.BuildpackInfo{ + ID: "buildpack.1.id", + Version: "buildpack.1.version", + }, + }}, + }, { + Group: []dist.BuildpackRef{{ + BuildpackInfo: dist.BuildpackInfo{ + ID: "buildpack.2.id", + Version: "buildpack.2.version", + }, + }}, + }}, ) h.AssertNil(t, defaultBuilderImage.SetLabel("io.buildpacks.stack.mixins", `["mixinA", "build:mixinB", "mixinX", "build:mixinY"]`)) fakeImageFetcher.LocalImages[defaultBuilderImage.Name()] = defaultBuilderImage @@ -373,6 +403,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }, }, nil, + nil, ) fakeImageFetcher.LocalImages[customBuilderImage.Name()] = customBuilderImage @@ -550,186 +581,329 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }) when("Buildpacks option", func() { - var ( - additionalBuildpackPath string - additionalBuildpackInfo dist.BuildpackInfo - ) - - it.Before(func() { - additionalBuildpackInfo = dist.BuildpackInfo{ - ID: "buildpack.add.1.id", - Version: "buildpack.add.1.version", - } - - buildpack, err := ifakes.NewFakeBuildpackBlob(dist.BuildpackDescriptor{ - API: api.MustParse("0.2"), - Info: additionalBuildpackInfo, - Stacks: []dist.Stack{{ID: defaultBuilderStackID}}, - Order: nil, - }, 0777) - h.AssertNil(t, err) - - tempFile, err := ioutil.TempFile(tmpDir, "additional-bp-*.tar") - h.AssertNil(t, err) - defer tempFile.Close() + assertOrderEquals := func(content string) { + t.Helper() - reader, err := buildpack.Open() + orderLayer, err := defaultBuilderImage.FindLayerWithPath("/cnb/order.toml") h.AssertNil(t, err) - - _, err = io.Copy(tempFile, reader) - h.AssertNil(t, err) - - additionalBuildpackPath = tempFile.Name() - }) + h.AssertOnTarEntry(t, orderLayer, "/cnb/order.toml", h.ContentEquals(content)) + } it("builder order is overwritten", func() { + additionalBP := createBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + API: api.MustParse("0.3"), + Info: dist.BuildpackInfo{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", + }, + Stacks: []dist.Stack{{ID: defaultBuilderStackID}}, + Order: nil, + }) + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ Image: "some/app", Builder: builderName, ClearCache: true, - Buildpacks: []string{additionalBuildpackPath}, + Buildpacks: []string{additionalBP}, })) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) - h.AssertNil(t, err) - h.AssertEq(t, bldr.Order(), dist.Order{ - {Group: []dist.BuildpackRef{ - {BuildpackInfo: additionalBuildpackInfo}, - }}, + + assertOrderEquals(`[[order]] + + [[order.group]] + id = "buildpack.add.1.id" + version = "buildpack.add.1.version" +`) + }) + + when("id - no version is provided", func() { + it("resolves version", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: builderName, + ClearCache: true, + Buildpacks: []string{"buildpack.1.id"}, + })) + h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) + + assertOrderEquals(`[[order]] + + [[order.group]] + id = "buildpack.1.id" + version = "buildpack.1.version" +`) + }) + }) + + when("id@latest", func() { + it("resolves version and prints a warning", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: builderName, + ClearCache: true, + Buildpacks: []string{"buildpack.1.id@latest"}, + })) + h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) + h.AssertContains(t, outBuf.String(), "Warning: @latest syntax is deprecated, will not work in future releases") + + assertOrderEquals(`[[order]] + + [[order.group]] + id = "buildpack.1.id" + version = "buildpack.1.version" +`) }) }) - when("buildpack `from-builder=all` is set first", func() { + when("from=builder:id@version", func() { it("builder order is prepended", func() { h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ Image: "some/app", Builder: builderName, ClearCache: true, Buildpacks: []string{ - "from-builder=all", - additionalBuildpackPath, + "from=builder:buildpack.1.id@buildpack.1.version", }, })) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) - h.AssertNil(t, err) - h.AssertEq(t, bldr.Order(), dist.Order{{Group: []dist.BuildpackRef{ - { - BuildpackInfo: dist.BuildpackInfo{ - ID: "io.buildpacks.builder-buildpacks", - Version: "embedded", - }, - Optional: false, + + assertOrderEquals(`[[order]] + + [[order.group]] + id = "buildpack.1.id" + version = "buildpack.1.version" +`) + }) + }) + + when("from=builder is set first", func() { + it("builder order is prepended", func() { + additionalBP1 := createBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + API: api.MustParse("0.3"), + Info: dist.BuildpackInfo{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", }, - { - BuildpackInfo: additionalBuildpackInfo, - Optional: false, + Stacks: []dist.Stack{{ID: defaultBuilderStackID}}, + Order: nil, + }) + + additionalBP2 := createBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + API: api.MustParse("0.3"), + Info: dist.BuildpackInfo{ + ID: "buildpack.add.2.id", + Version: "buildpack.add.2.version", }, - }}}) - - h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{ - {BuildpackInfo: dist.BuildpackInfo{ - ID: "buildpack.id", - Version: "buildpack.version", - }, Latest: true}, - {BuildpackInfo: dist.BuildpackInfo{ - ID: "io.buildpacks.builder-buildpacks", - Version: "embedded", - }, Latest: true}, - {BuildpackInfo: additionalBuildpackInfo, Latest: true}, + Stacks: []dist.Stack{{ID: defaultBuilderStackID}}, + Order: nil, }) - }) - }) - when("buildpack `from-builder=all` is set last", func() { - it("builder order is appended", func() { h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ Image: "some/app", Builder: builderName, ClearCache: true, Buildpacks: []string{ - additionalBuildpackPath, - "from-builder=all", + "from=builder", + additionalBP1, + additionalBP2, }, })) - h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) - h.AssertNil(t, err) - h.AssertEq(t, bldr.Order(), dist.Order{{Group: []dist.BuildpackRef{ - { - BuildpackInfo: additionalBuildpackInfo, - Optional: false, - }, { - BuildpackInfo: dist.BuildpackInfo{ - ID: "io.buildpacks.builder-buildpacks", - Version: "embedded", - }, - Optional: false, - }, - }}}) - - h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{ - {BuildpackInfo: dist.BuildpackInfo{ - ID: "buildpack.id", - Version: "buildpack.version", - }, Latest: true}, - {BuildpackInfo: additionalBuildpackInfo, Latest: true}, - {BuildpackInfo: dist.BuildpackInfo{ - ID: "io.buildpacks.builder-buildpacks", - Version: "embedded", - }, Latest: true}, - }) + + assertOrderEquals(`[[order]] + + [[order.group]] + id = "buildpack.1.id" + version = "buildpack.1.version" + + [[order.group]] + id = "buildpack.add.1.id" + version = "buildpack.add.1.version" + + [[order.group]] + id = "buildpack.add.2.id" + version = "buildpack.add.2.version" + +[[order]] + + [[order.group]] + id = "buildpack.2.id" + version = "buildpack.2.version" + + [[order.group]] + id = "buildpack.add.1.id" + version = "buildpack.add.1.version" + + [[order.group]] + id = "buildpack.add.2.id" + version = "buildpack.add.2.version" +`) }) }) - when("no version is provided", func() { - it("resolves version", func() { + when("from=builder is set in middle", func() { + it("builder order is appended", func() { + additionalBP1 := createBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + API: api.MustParse("0.3"), + Info: dist.BuildpackInfo{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", + }, + Stacks: []dist.Stack{{ID: defaultBuilderStackID}}, + Order: nil, + }) + + additionalBP2 := createBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + API: api.MustParse("0.3"), + Info: dist.BuildpackInfo{ + ID: "buildpack.add.2.id", + Version: "buildpack.add.2.version", + }, + Stacks: []dist.Stack{{ID: defaultBuilderStackID}}, + Order: nil, + }) + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ Image: "some/app", Builder: builderName, ClearCache: true, - Buildpacks: []string{"buildpack.id"}, + Buildpacks: []string{ + additionalBP1, + "from=builder", + additionalBP2, + }, })) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - orderLayer, err := defaultBuilderImage.FindLayerWithPath("/cnb/order.toml") - h.AssertNil(t, err) + assertOrderEquals(`[[order]] - h.AssertOnTarEntry(t, - orderLayer, - "/cnb/order.toml", - h.ContentEquals(`[[order]] + [[order.group]] + id = "buildpack.add.1.id" + version = "buildpack.add.1.version" [[order.group]] - id = "buildpack.id" - version = "buildpack.version" -`)) + id = "buildpack.1.id" + version = "buildpack.1.version" + + [[order.group]] + id = "buildpack.add.2.id" + version = "buildpack.add.2.version" + +[[order]] + + [[order.group]] + id = "buildpack.add.1.id" + version = "buildpack.add.1.version" + + [[order.group]] + id = "buildpack.2.id" + version = "buildpack.2.version" + + [[order.group]] + id = "buildpack.add.2.id" + version = "buildpack.add.2.version" +`) }) }) - when("latest is explicitly provided", func() { - it("resolves version and prints a warning", func() { + when("from=builder is set last", func() { + it("builder order is appended", func() { + additionalBP1 := createBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + API: api.MustParse("0.3"), + Info: dist.BuildpackInfo{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", + }, + Stacks: []dist.Stack{{ID: defaultBuilderStackID}}, + Order: nil, + }) + + additionalBP2 := createBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + API: api.MustParse("0.3"), + Info: dist.BuildpackInfo{ + ID: "buildpack.add.2.id", + Version: "buildpack.add.2.version", + }, + Stacks: []dist.Stack{{ID: defaultBuilderStackID}}, + Order: nil, + }) + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ Image: "some/app", Builder: builderName, ClearCache: true, - Buildpacks: []string{"buildpack.id@latest"}, + Buildpacks: []string{ + additionalBP1, + additionalBP2, + "from=builder", + }, })) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - h.AssertContains(t, outBuf.String(), "Warning: @latest syntax is deprecated, will not work in future releases") - orderLayer, err := defaultBuilderImage.FindLayerWithPath("/cnb/order.toml") - h.AssertNil(t, err) + assertOrderEquals(`[[order]] + + [[order.group]] + id = "buildpack.add.1.id" + version = "buildpack.add.1.version" + + [[order.group]] + id = "buildpack.add.2.id" + version = "buildpack.add.2.version" - h.AssertOnTarEntry(t, - orderLayer, - "/cnb/order.toml", - h.ContentEquals(`[[order]] + [[order.group]] + id = "buildpack.1.id" + version = "buildpack.1.version" + +[[order]] [[order.group]] - id = "buildpack.id" - version = "buildpack.version" -`)) + id = "buildpack.add.1.id" + version = "buildpack.add.1.version" + + [[order.group]] + id = "buildpack.add.2.id" + version = "buildpack.add.2.version" + + [[order.group]] + id = "buildpack.2.id" + version = "buildpack.2.version" +`) + }) + }) + + when("meta-buildpack is used", func() { + it("resolves buildpack from builder", func() { + buildpackTar := createBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + API: api.MustParse("0.3"), + Info: dist.BuildpackInfo{ + ID: "metabuildpack.id", + Version: "metabuildpack.version", + }, + Stacks: nil, + Order: dist.Order{{ + Group: []dist.BuildpackRef{{ + BuildpackInfo: dist.BuildpackInfo{ + ID: "buildpack.1.id", + Version: "buildpack.1.version", + }, + Optional: false, + }, { + BuildpackInfo: dist.BuildpackInfo{ + ID: "buildpack.2.id", + Version: "buildpack.2.version", + }, + Optional: false, + }}, + }}, + }) + + err := subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: builderName, + ClearCache: true, + Buildpacks: []string{buildpackTar}, + }) + + h.AssertNil(t, err) }) }) @@ -766,7 +940,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { Builder: builderName, ClearCache: true, Buildpacks: []string{ - "buildid@buildpack.version", + "buildid@buildpack.1.version", filepath.Join("testdata", "buildpack"), }, }) @@ -780,7 +954,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { Builder: builderName, ClearCache: true, Buildpacks: []string{ - "buildpack.id@buildpack.version", + "buildpack.1.id@buildpack.1.version", buildpackTgz, }, }) @@ -791,15 +965,22 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.BuildpackRef{ - {BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.id", Version: "buildpack.version"}}, + {BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"}}, {BuildpackInfo: dist.BuildpackInfo{ID: "some-other-buildpack-id", Version: "some-other-buildpack-version"}}, }}, }) h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{ { BuildpackInfo: dist.BuildpackInfo{ - ID: "buildpack.id", - Version: "buildpack.version", + ID: "buildpack.1.id", + Version: "buildpack.1.version", + }, + Latest: true, + }, + { + BuildpackInfo: dist.BuildpackInfo{ + ID: "buildpack.2.id", + Version: "buildpack.2.version", }, Latest: true, }, @@ -825,7 +1006,8 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { Builder: builderName, ClearCache: true, Buildpacks: []string{ - "buildpack.id@buildpack.version", + "buildpack.1.id@buildpack.1.version", + "buildpack.2.id@buildpack.2.version", filepath.Join("testdata", "buildpack"), buildpackTgz, }, @@ -835,18 +1017,21 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) - buildpackInfo := dist.BuildpackInfo{ID: "buildpack.id", Version: "buildpack.version"} + buildpack1Info := dist.BuildpackInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"} + buildpack2Info := dist.BuildpackInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"} dirBuildpackInfo := dist.BuildpackInfo{ID: "bp.one", Version: "1.2.3"} tgzBuildpackInfo := dist.BuildpackInfo{ID: "some-other-buildpack-id", Version: "some-other-buildpack-version"} h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.BuildpackRef{ - {BuildpackInfo: buildpackInfo}, + {BuildpackInfo: buildpack1Info}, + {BuildpackInfo: buildpack2Info}, {BuildpackInfo: dirBuildpackInfo}, {BuildpackInfo: tgzBuildpackInfo}, }}, }) h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{ - {BuildpackInfo: buildpackInfo, Latest: true}, + {BuildpackInfo: buildpack1Info, Latest: true}, + {BuildpackInfo: buildpack2Info, Latest: true}, {BuildpackInfo: dirBuildpackInfo, Latest: true}, {BuildpackInfo: tgzBuildpackInfo, Latest: true}, }) @@ -873,7 +1058,8 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { Builder: builderName, ClearCache: true, Buildpacks: []string{ - "buildpack.id@buildpack.version", + "buildpack.1.id@buildpack.1.version", + "buildpack.2.id@buildpack.2.version", server.URL(), }, }) @@ -884,12 +1070,14 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.BuildpackRef{ - {BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.id", Version: "buildpack.version"}}, + {BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"}}, + {BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"}}, {BuildpackInfo: dist.BuildpackInfo{ID: "some-other-buildpack-id", Version: "some-other-buildpack-version"}}, }}, }) h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{ - {BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.id", Version: "buildpack.version"}, Latest: true}, + {BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"}, Latest: true}, + {BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"}, Latest: true}, {BuildpackInfo: dist.BuildpackInfo{ID: "some-other-buildpack-id", Version: "some-other-buildpack-version"}, Latest: true}, }) }) @@ -1135,12 +1323,13 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }, }, API: builder.LifecycleAPI{ - BuildpackVersion: api.MustParse("0.2"), + BuildpackVersion: api.MustParse("0.3"), PlatformVersion: api.MustParse("0.9"), }, }, }, nil, + nil, ) fakeImageFetcher.LocalImages[incompatibleBuilderImage.Name()] = incompatibleBuilderImage @@ -1193,7 +1382,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { Builder: builderName, }) - h.AssertError(t, err, "validating stack mixins: buildpack 'buildpack.id@buildpack.version' requires missing mixin(s): build:mixinY, mixinX, run:mixinZ") + h.AssertError(t, err, "validating stack mixins: buildpack 'buildpack.1.id@buildpack.1.version' requires missing mixin(s): build:mixinY, mixinX, run:mixinZ") }) }) }) @@ -1231,3 +1420,20 @@ func tarFileContents(t *testing.T, tarfile, path string) (exist bool, contents s } return false, "" } + +func createBuildpackTar(t *testing.T, tmpDir string, descriptor dist.BuildpackDescriptor) string { + buildpack, err := ifakes.NewFakeBuildpackBlob(descriptor, 0777) + h.AssertNil(t, err) + + tempFile, err := ioutil.TempFile(tmpDir, "bp-*.tar") + h.AssertNil(t, err) + defer tempFile.Close() + + reader, err := buildpack.Open() + h.AssertNil(t, err) + + _, err = io.Copy(tempFile, reader) + h.AssertNil(t, err) + + return tempFile.Name() +} diff --git a/create_builder_test.go b/create_builder_test.go index 35fc1a279..8a0726cbd 100644 --- a/create_builder_test.go +++ b/create_builder_test.go @@ -501,7 +501,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { mockImageFactory.EXPECT().NewImage(packageImage.Name(), false).Return(packageImage, nil) bpd := dist.BuildpackDescriptor{ - API: api.MustParse("0.2"), + API: api.MustParse("0.3"), Info: dist.BuildpackInfo{ID: "some.pkg.bp", Version: "2.3.4"}, Stacks: []dist.Stack{{ID: "some.stack.id"}}, } diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index a1d52bcf8..94a4b0d6c 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -685,7 +685,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { assertImageHasBPLayer(t, baseImage, bp1v1) assertImageHasBPLayer(t, baseImage, bp1v2) assertImageHasBPLayer(t, baseImage, bp2v1) - assertImageHasBPLayer(t, baseImage, bpOrder) + assertImageHasOrderBpLayer(t, baseImage, bpOrder) }) it("adds the buildpack metadata", func() { @@ -1087,3 +1087,19 @@ func assertImageHasBPLayer(t *testing.T, image *fakes.Image, bp dist.Buildpack) h.ContentEquals("detect-contents"), ) } + +func assertImageHasOrderBpLayer(t *testing.T, image *fakes.Image, bp dist.Buildpack) { + t.Helper() + + dirPath := fmt.Sprintf("/cnb/buildpacks/%s/%s", bp.Descriptor().Info.ID, bp.Descriptor().Info.Version) + layerTar, err := image.FindLayerWithPath(dirPath) + h.AssertNil(t, err) + + h.AssertOnTarEntry(t, layerTar, dirPath, + h.IsDirectory(), + ) + + h.AssertOnTarEntry(t, layerTar, path.Dir(dirPath), + h.IsDirectory(), + ) +} diff --git a/internal/buildpackage/builder_test.go b/internal/buildpackage/builder_test.go index 338240a16..51d578e3e 100644 --- a/internal/buildpackage/builder_test.go +++ b/internal/buildpackage/builder_test.go @@ -378,6 +378,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) buildpackExists := func(name, version string) { + t.Helper() dirPath := fmt.Sprintf("/cnb/buildpacks/%s/%s", name, version) layerTar, err := fakePackageImage.FindLayerWithPath(dirPath) h.AssertNil(t, err) diff --git a/internal/fakes/fake_buildpack.go b/internal/fakes/fake_buildpack.go index 5e454c535..ea635d651 100644 --- a/internal/fakes/fake_buildpack.go +++ b/internal/fakes/fake_buildpack.go @@ -49,9 +49,12 @@ func (b *fakeBuildpack) Open() (io.ReadCloser, error) { bpDir := fmt.Sprintf("/cnb/buildpacks/%s/%s", b.descriptor.EscapedID(), b.descriptor.Info.Version) tarBuilder.AddDir(bpDir, b.chmod, ts) tarBuilder.AddFile(bpDir+"/buildpack.toml", b.chmod, ts, buf.Bytes()) - tarBuilder.AddDir(bpDir+"/bin", b.chmod, ts) - tarBuilder.AddFile(bpDir+"/bin/build", b.chmod, ts, []byte("build-contents")) - tarBuilder.AddFile(bpDir+"/bin/detect", b.chmod, ts, []byte("detect-contents")) + + if len(b.descriptor.Order) == 0 { + tarBuilder.AddDir(bpDir+"/bin", b.chmod, ts) + tarBuilder.AddFile(bpDir+"/bin/build", b.chmod, ts, []byte("build-contents")) + tarBuilder.AddFile(bpDir+"/bin/detect", b.chmod, ts, []byte("detect-contents")) + } return tarBuilder.Reader(), nil } diff --git a/internal/fakes/fake_images.go b/internal/fakes/fake_images.go index 7c22f2320..169bc0572 100644 --- a/internal/fakes/fake_images.go +++ b/internal/fakes/fake_images.go @@ -1,43 +1,42 @@ package fakes import ( - "encoding/json" + "bytes" "fmt" "io" "os" "path/filepath" "testing" + "github.com/BurntSushi/toml" "github.com/buildpacks/imgutil/fakes" + "github.com/buildpacks/pack/internal/archive" "github.com/buildpacks/pack/internal/builder" "github.com/buildpacks/pack/internal/dist" h "github.com/buildpacks/pack/testhelpers" ) -func NewFakeBuilderImage(t *testing.T, tmpDir, name string, stackID, uid, gid string, metadata builder.Metadata, bpLayers dist.BuildpackLayers) *fakes.Image { +func NewFakeBuilderImage(t *testing.T, tmpDir, name string, stackID, uid, gid string, metadata builder.Metadata, bpLayers dist.BuildpackLayers, order dist.Order) *fakes.Image { fakeBuilderImage := fakes.NewImage(name, "", nil) h.AssertNil(t, fakeBuilderImage.SetLabel("io.buildpacks.stack.id", stackID)) h.AssertNil(t, fakeBuilderImage.SetEnv("CNB_USER_ID", uid)) h.AssertNil(t, fakeBuilderImage.SetEnv("CNB_GROUP_ID", gid)) - label, err := json.Marshal(&metadata) - h.AssertNil(t, err) - h.AssertNil(t, fakeBuilderImage.SetLabel("io.buildpacks.builder.metadata", string(label))) - - label, err = json.Marshal(&bpLayers) - h.AssertNil(t, err) - h.AssertNil(t, fakeBuilderImage.SetLabel("io.buildpacks.buildpack.layers", string(label))) + h.AssertNil(t, dist.SetLabel(fakeBuilderImage, "io.buildpacks.builder.metadata", metadata)) + h.AssertNil(t, dist.SetLabel(fakeBuilderImage, "io.buildpacks.buildpack.layers", bpLayers)) for bpID, v := range bpLayers { for bpVersion, bpLayerInfo := range v { + bpInfo := dist.BuildpackInfo{ + ID: bpID, + Version: bpVersion, + } + buildpack, err := NewFakeBuildpack(dist.BuildpackDescriptor{ - API: bpLayerInfo.API, - Info: dist.BuildpackInfo{ - ID: bpID, - Version: bpVersion, - }, + API: bpLayerInfo.API, + Info: bpInfo, Stacks: bpLayerInfo.Stacks, Order: bpLayerInfo.Order, }, 0755) @@ -49,9 +48,24 @@ func NewFakeBuilderImage(t *testing.T, tmpDir, name string, stackID, uid, gid st } } + h.AssertNil(t, dist.SetLabel(fakeBuilderImage, "io.buildpacks.buildpack.order", order)) + + tarBuilder := archive.TarBuilder{} + orderTomlBytes := &bytes.Buffer{} + h.AssertNil(t, toml.NewEncoder(orderTomlBytes).Encode(orderTOML{Order: order})) + tarBuilder.AddFile("/cnb/order.toml", 0777, archive.NormalizedDateTime, orderTomlBytes.Bytes()) + + orderTar := filepath.Join(tmpDir, fmt.Sprintf("order.%s.toml", h.RandString(8))) + h.AssertNil(t, tarBuilder.WriteToPath(orderTar)) + h.AssertNil(t, fakeBuilderImage.AddLayer(orderTar)) + return fakeBuilderImage } +type orderTOML struct { + Order dist.Order `toml:"order"` +} + func createBuildpackTar(t *testing.T, tmpDir string, buildpack dist.Buildpack) string { f, err := os.Create(filepath.Join(tmpDir, fmt.Sprintf( "%s.%s.tar", diff --git a/testdata/buildpack/buildpack.toml b/testdata/buildpack/buildpack.toml index bd340a94b..366fa46b3 100644 --- a/testdata/buildpack/buildpack.toml +++ b/testdata/buildpack/buildpack.toml @@ -1,4 +1,4 @@ -api = "0.2" +api = "0.3" [buildpack] id = "bp.one" diff --git a/testdata/buildpack2/buildpack.toml b/testdata/buildpack2/buildpack.toml index 4ecc1362a..61a175d81 100644 --- a/testdata/buildpack2/buildpack.toml +++ b/testdata/buildpack2/buildpack.toml @@ -1,4 +1,4 @@ -api = "0.2" +api = "0.3" [buildpack] id = "some-other-buildpack-id" diff --git a/testdata/lifecycle-platform-0.1/lifecycle.toml b/testdata/lifecycle-platform-0.1/lifecycle.toml index 6c5d87299..a04d09398 100644 --- a/testdata/lifecycle-platform-0.1/lifecycle.toml +++ b/testdata/lifecycle-platform-0.1/lifecycle.toml @@ -1,6 +1,6 @@ [api] platform = "0.1" - buildpack = "0.2" + buildpack = "0.3" [lifecycle] version = "3.4.5" diff --git a/testdata/lifecycle/lifecycle.toml b/testdata/lifecycle/lifecycle.toml index 8dbd0b379..2518f16ad 100644 --- a/testdata/lifecycle/lifecycle.toml +++ b/testdata/lifecycle/lifecycle.toml @@ -1,6 +1,6 @@ [api] platform = "0.2" - buildpack = "0.2" + buildpack = "0.3" [lifecycle] version = "3.4.5" \ No newline at end of file From a0d0725ae33d9972a8a6ed6b891ee5820ca78edd Mon Sep 17 00:00:00 2001 From: Javier Romero Date: Mon, 13 Jan 2020 09:10:04 -0800 Subject: [PATCH 3/3] Fix flakiness in tests by sorting output of method Signed-off-by: Javier Romero --- build.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/build.go b/build.go index cd89a5794..a2af516e0 100644 --- a/build.go +++ b/build.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "runtime" + "sort" "strings" "github.com/buildpacks/imgutil" @@ -233,6 +234,8 @@ func assembleAvailableMixins(buildMixins, runMixins []string) []string { return append(common, append(buildOnly, runOnly...)...) } +// allBuildpacks aggregates all buildpacks declared on the image with additional buildpacks passed in. They are sorted +// by ID then Version. func allBuildpacks(builderImage imgutil.Image, additionalBuildpacks []dist.Buildpack) ([]dist.BuildpackDescriptor, error) { var all []dist.BuildpackDescriptor var bpLayers dist.BuildpackLayers @@ -255,6 +258,14 @@ func allBuildpacks(builderImage imgutil.Image, additionalBuildpacks []dist.Build for _, bp := range additionalBuildpacks { all = append(all, bp.Descriptor()) } + + sort.Slice(all, func(i, j int) bool { + if all[i].Info.ID != all[j].Info.ID { + return all[i].Info.ID < all[j].Info.ID + } + return all[i].Info.Version < all[j].Info.Version + }) + return all, nil }