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

Don't sign yet, plus attempt to shell to cosign version at start + fix install in gcb #62

Merged
merged 3 commits into from
Oct 14, 2021

Conversation

SgtCoDFish
Copy link
Member

@SgtCoDFish SgtCoDFish commented Oct 13, 2021

quay.io needs to be updated to add support for "custom mime types" before we can sign containers. Most other registries seems to support signatures and have for a long time, but we use quay and we'll have to wait for it.

In the meantime, shelling to cosign version at the start of a publish gives information about the version of cosign being used as well as providing a soft early check that the binary exists and is callable. Even though we're not gonna actually call cosign sign until Quay catches up, there's no reason I see that we shouldn't check that cosign is working and installed correctly at least.

The changes to the GCB yaml took a lot of trial and painful error to get to. In the end I've ended up just copying what we already did for cmrel - more the fool's me for thinking that using go install might be the way to go.

this gives information about the version of cosign being used as well as
providing a soft early check that the binary exists and is callable

Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
there aren't enough words in the english language to describe
how painful this process was

Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 13, 2021
Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
@SgtCoDFish SgtCoDFish changed the title Attempt to shell to cosign version at start + fix install in gcb Don't sign yet, plus attempt to shell to cosign version at start + fix install in gcb Oct 13, 2021
Copy link
Contributor

@irbekrm irbekrm left a comment

Choose a reason for hiding this comment

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

My main comment is about whether it wouldn't have been easier to build our own image with cosign built-in? https://cloud.google.com/build/docs/cloud-builders

I think the only reason why to install a tool rather than build it into an image would be if we want to run the same thing locally as well as in a container somewhere, but this is not the case with these build steps I guess?

Otherwise, this looks harmless to me, happy for you to merge when ready

/hold
/lgtm
/approve

gcb/publish/cloudbuild.yaml Outdated Show resolved Hide resolved
@jetstack-bot jetstack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 13, 2021
@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2021
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irbekrm, SgtCoDFish

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@SgtCoDFish
Copy link
Member Author

/unhold

Thanks for the review!

@jetstack-bot jetstack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 14, 2021
@jetstack-bot jetstack-bot merged commit 6ce6131 into cert-manager:master Oct 14, 2021
@SgtCoDFish SgtCoDFish deleted the whynocosign branch October 14, 2021 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants