Skip to content
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

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

candrews
Copy link
Contributor

@candrews candrews commented Jan 30, 2023

Copy link
Member

@crazy-max crazy-max left a 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:/moby/buildkit/pull/3204/files#diff-ffe3a297ed15e9a79846161a3731a580511bf13c6b5cc5a9e03ddcbd8515fdeeR205-R234 would help though for the BuildKit image.

-
name: Build
if: steps.build.outputs.result == 'true'
uses: docker/build-push-action@v3
with:
labels: ${{ steps.meta.outputs.labels }}
Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Contributor Author

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

@candrews
Copy link
Contributor Author

Do you have specific needs for these labels?

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:/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.

@crazy-max
Copy link
Member

CI looks broken, I don't see any checks. Can you rebase?

@candrews candrews changed the title ci: add labels to docker images ci: add OCI image annotatons to docker images Jul 12, 2024
@candrews candrews changed the title ci: add OCI image annotatons to docker images ci: add OCI image annotations to docker images Jul 12, 2024
@candrews
Copy link
Contributor Author

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
Comment on lines 55 to 67
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
Copy link
Member

@crazy-max crazy-max Jul 18, 2024

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:/moby/buildkit/pull/3554/files#diff-d131ef7e8954a6ffbaa57cfce3e105863587a19e310500d0c7c55b9f52038bfdR47

or if we also want to set annotations on pr for testing check localmode is not set

Copy link
Contributor Author

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?

Copy link
Member

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

Copy link
Member

@crazy-max crazy-max Jul 18, 2024

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

: "${GITHUB_REPOSITORY=}"

Copy link
Member

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" ]

Copy link
Contributor Author

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?

@candrews
Copy link
Contributor Author

@crazy-max I saw that the shfmt test failed due to an extra space; I updated the MR to fix that.

I don't understand the other failing test, https:/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!

@candrews
Copy link
Contributor Author

@crazy-max friendly bump

@crazy-max
Copy link
Member

crazy-max commented Jul 29, 2024

I don't understand the other failing test, https:/moby/buildkit/actions/runs/9996089167/job/27633669860?pr=3554 - it doesn't seem related.

This one is related to docker/buildx#2598 which has been fixed in v0.16.1: https:/docker/buildx/releases/tag/v0.16.1.

Rerunning the workflow with your PR and should be good

Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@crazy-max crazy-max merged commit d75ba52 into moby:master Jul 29, 2024
75 checks passed
@crazy-max
Copy link
Member

@candrews This might need a follow-up if you want the same for the Dockerfile frontend image.

@candrews
Copy link
Contributor Author

@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?

@crazy-max
Copy link
Member

@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?

https:/moby/buildkit/blob/master/frontend/dockerfile/cmd/dockerfile-frontend/hack/release

@candrews
Copy link
Contributor Author

@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?

https:/moby/buildkit/blob/master/frontend/dockerfile/cmd/dockerfile-frontend/hack/release

Thank you! Here's the PR: #5197

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci area/hack building buildkit itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add OCI image annotations to docker hub published moby/buildkit images
3 participants