-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add metadata to our publish task for Tekton Chains to observe & sign #4156
Conversation
@@ -177,6 +183,8 @@ spec: | |||
IMAGE_WITHOUT_SHA_AND_TAG=${IMAGE_WITHOUT_SHA%%:*} | |||
IMAGE_WITH_SHA=${IMAGE_WITHOUT_SHA_AND_TAG}@${IMAGE##*@} | |||
|
|||
echo $IMAGE_WITH_SHA, >> $(results.IMAGES.path) |
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 is probably a silly question but will a trailing comma be okay? im guessing yes and that the effort to remove it isn't worth it XD
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.
ps do you know if the crane cp commands below will also produce more unique shas?
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.
Yeah I think so - this is taken verbatim from chains own publish script: https://github.com/tektoncd/chains/blob/main/release/publish.yaml
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.
/cc @priyawadhwa
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.
ps do you know if the crane cp commands below will also produce more unique shas?
Oo, this I don't know much about unfortunately.
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.
trailing comma should be fine!
& thecrane cp
command should result in the same digest!
VERY EXCITING!!!! /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bobcatfish The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@sbwsg: GitHub didn't allow me to request PR reviews from the following users: priyawadhwa. Note that only tektoncd members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
tekton/publish.yaml
Outdated
@@ -1,7 +1,9 @@ | |||
apiVersion: tekton.dev/v1beta1 | |||
gpiVersion: tekton.dev/v1beta1 |
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.
typo? 🤦♀️ 🤣
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.
Oh. Wow. Vim.
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.
Good catch, fixed, thanks!
@@ -177,6 +183,8 @@ spec: | |||
IMAGE_WITHOUT_SHA_AND_TAG=${IMAGE_WITHOUT_SHA%%:*} | |||
IMAGE_WITH_SHA=${IMAGE_WITHOUT_SHA_AND_TAG}@${IMAGE##*@} | |||
|
|||
echo $IMAGE_WITH_SHA, >> $(results.IMAGES.path) | |||
|
|||
if [[ "$(params.releaseAsLatest)" == "true" ]] | |||
then | |||
crane cp ${IMAGE_WITH_SHA} ${IMAGE_WITHOUT_SHA_AND_TAG}:latest |
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.
Comparing this to Chains publish task, we are missing REGION specific sha in the result here
https://github.com/tektoncd/chains/blob/main/release/publish.yaml#L202
Do we need to add one more echo
command here?
Edit: replaced L184 with L202 in the URL
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'd recommend adding that in so that all images are signed
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.
Amazing, thanks again - I didn't copy verbatim enough 😆
Added the additional region echo
This commit adds an annotation to indicate that build provenance should be generated and an `IMAGES` result composed of a comma-separated list of imageNames+digest to be signed. This change is based on https://github.com/tektoncd/chains/blob/main/release/publish.yaml and https://github.com/tektoncd/plumbing/blob/main/docs/signing.md
thanks @sbwsg |
Fixes #4153
Changes
This commit adds an annotation to indicate that build provenance should be
generated and an
IMAGES
result composed of a comma-separated list ofimageNames+digest to be signed.
This change is based on
https://github.com/tektoncd/chains/blob/main/release/publish.yaml and
https://github.com/tektoncd/plumbing/blob/main/docs/signing.md
/kind misc
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes