docker / buildx

Docker CLI plugin for extended build capabilities with BuildKit
Apache License 2.0
3.41k stars 463 forks source link

imagetools: cryptic error messages for annotation flag #2148

Open dvdksn opened 8 months ago

dvdksn commented 8 months ago

Contributing guidelines

I've found a bug and checked that ...

Description

When I try to create a new image with annotation, using buildx imagetools create --annotation, I get the following error

$ docker buildx imagetools create --annotation "country=sweden" -t davidkarlsson416/build:annotations davidkarlsson416/build:annotations
ERROR: "" annotations are not supported yet
$ docker buildx imagetools create --annotation "manifest:country=sweden" -t davidkarlsson416/build:annotations davidkarlsson416/build:annotations
ERROR: "manifest" annotations are not supported yet

Expected behaviour

I expected to be able to create an image with the country=sweden annotation

Actual behaviour

error

Buildx version

github.com/docker/buildx v0.12.0 542e5d810e4a1a155684f5f3c5bd7e797632a12f

Docker info

No response

Builders list

NAME/NODE         DRIVER/ENDPOINT         STATUS   BUILDKIT             PLATFORMS
remote-builder    remote                                                
  remote-builder0 tcp://3.135.10.165:1234 inactive                      
default           docker                                                
  default         default                 running  v0.12.2+6560bb937e8c linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/mips64
desktop-linux *   docker                                                
  desktop-linux   desktop-linux           running  v0.12.2+6560bb937e8c linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/mips64

Configuration

n/a

Build logs

No response

Additional info

No response

tonistiigi commented 8 months ago
docker buildx imagetools create --annotation "index:country=estonia" --dry-run davidkarlsson416/build:annotations

docker buildx imagetools create --annotation "manifest-descriptor:country=estonia" --dry-run davidkarlsson416/build:annotations

docker buildx imagetools create --annotation "manifest-descriptor[linux/amd64]:country=estonia" --dry-run davidkarlsson416/build:annotations

But yeah, these error messages are really confusing.

dvdksn commented 8 months ago

@tonistiigi your examples work, the ones I tried (no prefix, and the manifest: prefix) should also work if imagetools create worked the same way as with build or bake -- it would create an annotation on the manifest(s). But currently it just seems to fail.

jedevc commented 8 months ago

Support for manifest is more complex than the other cases, since it requires modifying the manifest, see here from the original PR: https://github.com/docker/buildx/pull/1965#discussion_r1281623654.

I remember discussing this internally at some point, and I think we had mixed feelings on whether imagetools create should modify manifest digests... though I suppose that the user is explicitly requesting this behavior, so maybe it should be fine.

tonistiigi commented 8 months ago

Yes, imagetools create does not make modifications to the manifest. It either copies indexes/manifests with guaranteed immutability or creates new indexes from existing descriptors. I don't think we should change that. Changing manifest is a different kind of tool that would change for example, env or layers as well and change the digest. Note that any modification of the manifest also invalidates any attestation.

dvdksn commented 8 months ago

Got it, thanks. In addition to improving the error message, would it make sense to specify a default level for imagetools create, such that if no prefix type is specified, the annotation is created on e.g. index ?