build: error when using docker exporter and attestations#1511
build: error when using docker exporter and attestations#1511tonistiigi merged 2 commits intodocker:masterfrom
Conversation
| return err | ||
| } | ||
| for _, out := range outputs { | ||
| if out.Type == "docker" { |
There was a problem hiding this comment.
this should use driver.MultiPlatform
There was a problem hiding this comment.
Or I guess because this is container driver with a Docker endpoint for exporting that endpoint should be checked similarly as driver.MultiPlatform is detected. If this gets complicated and doesn't work for regular multi-platform atm then it can be follow-up.
There was a problem hiding this comment.
I've reworked the patch to be handled in toSolveOpt, where we handle the exporter and multi-platform case like this today - this means it'll also work in bake.
I think you're right, we should check multi-platform capabilities in toSolveOpt, to see if we need to error with "docker exporter does not currently support exporting manifest lists" for either attestations or multi-platform?
But we can do that as a follow-up, it seems orthogonal to this - it makes sense for the behavior for attestations and multi-platform should be linked together in the same place (that both generate indexes).
We should avoid erroring with attestations support compatability errors when a user has specified --provenance=false. A user may wish to enable --provenance=false that works across buildkit versions, but currently it will fail on old versions - this patch fixes this, to silently ignore the provenance flag for this check if it's set to disabled. Signed-off-by: Justin Chadwell <[email protected]>
Signed-off-by: Justin Chadwell <[email protected]>
jedevc
force-pushed
the
error-on-attestations-docker
branch
from
9415350 to
43a748f
Compare
| } | ||
| } | ||
| if _, ok := opt.Attests["attest:provenance"]; !ok && supportsAttestations { | ||
| so.FrontendAttrs["attest:provenance"] = "mode=min,inline-only=true" |
There was a problem hiding this comment.
I'm not sure I understand what stops the inline attestation to be added when loading to docker? Or did we block this in buildkit side?
There was a problem hiding this comment.
Yup, this is blocked buildkit side - https://github.com/moby/buildkit/pull/3466/files#diff-93b81ebb2d74a7ff89a643dfd137603e309b87754076de895b0c15dd4626a169R76.
We will attach inline attestations - but only if we would are already exporting an image index. For cases where we need to transform the manifest into the index, we only perform that upgrade for the image exporter.
3 participants
Bug reported by @crazy-max:
This occurs when attestations are explicitly requested, and buildkit generates an index - which docker will not import. We should explicitly error in this case, and not allow the user to
--loadan image that contains explicit attestation flags.