Skip to content

Commit

Permalink
Also consider buildpacks that are added from project descriptor (but …
Browse files Browse the repository at this point in the history 
…exclude inline buildpacks)

Signed-off-by: Natalie Arellano <narellano@vmware.com>
  • Loading branch information
@natalieparellano
natalieparellano committed May 2, 2024
1 parent 1ab72dd commit 2320c96
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 29 deletions.
73 changes: 44 additions & 29 deletions pkg/client/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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\"}}",
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)
}
Expand All @@ -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)
Expand All @@ -1152,15 +1167,15 @@ 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)
}
}
}

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) {
Expand Down Expand Up @@ -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")
}
}

Expand Down
55 changes: 55 additions & 0 deletions pkg/client/build_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
})
})

Expand Down

0 comments on commit 2320c96

Please sign in to comment.