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

binary: add sha256 checksum to downloads #4109

Merged

Conversation

johanneslarsson
Copy link
Contributor

Adds sha256 checksum to all binaries

These checksums can be used to verify file integrity.

Fixes #3448

srenatus
srenatus previously approved these changes Dec 7, 2021
Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

✔️ So in this PR, the GHA binaries.zip already contains the checksums and they're OK.
✔️ Via this workflow, those shasums should end up on the github release, too.

Traditionally, anything touching the release process breaks the release process. 😅 But lets merge this and deal with that when we get there.

Could you fix DCO or just squash it all into one commit, please?

docs/content/_index.md Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@johanneslarsson
Copy link
Contributor Author

✔️ So in this PR, the GHA binaries.zip already contains the checksums and they're OK. ✔️ Via this workflow, those shasums should end up on the github release, too.

Traditionally, anything touching the release process breaks the release process. 😅 But lets merge this and deal with that when we get there.

Could you fix DCO or just squash it all into one commit, please?

I'm quite sure I probably missed something 😅, but these changes was the obvious ones.

@johanneslarsson
Copy link
Contributor Author

I read in the issue that checksums where wanted in the release notes, but I'm not sure how it would work.

@srenatus
Copy link
Contributor

srenatus commented Dec 8, 2021

I read in the issue that checksums where wanted in the release notes, but I'm not sure how it would work.

I suppose we could adapt the github release script (that creates the draft release for each push of a tag) to also generate a draft message that would include the checksums. It would then be the starting point, and the manually created release notes would be paste above it or something.

But also we can worry about that next. (From a security perspective, I'm not sure I understand the difference between having the shasums in assets vs having them mentioned in the release notes. I think if you have the privilege to change one of them, you likely have it to change the other, too. I might be missing something, though.)

@srenatus
Copy link
Contributor

srenatus commented Dec 8, 2021

Unrelated netlify failure... I've retried the deployment.

@johanneslarsson
Copy link
Contributor Author

I read in the issue that checksums where wanted in the release notes, but I'm not sure how it would work.

I suppose we could adapt the github release script (that creates the draft release for each push of a tag) to also generate a draft message that would include the checksums. It would then be the starting point, and the manually created release notes would be paste above it or something.

But also we can worry about that next. (From a security perspective, I'm not sure I understand the difference between having the shasums in assets vs having them mentioned in the release notes. I think if you have the privilege to change one of them, you likely have it to change the other, too. I might be missing something, though.)

Yes, I agree.

@johanneslarsson
Copy link
Contributor Author

Unrelated netlify failure... I've retried the deployment.

I'll re-phrase my doc change to trigger a retry

Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

Thanks again!

I'm afraid I might not have the proper AWS keys to fix this if there's any fallout, so let's get this merged when people with 🔑 are online 😅

Copy link
Member

@tsandall tsandall left a comment

Choose a reason for hiding this comment

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

@johanneslarsson this looks good to me. Thanks! I'll merge now and then verify the release automation is working.

@tsandall tsandall merged commit 9ec964b into open-policy-agent:main Dec 8, 2021
@johanneslarsson johanneslarsson deleted the add-sha256-checksums branch January 27, 2022 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Consider adding validation of checksums or signatures to OPA install documentation
3 participants