Golang API users do not get proper trusted builder behavior

[aemengo]

Summary

Using the pack golang API does not have the same behavior as the pack CLI regarding trusted builders. In particular, trusted builders are not automatically detected as trusted. The ramification is that the creator workflow does not run, when it should.

---

Reproduction

Example

        // ...

	client, err := pack.NewClient()
	if err != nil {
		return nil, err
	}

	opts := pack.BuildOptions{
		Image:      "aemengo/test",
		Builder:    "heroku/buildpacks:18",
		AppPath:    appPath,
	}

	err = client.Build(ctx, opts)
	if err != nil {
		return nil, err
	}

        // ...

Expected behavior

In the above example, heroku/buildpacks:18 is a trusted builder and the creator workflow would run using the CLI. But since opts.TrustBuilder defaults to false here, the same code path does not run. In the following output, the lifecycle gets downloaded when it shouldn't be

# $ ./run-program
2021/08/11 13:23:44.042847 DEBUG:  Pulling image index.docker.io/heroku/buildpacks:18
18: Pulling from heroku/buildpacks
Digest: sha256:447ce9f5bce9a75ccdde70766e5b7114254dad513cf30869ca4bc8bfebb5edf8
Status: Image is up to date for heroku/buildpacks:18
2021/08/11 13:23:44.473495 DEBUG:  Selected run image heroku/pack:18
2021/08/11 13:23:44.473508 DEBUG:  Pulling image heroku/pack:18
18: Pulling from heroku/pack
Digest: sha256:73dd079b11ee17a657278f348b86aac2efe4432d03b9c79c946afa59799a82d5
Status: Image is up to date for heroku/pack:18

# ...

2021/08/11 13:23:44.889620 DEBUG:  -> heroku/nodejs-npm@0.4.5
2021/08/11 13:23:45.172248 DEBUG:  Pulling image buildpacksio/lifecycle:0.11.4
0.11.4: Pulling from buildpacksio/lifecycle
Digest: sha256:7241fe488c54902bd0d2c95e664456d18d710bc2156383b50fc6786e0126218b
Status: Image is up to date for buildpacksio/lifecycle:0.11.4

# ...

---

Environment

pack info

v0.20.0

[jromero]

I believe that this is by design. At least in it's initial implementation. IIRC, there was a discussion about delegating the fact of whether a builder should be trusted to the platform that may be using the pack client. I can see an improvement where we can have both. We can have a default trust mechanism that behaves as pack does but that can then be replaced by the user of the library.

[jimil749]

Hey @jromero, @aemengo. New contributor here! If I understand the this correctly, do we need to set opts.TrustBuilder to true if we pass any trusted builders? This check could be added in the client.Build method.

[aemengo]

@jimil749 Hey 👋🏾. Appreciate you posting on here!

Here's how the "normal" behavior works, if you're looking for some inspiration

[jimil749]

@aemengo, thanks for the response. Does it make sense to add the check (the one you mentioned) in pack/build.go? Because, the CLI way behaves "correctly" because of the check you mentioned. But this won't happen if one is using the API to create a builder (as we are directly calling Build() when using the API).

[aemengo]

@jimil749 Yes, those are my thoughts. That the check should be moved, for the reason that you mentioned. @jromero Any other opinions here?

[jimil749]

/cc @jromero, do you think it would be wise to move the check? 😅

[jromero]

/cc @jromero, do you think it would be wise to move the check? 😅

Yes. In general I agree that it should be moved. I think the only requirement that I would ask for is that it could be replaced.

Basically the two supported use cases should be:

1. (Default) If I don't do anything special, calling packClient.Build() should trust the suggested builders.
2. I would like to set different behavior, ie trust different builders or trust no builders at all, when I call packClient.Build()

I hope that makes sense.

[jimil749]

2. I would like to set different behavior, ie trust different builders or trust no builders at all, when I call packClient.Build()

Does this imply that, even if I have one of the trusted builders and the user chooses to NOT trust the builder, the builder shouldn't be trusted? But then how would we differentiate between the 2 cases, by default the Builder.TrustBuilder is false, hence there is no way to check it is the user that configured it to false or by default.

[jimil749]

/cc @jromero, it'd be great if you can clear this up!

[jromero]

@jimil749 one way to achieve this is to replace TrustBuilder option from being a boolean to being a function that returns a boolean. The default function could look at the config you just mentioned (along with any existing logic in the cmd package) but the user of the library can the provide a very different function with their own logic as to whether a builder should be trusted.

It's a lot like providing a filter function in JavaScript or other functional programming languages.

[jimil749]

Having a func makes sense @jromero! Just a final question, since we have the suggestedBuilders stored in the internal/commands package, we cannot just import the package in the pack package because it would cause import cycles! Does it make sense to move the suggested builders into a separate package?

[jromero]

Having a func makes sense @jromero! Just a final question, since we have the suggestedBuilders stored in the internal/commands package, we cannot just import the package in the pack package because it would cause import cycles! Does it make sense to move the suggested builders into a separate package?

@jimil749 ugh, import cycles. 🤢 Yes, do what you must. LOL. We can discuss it further as part of the PR if we can somehow improve it. It's typically easier to discuss once we have something to look at.

[jimil749]

Alright! 👍

[jimil749]

@jromero #1274