Trusted builders fix

[colincasey]

Summary

With the changes introduced in #2043 for separating suggested 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.

Output

There is a small output change when calling pack config trusted-builders remove <x> with a builder in the known trusted list. The previous output gave the following message:

Builder <x> is a suggested builder, and is trusted by default. Currently pack doesn't support making these builders untrusted

Which now says:

Builder <x> is a known trusted builder. Currently pack doesn't support making these builders untrusted

Documentation

• Should this change be documented?
  • Yes, see #___
  • No

Related

Resolves #2198

[edmorley]

Thank you for fixing this! The changes look good to me to fix the issue, though it would be good to have a test for this. I presume something similar to the tests here?

pack/internal/commands/build_test.go

Lines 93 to 128 in ce8db3c

	when("the builder is trusted", func() {
	it.Before(func() {
	mockClient.EXPECT().
	Build(gomock.Any(), EqBuildOptionsWithTrustedBuilder(true)).
	Return(nil)
	cfg := config.Config{TrustedBuilders: []config.TrustedBuilder{{Name: "my-builder"}}}
	command = commands.Build(logger, cfg, mockClient)
	})
	it("sets the trust builder option", func() {
	logger.WantVerbose(true)
	command.SetArgs([]string{"image", "--builder", "my-builder"})
	h.AssertNil(t, command.Execute())
	h.AssertContains(t, outBuf.String(), "Builder 'my-builder' is trusted")
	})
	when("a lifecycle-image is provided", func() {
	it("ignoring the mentioned lifecycle image, going with default version", func() {
	command.SetArgs([]string{"--builder", "my-builder", "image", "--lifecycle-image", "some-lifecycle-image"})
	h.AssertNil(t, command.Execute())
	h.AssertContains(t, outBuf.String(), "Warning: Ignoring the provided lifecycle image as the builder is trusted, running the creator in a single container using the provided builder")
	})
	})
	})
	when("the builder is 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:24"})
	h.AssertNil(t, command.Execute())
	h.AssertContains(t, outBuf.String(), "Builder 'heroku/builder:24' is trusted")
	})
	})

[jjbustamante]

I am ok with the change, I agree with Ed, add the missing test case

[edmorley]

Thank you - looks good to me :-)