Skip to content

CI(docker): Add docker labels from metadata#6667

Merged
echoix merged 2 commits intoOSGeo:mainfrom
Dasux:add_docker_labels_from_metadata
Mar 12, 2026
Merged

CI(docker): Add docker labels from metadata#6667
echoix merged 2 commits intoOSGeo:mainfrom
Dasux:add_docker_labels_from_metadata

Conversation

@Dasux
Copy link
Copy Markdown
Contributor

@Dasux Dasux commented Nov 25, 2025

This PR addresses issue #6511.

Following the maintainer’s guidance, this updates the Docker workflow to use the outputs of docker/metadata-action not only for annotations but also for image labels.
The goal is to ensure the metadata generated by the action is fully utilized in the build-and-push step, aligning the workflow with the examples shown in the upstream docker metadata documentation.

If anything else should be added or adjusted, I’m happy to iterate further.

@github-actions github-actions bot added the CI Continuous integration label Nov 25, 2025
Comment thread .github/workflows/add-docker-labels.yml Fixed
@echoix
Copy link
Copy Markdown
Member

echoix commented Nov 25, 2025

It still looks like it is adding a new workflow. Can you review your PR, try it on your fork, and let us know when we should look at it again?

@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Nov 26, 2025

Thanks for pointing that out — I’ve reviewed the PR again and removed the stray workflow file.
I’ve verified the updated branch on my fork as well.
One thing I'd like to mention...

I tested this on my fork, and the workflow runs fine — but the labels: field in the build output still shows empty values.
Is this expected behavior inside forks, or am I missing something about when these metadata values actually populate?
image

Copy link
Copy Markdown
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im reading about how labels should be created by the action. In case I still can't be sure, I would be comfortable to merge and try to fix after

Comment thread .github/workflows/docker.yml Outdated
@echoix echoix changed the title Add docker labels from metadata CI(docker): Add docker labels from metadata Nov 29, 2025
@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Feb 3, 2026

Hi,

This PR adds Docker image labels via docker/metadata-action outputs and wires them into the docker/build-push-action step.

The labels are generated correctly during CI, but they are only applied to images pushed from the main repository (not from forks), since Docker images are not published for pull requests or forked repos. This also explains why I couldn't see them on my fork earlier.

I also resolved the recent workflow conflict by merging upstream changes (build-args and image annotations) while preserving the label injection.

Could you please review the workflow changes?

@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 3, 2026

hi @echoix ...

just a ping..
could you please approve the workflows ... i'll verify if its working here

@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 4, 2026

Why does this PR remove some explanation comments not directly related to the changes?

And can you tell if the permission change for attesting the artifacts is 100% needed for not failing the builds? If it isn't necessary, you could remove it here, and create a new PR concerning attestations, since anyways the attest-build-provenance should be migrated to "actions/attest"
https://github.com/actions/attest, as it is now only a wrapper in order to not stop working. We would review that part there

@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 9, 2026

Why does this PR remove some explanation comments not directly related to the changes?

And can you tell if the permission change for attesting the artifacts is 100% needed for not failing the builds? If it isn't necessary, you could remove it here, and create a new PR concerning attestations, since anyways the attest-build-provenance should be migrated to "actions/attest" https://github.com/actions/attest, as it is now only a wrapper in order to not stop working. We would review that part there

thanks for pointing that out... the comments were unintentionally removed while modifying the workflow... i didn't intend to change unrelated parts of the file...

i've restored the removed comments in the next commit.. so it only contains the relevant changes

regarding the attestations perms..... I added it from the examples from github actions... im not sure if they're strictly necessary here...
i have removed those in this current commit aswell
we can handle attestations separately as suggested...

@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 9, 2026

You removed everything, not just the extraneous changes you made. Try to review your PR here against n before, then come back to us when ready.

@Dasux Dasux force-pushed the add_docker_labels_from_metadata branch from 5b86ea7 to fd4bc78 Compare March 10, 2026 14:00
Copy link
Copy Markdown
Contributor Author

@Dasux Dasux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the diff should show only adding the labels using meta-data...

there is one more thing i noticed... some actions are currently pinned to specific commit SHAs...

  • uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5.10.0
  • uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0

In the official docs, these are referenced using major version tags (v6 and v7), which are newer than the old v5.10.0 and v6.18.0

for this PR i left them untouched, but do you think these should be updated in another PR??

@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 10, 2026

Now the diff should show only adding the labels using meta-data...

there is one more thing i noticed... some actions are currently pinned to specific commit SHAs...

  • uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5.10.0
  • uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0

In the official docs, these are referenced using major version tags (v6 and v7), which are newer than the old v5.10.0 and v6.18.0

for this PR i left them untouched, but do you think these should be updated in another PR??

We take care of of the updates with renovate. The new major versions are like a week or so old. Many actions recently switched to the latest node LTS versions. There are reasons why we pin to specific hashes. Until recently, GitHub actions didn't have immutable releases. Pinning by hash was the only way to know you are not getting another content with the same tag label (a new tag pointing to a different commit instead).

@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 11, 2026

We take care of of the updates with renovate. The new major versions are like a week or so old. Many actions recently switched to the latest node LTS versions. There are reasons why we pin to specific hashes. Until recently, GitHub actions didn't have immutable releases. Pinning by hash was the only way to know you are not getting another content with the same tag label (a new tag pointing to a different commit instead).

got it--- stability matters more...

@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 12, 2026

Can we merge this?

@echoix echoix enabled auto-merge (squash) March 12, 2026 10:54
@Dasux
Copy link
Copy Markdown
Contributor Author

Dasux commented Mar 12, 2026

I think it should be the other way around?? Why are we merging main into this branch?
Correct me if I'm wrong..

@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 12, 2026

I think it should be the other way around?? Why are we merging main into this branch? Correct me if I'm wrong..

CI wasn’t approved/ran yet, so might as well run it with fresh changes if it needed to be ran to have the required successful checks

@echoix
Copy link
Copy Markdown
Member

echoix commented Mar 12, 2026

One failed (macOS), so I triggered a rerun only for that

@echoix echoix merged commit 26a670c into OSGeo:main Mar 12, 2026
31 of 32 checks passed
@github-actions github-actions bot added this to the 8.6.0 milestone Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants