-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Support uppercase repository names with cosign. #1294
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.
Looks good to me
@@ -90,4 +90,4 @@ jobs: | |||
# This step uses the identity token to provision an ephemeral certificate | |||
# against the sigstore community Fulcio instance, and records it to the | |||
# sigstore community Rekor transparency log. | |||
run: cosign sign ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@${{ steps.build-and-push.outputs.digest }} | |||
run: cosign sign ${{ steps.meta.outputs.tags }}@${{ steps.build-and-push.outputs.digest }} |
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.
Could it be a problem if tags
points to a list of tags?
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.
Given line 66, I don't think that's possible today, but it could be a problem as folks alter the workflow.
7436922
to
078c8f1
Compare
👋 Looks like there are some merge conflicts, possibly from the comment change that went with #1295 |
I'll update this shortly |
078c8f1
to
08df7b2
Compare
Ok, this is rebased! |
This pull request has become stale and will be closed automatically within a period of time. Sorry about that. |
@mattmoor Do we still need this change to get merged in? I was surprised to see we never merged it back in December, but I guess holidays got in the way. |
Yeah, I can rebase it, but it had fallen off my radar. thanks for the ping. |
My previous PR didn't properly handle uppercase usernames (or repository names) when signing container images with `cosign`. It seems that the `docker buildx --push` doesn't like this either, but it's passed the output of the `docker/metadata-action` which seems to lowercase things. Fixes: actions#1293 Signed-off-by: Matt Moore <mattmoor@chainguard.dev>
08df7b2
to
002e1a4
Compare
your welcome
Sent from ProtonMail mobile
…-------- Original Message --------
On Mar. 16, 2022, 12:48 p.m., Matt Moore wrote:
Yeah, I can rebase it, but it had fallen off my radar. thanks for the ping.
—
Reply to this email directly, [view it on GitHub](#1294 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ASADIUN5R5SDHBWXO3LNEZTVAIUITANCNFSM5JPQBQ6Q).
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
Just noticed this still on my dashboard. I'm happy to rebase, but it'd be great to coordinate with someone that can merge this to avoid spinning my wheels rebasing 🙏 |
@mattmoor should we go ahead with merge, if there are no changes required? |
It's been a while, but I'm pretty sure this is good to go. thanks! |
My previous PR didn't properly handle uppercase usernames (or repository names) when signing container images with
cosign
.It seems that the
docker buildx --push
doesn't like this either, but it's passed the output of thedocker/metadata-action
which seems to lowercase things.Fixes: #1293
Signed-off-by: Matt Moore mattmoor@chainguard.dev
thanks to @imjasonh for the suggestion!
Pre-requisites
Tasks
For all workflows, the workflow:
.yml
file with the language or platform as its filename, in lower, kebab-cased format (for example,docker-image.yml
). Special characters should be removed or replaced with words as appropriate (for example, "dotnet" instead of ".NET").For CI workflows, the workflow:
ci
directory.ci/properties/*.properties.json
file (for example,ci/properties/docker-publish.properties.json
).push
tobranches: [ $default-branch ]
andpull_request
tobranches: [ $default-branch ]
.release
withtypes: [ created ]
.docker-publish.yml
).For Code Scanning workflows, the workflow:
code-scanning
directory.code-scanning/properties/*.properties.json
file (for example,code-scanning/properties/codeql.properties.json
), with properties set as follows:name
: Name of the Code Scanning integration.organization
: Name of the organization producing the Code Scanning integration.description
: Short description of the Code Scanning integration.categories
: Array of languages supported by the Code Scanning integration.iconName
: Name of the SVG logo representing the Code Scanning integration. This SVG logo must be present in theicons
directory.push
tobranches: [ $default-branch, $protected-branches ]
andpull_request
tobranches: [ $default-branch ]
. We also recommend aschedule
trigger ofcron: $cron-weekly
(for example,codeql.yml
).Some general notes:
actions
organization, or