implement Cosign verification for HelmCharts

[souleb]

fixes #914

If implemented, users will be able to enable chart verification for OCI based helm charts.

The remote builder will not attempt to download the chart if an artifact exist with the same name and version and the force option is false. It will try to verify the chart if:

• we are on the first reconciliation
• the HelmtChartSpec has changed (generation drift)
• the previous reconciliation resulted in a failed artifact verification
• there is no artifact in storage

The helmChart metadata does not contain the digest so we cannot rely on it to detect drift on the remote.

⚠️ We do not enable verification for umbrella charts stored in Git with dependencies resolved from OCI registries, as we assume that each dependency need its own key.

Tests

• [x] aws + ecr with credentials and autologin
• [x] gcp + artifact registry with autologin and keyless

Signed-off-by: Soule BA [email protected]

[souleb]

In the OCIRepository controller there is a loop to collect keys from a secret and perform verification using that list. The thing is that we break from the list as soon as a signature verification is successful with a key from the list. Is that the intended behavior, or shall we successful verify with all keys?

[stefanprodan]

Is that the intended behavior, or shall we successful verify with all keys?

We shouldn't verify all keys, if one is a match than it's all good IMO. This way we allow people to do key rotation, old artifacts will still be valid if the old key is in the secret. The same we do for SOPS and decryption keys.

[souleb]

We shouldn't verify all keys, if one is a match than it's all good IMO. This way we allow people to do key rotation, old > artifacts will still be valid if the old key is in the secret. The same we do for SOPS and decryption keys.

I have change the code and documentation to take this into account.

[souleb]

We discussed this with @darkowlzz and we think that we could persist a given chart digest after successfully building in the Status of the resource. We could then pass it to the chartBuilder in future reconciliation in order to enable skipping verification.

Thoughts @stefanprodan @hiddeco @makkes

Edit: The other option is to have Helm surface a digest field in the chart metadata as it does with the version. We would store it as part of the generated artifact.

[stefanprodan]

In OCIRepository we persist the digest in the revision field in the format <tag>/<digest>, we could do the same for HelmCharts.

[hiddeco]

Can someone remind me why we continue to prefer <tag>/<digest> over <tag>@<digest>? Which is the more common format used to tie a named reference to a specific unique reference/digest, including while working with the OCI spec itself.

[stefanprodan]

Can someone remind me why we continue to prefer / over @?

To be consistent with how we report the revision to consumers. The current format is <label>/<sha-hex> without having the SHA type encoded. Please create an RFC if you wish to change this in across Flux, also NC that relies on this format. If we do change it, then I propose we add the SHA type as well, so for Git it would be <branch|tag>@sha1:<commit-sha> and for OCI would be <tag>@sha256:<digest-hex>.

[hiddeco]

If we do change it, then I propose we add the SHA type as well

This has overlap with the already proposed https://github.com/fluxcd/source-controller/issues/855.

[souleb]

Following this comment, it has bee decided to always perform verification for now.

In a follow up Pull request we will propose pulling the digest in remote.GetChartVersion() and persist it in the spec.Status in the format <tag>/<digest>. We could then use it in the remoteBuilder to decide whether to verify.