buildx icon indicating copy to clipboard operation
buildx copied to clipboard

Proposal: annotations helpers

Open jedevc opened this issue 2 years ago • 11 comments

With https://github.com/moby/buildkit/pull/2879 close to merging, I thought I'd propose some new syntax here :tada:

When merged, it'll be possible to use annotations as follows (with no changes to buildx):

docker buildx build . --output type=image,name=example/foo,push=true,annotation.x=1 --platform=linux/arm64,linux/amd64

However, this isn't ideal, and it would be nice to provide a shorthand, similar to how --push appends push=true.

Buildx

For buildx, we can provide the --annotation flag:

docker buildx build . --output type=image,name=example/foo,push=true --annotation x=1 --platform=linux/arm64,linux/amd64

For platform-specific annotations, we can allow prefixing with the platform name:

docker buildx build . --output type=image,name=example/foo,push=true --annotation linux/amd64:x=1 --platform=linux/arm64,linux/amd64

Not super sure about the exact syntax, but something along those lines should work.

I think we should explicitly avoid allowing using anything other than the manifest annotation type, directing users to manually add the output attribute if they want more advanced behavior.

Bake

For bake, we can add an annotations key:

target "x" {
    ...
    annotations = {
        "linux/amd64:x" = "1"
    }
    ...
}

Comments appreciated :tada:

jedevc avatar Jun 16 '22 11:06 jedevc

This is nice work @jedevc! I think the experience of allowing multiple --annotation flags (like the example below) would allow image builders to support OCI Annotations.

docker buildx build \
  --annotation 'org.opencontainers.image.authors=authorName' \
  --annotation 'org.opencontainers.image.licenses=Apache' \
  -f Dockerfile -t myimage:latest .

I suppose the annotations should be added at the manifest type.


For multi-arch builds, would the annotations be only in the manifestList, or should they also be in each manifest within the multi-arch manifestList?

docker buildx build \
  --annotation 'org.opencontainers.image.authors=authorName' \
  --annotation 'org.opencontainers.image.licenses=Apache' \
  --platform=linux/arm64,linux/amd64 \
  -f Dockerfile -t myimage:latest .

johnsonshi avatar Jun 20 '22 21:06 johnsonshi

I think we should explicitly avoid allowing using anything other than the manifest annotation type, directing users to manually add the output attribute if they want more advanced behavior.

Scenario: image build users may want to add annotations to manifest configs and layers. The problem is that there is no tool out there today that supports annotating a manifest config or a layer if a user wants to.

I think we need to explore the experience around specifying annotations for any OCI spec (manifest, manifestList, config, layer, etc) during image build.

johnsonshi avatar Jun 20 '22 21:06 johnsonshi

Yeah, this is fiddly :smile:

Buildkit now supports 4 different locations for attaching annotations:

  • At the manifest level (the default)
  • At the index level (for multi-arch images, etc)
  • At the manifest descriptor level (the manifest descriptor in the index)
  • At the index descriptor level (in the OCI layout)

I don't see the benefit on supporting that many more (maybe the root of the OCI layout, though I struggle to see the use case for that one), since it's already quite a challenge to provide syntax that allows distinguishing between the different locations. For example, we have annotation-manifest[linux/amd64].org.opencontainers.image.authors=authorName - I'm not sure how well that will extend to layers and configs. I think until a concrete use case for manually attaching annotations at those levels appears, we should hold off on adding support?

It might be worth splitting the discussion around configs and layers into another issue on the buildkit repo, since annotation support is implemented there, I think this is more around working out what the default user-experience should be (though they'll of course still have all the dials to mess with if they want).

jedevc avatar Jun 21 '22 09:06 jedevc

So in chatting with @tonistiigi and @crazy-max - I think we want to hold off on adding any buildx-specific syntax just for the time being, at least until we can gather some more concrete use cases of how we expect users to interact with this feature.

The buildkit is still accessible by attaching annotations using the exporter options, e.g. type=image,name=example/foo,push=true,annotation.x=1.

@developer-guy @chrisdostert since you were both active on the original buildkit issue https://github.com/moby/buildkit/issues/1220, did you have any concrete use cases you were wanting to use annotations for?

jedevc avatar Jun 22 '22 10:06 jedevc

This would be useful for the metadata-action where we use labels atm to provide OCI Image Format Specification: https://github.com/docker/metadata-action/blob/97c170d70b5f8bfc77e6b23e68381f217cb64ded/src/meta.ts#L440-L453

crazy-max avatar Jul 21 '22 09:07 crazy-max

If this was added am I correct in understanding that we can annotate architecture-specific annotations/labels?

I've just been exploring how to set different labels per architecture on a multi-architecture build. I have not had any luck without creating each architecture separately and modifying the final manifest.

If the proposed annotation structure was introduced which is prefixed with the platform e.g: linux/amd64:org.opencontainers.image.base.digest=xyz we potentially could at build time?

nightah avatar Aug 11 '22 03:08 nightah

@nightah https://github.com/moby/buildkit/blob/master/docs/annotations.md

tonistiigi avatar Aug 11 '22 03:08 tonistiigi

@tonistiigi thanks for pointing me in the right direction.

I've tried using the annotations per the documentation and I'm not sure if it's just perhaps me doing something wrong but I'm not seeing any of the annotations that I'm attempting to include in a build.

Firstly - should all annotations (especially those that are specified with annotation-index) be visible in a docker inspect? If not how should I be validating annotations actually being included?

I think the answer to the above question should likely shed light on my next but it seems whenever I run a build with the following command:

docker build -t nightah/test:test --label org.opencontainers.image.base.name=docker.io/library/alpine:3.16.0 --output "type=image,name=docker.io/nightah/test,push=true,annotation[linux/amd64].org.opencontainers.image.base.digest=sha256:9b2a28eb47540823042a2ba401386845089bb7b62a9637d55816132c4c3c36eb,annotation[linux/arm/v7].org.opencontainers.image.base.digest=sha256:0dc112f0cf79af2654a164af9223723348b07ce2b798bbcb858984fb64d8e13b,annotation[linux/arm64].org.opencontainers.image.base.digest=sha256:d66d8a4b754d1e4da73ed711f0df63b3f19403f4e0711e4edc97ac87d20d707a" --platform linux/amd64,linux/arm/v7,linux/arm64 --builder buildx .

I'm only seeing the label that I specified and none of the annotations, I've also attempted to utilise the other annotation types (index, manifest-descriptor) without success (I've left the image there in the interim too).

nightah avatar Aug 11 '22 23:08 nightah

but I'm not seeing any of the annotations that I'm attempting to include in a build.

Are you using master buildkit? This feature is not in any release yet. @jedevc Maybe we can make it more clear in the docs.

docker buildx create --driver-opt image=moby/buildkit:master --use

especially those that are specified with annotation-index) be visible in a docker inspect

No (at least not with the current version). You would need to use docker buildx imagetools inspect, ctr or any other tool that can show you the blobs in the registry storage.

I did check your nightah/test:test. It does not have any annotations atm.

tonistiigi avatar Aug 11 '22 23:08 tonistiigi

Are you using master buildkit? This feature is not in any release yet. @jedevc Maybe we can make it more clear in the docs.

Looks like this was the key, specifying the master version of buildkit now has annotations on the respective blob.

No (at least not with the current version).

Does this imply that at some point this is going to change, or already is being considered to change?

nightah avatar Aug 11 '22 23:08 nightah

Are you using master buildkit? This feature is not in any release yet. @jedevc Maybe we can make it more clear in the docs.

CC @crazy-max, we've discussed making versioning more clear across all of our docs, not sure if we came up with any solutions?

jedevc avatar Aug 12 '22 07:08 jedevc

My 2 cents for a concrete proposal. I think this is far from being perfect, but it mimics the --output design, and I would love to see progress in this discussion.

Helper Output
--annotation "key=value" annotation.key=value
--annotation "type:key=value" annotation-type.key=value
--annotation "[platform]:key=value" annotation[platform].key=value
--annotation "type[platform]:key=value" annotation-type[platform].key=value

favonia avatar Apr 06 '23 21:04 favonia

@favonia sorry for the delay in getting back to you - I think the design you suggest is good - I'd be happy with that.

I currently don't have the time to work on this, but I'll mark this issue as help wanted - if anyone wants to help by putting together a PR, I'm happy to help out where I can :heart:

jedevc avatar Jul 24 '23 09:07 jedevc

I am working on annotation for OCI index in https://github.com/docker/buildx/pull/1965 via buildx imagetool create. So I was wondering it would be great to finalize the design :)

The syntax proposed in https://github.com/docker/buildx/issues/1171#issuecomment-1499667083 looks fine to me as well. For index it would look as (https://github.com/docker/buildx/pull/1965#issuecomment-1647571108) follow:

$ buildx imagetools create -t hello-world hello-world hello-world --dry-run --annotation index:org.opencontainers.image.description="I am a description"

Do we see any major issues with it? Otherwise I will proceed in this direction

mqasimsarfraz avatar Jul 24 '23 13:07 mqasimsarfraz