From 2320c964f01c5ce0f051fc78a67c47fa63fe49be Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 2 May 2024 13:42:06 -0400 Subject: [PATCH] Also consider buildpacks that are added from project descriptor (but exclude inline buildpacks) Signed-off-by: Natalie Arellano --- pkg/client/build.go | 73 ++++++++++++++++++++++++---------------- pkg/client/build_test.go | 55 ++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 29 deletions(-) diff --git a/pkg/client/build.go b/pkg/client/build.go index 5429ed86..28489b43 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -372,7 +372,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return err } - fetchedBPs, order, err := c.processBuildpacks(ctx, bldr.Image(), bldr.Buildpacks(), bldr.Order(), bldr.StackID, opts) + fetchedBPs, nInlineBPs, order, err := c.processBuildpacks(ctx, bldr.Image(), bldr.Buildpacks(), bldr.Order(), bldr.StackID, opts) if err != nil { return err } @@ -400,7 +400,16 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { // Get the platform API version to use lifecycleVersion := bldr.LifecycleDescriptor().Info.Version useCreator := supportsCreator(lifecycleVersion) && opts.TrustBuilder(opts.Builder) - if useCreator && hasAdditionalModules(opts) { + hasAdditionalModules := func() bool { + if len(fetchedBPs) == 0 && len(fetchedExs) == 0 { + return false + } + if len(fetchedBPs) == nInlineBPs && len(fetchedExs) == 0 { + return false + } + return true + }() + if useCreator && hasAdditionalModules { c.logger.Warnf("Builder is trusted but additional modules were added; using the untrusted (5 phases) build flow") useCreator = false } @@ -707,11 +716,6 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return c.logImageNameAndSha(ctx, opts.Publish, imageRef) } -func hasAdditionalModules(opts BuildOptions) bool { - return !(len(opts.Buildpacks) == 0 && len(opts.Extensions) == 0 && - len(opts.PreBuildpacks) == 0 && len(opts.PostBuildpacks) == 0) -} - func extractSupportedLifecycleApis(labels map[string]string) ([]string, error) { // sample contents of labels: // {io.buildpacks.builder.metadata:\"{\"lifecycle\":{\"version\":\"0.15.3\"},\"api\":{\"buildpack\":\"0.2\",\"platform\":\"0.3\"}}", @@ -1064,18 +1068,21 @@ func (c *Client) processProxyConfig(config *ProxyConfig) ProxyConfig { // ---------- // - group: // - A -func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Image, builderBPs []dist.ModuleInfo, builderOrder dist.Order, stackID string, opts BuildOptions) (fetchedBPs []buildpack.BuildModule, order dist.Order, err error) { +func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Image, builderBPs []dist.ModuleInfo, builderOrder dist.Order, stackID string, opts BuildOptions) (fetchedBPs []buildpack.BuildModule, nInlineBPs int, order dist.Order, err error) { relativeBaseDir := opts.RelativeBaseDir declaredBPs := opts.Buildpacks - // declare buildpacks provided by project descriptor when no buildpacks are declared + // Buildpacks from --buildpack override buildpacks from project descriptor if len(declaredBPs) == 0 && len(opts.ProjectDescriptor.Build.Buildpacks) != 0 { relativeBaseDir = opts.ProjectDescriptorBaseDir for _, bp := range opts.ProjectDescriptor.Build.Buildpacks { - buildpackLocator, err := getBuildpackLocator(bp, stackID) + buildpackLocator, isInline, err := getBuildpackLocator(bp, stackID) if err != nil { - return nil, nil, err + return nil, 0, nil, err + } + if isInline { + nInlineBPs++ } declaredBPs = append(declaredBPs, buildpackLocator) } @@ -1085,7 +1092,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima for _, bp := range declaredBPs { locatorType, err := buildpack.GetLocatorType(bp, relativeBaseDir, builderBPs) if err != nil { - return nil, nil, err + return nil, 0, nil, err } switch locatorType { @@ -1095,7 +1102,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima 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") + return nil, 0, nil, errors.New("buildpacks from builder can only be defined once") default: newOrder := dist.Order{} groupToAdd := order[0].Group @@ -1109,7 +1116,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima default: newFetchedBPs, moduleInfo, err := c.fetchBuildpack(ctx, bp, relativeBaseDir, builderImage, builderBPs, opts, buildpack.KindBuildpack) if err != nil { - return fetchedBPs, order, err + return fetchedBPs, 0, order, err } fetchedBPs = append(fetchedBPs, newFetchedBPs...) order = appendBuildpackToOrder(order, *moduleInfo) @@ -1119,20 +1126,28 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima if (len(order) == 0 || len(order[0].Group) == 0) && len(builderOrder) > 0 { preBuildpacks := opts.PreBuildpacks postBuildpacks := opts.PostBuildpacks + // Pre-buildpacks from --pre-buildpack override pre-buildpacks from project descriptor if len(preBuildpacks) == 0 && len(opts.ProjectDescriptor.Build.Pre.Buildpacks) > 0 { for _, bp := range opts.ProjectDescriptor.Build.Pre.Buildpacks { - buildpackLocator, err := getBuildpackLocator(bp, stackID) + buildpackLocator, isInline, err := getBuildpackLocator(bp, stackID) if err != nil { - return nil, nil, errors.Wrap(err, "get pre-buildpack locator") + return nil, 0, nil, errors.Wrap(err, "get pre-buildpack locator") + } + if isInline { + nInlineBPs++ } preBuildpacks = append(preBuildpacks, buildpackLocator) } } + // Post-buildpacks from --post-buildpack override post-buildpacks from project descriptor if len(postBuildpacks) == 0 && len(opts.ProjectDescriptor.Build.Post.Buildpacks) > 0 { for _, bp := range opts.ProjectDescriptor.Build.Post.Buildpacks { - buildpackLocator, err := getBuildpackLocator(bp, stackID) + buildpackLocator, isInline, err := getBuildpackLocator(bp, stackID) if err != nil { - return nil, nil, errors.Wrap(err, "get post-buildpack locator") + return nil, 0, nil, errors.Wrap(err, "get post-buildpack locator") + } + if isInline { + nInlineBPs++ } postBuildpacks = append(postBuildpacks, buildpackLocator) } @@ -1143,7 +1158,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima for _, bp := range preBuildpacks { newFetchedBPs, moduleInfo, err := c.fetchBuildpack(ctx, bp, relativeBaseDir, builderImage, builderBPs, opts, buildpack.KindBuildpack) if err != nil { - return fetchedBPs, order, err + return fetchedBPs, 0, order, err } fetchedBPs = append(fetchedBPs, newFetchedBPs...) order = prependBuildpackToOrder(order, *moduleInfo) @@ -1152,7 +1167,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima for _, bp := range postBuildpacks { newFetchedBPs, moduleInfo, err := c.fetchBuildpack(ctx, bp, relativeBaseDir, builderImage, builderBPs, opts, buildpack.KindBuildpack) if err != nil { - return fetchedBPs, order, err + return fetchedBPs, 0, order, err } fetchedBPs = append(fetchedBPs, newFetchedBPs...) order = appendBuildpackToOrder(order, *moduleInfo) @@ -1160,7 +1175,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima } } - return fetchedBPs, order, nil + return fetchedBPs, nInlineBPs, order, nil } func (c *Client) fetchBuildpack(ctx context.Context, bp string, relativeBaseDir string, builderImage imgutil.Image, builderBPs []dist.ModuleInfo, opts BuildOptions, kind string) ([]buildpack.BuildModule, *dist.ModuleInfo, error) { @@ -1250,26 +1265,26 @@ func (c *Client) fetchBuildpackDependencies(ctx context.Context, bp string, pack return nil, err } -func getBuildpackLocator(bp projectTypes.Buildpack, stackID string) (string, error) { +func getBuildpackLocator(bp projectTypes.Buildpack, stackID string) (locator string, isInline bool, err error) { switch { case bp.ID != "" && bp.Script.Inline != "" && bp.URI == "": if bp.Script.API == "" { - return "", errors.New("Missing API version for inline buildpack") + return "", false, errors.New("Missing API version for inline buildpack") } pathToInlineBuildpack, err := createInlineBuildpack(bp, stackID) if err != nil { - return "", errors.Wrap(err, "Could not create temporary inline buildpack") + return "", false, errors.Wrap(err, "Could not create temporary inline buildpack") } - return pathToInlineBuildpack, nil + return pathToInlineBuildpack, true, nil case bp.URI != "": - return bp.URI, nil + return bp.URI, false, nil case bp.ID != "" && bp.Version != "": - return fmt.Sprintf("%s@%s", bp.ID, bp.Version), nil + return fmt.Sprintf("%s@%s", bp.ID, bp.Version), false, nil case bp.ID != "" && bp.Version == "": - return bp.ID, nil + return bp.ID, false, nil default: - return "", errors.New("Invalid buildpack definition") + return "", false, errors.New("Invalid buildpack definition") } } diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 85265d2f..01b29890 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -2176,6 +2176,61 @@ api = "0.2" h.AssertContains(t, outBuf.String(), "Builder is trusted but additional modules were added; using the untrusted (5 phases) build flow") }) + + when("from project descriptor", func() { + it("uses the 5 phases with the lifecycle image", func() { + additionalBP := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + WithAPI: api.MustParse("0.3"), + WithInfo: dist.ModuleInfo{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", + }, + WithStacks: []dist.Stack{{ID: defaultBuilderStackID}}, + WithOrder: nil, + }) + + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: defaultBuilderName, + Publish: true, + TrustBuilder: func(string) bool { return true }, + ProjectDescriptor: projectTypes.Descriptor{Build: projectTypes.Build{ + Buildpacks: []projectTypes.Buildpack{{ + URI: additionalBP, + }}, + }}, + })) + h.AssertEq(t, fakeLifecycle.Opts.UseCreator, false) + h.AssertEq(t, fakeLifecycle.Opts.LifecycleImage, fakeLifecycleImage.Name()) + + h.AssertContains(t, outBuf.String(), "Builder is trusted but additional modules were added; using the untrusted (5 phases) build flow") + }) + + when("inline buildpack", func() { + it("uses the creator with the provided builder", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: defaultBuilderName, + Publish: true, + TrustBuilder: func(string) bool { return true }, + ProjectDescriptor: projectTypes.Descriptor{Build: projectTypes.Build{ + Buildpacks: []projectTypes.Buildpack{{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", + Script: projectTypes.Script{ + API: "0.10", + Inline: "echo hello", + }, + }}, + }}, + })) + h.AssertEq(t, fakeLifecycle.Opts.UseCreator, true) + + args := fakeImageFetcher.FetchCalls[fakeLifecycleImage.Name()] + h.AssertNil(t, args) + }) + }) + }) }) })