Better attestations default parameters using new exporter attestation options

[jedevc]

🛠️ Fixes moby/buildkit#3380 (comment)

This PR refactors the buildx attestations addition to improve the defaults for what attestations are added.

The main part of the patch includes the addition of explicit filtering to all exporters -- if the user has not explicitly enabled an attestation option, then no attestations of that type will be exported, even if they are generated (e.g. a frontend that might unconditionally produce SBOMs).

The exception to the above rule is for the type=image exporter: for this case only, we additionally ensure that provenance is always generated (with at least mode=min) and attach this provenance to the generated image (unless the user explicitly opts out, either by specifying --provenance=false or setting the exporter attestation option to not include provenance). Because the end destination of the image exporter should be a registry (or, I think, a containerd image store?), then it will support the image indexes we use for storing attestations.

[tonistiigi]

Is this actually fixing any cases? I think the new logic is much more complicated and makes assumptions about buildkit implementation.

[jedevc]

Yup, so there's a couple of issues fixes:

• We only add this to the image exporter - not oci or docker, since the user might take these and load them into the daemon at some point. We can add this attestation safely to the image exporter as the image exporter is either used for: generating images to push, or generating images to load to a containerd content store - both of which support indexes.
  We could do this by updating inline-only on buildkit - but then inline-only is just a synonym for "just the image exporter", and seems strange to couple the exporter that we will use so tightly with the attestation generation.
  We also could do this by conditionally adding the inline-only attestation in buildx - but then this is quite tricky logic, we have to understand why inline-only is being added in buildx, and then what inline-only does in buildkit. It's also weird to have an option inline-only that we wouldn't add to every request, since the point of it was to defer decision making from buildx (which doesn't seem possible to me at the moment).

• We'll need to do something like this at some point anyways - when we support multiple exporters, how should we handle a docker export and an image export at the same time? inline-only isn't good enough (if we don't change how it works on buildkit atm), since if we set it, we'd get the attestation in docker, if we don't set it, we don't get the attestation in image - each exporter needs to be individually configured to do the right thing. We might as well take this approach from the beginning.

• There's a clear distinction between attestations generated and attestations exported - this would let us more easily add functionality in the future to extract the attestations out-of-band, even if they aren't exported, e.g. we could generate an sbom for a build, though it stays not exported - this could be useful for the build history API, or for debugging support.

[jedevc]

Another improvement from this change worth calling out - the version currently in master isn't 100% correct. If we only specified inline attestations, then even for a pushed image, we won't change a manifest into an index and include those attestations. This as mentioned in moby/buildkit#3389 (comment) - this is what currently prevents the error for loading to docker with index support - ideally the proper fix for this needs to be client led, since it's the only component that can be aware of the capabilities of the store where the image is being sent to.

[jedevc]

Have dropped the commit to set the additional attestation options, we don't need to change any buildx logic with moby/buildkit#3466.

However, we can still take the refactoring commit in here.