-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: add OCI image annotations to docker images #3554
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have specific needs for these labels? They will not be used anyway, see my comment.
https://github.com/moby/buildkit/pull/3204/files#diff-ffe3a297ed15e9a79846161a3731a580511bf13c6b5cc5a9e03ddcbd8515fdeeR205-R234 would help though for the BuildKit image.
.github/workflows/dockerd.yml
Outdated
- | ||
name: Build | ||
if: steps.build.outputs.result == 'true' | ||
uses: docker/build-push-action@v3 | ||
with: | ||
labels: ${{ steps.meta.outputs.labels }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This step only produces BuildKit artifacts, not an image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have misunderstood the CI implementation, sorry!
Where would I make the change and have it actually apply then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated this PR so it adds the annotations in the right place
Yes, I do. I use these images in a GitLab CI pipelines and I have Renovate submitting MRs to update the image references. Currently, the Renovate MRs do not have a changelog because Renovate doesn't know the GitHub URL for the project. By adding these labels, Renovate will be able to find the GitHub project and therefore include release notes. See https://github.com/renovatebot/renovate/blob/34.115.1/lib/modules/datasource/docker/readme.md I also use other tools that would use the labels, including Snyk which uses them in its UI. So having these labels populated would make life for myself and my colleagues just a little bit nicer, and I can only assume they would help others as well. |
CI looks broken, I don't see any checks. Can you rebase? |
I've rebased and taken a different approach that addresses the previously expressed concerns. Can you please run the workflow and review this PR? |
hack/images
Outdated
outputFlag="${outputFlag},annotation.org.opencontainers.image.title=BuildKit" | ||
if [ -n "$GITHUB_SHA" ]; then | ||
outputFlag="${outputFlag},annotation.org.opencontainers.image.revision=$GITHUB_SHA" | ||
fi | ||
if [ -n "$GITHUB_REPOSITORY" ]; then | ||
outputFlag="${outputFlag},annotation.org.opencontainers.image.source=$GITHUB_SERVER_URL/$GITHUB_REPOSITORY" | ||
outputFlag="${outputFlag},annotation.org.opencontainers.image.url=$GITHUB_SERVER_URL/$GITHUB_REPOSITORY" | ||
fi | ||
if [ -n "$versionTag" ]; then | ||
outputFlag="${outputFlag},annotation.org.opencontainers.image.version=$versionTag" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be done only on push so moved in https://github.com/moby/buildkit/pull/3554/files#diff-d131ef7e8954a6ffbaa57cfce3e105863587a19e310500d0c7c55b9f52038bfdR47
or if we also want to set annotations on pr for testing check localmode
is not set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, thank you!
Is there any more to do, or is this now ready to be merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry forgot to set "request for changes", we should check localmode
is not set for this code otherwise it would break as docker exporter does not support annotations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also add GITHUB_SERVER_URL
and GITHUB_SHA
before
Line 7 in 36ffdf7
: "${GITHUB_REPOSITORY=}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should also check if [ "$GITHUB_ACTIONS" = "true" ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated this MR addressing your concerns. How does it look now?
See: https://github.com/opencontainers/image-spec/blob/main/annotations.md#pre-defined-annotation-keys Closes: moby#3553 Signed-off-by: Craig Andrews <candrews@integralblue.com>
@crazy-max I saw that the I don't understand the other failing test, https://github.com/moby/buildkit/actions/runs/9996089167/job/27633669860?pr=3554 - it doesn't seem related. Please let me know what else, if anything, I can do to get this across the finish line. Thank you again! |
@crazy-max friendly bump |
This one is related to docker/buildx#2598 which has been fixed in v0.16.1: https://github.com/docker/buildx/releases/tag/v0.16.1. Rerunning the workflow with your PR and should be good |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks!
@candrews This might need a follow-up if you want the same for the Dockerfile frontend image. |
That would be ideal - can you point me in the right direction and I'll get on it? |
|
See: https://github.com/opencontainers/image-spec/blob/main/annotations.md#pre-defined-annotation-keys
Closes: #3553
Signed-off-by: Craig Andrews candrews@integralblue.com