Add support for signing Builders and ClusterBuilders

[stormqueen1990]

Changes

• Implement Builder and ClusterBuilder signing with cosign, following the specification defined in RFC 0007 - Integrate cosign with kpack.

Fixes #942

[codecov-commenter]

Codecov Report

Merging #1208 (4a76d00) into main (4d30b2a) will decrease coverage by 0.10%.
Report is 3 commits behind head on main.
The diff coverage is 65.75%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main    #1208      +/-   ##
==========================================
- Coverage   67.41%   67.32%   -0.10%     
==========================================
  Files         133      133              
  Lines        8093     8205     +112     
==========================================
+ Hits         5456     5524      +68     
- Misses       2200     2231      +31     
- Partials      437      450      +13     

Files	Coverage Δ
pkg/apis/build/v1alpha2/builder_types.go	22.22% <ø> (ø)
pkg/blob/fetch.go	58.90% <100.00%> (ø)
pkg/cnb/build_metadata.go	57.89% <100.00%> (ø)
pkg/cnb/create_builder.go	79.72% <100.00%> (+3.53%)	⬆️
pkg/cnb/env_vars.go	84.00% <100.00%> (ø)
pkg/dockercreds/parse_secrets.go	59.21% <100.00%> (ø)
pkg/registry/fetch.go	49.61% <100.00%> (ø)
pkg/secret/volume_secret_reader.go	67.56% <100.00%> (ø)
pkg/notary/repository.go	0.00% <0.00%> (ø)
pkg/apis/build/v1alpha2/builder_lifecycle.go	0.00% <0.00%> (ø)
... and 3 more

[chenbh]

What do we want to happen if we add a cosign secret after the [Cluster]Builder has been created?

Right now nothing happens, but we could get the builder and clusterbuilder reconcilers to track secrets? It does feel a little heavy hand to continuously read all new secrets in a namespace, but would that provide a better user experience?

(I don't have an opinion either way, just want to pick your brains a bit and maybe explicitly document this)

[stormqueen1990]

What do we want to happen if we add a cosign secret after the [Cluster]Builder has been created?

Right now nothing happens, but we could get the builder and clusterbuilder reconcilers to track secrets? It does feel a little heavy hand to continuously read all new secrets in a namespace, but would that provide a better user experience?

(I don't have an opinion either way, just want to pick your brains a bit and maybe explicitly document this)

For UX purposes, it sounds to me like that would be the best way to go forward. I'll wait for other opinions before making changes though.

[chenbh]

What do we want to happen if we add a cosign secret after the [Cluster]Builder has been created?

We're going to bring this up at the next kpack wg, but from early talks with @tomkennedy513 and @matthewmcnew, we have some ideas:

• Do nothing, the user will have to recreate the Builder to get it signed by every secret
• Rely on the natural resync period for Builders (once every 10~12 hrs) and diff the current secrets found vs secrets used in the last run to see if we need a rebuild/resign
• Watch all service accounts referenced by Builders and queue for reconcile if any service account changes
• Watch all secrets and queue for reconcile if any cosign secrets (i.e. those that have cosign.blah in the data fields)

[chenbh]

We're going to bring this up at the next kpack wg, but from early talks with @tomkennedy513 and @matthewmcnew, we have some ideas:

We ended up deciding that because there's no exit-early when reconciling builders, it's ok to rely on the natural (10-hour) resync period. So if the cosign secrets changed, the users will have to wait up to 10 hours before kpack pushes a new builder with the updated signature(s).

[chenbh]

I think this PR is in a good place once #1208 (comment) is addressed, @stormqueen1990 do you mind cleaning/squashing up the commits a little? There are several of them titled "changes from code review" with wildly different changes in them that should really be separated into different commits or squashed into another commit.

Just to be clear, I don't care about the amount of commits as long as each individual commit is immediately obvious to what it's trying to do and is focused on one "unit of work"

[stormqueen1990]

I think this PR is in a good place once #1208 (comment) is addressed, @stormqueen1990 do you mind cleaning/squashing up the commits a little? There are several of them titled "changes from code review" with wildly different changes in them that should really be separated into different commits or squashed into another commit.

Just to be clear, I don't care about the amount of commits as long as each individual commit is immediately obvious to what it's trying to do and is focused on one "unit of work"

Hi there, @chenbh! 👋

Sure thing! I was planning to squash commits into more meaningful chunks once this is good to go. I am a bit busy this week but will try to make all changes (squashing included) by EOD Thursday.

[stormqueen1990]

Hi there, @chenbh! Sorry for the delay, I started facing issues after rebasing the branch for the last time. I'll have to investigate it and will push the updated and squashed branch as soon as possible.

[chenbh]

lgtm!

[chenbh]

@tomkennedy513 @matthewmcnew @samj1912 do one of you mind taking a quick 👀 at this PR? It's a pretty beefy one and I would like at least one more review on it before merging

[tomkennedy513]

Hey @stormqueen1990 would you be able to rebase this pr?

[stormqueen1990]

Hey @stormqueen1990 would you be able to rebase this pr?

Hi there, @tomkennedy513! Yes, just rebased it. Please let me know if there's anything else that's needed to move this PR forward 🙏

I also squashed it to a single commit as my previous squash didn't really break this work into atomic pieces.

[tomkennedy513]

Thank you so much for this work! @stormqueen1990