Skip to content

Commit

Permalink
With the changes introduced in buildpacks#2043 for separating suggest…
Browse files Browse the repository at this point in the history
…ed builders and trusted builders, there were several places that still had logic referencing suggested builders in the trusted context. This PR updates those code paths to only consider trusted builders and extracts out a shared function `IsKnownTrustedBuilder` that can be used for "is this a trusted builder" checks.

Fixes buildpacks#2198

Signed-off-by: Colin Casey <casey.colin@gmail.com>
  • Loading branch information
colincasey committed Jul 4, 2024
1 parent ce8db3c commit 1e79cc8
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 29 deletions.
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
}
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

0 comments on commit 1e79cc8

Please sign in to comment.