Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trusted builders fix #2205

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions internal/builder/known_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,12 @@ var KnownBuilders = []KnownBuilder{
Trusted: true,
},
}

var IsKnownTrustedBuilder = func(b string) bool {
for _, knownBuilder := range KnownBuilders {
if b == knownBuilder.Image && knownBuilder.Trusted {
return true
}
}
return false
}
28 changes: 27 additions & 1 deletion internal/commands/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) {
})
})

when("the builder is suggested", func() {
when("the builder is known to be trusted and suggested", func() {
it("sets the trust builder option", func() {
mockClient.EXPECT().
Build(gomock.Any(), EqBuildOptionsWithTrustedBuilder(true)).
Expand All @@ -126,6 +126,32 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) {
h.AssertContains(t, outBuf.String(), "Builder 'heroku/builder:24' is trusted")
})
})

when("the builder is known to be trusted but not suggested", func() {
it("sets the trust builder option", func() {
mockClient.EXPECT().
Build(gomock.Any(), EqBuildOptionsWithTrustedBuilder(true)).
Return(nil)

logger.WantVerbose(true)
command.SetArgs([]string{"image", "--builder", "heroku/builder:22"})
h.AssertNil(t, command.Execute())
h.AssertContains(t, outBuf.String(), "Builder 'heroku/builder:22' is trusted")
})
})

when("the builder is not trusted", func() {
colincasey marked this conversation as resolved.
Show resolved Hide resolved
it("warns the user that the builder is untrusted", func() {
mockClient.EXPECT().
Build(gomock.Any(), EqBuildOptionsWithImage("org/builder:unknown", "image")).
colincasey marked this conversation as resolved.
Show resolved Hide resolved
Return(nil)

logger.WantVerbose(true)
command.SetArgs([]string{"image", "--builder", "org/builder:unknown"})
h.AssertNil(t, command.Execute())
h.AssertContains(t, outBuf.String(), "Builder 'org/builder:unknown' is untrusted")
})
})
})

when("--buildpack-registry flag is specified but experimental isn't set in the config", func() {
Expand Down
8 changes: 5 additions & 3 deletions internal/commands/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"os/signal"
"syscall"

"github.com/buildpacks/pack/internal/builder"

"github.com/google/go-containerregistry/pkg/v1/types"
"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -105,14 +107,14 @@ func getMirrors(config config.Config) map[string][]string {
return mirrors
}

func isTrustedBuilder(cfg config.Config, builder string) bool {
func isTrustedBuilder(cfg config.Config, builderName string) bool {
for _, trustedBuilder := range cfg.TrustedBuilders {
if builder == trustedBuilder.Name {
if builderName == trustedBuilder.Name {
return true
}
}

return isSuggestedBuilder(builder)
return builder.IsKnownTrustedBuilder(builderName)
}

func deprecationWarning(logger logging.Logger, oldCmd, replacementCmd string) {
Expand Down
6 changes: 3 additions & 3 deletions internal/commands/config_trusted_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ func removeTrustedBuilder(args []string, logger logging.Logger, cfg config.Confi

// Builder is not in the trusted builder list
if len(existingTrustedBuilders) == len(cfg.TrustedBuilders) {
if isSuggestedBuilder(builder) {
// Attempted to untrust a suggested builder
return errors.Errorf("Builder %s is a suggested builder, and is trusted by default. Currently pack doesn't support making these builders untrusted", style.Symbol(builder))
if bldr.IsKnownTrustedBuilder(builder) {
// Attempted to untrust a known trusted builder
return errors.Errorf("Builder %s is a known trusted builder. Currently pack doesn't support making these builders untrusted", style.Symbol(builder))
}

logger.Infof("Builder %s wasn't trusted", style.Symbol(builder))
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/config_trusted_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func testTrustedBuilderCommand(t *testing.T, when spec.G, it spec.S) {
command.SetArgs(append(args, builder))

err := command.Execute()
h.AssertError(t, err, fmt.Sprintf("Builder %s is a suggested builder, and is trusted by default", style.Symbol(builder)))
h.AssertError(t, err, fmt.Sprintf("Builder %s is a known trusted builder. Currently pack doesn't support making these builders untrusted", style.Symbol(builder)))
})
})
})
Expand Down
10 changes: 0 additions & 10 deletions internal/commands/suggest_builders.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,3 @@ func getBuilderDescription(builder bldr.KnownBuilder, inspector BuilderInspector

return builder.DefaultDescription
}

func isSuggestedBuilder(builder string) bool {
for _, knownBuilder := range bldr.KnownBuilders {
if builder == knownBuilder.Image && knownBuilder.Suggested {
return true
}
}

return false
}
2 changes: 1 addition & 1 deletion internal/commands/untrust_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func testUntrustBuilderCommand(t *testing.T, when spec.G, it spec.S) {
command.SetArgs([]string{builder})

err := command.Execute()
h.AssertError(t, err, fmt.Sprintf("Builder %s is a suggested builder, and is trusted by default", style.Symbol(builder)))
h.AssertError(t, err, fmt.Sprintf("Builder %s is a known trusted builder. Currently pack doesn't support making these builders untrusted", style.Symbol(builder)))
})
})
})
Expand Down
13 changes: 2 additions & 11 deletions pkg/client/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,6 @@ type layoutPathConfig struct {
targetRunImagePath string
}

var IsTrustedBuilderFunc = func(b string) bool {
for _, knownBuilder := range builder.KnownBuilders {
if b == knownBuilder.Image && knownBuilder.Trusted {
return true
}
}
return false
}

// Build configures settings for the build container(s) and lifecycle.
// It then invokes the lifecycle to build an app image.
// If any configuration is deemed invalid, or if any lifecycle phases fail,
Expand Down Expand Up @@ -409,9 +400,9 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
return err
}

// Default mode: if the TrustBuilder option is not set, trust the suggested builders.
// Default mode: if the TrustBuilder option is not set, trust the known trusted builders.
if opts.TrustBuilder == nil {
opts.TrustBuilder = IsTrustedBuilderFunc
opts.TrustBuilder = builder.IsKnownTrustedBuilder
}

// Ensure the builder's platform APIs are supported
Expand Down
Loading