Merge pull request #2205 from colincasey/fix_trusting_of_trusted_buil…

…ders

Trusted builders fix

internal/builder/known_builder.go

@@ -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
+}

internal/commands/build_test.go

@@ -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)).
@@ -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() {
+				it("warns the user that the builder is untrusted", func() {
+					mockClient.EXPECT().
+						Build(gomock.Any(), EqBuildOptionsWithTrustedBuilder(false)).
+						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() {
@@ -1036,7 +1062,7 @@ func EqBuildOptionsWithTrustedBuilder(trustBuilder bool) gomock.Matcher {
 	return buildOptionsMatcher{
 		description: fmt.Sprintf("Trust Builder=%t", trustBuilder),
 		equals: func(o client.BuildOptions) bool {
-			return o.TrustBuilder(o.Builder)
+			return o.TrustBuilder(o.Builder) == trustBuilder
 		},
 	}
 }

internal/commands/commands.go

@@ -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"
@@ -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) {

internal/commands/config_trusted_builder.go

@@ -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))

internal/commands/config_trusted_builder_test.go

@@ -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)))
 			})
 		})
 	})

internal/commands/suggest_builders.go

@@ -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
-}

internal/commands/untrust_builder_test.go

@@ -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)))
 			})
 		})
 	})

pkg/client/build.go

@@ -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,
@@ -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