Skip to content

labelPrefix flavor for annotations support#351

Closed
crazy-max wants to merge 2 commits intodocker:masterfrom
crazy-max:annotations
Closed

labelPrefix flavor for annotations support#351
crazy-max wants to merge 2 commits intodocker:masterfrom
crazy-max:annotations

Conversation

@crazy-max
Copy link
Member

fixes #332

With docker/build-push-action#992 and Buildx v0.12, it will be possible to set annotations to an image through the --annotation flag.

With this change in the metadata-action we can now set the manifest: or index: prefix to each label using the labelPrefix attribute in the flavor input. When used with the build-push-action and the annotations input, it will either set annotations at the manifest or index level like:

      -
        name: Docker meta
        uses: docker/metadata-action@v5
        with:
          images: name/app
          flavor: |
            labelPrefix=index:
      -
        name: Build and push
        uses: docker/build-push-action@v5
        with:
          tags: ${{ steps.meta.outputs.tags }}
          annotations: ${{ steps.meta.outputs.labels }}

@favonia
Copy link
Contributor

favonia commented Oct 25, 2023

@crazy-max I think this solution might be the best one to this tricky problem, but I start to worry that it'll be difficult for a beginner to get it right. For example, I feel many will use manifest instead of manifest: in their first times (missing the colon), and many will think they can use it for any prefix (in fact, something like org.opencontainers.image. seems to work with the current code, and they may be surprised later that it's not the "intended" usage). I wonder if we can give a warning or an error if an non-empty prefix does not end with :? (Otherwise, thank you for making these changes quickly!)

@crazy-max
Copy link
Member Author

@crazy-max I think this solution might be the best one to this tricky problem, but I start to worry that it'll be difficult for a beginner to get it right. For example, I feel many will use manifest instead of manifest: in their first times (missing the colon), and many will think they can use it for any prefix (in fact, something like org.opencontainers.image. seems to work with the current code, and they may be surprised later that it's not the "intended" usage). I wonder if we can give a warning or an error if an non-empty prefix does not end with :? (Otherwise, thank you for making these changes quickly!)

Yes I opened this PR as draft because of something similar. What I was thinking was instead providing new outputs such as annotations-manifest and annotations-index instead of this new attribute in flavor. So instead you could just do:

      -
        name: Docker meta
        uses: docker/metadata-action@v5
        with:
          images: name/app
      -
        name: Build and push
        uses: docker/build-push-action@v5
        with:
          tags: ${{ steps.meta.outputs.tags }}
          labels: ${{ steps.meta.outputs.labels }}
          annotations: ${{ steps.meta.outputs.annotations-manifest }}

You can then keep labels but also set annotations.

@favonia
Copy link
Contributor

favonia commented Oct 25, 2023

@crazy-max I like your new solution. It "only" covers 99.9% of the cases but that seems to match the philosophy of this GitHub Actions perfectly!

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request and discussion: support of annotations

2 participants