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

Cosign #81

Merged
merged 1 commit into from Feb 4, 2022
Merged

Cosign #81

merged 1 commit into from Feb 4, 2022

Conversation

hslatman
Copy link
Contributor

@hslatman hslatman commented Feb 2, 2022

Closes #69

@fabpot
Copy link
Contributor

fabpot commented Feb 2, 2022

Thank you for working on this.

Would it make sense to add a small note in the README to tell people how they can verify their binaries?

@hslatman
Copy link
Contributor Author

hslatman commented Feb 2, 2022

@fabpot: that absolutely makes sense!

There are a couple of things to consider going forward and I think these are things that need your approval. I've described these in the issue: #69 (comment). Decision made will also affect the instructions for the user.

It seems the workflow execution failed in this repo. I think you approved of it manually? It seems that cosign didn't have access to an identity token and started an OIDC flow as part of the GH action. This probably has something to do with the GITHUB_TOKEN not having the right permissions, but I'm not sure about it. I did a test run on my fork and there it did continue without doing an OIDC device flow.

@fabpot
Copy link
Contributor

fabpot commented Feb 3, 2022

Regarding your questions, I would do the simplest thing for our users, so signing the binaries themselves. For SBOMs, if nobody asked for them, let's not do that for now. I would like to keep this PR as simple as possible.

@hslatman hslatman force-pushed the cosign branch 2 times, most recently from 7bb670b to d0a6f67 Compare February 3, 2022 22:49
@hslatman hslatman marked this pull request as ready for review February 3, 2022 22:59
@hslatman
Copy link
Contributor Author

hslatman commented Feb 3, 2022

@fabpot: I've addressed your comments. Is the README OK like this?

I haven't been able to test the full action flow when using a version tag, so I'm curious to see if it works as expected 🙂

@fabpot
Copy link
Contributor

fabpot commented Feb 4, 2022

The build fails.

@hslatman
Copy link
Contributor Author

hslatman commented Feb 4, 2022

@fabpot: it's likely due to using GH_PAT instead of GITHUB_TOKEN. I don't think a PAT can be used to get an identity token; the temporary GITHUB_TOKEN can.

Do you want to go forward using the GH_PAT for other purposes? If so, I'll try to pass the GITHUB_TOKEN to just the signing operation, so that it can get an identity token. Otherwise I'll replace GH_PAT with the GITHUB_TOKEN and then it should work.

To be complete: these tokens wouldn't be necessary if I didn't use the keyless method; a private key with password would be enough instead. But from what I gather, keyless will likely be the recommended method in the future.

@fabpot
Copy link
Contributor

fabpot commented Feb 4, 2022

Let's keep it simple by using the GITHUB_TOKEN everywhere then.

@hslatman
Copy link
Contributor Author

hslatman commented Feb 4, 2022

@fabpot: changed it. Let's see if it works now. I don't know if your PAT had certain permissions that the GITHUB_TOKEN currently does not, so that could cause an issue. Updating the permissions with the ones required should help then.

@hslatman
Copy link
Contributor Author

hslatman commented Feb 4, 2022

@fabpot
Copy link
Contributor

fabpot commented Feb 4, 2022

I've just created a branch with your changes to see if that would work and indeed, that works well: https://github.com/symfony-cli/symfony-cli/runs/5065888193?check_suite_focus=true

@fabpot fabpot merged commit b7b6b33 into symfony-cli:main Feb 4, 2022
@fabpot
Copy link
Contributor

fabpot commented Feb 4, 2022

Thank you @hslatman

@fabpot
Copy link
Contributor

fabpot commented Feb 4, 2022

@hslatman But unfortunately, the released failed: https://github.com/symfony-cli/symfony-cli/runs/5065962537?check_suite_focus=true
Can you have a look? Do we really need the proxy?

@hslatman
Copy link
Contributor Author

hslatman commented Feb 4, 2022

I noticed; too bad 😞

Using the Go module proxy is not necessarily required, no. Disabling that in the GoReleaser config should make it run.

I tried it before and thought it was because the tag wasn't on the original repo. That apparently wasn't the issue. It's likely due to Go module versioning. I'll have a look at doing that the proper way, but for now disabling the proxy is OK.

EDIT: PR is here: #85

@hslatman
Copy link
Contributor Author

hslatman commented Feb 4, 2022

I'll fix the issue with the Homebrew tap.

EDIT: this should fix it: #86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Sign release artifacts using sigstore/cosign
2 participants