imagetools: Allow annotations for OCI image index #1965

mqasimsarfraz · 2023-07-20T22:05:29Z

Currently there isn't any way to add annotations to OCI image index for multi-arch. The use case is to have a description in "About this version" in GitHub. It will introduce an optional flag of --annotation to imagetools create to allow adding OCI annotations.

Related: #1171

Testing Done:

OCI Image Index (annotation added)

$ ./bin/build/buildx  imagetools create  -t ghcr.io/inspektor-gadget/inspektor-gadget:v0.18.1 ghcr.io/inspektor-gadget/inspektor-gadget:v0.18.1@sha256:c3780808973801b24c80f8b46835b882fe0d69c08a4460333f28143569665861 ghcr.io/inspektor-gadget/inspektor-gadget:v0.18.1@sha256:d6d5661b02002aa8035e035e9210e3476e0401a948a86fabf104f71c2cbe0efe  --dry-run --annotation index:org.opencontainers.image.description="I am a description"
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:c3780808973801b24c80f8b46835b882fe0d69c08a4460333f28143569665861",
      "size": 2963,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:d6d5661b02002aa8035e035e9210e3476e0401a948a86fabf104f71c2cbe0efe",
      "size": 2962,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ],
  "annotations": {
    "org.opencontainers.image.description": "I am a description"
  }
}

Manifest Descriptor

$ ./bin/build/buildx  imagetools create  -t ghcr.io/inspektor-gadget/inspektor-gadget:v0.18.1 ghcr.io/inspektor-gadget/inspektor-gadget:v0.18.1@sha256:c3780808973801b24c80f8b46835b882fe0d69c08a4460333f28143569665861 ghcr.io/inspektor-gadget/inspektor-gadget:v0.18.1@sha256:d6d5661b02002aa8035e035e9210e3476e0401a948a86fabf104f71c2cbe0efe  --dry-run --annotation manifest-descriptor:org.opencontainers.image.description="I am a description"
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:c3780808973801b24c80f8b46835b882fe0d69c08a4460333f28143569665861",
      "size": 2963,
      "annotations": {
        "org.opencontainers.image.description": "I am a description"
      },
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:d6d5661b02002aa8035e035e9210e3476e0401a948a86fabf104f71c2cbe0efe",
      "size": 2962,
      "annotations": {
        "org.opencontainers.image.description": "I am a description"
      },
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ]
}

Docker manifest list (noop)

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

eiffel-fl

Thank you for it! I tested it and it works fine:

$ ./bin/build/buildx imagetools create -t ghcr.io/eiffel-fl/inspektor-gadget:vtest ghcr.io/inspektor-gadget/inspektor-gadget:v0.18.1@sha256:c3780808973801b24c80f8b46835b882fe0d69c08a4460333f28143569665861 ghcr.io/inspektor-gadget/inspektor-gadget:v0.18.1@sha256:d6d5661b02002aa8035e035e9210e3476e0401a948a86fabf104f71c2cbe0efe --annotations foo="bar"                                       
[+] Building 2.4s (1/1) FINISHED                                                                                                                                           
 => [internal] pushing ghcr.io/eiffel-fl/inspektor-gadget:vtest
$ ./bin/build/buildx imagetools inspect --raw ghcr.io/eiffel-fl/inspektor-gadget:vtest                    qasim/oci-annotations u=
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:c3780808973801b24c80f8b46835b882fe0d69c08a4460333f28143569665861",
      "size": 2963,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:d6d5661b02002aa8035e035e9210e3476e0401a948a86fabf104f71c2cbe0efe",
      "size": 2962,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ],
  "annotations": {
    "foo": "bar"
  }
}

I think we are OK regarding the rules, as the reversed domain name is only an advise and due to using map we cannot have two times the same key.

util/imagetools/create.go

mqasimsarfraz · 2023-07-24T08:01:03Z

@crazy-max @jedevc any thoughts on this? :)

jedevc · 2023-07-24T09:41:55Z

Hm, in general I think the idea for this is alright 🎉

However, I think we'd probably want to retain consistency with how buildkit sets annotations: https://github.com/moby/buildkit/blob/master/docs/annotations.md.

There's not really any strong consensus on how we'd expose this in buildx build, but imagetools should probably follow an identical setup to avoid confusion.

#1171 (comment) gets pretty close to an ideal syntax imo. With, that to annotate your multi-arch image, you'd instead have:

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

Would this kind of syntax work for you? I think we need consistency through buildx, so maybe good to continue the discussion there.

crazy-max · 2023-07-24T10:02:22Z

For the sake of parity we should consider supporting labels as well in follow-up.

mqasimsarfraz · 2023-07-24T13:04:18Z

in general I think the idea for this is alright tada

Great

Would this kind of syntax work for you? I think we need consistency through buildx.

Agreed, we should have consistency through buildx. Also, I was already thinking how can we give user a hint that these annotations are for index and not manifest/descriptor so using index: prefix works well for that. I will proceed the PR in this direction then!

mqasimsarfraz · 2023-07-27T16:15:07Z

@jedevc I have updated the PR to use the new syntax with index:. I still kept the scope to index only. Please feel free to let me know if you think we should handle descriptors as well.

tonistiigi · 2023-07-31T14:20:17Z

I've opened #1978 that you can use as a base for adding integration tests for checking the --annotation flag behavior.

mqasimsarfraz · 2023-08-01T12:31:17Z

I've opened #1978 that you can use as a base for adding integration tests for checking the --annotation flag behavior.

@tonistiigi great thanks. I updated the PR to include testImagetoolsAnnotation. Had to use oci-mediatypes=true to ensure we push an index and not a manifest list in the test.

tests/imagetools.go

util/imagetools/create.go

tests/imagetools.go

jedevc · 2023-08-03T08:47:40Z

util/imagetools/create.go

+	indexAnnotations := make(map[string]string)
+	manifestDescriptorAnnotations := make(map[string]string)
+	for k, v := range ann {
+		groups := annotationRegexp.FindStringSubmatch(k)


Instead of using a regexp here, wdyt about using https://github.com/moby/buildkit/blob/dd0053cdce470b1355fdb0bd5a8f2b0fc506d842/exporter/containerimage/exptypes/annotations.go#L85 instead?

Obviously, it's not perfect, since we'd need to add the annotation- prefix here, which might make the error message a bit odd, but we could always upstream a buildkit fix later that would rework the function to have the caller remove the prefix there (so we wouldn't have to do that).

Instead of using a regexp here, wdyt about using https://github.com/moby/buildkit/blob/dd0053cdce470b1355fdb0bd5a8f2b0fc506d842/exporter/containerimage/exptypes/annotations.go#L85 instead?

Initially I had the same idea but we are using : as a separator for type and annotation key compared to . expected here. So it needs more work than just appending annotation- prefix.

but we could always upstream a buildkit fix later that would rework the function

Does it make sense to make buildkit parser to also handle : separator. It sounded specific to buildx so I feel we should keep it here. wdyt?

Personally, I'd prefer the conversion to buildkit's form for now, so that we only have one source of truth for parsing these keys - I'm happy to follow up in buildkit to rework the logic to be a bit more reusable for this case.

I also just noticed (sorry), that we aren't handling platforms? manifest-descriptor supports a platform, and should only be attached to the descriptor for those platforms.

I'm happy to follow up in buildkit to rework the logic to be a bit more reusable for this case.

Agreed. That should be the way moving forward. I added a comment regrading using buildkit once it supports our use-case here.

I also just noticed (sorry), that we aren't handling platforms?

Great Catch. Done.

Signed-off-by: Qasim Sarfraz <[email protected]>

jedevc

LGTM 🎉 Thanks @mqasimsarfraz!

mqasimsarfraz · 2023-08-08T12:59:37Z

Can you please merge this ? or do we have any open items against it? :)

We were using a specific version for docker buildx to have support for adding annotaions ( docker/buildx#1965). But it has been released into stable already so removing workaround. Signed-off-by: Qasim Sarfraz <[email protected]>

…tion support Root cause: OCI annotations were not persisted to multi-arch manifest index, causing GitHub Container Registry to show "No description provided". The --annotation flag for docker buildx imagetools create requires BuildKit v0.12.0+ / Buildx v0.11.0+. The merge and release jobs were missing explicit BuildKit configuration, defaulting to older versions that silently ignored --annotation flags. Changes: - merge job: Add driver-opts with moby/buildkit:latest + network=host - release job: Add driver-opts with moby/buildkit:latest + network=host - Ensures consistency with build job which already had this configuration This fixes the verification step failure: ❌ ERROR: No annotations found on manifest index Related: - Fixes https://github.com/fluxo-kt/aza-pg/actions/runs/19348853813/job/55357635596#step:6:109 - Docker docs: https://docs.docker.com/build/metadata/annotations/ - BuildKit PR: docker/buildx#1965 Testing: Validated with yamllint and bun run validate (all checks passed)

mqasimsarfraz force-pushed the qasim/oci-annotations branch from 6d283d0 to 42ee388 Compare July 20, 2023 22:06

eiffel-fl approved these changes Jul 21, 2023

View reviewed changes

util/imagetools/create.go Outdated Show resolved Hide resolved

mqasimsarfraz force-pushed the qasim/oci-annotations branch from 42ee388 to b071d65 Compare July 21, 2023 14:57

mqasimsarfraz mentioned this pull request Jul 24, 2023

Proposal: annotations helpers #1171

Closed

mqasimsarfraz force-pushed the qasim/oci-annotations branch from b071d65 to e5e178f Compare July 27, 2023 14:58

mqasimsarfraz force-pushed the qasim/oci-annotations branch from e5e178f to f8d9676 Compare August 1, 2023 12:26

eiffel-fl reviewed Aug 2, 2023

View reviewed changes

tests/imagetools.go Outdated Show resolved Hide resolved

eiffel-fl reviewed Aug 2, 2023

View reviewed changes

tests/imagetools.go Outdated Show resolved Hide resolved

mqasimsarfraz force-pushed the qasim/oci-annotations branch from f8d9676 to 95b6a2f Compare August 2, 2023 08:38

jedevc requested changes Aug 2, 2023

View reviewed changes

util/imagetools/create.go Outdated Show resolved Hide resolved

util/imagetools/create.go Outdated Show resolved Hide resolved

tonistiigi reviewed Aug 2, 2023

View reviewed changes

tests/imagetools.go Outdated Show resolved Hide resolved

tests/imagetools.go Outdated Show resolved Hide resolved

mqasimsarfraz force-pushed the qasim/oci-annotations branch from 95b6a2f to 11c0502 Compare August 3, 2023 07:55

jedevc reviewed Aug 3, 2023

View reviewed changes

jedevc added status/needs-docs-follow-up status/needs-release-notes area/imagetools labels Aug 3, 2023

mqasimsarfraz added 2 commits August 3, 2023 12:08

allow annotations for OCI image index

18894a8

Signed-off-by: Qasim Sarfraz <[email protected]>

tests: add testImagetoolsAnnotation integration test

3ef93e0

Signed-off-by: Qasim Sarfraz <[email protected]>

mqasimsarfraz force-pushed the qasim/oci-annotations branch from 11c0502 to 3ef93e0 Compare August 3, 2023 10:20

jedevc approved these changes Aug 3, 2023

View reviewed changes

jedevc merged commit e5cee89 into docker:master Aug 8, 2023

mqasimsarfraz mentioned this pull request Aug 8, 2023

container images: Add description to image index via annotations inspektor-gadget/inspektor-gadget#1946

Merged

jedevc mentioned this pull request Aug 24, 2023

build: add --annotation shortcut flag #2020

Merged

crazy-max removed status/needs-docs-follow-up status/needs-release-notes labels Nov 17, 2023

jedevc mentioned this pull request Dec 10, 2023

imagetools: cryptic error messages for annotation flag #2148

Open

3 tasks

mqasimsarfraz mentioned this pull request Feb 14, 2024

ci: unpin docker buildx version inspektor-gadget/inspektor-gadget#2503

Merged

crazy-max mentioned this pull request Apr 5, 2024

Improve buildx test coverage #1857

Open

35 tasks

wouterdebruijn mentioned this pull request Feb 3, 2025

🔧 Added docker labels and PR builds retigra/ZabbixCI#100

Merged

imagetools: Allow annotations for OCI image index #1965

imagetools: Allow annotations for OCI image index #1965

Uh oh!

Conversation

mqasimsarfraz commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing Done:

OCI Image Index (annotation added)

Manifest Descriptor

Docker manifest list (noop)

Uh oh!

eiffel-fl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mqasimsarfraz commented Jul 24, 2023

Uh oh!

jedevc commented Jul 24, 2023

Uh oh!

crazy-max commented Jul 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mqasimsarfraz commented Jul 24, 2023

Uh oh!

mqasimsarfraz commented Jul 27, 2023

Uh oh!

tonistiigi commented Jul 31, 2023

Uh oh!

mqasimsarfraz commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jedevc Aug 3, 2023

Choose a reason for hiding this comment

Uh oh!

mqasimsarfraz Aug 3, 2023

Choose a reason for hiding this comment

Uh oh!

jedevc Aug 3, 2023

Choose a reason for hiding this comment

Uh oh!

mqasimsarfraz Aug 3, 2023

Choose a reason for hiding this comment

Uh oh!

jedevc left a comment

Choose a reason for hiding this comment

Uh oh!

mqasimsarfraz commented Aug 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mqasimsarfraz commented Jul 20, 2023 •

edited

Loading

crazy-max commented Jul 24, 2023 •

edited

Loading

mqasimsarfraz commented Aug 1, 2023 •

edited

Loading