Skip to content

Commit

Permalink
Merge pull request #30 from buildpacks/fix/untrusted-flow
Browse files Browse the repository at this point in the history 
Use the untrusted flow when buildpacks are added to a trusted builder
  • Loading branch information
@natalieparellano
natalieparellano committed Jul 10, 2024
2 parents dded2a0 + 5dbbb52 commit 16f4932
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 26 deletions.
1 change: 1 addition & 0 deletions acceptance/acceptance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1627,6 +1627,7 @@ func testAcceptance(

it.Before(func() {
h.SkipIf(t, os.Getenv("DOCKER_HOST") != "", "cannot mount volume when DOCKER_HOST is set")
h.SkipIf(t, imageManager.HostOS() == "windows", "These tests are broken on Windows Containers on Windows when not using the creator; see https://github.com/buildpacks/pack/issues/2147")

if imageManager.HostOS() == "windows" {
volumeRoot = `c:\`
Expand Down
70 changes: 47 additions & 23 deletions pkg/client/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
return err
}

fetchedBPs, order, err := c.processBuildpacks(ctx, bldr.Buildpacks(), bldr.Order(), bldr.StackID, opts, targetToUse)
fetchedBPs, nInlineBPs, order, err := c.processBuildpacks(ctx, bldr.Buildpacks(), bldr.Order(), bldr.StackID, opts, targetToUse)
if err != nil {
return err
}
Expand Down Expand Up @@ -418,6 +418,19 @@ 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)
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
}
var (
lifecycleOptsLifecycleImage string
lifecycleAPIs []string
Expand Down Expand Up @@ -1140,18 +1153,21 @@ func (c *Client) processProxyConfig(config *ProxyConfig) ProxyConfig {
// ----------
// - group:
// - A
func (c *Client) processBuildpacks(ctx context.Context, builderBPs []dist.ModuleInfo, builderOrder dist.Order, stackID string, opts BuildOptions, targetToUse *dist.Target) (fetchedBPs []buildpack.BuildModule, order dist.Order, err error) {
func (c *Client) processBuildpacks(ctx context.Context, builderBPs []dist.ModuleInfo, builderOrder dist.Order, stackID string, opts BuildOptions, targetToUse *dist.Target) (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 @@ -1161,7 +1177,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderBPs []dist.Module
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 @@ -1171,7 +1187,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderBPs []dist.Module
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 @@ -1185,7 +1201,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderBPs []dist.Module
default:
newFetchedBPs, moduleInfo, err := c.fetchBuildpack(ctx, bp, relativeBaseDir, builderBPs, opts, buildpack.KindBuildpack, targetToUse)
if err != nil {
return fetchedBPs, order, err
return fetchedBPs, 0, order, err
}
fetchedBPs = append(fetchedBPs, newFetchedBPs...)
order = appendBuildpackToOrder(order, *moduleInfo)
Expand All @@ -1195,20 +1211,28 @@ func (c *Client) processBuildpacks(ctx context.Context, builderBPs []dist.Module
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 @@ -1219,7 +1243,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderBPs []dist.Module
for _, bp := range preBuildpacks {
newFetchedBPs, moduleInfo, err := c.fetchBuildpack(ctx, bp, relativeBaseDir, builderBPs, opts, buildpack.KindBuildpack, targetToUse)
if err != nil {
return fetchedBPs, order, err
return fetchedBPs, 0, order, err
}
fetchedBPs = append(fetchedBPs, newFetchedBPs...)
order = prependBuildpackToOrder(order, *moduleInfo)
Expand All @@ -1228,15 +1252,15 @@ func (c *Client) processBuildpacks(ctx context.Context, builderBPs []dist.Module
for _, bp := range postBuildpacks {
newFetchedBPs, moduleInfo, err := c.fetchBuildpack(ctx, bp, relativeBaseDir, builderBPs, opts, buildpack.KindBuildpack, targetToUse)
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, builderBPs []dist.ModuleInfo, opts BuildOptions, kind string, targetToUse *dist.Target) ([]buildpack.BuildModule, *dist.ModuleInfo, error) {
Expand Down Expand Up @@ -1315,26 +1339,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
84 changes: 81 additions & 3 deletions pkg/client/build_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2050,9 +2050,6 @@ api = "0.2"
h.AssertEq(t, args.PullPolicy, image.PullAlways)
h.AssertEq(t, args.Target.ValuesAsPlatform(), "linux/amd64")
})
it("uses the api versions of the lifecycle image", func() {
h.AssertTrue(t, true)
})
it("parses the versions correctly", func() {
fakeLifecycleImage.SetLabel("io.buildpacks.lifecycle.apis", "{\"platform\":{\"deprecated\":[\"0.1\",\"0.2\",\"0.3\",\"0.4\",\"0.5\",\"0.6\"],\"supported\":[\"0.7\",\"0.8\",\"0.9\",\"0.10\",\"0.11\",\"0.12\"]}}")

Expand Down Expand Up @@ -2092,6 +2089,87 @@ api = "0.2"
args := fakeImageFetcher.FetchCalls[fakeLifecycleImage.Name()]
h.AssertNil(t, args)
})

when("additional buildpacks were added", 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 },
Buildpacks: []string{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("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)
})
})
})
})
})

when("lifecycle doesn't support creator", func() {
Expand Down

0 comments on commit 16f4932

Please sign in to comment.