From d8ea58810b45ece0c3ff810937b0873c9f4976d5 Mon Sep 17 00:00:00 2001 From: Pratham Agarwal Date: Thu, 11 Jan 2024 14:18:22 +0530 Subject: [PATCH 1/6] resolve issue #1964 Signed-off-by: Pratham Agarwal --- internal/builder/builder.go | 17 ++++++++++++----- pkg/client/build.go | 19 +++++++++++-------- pkg/client/build_test.go | 24 ++++++++++++------------ pkg/client/create_builder.go | 3 ++- pkg/client/create_builder_test.go | 2 +- 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 88330845f..757a349e7 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -107,16 +107,18 @@ type options struct { } // FromImage constructs a builder from a builder image -func FromImage(img imgutil.Image) (*Builder, error) { - return constructBuilder(img, "", true) +func FromImage(img imgutil.Image, runImage string, registry string) (*Builder, error) { + return constructBuilder(img, runImage, registry, "", true) } // New constructs a new builder from a base image -func New(baseImage imgutil.Image, name string, ops ...BuilderOption) (*Builder, error) { - return constructBuilder(baseImage, name, false, ops...) +func New(baseImage imgutil.Image, runImage string, registry string, name string, ops ...BuilderOption) (*Builder, error) { + + return constructBuilder(baseImage, runImage, registry, name, false, ops...) } -func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, ops ...BuilderOption) (*Builder, error) { +func constructBuilder(img imgutil.Image, runImage string, registry string, newName string, errOnMissingLabel bool, ops ...BuilderOption) (*Builder, error) { + var metadata Metadata if ok, err := dist.GetLabel(img, metadataLabel, &metadata); err != nil { return nil, errors.Wrapf(err, "getting label %s", metadataLabel) @@ -147,6 +149,11 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, } } + if registry != "" && runImage != "" { + metadata.RunImages[0].Image = registry + "/" + runImage + metadata.Stack.RunImage.Image = registry + "/" + runImage + } + bldr := &Builder{ baseImageName: img.Name(), image: img, diff --git a/pkg/client/build.go b/pkg/client/build.go index 5d63317c1..cdf975668 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -319,7 +319,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { if err != nil { return errors.Wrapf(err, "invalid builder '%s'", opts.Builder) } - + rawBuilderImage, err := c.imageFetcher.Fetch(ctx, builderRef.Name(), image.FetchOptions{Daemon: true, PullPolicy: opts.PullPolicy}) if err != nil { return errors.Wrapf(err, "failed to fetch builder image '%s'", builderRef.Name()) @@ -334,8 +334,8 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { if err != nil { return errors.Wrapf(err, "getting builder architecture") } - - bldr, err := c.getBuilder(rawBuilderImage) + + bldr, err := c.getBuilder(rawBuilderImage,opts.RunImage,imgRegistry) if err != nil { return errors.Wrapf(err, "invalid builder %s", style.Symbol(opts.Builder)) } @@ -460,7 +460,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { buildEnvs[k] = v } - ephemeralBuilder, err := c.createEphemeralBuilder(rawBuilderImage, buildEnvs, order, fetchedBPs, orderExtensions, fetchedExs, usingPlatformAPI.LessThan("0.12")) + ephemeralBuilder, err := c.createEphemeralBuilder(rawBuilderImage, buildEnvs, order, fetchedBPs, orderExtensions, fetchedExs, usingPlatformAPI.LessThan("0.12"),opts.RunImage,imgRegistry) if err != nil { return err } @@ -552,7 +552,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { Layout: opts.Layout(), Keychain: c.keychain, } - + // fmt.Printf("%v",projectMetadata) switch { case useCreator: lifecycleOpts.UseCreator = true @@ -776,11 +776,12 @@ func (c *Client) processBuilderName(builderName string) (name.Reference, error) return name.ParseReference(builderName, name.WeakValidation) } -func (c *Client) getBuilder(img imgutil.Image) (*builder.Builder, error) { - bldr, err := builder.FromImage(img) +func (c *Client) getBuilder(img imgutil.Image,runImage string,registry string) (*builder.Builder, error) { + bldr, err := builder.FromImage(img,runImage,registry) if err != nil { return nil, err } + if bldr.Stack().RunImage.Image == "" && len(bldr.RunImages()) == 0 { return nil, errors.New("builder metadata is missing run-image") } @@ -1328,9 +1329,11 @@ func (c *Client) createEphemeralBuilder( orderExtensions dist.Order, extensions []buildpack.BuildModule, validateMixins bool, + runImage string, + registry string, ) (*builder.Builder, error) { origBuilderName := rawBuilderImage.Name() - bldr, err := builder.New(rawBuilderImage, fmt.Sprintf("pack.local/builder/%x:latest", randString(10))) + bldr, err := builder.New(rawBuilderImage,runImage,registry, fmt.Sprintf("pack.local/builder/%x:latest", randString(10))) if err != nil { return nil, errors.Wrapf(err, "invalid builder %s", style.Symbol(origBuilderName)) } diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 27cd02d23..329e57019 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -1048,7 +1048,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) buildpack1Info := dist.ModuleInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"} @@ -1217,7 +1217,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1338,7 +1338,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) buildpack1Info := dist.ModuleInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"} buildpack2Info := dist.ModuleInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"} @@ -1388,7 +1388,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1420,7 +1420,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1452,7 +1452,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1489,7 +1489,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1529,7 +1529,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1571,7 +1571,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1657,7 +1657,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1693,7 +1693,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1869,7 +1869,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage) + bldr, err := builder.FromImage(defaultBuilderImage,"","") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index ee1062bc4..a4f7d6516 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -157,11 +157,12 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption if opts.Flatten != nil && len(opts.Flatten.FlattenModules()) > 0 { builderOpts = append(builderOpts, builder.WithFlattened(opts.Flatten)) } + if opts.Labels != nil && len(opts.Labels) > 0 { builderOpts = append(builderOpts, builder.WithLabels(opts.Labels)) } - bldr, err := builder.New(baseImage, opts.BuilderName, builderOpts...) + bldr, err := builder.New(baseImage, "", "", opts.BuilderName, builderOpts...) if err != nil { return nil, errors.Wrap(err, "invalid build-image") } diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index 7c51fa309..46eb91cf6 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -200,7 +200,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) h.AssertEq(t, fakeBuildImage.IsSaved(), true) - bldr, err := builder.FromImage(fakeBuildImage) + bldr, err := builder.FromImage(fakeBuildImage,"","") h.AssertNil(t, err) return bldr From e778646e64e35eddc411b9d25db3575e5348b310 Mon Sep 17 00:00:00 2001 From: Pratham Agarwal Date: Tue, 16 Jan 2024 23:37:45 +0530 Subject: [PATCH 2/6] fix:fixed tests Signed-off-by: Pratham Agarwal --- internal/builder/builder.go | 2 -- internal/builder/builder_test.go | 40 +++++++++++++++---------------- pkg/client/build.go | 16 ++++++------- pkg/client/build_test.go | 24 +++++++++---------- pkg/client/create_builder_test.go | 2 +- 5 files changed, 41 insertions(+), 43 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 757a349e7..98f9befab 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -113,12 +113,10 @@ func FromImage(img imgutil.Image, runImage string, registry string) (*Builder, e // New constructs a new builder from a base image func New(baseImage imgutil.Image, runImage string, registry string, name string, ops ...BuilderOption) (*Builder, error) { - return constructBuilder(baseImage, runImage, registry, name, false, ops...) } func constructBuilder(img imgutil.Image, runImage string, registry string, newName string, errOnMissingLabel bool, ops ...BuilderOption) (*Builder, error) { - var metadata Metadata if ok, err := dist.GetLabel(img, metadataLabel, &metadata); err != nil { return nil, errors.Wrapf(err, "getting label %s", metadataLabel) diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 80d39d03f..df2317df8 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -179,7 +179,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { `{"something-random": ,}`, )) - _, err := builder.FromImage(baseImage) + _, err := builder.FromImage(baseImage, "", "") h.AssertError(t, err, "getting label") }) }) @@ -193,14 +193,14 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { `{"something-random": ,}`, )) - _, err := builder.FromImage(baseImage) + _, err := builder.FromImage(baseImage, "", "") h.AssertError(t, err, "getting label") }) }) when("missing CNB_USER_ID", func() { it("returns an error", func() { - _, err := builder.New(baseImage, "some/builder") + _, err := builder.New(baseImage, "some/builder", "", "") h.AssertError(t, err, "image 'base/image' missing required env var 'CNB_USER_ID'") }) }) @@ -211,7 +211,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("returns an error", func() { - _, err := builder.New(baseImage, "some/builder") + _, err := builder.New(baseImage, "some/builder", "", "") h.AssertError(t, err, "image 'base/image' missing required env var 'CNB_GROUP_ID'") }) }) @@ -223,7 +223,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("returns an error", func() { - _, err := builder.New(baseImage, "some/builder") + _, err := builder.New(baseImage, "some/builder", "", "") h.AssertError(t, err, "failed to parse 'CNB_USER_ID', value 'not an int' should be an integer") }) }) @@ -235,7 +235,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("returns an error", func() { - _, err := builder.New(baseImage, "some/builder") + _, err := builder.New(baseImage, "some/builder", "", "") h.AssertError(t, err, "failed to parse 'CNB_GROUP_ID', value 'not an int' should be an integer") }) }) @@ -247,7 +247,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("does not return an error", func() { - _, err := builder.New(baseImage, "some/builder") + _, err := builder.New(baseImage, "some/builder", "", "") h.AssertNilE(t, err) }) }) @@ -261,7 +261,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("returns an error", func() { h.AssertNil(t, baseImage.SetLabel("io.buildpacks.stack.mixins", `{"mixinX", "mixinY", "build:mixinA"}`)) - _, err := builder.New(baseImage, "some/builder") + _, err := builder.New(baseImage, "some/builder", "", "") h.AssertError(t, err, "getting label io.buildpacks.stack.mixins") }) }) @@ -275,7 +275,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("returns an error", func() { h.AssertNil(t, baseImage.SetLabel("io.buildpacks.buildpack.order", `{"something", }`)) - _, err := builder.New(baseImage, "some/builder") + _, err := builder.New(baseImage, "some/builder", "", "") h.AssertError(t, err, "getting label io.buildpacks.buildpack.order") }) }) @@ -289,7 +289,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, baseImage.SetEnv("CNB_GROUP_ID", "4321")) h.AssertNil(t, baseImage.SetLabel("io.buildpacks.stack.id", "some.stack.id")) h.AssertNil(t, baseImage.SetLabel("io.buildpacks.stack.mixins", `["mixinX", "mixinY", "build:mixinA"]`)) - subject, err = builder.New(baseImage, "some/builder") + subject, err = builder.New(baseImage, "some/builder", "", "") h.AssertNil(t, err) subject.SetLifecycle(mockLifecycle) @@ -872,7 +872,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it.Before(func() { var err error fakeLayerImage = &h.FakeAddedLayerImage{Image: baseImage} - subject, err = builder.New(fakeLayerImage, "some/builder") + subject, err = builder.New(fakeLayerImage, "some/builder", "", "") h.AssertNil(t, err) subject.SetLifecycle(mockLifecycle) @@ -1105,7 +1105,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "some/builder") + subject, err = builder.New(baseImage, "some/builder", "", "") h.AssertNil(t, err) subject.AddBuildpack(bp1v2) @@ -1190,7 +1190,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "some/builder") + subject, err = builder.New(baseImage, "some/builder", "", "") h.AssertNil(t, err) subject.AddBuildpack(bp1v1) @@ -1296,7 +1296,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "some/builder") + subject, err = builder.New(baseImage, "some/builder", "", "") h.AssertNil(t, err) subject.AddExtension(ext1v2) @@ -1375,7 +1375,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "some/builder") + subject, err = builder.New(baseImage, "some/builder", "", "") h.AssertNil(t, err) subject.AddExtension(ext1v1) @@ -1752,7 +1752,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it.Before(func() { var err error - bldr, err = builder.FromImage(builderImage) + bldr, err = builder.FromImage(builderImage, "", "") h.AssertNil(t, err) }) @@ -1784,7 +1784,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("should error", func() { - _, err := builder.FromImage(builderImage) + _, err := builder.FromImage(builderImage, "", "") h.AssertError(t, err, "missing label 'io.buildpacks.builder.metadata'") }) }) @@ -1876,7 +1876,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("buildpacks to be flattened are NOT defined", func() { it.Before(func() { var err error - bldr, err = builder.New(builderImage, "some-builder") + bldr, err = builder.New(builderImage, "some-builder", "", "") h.AssertNil(t, err) // Let's add the buildpacks @@ -1910,7 +1910,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { flattenModules, err := buildpack.ParseFlattenBuildModules([]string{"buildpack-1-id@buildpack-1-version-1,buildpack-1-id@buildpack-1-version-2,buildpack-2-id@buildpack-2-version-1"}) h.AssertNil(t, err) - bldr, err = builder.New(builderImage, "some-builder", builder.WithFlattened(flattenModules)) + bldr, err = builder.New(builderImage, "some-builder", "", "", builder.WithFlattened(flattenModules)) h.AssertNil(t, err) // Let's add the buildpacks @@ -1958,7 +1958,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("should set labels to the image", func() { customLabels = map[string]string{"test.label.one": "1", "test.label.two": "2"} - subject, err = builder.New(baseImage, "some/builder", builder.WithLabels(customLabels)) + subject, err = builder.New(baseImage, "some/builder", "", "", builder.WithLabels(customLabels)) h.AssertNil(t, err) imageLabels, err = baseImage.Labels() diff --git a/pkg/client/build.go b/pkg/client/build.go index cdf975668..ae3181fe0 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -319,7 +319,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { if err != nil { return errors.Wrapf(err, "invalid builder '%s'", opts.Builder) } - + rawBuilderImage, err := c.imageFetcher.Fetch(ctx, builderRef.Name(), image.FetchOptions{Daemon: true, PullPolicy: opts.PullPolicy}) if err != nil { return errors.Wrapf(err, "failed to fetch builder image '%s'", builderRef.Name()) @@ -334,8 +334,8 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { if err != nil { return errors.Wrapf(err, "getting builder architecture") } - - bldr, err := c.getBuilder(rawBuilderImage,opts.RunImage,imgRegistry) + + bldr, err := c.getBuilder(rawBuilderImage, opts.RunImage, imgRegistry) if err != nil { return errors.Wrapf(err, "invalid builder %s", style.Symbol(opts.Builder)) } @@ -460,7 +460,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { buildEnvs[k] = v } - ephemeralBuilder, err := c.createEphemeralBuilder(rawBuilderImage, buildEnvs, order, fetchedBPs, orderExtensions, fetchedExs, usingPlatformAPI.LessThan("0.12"),opts.RunImage,imgRegistry) + ephemeralBuilder, err := c.createEphemeralBuilder(rawBuilderImage, buildEnvs, order, fetchedBPs, orderExtensions, fetchedExs, usingPlatformAPI.LessThan("0.12"), opts.RunImage, imgRegistry) if err != nil { return err } @@ -776,12 +776,12 @@ func (c *Client) processBuilderName(builderName string) (name.Reference, error) return name.ParseReference(builderName, name.WeakValidation) } -func (c *Client) getBuilder(img imgutil.Image,runImage string,registry string) (*builder.Builder, error) { - bldr, err := builder.FromImage(img,runImage,registry) +func (c *Client) getBuilder(img imgutil.Image, runImage string, registry string) (*builder.Builder, error) { + bldr, err := builder.FromImage(img, runImage, registry) if err != nil { return nil, err } - + if bldr.Stack().RunImage.Image == "" && len(bldr.RunImages()) == 0 { return nil, errors.New("builder metadata is missing run-image") } @@ -1333,7 +1333,7 @@ func (c *Client) createEphemeralBuilder( registry string, ) (*builder.Builder, error) { origBuilderName := rawBuilderImage.Name() - bldr, err := builder.New(rawBuilderImage,runImage,registry, fmt.Sprintf("pack.local/builder/%x:latest", randString(10))) + bldr, err := builder.New(rawBuilderImage, runImage, registry, fmt.Sprintf("pack.local/builder/%x:latest", randString(10))) if err != nil { return nil, errors.Wrapf(err, "invalid builder %s", style.Symbol(origBuilderName)) } diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 329e57019..d668d5b85 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -1048,7 +1048,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) buildpack1Info := dist.ModuleInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"} @@ -1217,7 +1217,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1338,7 +1338,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) buildpack1Info := dist.ModuleInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"} buildpack2Info := dist.ModuleInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"} @@ -1388,7 +1388,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1420,7 +1420,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1452,7 +1452,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1489,7 +1489,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1529,7 +1529,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1571,7 +1571,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1657,7 +1657,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1693,7 +1693,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1869,7 +1869,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage,"","") + bldr, err := builder.FromImage(defaultBuilderImage, "", "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index 46eb91cf6..489ab4433 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -200,7 +200,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) h.AssertEq(t, fakeBuildImage.IsSaved(), true) - bldr, err := builder.FromImage(fakeBuildImage,"","") + bldr, err := builder.FromImage(fakeBuildImage, "", "") h.AssertNil(t, err) return bldr From fdbbe2b209390e89c272de9be2f49c47008580ec Mon Sep 17 00:00:00 2001 From: Pratham Agarwal Date: Thu, 18 Jan 2024 01:18:30 +0530 Subject: [PATCH 3/6] fix:minor changes Signed-off-by: Pratham Agarwal --- internal/builder/builder.go | 25 ++++++++++++------- internal/builder/builder_test.go | 40 +++++++++++++++---------------- pkg/client/build.go | 9 ++++--- pkg/client/build_test.go | 24 +++++++++---------- pkg/client/create_builder.go | 2 +- pkg/client/create_builder_test.go | 2 +- 6 files changed, 55 insertions(+), 47 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 98f9befab..ef81800ca 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -104,19 +104,28 @@ type BuilderOption func(*options) error type options struct { toFlatten buildpack.FlattenModuleInfos labels map[string]string + runImage string +} + +func WithRunImage(name string) BuilderOption { + return func(o *options) error { + o.runImage = name + return nil + } } // FromImage constructs a builder from a builder image -func FromImage(img imgutil.Image, runImage string, registry string) (*Builder, error) { - return constructBuilder(img, runImage, registry, "", true) +func FromImage(img imgutil.Image, runImage string) (*Builder, error) { + return constructBuilder(img, "", true, WithRunImage(runImage)) } // New constructs a new builder from a base image -func New(baseImage imgutil.Image, runImage string, registry string, name string, ops ...BuilderOption) (*Builder, error) { - return constructBuilder(baseImage, runImage, registry, name, false, ops...) +func New(baseImage imgutil.Image, runImage string, name string, ops ...BuilderOption) (*Builder, error) { + ops = append(ops, WithRunImage(runImage)) + return constructBuilder(baseImage, name, false, ops...) } -func constructBuilder(img imgutil.Image, runImage string, registry string, newName string, errOnMissingLabel bool, ops ...BuilderOption) (*Builder, error) { +func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, ops ...BuilderOption) (*Builder, error) { var metadata Metadata if ok, err := dist.GetLabel(img, metadataLabel, &metadata); err != nil { return nil, errors.Wrapf(err, "getting label %s", metadataLabel) @@ -147,9 +156,9 @@ func constructBuilder(img imgutil.Image, runImage string, registry string, newNa } } - if registry != "" && runImage != "" { - metadata.RunImages[0].Image = registry + "/" + runImage - metadata.Stack.RunImage.Image = registry + "/" + runImage + if opts.runImage != "" { + metadata.RunImages = []RunImageMetadata{{Image: opts.runImage}} + metadata.Stack.RunImage.Image = opts.runImage } bldr := &Builder{ diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index df2317df8..70fe6e44f 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -179,7 +179,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { `{"something-random": ,}`, )) - _, err := builder.FromImage(baseImage, "", "") + _, err := builder.FromImage(baseImage, "") h.AssertError(t, err, "getting label") }) }) @@ -193,14 +193,14 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { `{"something-random": ,}`, )) - _, err := builder.FromImage(baseImage, "", "") + _, err := builder.FromImage(baseImage, "") h.AssertError(t, err, "getting label") }) }) when("missing CNB_USER_ID", func() { it("returns an error", func() { - _, err := builder.New(baseImage, "some/builder", "", "") + _, err := builder.New(baseImage, "", "some/builder") h.AssertError(t, err, "image 'base/image' missing required env var 'CNB_USER_ID'") }) }) @@ -211,7 +211,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("returns an error", func() { - _, err := builder.New(baseImage, "some/builder", "", "") + _, err := builder.New(baseImage, "", "some/builder") h.AssertError(t, err, "image 'base/image' missing required env var 'CNB_GROUP_ID'") }) }) @@ -223,7 +223,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("returns an error", func() { - _, err := builder.New(baseImage, "some/builder", "", "") + _, err := builder.New(baseImage, "", "some/builder") h.AssertError(t, err, "failed to parse 'CNB_USER_ID', value 'not an int' should be an integer") }) }) @@ -235,7 +235,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("returns an error", func() { - _, err := builder.New(baseImage, "some/builder", "", "") + _, err := builder.New(baseImage, "", "some/builder") h.AssertError(t, err, "failed to parse 'CNB_GROUP_ID', value 'not an int' should be an integer") }) }) @@ -247,7 +247,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("does not return an error", func() { - _, err := builder.New(baseImage, "some/builder", "", "") + _, err := builder.New(baseImage, "", "some/builder") h.AssertNilE(t, err) }) }) @@ -261,7 +261,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("returns an error", func() { h.AssertNil(t, baseImage.SetLabel("io.buildpacks.stack.mixins", `{"mixinX", "mixinY", "build:mixinA"}`)) - _, err := builder.New(baseImage, "some/builder", "", "") + _, err := builder.New(baseImage, "", "some/builder") h.AssertError(t, err, "getting label io.buildpacks.stack.mixins") }) }) @@ -275,7 +275,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("returns an error", func() { h.AssertNil(t, baseImage.SetLabel("io.buildpacks.buildpack.order", `{"something", }`)) - _, err := builder.New(baseImage, "some/builder", "", "") + _, err := builder.New(baseImage, "", "some/builder") h.AssertError(t, err, "getting label io.buildpacks.buildpack.order") }) }) @@ -289,7 +289,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, baseImage.SetEnv("CNB_GROUP_ID", "4321")) h.AssertNil(t, baseImage.SetLabel("io.buildpacks.stack.id", "some.stack.id")) h.AssertNil(t, baseImage.SetLabel("io.buildpacks.stack.mixins", `["mixinX", "mixinY", "build:mixinA"]`)) - subject, err = builder.New(baseImage, "some/builder", "", "") + subject, err = builder.New(baseImage, "", "some/builder") h.AssertNil(t, err) subject.SetLifecycle(mockLifecycle) @@ -872,7 +872,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it.Before(func() { var err error fakeLayerImage = &h.FakeAddedLayerImage{Image: baseImage} - subject, err = builder.New(fakeLayerImage, "some/builder", "", "") + subject, err = builder.New(fakeLayerImage, "", "some/builder") h.AssertNil(t, err) subject.SetLifecycle(mockLifecycle) @@ -1105,7 +1105,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "some/builder", "", "") + subject, err = builder.New(baseImage, "", "some/builder") h.AssertNil(t, err) subject.AddBuildpack(bp1v2) @@ -1190,7 +1190,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "some/builder", "", "") + subject, err = builder.New(baseImage, "", "some/builder") h.AssertNil(t, err) subject.AddBuildpack(bp1v1) @@ -1296,7 +1296,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "some/builder", "", "") + subject, err = builder.New(baseImage, "", "some/builder") h.AssertNil(t, err) subject.AddExtension(ext1v2) @@ -1375,7 +1375,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "some/builder", "", "") + subject, err = builder.New(baseImage, "", "some/builder") h.AssertNil(t, err) subject.AddExtension(ext1v1) @@ -1752,7 +1752,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it.Before(func() { var err error - bldr, err = builder.FromImage(builderImage, "", "") + bldr, err = builder.FromImage(builderImage, "") h.AssertNil(t, err) }) @@ -1784,7 +1784,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("should error", func() { - _, err := builder.FromImage(builderImage, "", "") + _, err := builder.FromImage(builderImage, "") h.AssertError(t, err, "missing label 'io.buildpacks.builder.metadata'") }) }) @@ -1876,7 +1876,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("buildpacks to be flattened are NOT defined", func() { it.Before(func() { var err error - bldr, err = builder.New(builderImage, "some-builder", "", "") + bldr, err = builder.New(builderImage, "", "some-builder") h.AssertNil(t, err) // Let's add the buildpacks @@ -1910,7 +1910,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { flattenModules, err := buildpack.ParseFlattenBuildModules([]string{"buildpack-1-id@buildpack-1-version-1,buildpack-1-id@buildpack-1-version-2,buildpack-2-id@buildpack-2-version-1"}) h.AssertNil(t, err) - bldr, err = builder.New(builderImage, "some-builder", "", "", builder.WithFlattened(flattenModules)) + bldr, err = builder.New(builderImage, "", "some-builder", builder.WithFlattened(flattenModules)) h.AssertNil(t, err) // Let's add the buildpacks @@ -1958,7 +1958,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("should set labels to the image", func() { customLabels = map[string]string{"test.label.one": "1", "test.label.two": "2"} - subject, err = builder.New(baseImage, "some/builder", "", "", builder.WithLabels(customLabels)) + subject, err = builder.New(baseImage, "", "some/builder", builder.WithLabels(customLabels)) h.AssertNil(t, err) imageLabels, err = baseImage.Labels() diff --git a/pkg/client/build.go b/pkg/client/build.go index ae3181fe0..356ea3867 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -335,7 +335,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return errors.Wrapf(err, "getting builder architecture") } - bldr, err := c.getBuilder(rawBuilderImage, opts.RunImage, imgRegistry) + bldr, err := c.getBuilder(rawBuilderImage, opts.RunImage) if err != nil { return errors.Wrapf(err, "invalid builder %s", style.Symbol(opts.Builder)) } @@ -552,7 +552,6 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { Layout: opts.Layout(), Keychain: c.keychain, } - // fmt.Printf("%v",projectMetadata) switch { case useCreator: lifecycleOpts.UseCreator = true @@ -776,8 +775,8 @@ func (c *Client) processBuilderName(builderName string) (name.Reference, error) return name.ParseReference(builderName, name.WeakValidation) } -func (c *Client) getBuilder(img imgutil.Image, runImage string, registry string) (*builder.Builder, error) { - bldr, err := builder.FromImage(img, runImage, registry) +func (c *Client) getBuilder(img imgutil.Image, runImage string) (*builder.Builder, error) { + bldr, err := builder.FromImage(img, runImage) if err != nil { return nil, err } @@ -1333,7 +1332,7 @@ func (c *Client) createEphemeralBuilder( registry string, ) (*builder.Builder, error) { origBuilderName := rawBuilderImage.Name() - bldr, err := builder.New(rawBuilderImage, runImage, registry, fmt.Sprintf("pack.local/builder/%x:latest", randString(10))) + bldr, err := builder.New(rawBuilderImage, runImage, fmt.Sprintf("pack.local/builder/%x:latest", randString(10))) if err != nil { return nil, errors.Wrapf(err, "invalid builder %s", style.Symbol(origBuilderName)) } diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index d668d5b85..b6c8220c2 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -1048,7 +1048,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) buildpack1Info := dist.ModuleInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"} @@ -1217,7 +1217,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1338,7 +1338,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) buildpack1Info := dist.ModuleInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"} buildpack2Info := dist.ModuleInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"} @@ -1388,7 +1388,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1420,7 +1420,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1452,7 +1452,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1489,7 +1489,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1529,7 +1529,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1571,7 +1571,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1657,7 +1657,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1693,7 +1693,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1869,7 +1869,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "", "") + bldr, err := builder.FromImage(defaultBuilderImage, "") h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index a4f7d6516..1bd4013dc 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -161,8 +161,8 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption if opts.Labels != nil && len(opts.Labels) > 0 { builderOpts = append(builderOpts, builder.WithLabels(opts.Labels)) } + bldr, err := builder.New(baseImage, "", opts.BuilderName, builderOpts...) - bldr, err := builder.New(baseImage, "", "", opts.BuilderName, builderOpts...) if err != nil { return nil, errors.Wrap(err, "invalid build-image") } diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index 489ab4433..f4f211897 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -200,7 +200,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) h.AssertEq(t, fakeBuildImage.IsSaved(), true) - bldr, err := builder.FromImage(fakeBuildImage, "", "") + bldr, err := builder.FromImage(fakeBuildImage, "") h.AssertNil(t, err) return bldr From 81e50d5600e2b63dacf2ffdf735f9e4694fa27d4 Mon Sep 17 00:00:00 2001 From: Pratham Agarwal Date: Fri, 19 Jan 2024 00:36:38 +0530 Subject: [PATCH 4/6] fix:made changes as per comments Signed-off-by: Pratham Agarwal --- internal/builder/builder.go | 7 +++--- internal/builder/builder_test.go | 40 +++++++++++++++---------------- pkg/client/build.go | 7 +++--- pkg/client/build_test.go | 24 +++++++++---------- pkg/client/create_builder.go | 2 +- pkg/client/create_builder_test.go | 2 +- 6 files changed, 40 insertions(+), 42 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index ef81800ca..4298c6135 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -115,13 +115,12 @@ func WithRunImage(name string) BuilderOption { } // FromImage constructs a builder from a builder image -func FromImage(img imgutil.Image, runImage string) (*Builder, error) { - return constructBuilder(img, "", true, WithRunImage(runImage)) +func FromImage(img imgutil.Image) (*Builder, error) { + return constructBuilder(img, "", true) } // New constructs a new builder from a base image -func New(baseImage imgutil.Image, runImage string, name string, ops ...BuilderOption) (*Builder, error) { - ops = append(ops, WithRunImage(runImage)) +func New(baseImage imgutil.Image, name string, ops ...BuilderOption) (*Builder, error) { return constructBuilder(baseImage, name, false, ops...) } diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 70fe6e44f..80d39d03f 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -179,7 +179,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { `{"something-random": ,}`, )) - _, err := builder.FromImage(baseImage, "") + _, err := builder.FromImage(baseImage) h.AssertError(t, err, "getting label") }) }) @@ -193,14 +193,14 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { `{"something-random": ,}`, )) - _, err := builder.FromImage(baseImage, "") + _, err := builder.FromImage(baseImage) h.AssertError(t, err, "getting label") }) }) when("missing CNB_USER_ID", func() { it("returns an error", func() { - _, err := builder.New(baseImage, "", "some/builder") + _, err := builder.New(baseImage, "some/builder") h.AssertError(t, err, "image 'base/image' missing required env var 'CNB_USER_ID'") }) }) @@ -211,7 +211,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("returns an error", func() { - _, err := builder.New(baseImage, "", "some/builder") + _, err := builder.New(baseImage, "some/builder") h.AssertError(t, err, "image 'base/image' missing required env var 'CNB_GROUP_ID'") }) }) @@ -223,7 +223,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("returns an error", func() { - _, err := builder.New(baseImage, "", "some/builder") + _, err := builder.New(baseImage, "some/builder") h.AssertError(t, err, "failed to parse 'CNB_USER_ID', value 'not an int' should be an integer") }) }) @@ -235,7 +235,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("returns an error", func() { - _, err := builder.New(baseImage, "", "some/builder") + _, err := builder.New(baseImage, "some/builder") h.AssertError(t, err, "failed to parse 'CNB_GROUP_ID', value 'not an int' should be an integer") }) }) @@ -247,7 +247,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("does not return an error", func() { - _, err := builder.New(baseImage, "", "some/builder") + _, err := builder.New(baseImage, "some/builder") h.AssertNilE(t, err) }) }) @@ -261,7 +261,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("returns an error", func() { h.AssertNil(t, baseImage.SetLabel("io.buildpacks.stack.mixins", `{"mixinX", "mixinY", "build:mixinA"}`)) - _, err := builder.New(baseImage, "", "some/builder") + _, err := builder.New(baseImage, "some/builder") h.AssertError(t, err, "getting label io.buildpacks.stack.mixins") }) }) @@ -275,7 +275,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("returns an error", func() { h.AssertNil(t, baseImage.SetLabel("io.buildpacks.buildpack.order", `{"something", }`)) - _, err := builder.New(baseImage, "", "some/builder") + _, err := builder.New(baseImage, "some/builder") h.AssertError(t, err, "getting label io.buildpacks.buildpack.order") }) }) @@ -289,7 +289,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, baseImage.SetEnv("CNB_GROUP_ID", "4321")) h.AssertNil(t, baseImage.SetLabel("io.buildpacks.stack.id", "some.stack.id")) h.AssertNil(t, baseImage.SetLabel("io.buildpacks.stack.mixins", `["mixinX", "mixinY", "build:mixinA"]`)) - subject, err = builder.New(baseImage, "", "some/builder") + subject, err = builder.New(baseImage, "some/builder") h.AssertNil(t, err) subject.SetLifecycle(mockLifecycle) @@ -872,7 +872,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it.Before(func() { var err error fakeLayerImage = &h.FakeAddedLayerImage{Image: baseImage} - subject, err = builder.New(fakeLayerImage, "", "some/builder") + subject, err = builder.New(fakeLayerImage, "some/builder") h.AssertNil(t, err) subject.SetLifecycle(mockLifecycle) @@ -1105,7 +1105,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "", "some/builder") + subject, err = builder.New(baseImage, "some/builder") h.AssertNil(t, err) subject.AddBuildpack(bp1v2) @@ -1190,7 +1190,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "", "some/builder") + subject, err = builder.New(baseImage, "some/builder") h.AssertNil(t, err) subject.AddBuildpack(bp1v1) @@ -1296,7 +1296,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "", "some/builder") + subject, err = builder.New(baseImage, "some/builder") h.AssertNil(t, err) subject.AddExtension(ext1v2) @@ -1375,7 +1375,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { )) var err error - subject, err = builder.New(baseImage, "", "some/builder") + subject, err = builder.New(baseImage, "some/builder") h.AssertNil(t, err) subject.AddExtension(ext1v1) @@ -1752,7 +1752,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it.Before(func() { var err error - bldr, err = builder.FromImage(builderImage, "") + bldr, err = builder.FromImage(builderImage) h.AssertNil(t, err) }) @@ -1784,7 +1784,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("should error", func() { - _, err := builder.FromImage(builderImage, "") + _, err := builder.FromImage(builderImage) h.AssertError(t, err, "missing label 'io.buildpacks.builder.metadata'") }) }) @@ -1876,7 +1876,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("buildpacks to be flattened are NOT defined", func() { it.Before(func() { var err error - bldr, err = builder.New(builderImage, "", "some-builder") + bldr, err = builder.New(builderImage, "some-builder") h.AssertNil(t, err) // Let's add the buildpacks @@ -1910,7 +1910,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { flattenModules, err := buildpack.ParseFlattenBuildModules([]string{"buildpack-1-id@buildpack-1-version-1,buildpack-1-id@buildpack-1-version-2,buildpack-2-id@buildpack-2-version-1"}) h.AssertNil(t, err) - bldr, err = builder.New(builderImage, "", "some-builder", builder.WithFlattened(flattenModules)) + bldr, err = builder.New(builderImage, "some-builder", builder.WithFlattened(flattenModules)) h.AssertNil(t, err) // Let's add the buildpacks @@ -1958,7 +1958,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("should set labels to the image", func() { customLabels = map[string]string{"test.label.one": "1", "test.label.two": "2"} - subject, err = builder.New(baseImage, "", "some/builder", builder.WithLabels(customLabels)) + subject, err = builder.New(baseImage, "some/builder", builder.WithLabels(customLabels)) h.AssertNil(t, err) imageLabels, err = baseImage.Labels() diff --git a/pkg/client/build.go b/pkg/client/build.go index 356ea3867..157c69d4a 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -460,7 +460,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { buildEnvs[k] = v } - ephemeralBuilder, err := c.createEphemeralBuilder(rawBuilderImage, buildEnvs, order, fetchedBPs, orderExtensions, fetchedExs, usingPlatformAPI.LessThan("0.12"), opts.RunImage, imgRegistry) + ephemeralBuilder, err := c.createEphemeralBuilder(rawBuilderImage, buildEnvs, order, fetchedBPs, orderExtensions, fetchedExs, usingPlatformAPI.LessThan("0.12"), opts.RunImage) if err != nil { return err } @@ -776,7 +776,7 @@ func (c *Client) processBuilderName(builderName string) (name.Reference, error) } func (c *Client) getBuilder(img imgutil.Image, runImage string) (*builder.Builder, error) { - bldr, err := builder.FromImage(img, runImage) + bldr, err := builder.FromImage(img) if err != nil { return nil, err } @@ -1329,10 +1329,9 @@ func (c *Client) createEphemeralBuilder( extensions []buildpack.BuildModule, validateMixins bool, runImage string, - registry string, ) (*builder.Builder, error) { origBuilderName := rawBuilderImage.Name() - bldr, err := builder.New(rawBuilderImage, runImage, fmt.Sprintf("pack.local/builder/%x:latest", randString(10))) + bldr, err := builder.New(rawBuilderImage, fmt.Sprintf("pack.local/builder/%x:latest", randString(10)), builder.WithRunImage(runImage)) if err != nil { return nil, errors.Wrapf(err, "invalid builder %s", style.Symbol(origBuilderName)) } diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index b6c8220c2..27cd02d23 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -1048,7 +1048,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) buildpack1Info := dist.ModuleInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"} @@ -1217,7 +1217,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1338,7 +1338,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) buildpack1Info := dist.ModuleInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"} buildpack2Info := dist.ModuleInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"} @@ -1388,7 +1388,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1420,7 +1420,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1452,7 +1452,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1489,7 +1489,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1529,7 +1529,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1571,7 +1571,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1657,7 +1657,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1693,7 +1693,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ @@ -1869,7 +1869,7 @@ api = "0.2" h.AssertNil(t, err) h.AssertEq(t, fakeLifecycle.Opts.Builder.Name(), defaultBuilderImage.Name()) - bldr, err := builder.FromImage(defaultBuilderImage, "") + bldr, err := builder.FromImage(defaultBuilderImage) h.AssertNil(t, err) h.AssertEq(t, bldr.Order(), dist.Order{ {Group: []dist.ModuleRef{ diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index 1bd4013dc..dc3bf7d55 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -161,7 +161,7 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption if opts.Labels != nil && len(opts.Labels) > 0 { builderOpts = append(builderOpts, builder.WithLabels(opts.Labels)) } - bldr, err := builder.New(baseImage, "", opts.BuilderName, builderOpts...) + bldr, err := builder.New(baseImage, opts.BuilderName, builderOpts...) if err != nil { return nil, errors.Wrap(err, "invalid build-image") diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index f4f211897..7c51fa309 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -200,7 +200,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) h.AssertEq(t, fakeBuildImage.IsSaved(), true) - bldr, err := builder.FromImage(fakeBuildImage, "") + bldr, err := builder.FromImage(fakeBuildImage) h.AssertNil(t, err) return bldr From 1122cab269bbb19035075518206c363c424ee385 Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Mon, 4 Mar 2024 14:13:30 -0500 Subject: [PATCH 5/6] - Fixing formatting issues - Adding acceptance tests Signed-off-by: Juan Bustamante --- acceptance/acceptance_test.go | 29 +++++++++++++++++++++++++++-- acceptance/assertions/image.go | 9 +++++++++ acceptance/invoke/pack.go | 4 ++++ internal/builder/builder.go | 12 +++++++----- internal/builder/builder_test.go | 26 ++++++++++++++++++++++++++ pkg/client/build.go | 6 +++--- pkg/client/create_builder.go | 3 +-- 7 files changed, 77 insertions(+), 12 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 05ceea2e2..a667d489c 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1085,6 +1085,12 @@ func testAcceptance( assertOutput = assertions.NewOutputAssertionManager(t, output) assertOutput.ReportsSuccessfulImageBuild(repoName) assertOutput.ReportsSelectingRunImageMirrorFromLocalConfig(localRunImageMirror) + if pack.SupportsFeature(invoke.FixesRunImageMetadata) { + t.Log(fmt.Sprintf("image %s was NOT added into 'io.buildpacks.lifecycle.metadata' label", localRunImageMirror)) + assertImage.HasLabelWithNoData(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"%s"`, localRunImageMirror)) + t.Log(fmt.Sprintf("run-image %s was added into 'io.buildpacks.lifecycle.metadata' label", runImage)) + assertImage.HasLabelWithData(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"%s"`, runImage)) + } cachedLaunchLayer := "simple/layers:cached-launch-layer" assertLifecycleOutput := assertions.NewLifecycleOutputAssertionManager(t, output) @@ -1807,6 +1813,10 @@ func testAcceptance( t.Log("uses the run image as the base image") assertImage.HasBaseImage(repoName, runImageName) + if pack.SupportsFeature(invoke.FixesRunImageMetadata) { + t.Log(fmt.Sprintf("run-image %s was added into 'io.buildpacks.lifecycle.metadata' label", runImageName)) + assertImage.HasLabelWithData(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"%s"`, runImageName)) + } }) }) @@ -2842,6 +2852,14 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] var localRunImageMirror string it.Before(func() { + imageManager.CleanupImages(repoName) + pack.RunSuccessfully( + "build", repoName, + "-p", filepath.Join("testdata", "mock_app"), + "--builder", builderName, + "--pull-policy", "never", + ) + localRunImageMirror = registryConfig.RepoName("run-after/" + h.RandString(10)) buildRunImage(localRunImageMirror, "local-mirror-after-1", "local-mirror-after-2") pack.JustRunSuccessfully("config", "run-image-mirrors", "add", runImage, "-m", localRunImageMirror) @@ -2872,9 +2890,16 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] when("image metadata has a mirror", func() { it.Before(func() { // clean up existing mirror first to avoid leaking images - imageManager.CleanupImages(runImageMirror) - + imageManager.CleanupImages(runImageMirror, repoName) buildRunImage(runImageMirror, "mirror-after-1", "mirror-after-2") + + pack.RunSuccessfully( + "build", repoName, + "-p", filepath.Join("testdata", "mock_app"), + "--builder", builderName, + "--pull-policy", "never", + ) + }) it("selects the best mirror", func() { diff --git a/acceptance/assertions/image.go b/acceptance/assertions/image.go index f7b2540fa..2070b670c 100644 --- a/acceptance/assertions/image.go +++ b/acceptance/assertions/image.go @@ -69,6 +69,15 @@ func (a ImageAssertionManager) HasLabelWithData(image, label, data string) { a.assert.Contains(label, data) } +func (a ImageAssertionManager) HasLabelWithNoData(image, label, data string) { + a.testObject.Helper() + inspect, err := a.imageManager.InspectLocal(image) + a.assert.Nil(err) + label, ok := inspect.Config.Labels[label] + a.assert.TrueWithMessage(ok, fmt.Sprintf("expected label %s to exist", label)) + a.assert.NotContains(label, data) +} + func (a ImageAssertionManager) HasLengthLayers(image string, length int) { a.testObject.Helper() inspect, err := a.imageManager.InspectLocal(image) diff --git a/acceptance/invoke/pack.go b/acceptance/invoke/pack.go index e10bcdfe2..8c5379e60 100644 --- a/acceptance/invoke/pack.go +++ b/acceptance/invoke/pack.go @@ -237,6 +237,7 @@ const ( MetaBuildpackFolder PlatformRetries FlattenBuilderCreationV2 + FixesRunImageMetadata ) var featureTests = map[Feature]func(i *PackInvoker) bool{ @@ -270,6 +271,9 @@ var featureTests = map[Feature]func(i *PackInvoker) bool{ FlattenBuilderCreationV2: func(i *PackInvoker) bool { return i.atLeast("v0.33.1") }, + FixesRunImageMetadata: func(i *PackInvoker) bool { + return i.atLeast("v0.34.0") + }, } func (i *PackInvoker) SupportsFeature(f Feature) bool { diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 4298c6135..069a00935 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -148,6 +148,13 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, return nil, err } + if opts.runImage != "" { + // Do we need to look for available mirrors? for now the mirrors are gone if you override the run-image + // create an issue if you want to preserve the mirrors + metadata.RunImages = []RunImageMetadata{{Image: opts.runImage}} + metadata.Stack.RunImage = RunImageMetadata{Image: opts.runImage} + } + for labelKey, labelValue := range opts.labels { err = img.SetLabel(labelKey, labelValue) if err != nil { @@ -155,11 +162,6 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, } } - if opts.runImage != "" { - metadata.RunImages = []RunImageMetadata{{Image: opts.runImage}} - metadata.Stack.RunImage.Image = opts.runImage - } - bldr := &Builder{ baseImageName: img.Name(), image: img, diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 80d39d03f..5883c1e6b 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -1846,6 +1846,32 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) }) }) + + when("#New", func() { + when("#WithRunImage", func() { + // Current runImage information in builder image: + // "stack": {"runImage": {"image": "prev/run", "mirrors": ["prev/mirror"]}} + var newBuilder *builder.Builder + newRunImage := "another/run" + + it.Before(func() { + var err error + newBuilder, err = builder.New(builderImage, "newBuilder/image", builder.WithRunImage(newRunImage)) + h.AssertNil(t, err) + }) + + it("Overrides de RunImageMetadata", func() { + // RunImages() returns Stacks + RunImages metadata. + metadata := newBuilder.RunImages() + h.AssertTrue(t, len(metadata) == 2) + for _, m := range metadata { + // Both images must be equal to the expected run-image + h.AssertEq(t, m.Image, newRunImage) + h.AssertEq(t, len(m.Mirrors), 0) + } + }) + }) + }) }) when("flatten", func() { diff --git a/pkg/client/build.go b/pkg/client/build.go index 157c69d4a..670f53f2c 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -335,7 +335,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return errors.Wrapf(err, "getting builder architecture") } - bldr, err := c.getBuilder(rawBuilderImage, opts.RunImage) + bldr, err := c.getBuilder(rawBuilderImage) if err != nil { return errors.Wrapf(err, "invalid builder %s", style.Symbol(opts.Builder)) } @@ -552,6 +552,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { Layout: opts.Layout(), Keychain: c.keychain, } + switch { case useCreator: lifecycleOpts.UseCreator = true @@ -775,12 +776,11 @@ func (c *Client) processBuilderName(builderName string) (name.Reference, error) return name.ParseReference(builderName, name.WeakValidation) } -func (c *Client) getBuilder(img imgutil.Image, runImage string) (*builder.Builder, error) { +func (c *Client) getBuilder(img imgutil.Image) (*builder.Builder, error) { bldr, err := builder.FromImage(img) if err != nil { return nil, err } - if bldr.Stack().RunImage.Image == "" && len(bldr.RunImages()) == 0 { return nil, errors.New("builder metadata is missing run-image") } diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index dc3bf7d55..ee1062bc4 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -157,12 +157,11 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption if opts.Flatten != nil && len(opts.Flatten.FlattenModules()) > 0 { builderOpts = append(builderOpts, builder.WithFlattened(opts.Flatten)) } - if opts.Labels != nil && len(opts.Labels) > 0 { builderOpts = append(builderOpts, builder.WithLabels(opts.Labels)) } - bldr, err := builder.New(baseImage, opts.BuilderName, builderOpts...) + bldr, err := builder.New(baseImage, opts.BuilderName, builderOpts...) if err != nil { return nil, errors.Wrap(err, "invalid build-image") } From dfe0e73d732ca671aac13704c4c55308ea7072e5 Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Fri, 5 Apr 2024 16:08:15 -0500 Subject: [PATCH 6/6] renaming acceptance test helper method for consistency, adding feedback from Natalie Signed-off-by: Juan Bustamante --- acceptance/acceptance_test.go | 20 ++++++++++---------- acceptance/assertions/image.go | 4 ++-- internal/builder/builder_test.go | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index a667d489c..6b030cb04 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -801,11 +801,11 @@ func testAcceptance( bpSimpleLayersDiffID := "sha256:ade9da86859fa4ea50a513757f9b242bf1038667abf92dad3d018974a17f0ea7" bpReadEnvDiffID := "sha256:db0797077ba8deff7054ab5578133b8f0206b6393de34b5bfd795cf50f6afdbd" // extensions - assertImage.HasLabelWithData(builderName, "io.buildpacks.extension.layers", `{"read/env":{"read-env-version":{"api":"0.9","layerDiffID":"`+extReadEnvDiffID+`","name":"Read Env Extension"}},"simple/layers":{"simple-layers-version":{"api":"0.7","layerDiffID":"`+extSimpleLayersDiffID+`","name":"Simple Layers Extension"}}}`) - assertImage.HasLabelWithData(builderName, "io.buildpacks.buildpack.order-extensions", `[{"group":[{"id":"read/env","version":"read-env-version"},{"id":"simple/layers","version":"simple-layers-version"}]}]`) + assertImage.HasLabelContaining(builderName, "io.buildpacks.extension.layers", `{"read/env":{"read-env-version":{"api":"0.9","layerDiffID":"`+extReadEnvDiffID+`","name":"Read Env Extension"}},"simple/layers":{"simple-layers-version":{"api":"0.7","layerDiffID":"`+extSimpleLayersDiffID+`","name":"Simple Layers Extension"}}}`) + assertImage.HasLabelContaining(builderName, "io.buildpacks.buildpack.order-extensions", `[{"group":[{"id":"read/env","version":"read-env-version"},{"id":"simple/layers","version":"simple-layers-version"}]}]`) // buildpacks - assertImage.HasLabelWithData(builderName, "io.buildpacks.buildpack.layers", `{"read/env":{"read-env-version":{"api":"0.7","stacks":[{"id":"pack.test.stack"}],"layerDiffID":"`+bpReadEnvDiffID+`","name":"Read Env Buildpack"}},"simple/layers":{"simple-layers-version":{"api":"0.7","stacks":[{"id":"pack.test.stack"}],"layerDiffID":"`+bpSimpleLayersDiffID+`","name":"Simple Layers Buildpack"}}}`) - assertImage.HasLabelWithData(builderName, "io.buildpacks.buildpack.order", `[{"group":[{"id":"read/env","version":"read-env-version","optional":true},{"id":"simple/layers","version":"simple-layers-version","optional":true}]}]`) + assertImage.HasLabelContaining(builderName, "io.buildpacks.buildpack.layers", `{"read/env":{"read-env-version":{"api":"0.7","stacks":[{"id":"pack.test.stack"}],"layerDiffID":"`+bpReadEnvDiffID+`","name":"Read Env Buildpack"}},"simple/layers":{"simple-layers-version":{"api":"0.7","stacks":[{"id":"pack.test.stack"}],"layerDiffID":"`+bpSimpleLayersDiffID+`","name":"Simple Layers Buildpack"}}}`) + assertImage.HasLabelContaining(builderName, "io.buildpacks.buildpack.order", `[{"group":[{"id":"read/env","version":"read-env-version","optional":true},{"id":"simple/layers","version":"simple-layers-version","optional":true}]}]`) } }) @@ -1063,10 +1063,10 @@ func testAcceptance( assertImage.HasBaseImage(repoName, runImage) t.Log("sets the run image metadata") - assertImage.HasLabelWithData(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"pack-test/run","mirrors":["%s"]`, runImageMirror)) + assertImage.HasLabelContaining(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"pack-test/run","mirrors":["%s"]`, runImageMirror)) t.Log("sets the source metadata") - assertImage.HasLabelWithData(repoName, "io.buildpacks.project.metadata", (`{"source":{"type":"project","version":{"declared":"1.0.2"},"metadata":{"url":"https://github.com/buildpacks/pack"}}}`)) + assertImage.HasLabelContaining(repoName, "io.buildpacks.project.metadata", (`{"source":{"type":"project","version":{"declared":"1.0.2"},"metadata":{"url":"https://github.com/buildpacks/pack"}}}`)) t.Log("registry is empty") assertImage.NotExistsInRegistry(repo) @@ -1086,10 +1086,10 @@ func testAcceptance( assertOutput.ReportsSuccessfulImageBuild(repoName) assertOutput.ReportsSelectingRunImageMirrorFromLocalConfig(localRunImageMirror) if pack.SupportsFeature(invoke.FixesRunImageMetadata) { - t.Log(fmt.Sprintf("image %s was NOT added into 'io.buildpacks.lifecycle.metadata' label", localRunImageMirror)) - assertImage.HasLabelWithNoData(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"%s"`, localRunImageMirror)) + t.Log(fmt.Sprintf("run-image mirror %s was NOT added into 'io.buildpacks.lifecycle.metadata' label", localRunImageMirror)) + assertImage.HasLabelNotContaining(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"%s"`, localRunImageMirror)) t.Log(fmt.Sprintf("run-image %s was added into 'io.buildpacks.lifecycle.metadata' label", runImage)) - assertImage.HasLabelWithData(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"%s"`, runImage)) + assertImage.HasLabelContaining(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"%s"`, runImage)) } cachedLaunchLayer := "simple/layers:cached-launch-layer" @@ -1815,7 +1815,7 @@ func testAcceptance( assertImage.HasBaseImage(repoName, runImageName) if pack.SupportsFeature(invoke.FixesRunImageMetadata) { t.Log(fmt.Sprintf("run-image %s was added into 'io.buildpacks.lifecycle.metadata' label", runImageName)) - assertImage.HasLabelWithData(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"%s"`, runImageName)) + assertImage.HasLabelContaining(repoName, "io.buildpacks.lifecycle.metadata", fmt.Sprintf(`"image":"%s"`, runImageName)) } }) }) diff --git a/acceptance/assertions/image.go b/acceptance/assertions/image.go index 2070b670c..95c82d79c 100644 --- a/acceptance/assertions/image.go +++ b/acceptance/assertions/image.go @@ -60,7 +60,7 @@ func (a ImageAssertionManager) HasCreateTime(image string, expectedTime time.Tim a.assert.TrueWithMessage(actualTime.Sub(expectedTime) < 5*time.Second && expectedTime.Sub(actualTime) < 5*time.Second, fmt.Sprintf("expected image create time %s to match expected time %s", actualTime, expectedTime)) } -func (a ImageAssertionManager) HasLabelWithData(image, label, data string) { +func (a ImageAssertionManager) HasLabelContaining(image, label, data string) { a.testObject.Helper() inspect, err := a.imageManager.InspectLocal(image) a.assert.Nil(err) @@ -69,7 +69,7 @@ func (a ImageAssertionManager) HasLabelWithData(image, label, data string) { a.assert.Contains(label, data) } -func (a ImageAssertionManager) HasLabelWithNoData(image, label, data string) { +func (a ImageAssertionManager) HasLabelNotContaining(image, label, data string) { a.testObject.Helper() inspect, err := a.imageManager.InspectLocal(image) a.assert.Nil(err) diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 5883c1e6b..40fba7980 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -1860,7 +1860,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) - it("Overrides de RunImageMetadata", func() { + it("overrides the run image metadata (which becomes run.toml)", func() { // RunImages() returns Stacks + RunImages metadata. metadata := newBuilder.RunImages() h.AssertTrue(t, len(metadata) == 2)