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

[#726] feat(doc): Add document on how to sign and verify releases #729

Merged
merged 4 commits into from
Nov 16, 2023

Conversation

justinmclean
Copy link
Member

@justinmclean justinmclean commented Nov 13, 2023

What changes were proposed in this pull request?

Add documentation on how to sign and verify a release.

Why are the changes needed?

Signed releases are more trusted and a requirement for passing several security checks.

Fix: #726

Does this PR introduce any user-facing change?

No

How was this patch tested?

Tested shell commands on existing 0.2 release.

@justinmclean
Copy link
Member Author

Once this is merged I'll sign and update the 0.2 release.

Copy link

github-actions bot commented Nov 13, 2023

Code Coverage Report

Overall Project 67.25% 🟢

There is no coverage information present for the Files changed

docs/how-to-sign-releases.md Outdated Show resolved Hide resolved
docs/how-to-sign-releases.md Show resolved Hide resolved
@justinmclean justinmclean self-assigned this Nov 13, 2023

```shell
shasum -a 512 <filename>.[zip|tar.gz] > <filename>.[zip|tar.gz].sha512
```
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK, we don't need step 2-4 to sign a release binary, our gradle assembleDistribution will automatically generate a sha256 file corresponding to the release binary.

Copy link
Member Author

Choose a reason for hiding this comment

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

A hash is not a signature. Hashes can be used to verify the release contents, but a signature can be used to verify release authenticity and contents.

Copy link
Member Author

@justinmclean justinmclean Nov 13, 2023

Choose a reason for hiding this comment

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

The assembleDistribution only produces a ".tar.gz", we have both a "zip" and a ".tar.gz" in GitHub, and both will need hashes and signatures. Any reason why sha256 rather than sha512 was selected?

Copy link
Member

Choose a reason for hiding this comment

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

The assembleDistribution only produces a ".tar.gz", we have both a "zip" and a ".tar.gz" in GitHub, and both will need hashes and signatures.

If desired, we can support the generation of both tar.gz and .zip files with a few code changes in the build.gradle.kts script.

Any reason why sha256 rather than sha512 was selected?

There is no particular reason to choose sha256, we can change it to sha512 if needed

@jerryshao
Copy link
Contributor

@xunliu , please take a review, thanks.

Copy link
Member

@xunliu xunliu left a comment

Choose a reason for hiding this comment

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

The sha256 code generated by this command is the same as gravitino-0.3.0-SNAPSHOT-bin.tar.gz.sha256

shasum -a 512 <filename>.[zip|tar.gz] > <filename>.[zip|tar.gz].sha512
shasum -a 256 gravitino-0.3.0-SNAPSHOT-bin.tar.gz

Could we use the gradle signing plugin to perform this step?
Just like the Publish Gravitino related jar and documentation to OSSRH #543 method?

@justinmclean
Copy link
Member Author

I can change to using sha256, but you didn't seem to answer why sha256 was chosen on sha512. I also think you may also be confusing GitHub releases with Gradle distributions.

@justinmclean
Copy link
Member Author

@justinmclean
Copy link
Member Author

I also created a video https://youtu.be/P4F5oe6hIxA showing the process.

@jerryshao
Copy link
Contributor

I can change to using sha256, but you didn't seem to answer why sha256 was chosen on sha512. I also think you may also be confusing GitHub releases with Gradle distributions.

@xunliu please reply this.

@xunliu
Copy link
Member

xunliu commented Nov 15, 2023

The sha256 code generated by this command is the same as gravitino-0.3.0-SNAPSHOT-bin.tar.gz.sha256

shasum -a 512 <filename>.[zip|tar.gz] > <filename>.[zip|tar.gz].sha512
shasum -a 256 gravitino-0.3.0-SNAPSHOT-bin.tar.gz

Could we use the gradle signing plugin to perform this step? Just like the Publish Gravitino related jar and documentation to OSSRH #543 method?

My only question is whether it is possible to automate the steps in this PR via a gradle script.
If following the open source rules requires manually going through and following the documentation, I have no problem with that. :-)

@jerryshao
Copy link
Contributor

We can unify with sha512, @justinmclean can you please update the related gradle scripts?

@xunliu
Copy link
Member

xunliu commented Nov 15, 2023

Additionally, Github Action IT execution fails because docker is bound to a fixed port docker run -P 50070:50070, I use new docker container testing method to fixed this issue in the #711, So we can re-run Github Action temporarily ignore it.

@justinmclean
Copy link
Member Author

So I looked at modifying build.gradle.kts to produce both the zip and tar.gz, and sign and generate hashes and while it is certainly possible it's a bit more involved than I first thought. The signing requires user setup. I think it would be best to make automating this a separate issue as it is not a high priority in the short term.

@jerryshao jerryshao changed the title [#726] Add document on how to sign and verify releases [#726] feat(doc): Add document on how to sign and verify releases Nov 16, 2023
@jerryshao jerryshao merged commit 9634324 into apache:main Nov 16, 2023
2 checks passed
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.

[Improvement] Sign releases
4 participants