-
Notifications
You must be signed in to change notification settings - Fork 333
Shell script to verify staged release candidate artifacts #2824
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
base: main
Are you sure you want to change the base?
Conversation
9f81def to
29e8c26
Compare
b8f99e0 to
bc3f147
Compare
bc3f147 to
f9dde4b
Compare
adutra
left a comment
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.
Excellent improvement that will greatly simplify release verification!
My impression is that the tool is mostly focusing on artifact comparison and reproducible builds. A possible future addition would be to actually test the distribution by building and running basic tests, including tests for Helm chart.
| * Maven staging repository | ||
| * `DISCLAIMER`, `LICENSE` and `NOTICE` files are correct for the repository. | ||
| * Contents of jar artifacts `META-INF/LICENSE` and `META-INF/NOTICE` files are correct. | ||
| * All files have license headers if necessary. |
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.
Why not include a step to automate this?
I usually use the below to test the source distribution and check for licenses etc.
./gradlew rat test :checkForCopiedCode \
:polaris-server:assemble \
:polaris-server:quarkusAppPartsBuild --rerun \
-Dquarkus.container-image.tag=${version} \
-Dquarkus.container-image.build=trueThere 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.
The assumption is that CI, which performs all these checks, is "green".
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.
BTW: The script can keep (-k option) the (temporary) directory, which contains all the downloaded and locally built artifacts.
Tests can be run from there.
site/content/release-verify.md
Outdated
| * Contents of jar artifacts `META-INF/LICENSE` and `META-INF/NOTICE` files are correct. | ||
| * All files have license headers if necessary. | ||
| This is (mostly) verified using the "rat" tool during builds/CI. | ||
| * No compiled artifacts are bundled in the source archive. |
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 could also be automated:
executable_files=$(find . -type f ! -size 0 | perl -lne 'print if -B' | xargs file | grep executable)
if [ -n "$executable_files" ]; then
echo "Binary executable files found:"
echo "$executable_files"
exit 1
fiThere 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.
Hm, I think the line 52 should read No disallowed binary artifacts are bundled in the source archive..
A strict "no binary artifacts at all" is difficult to achieve, as png and jpeg are binary.
Bash scripts for example are permitted, as those are "text" files, despite being executable.
f55737d to
8ef4cb7
Compare
8ef4cb7 to
248911c
Compare
site/content/release-verify.md
Outdated
| bash <(curl \ | ||
| -s https://raw.githubusercontent.com/apache/polaris/refs/heads/main/tools/verify-release/verify-release.sh) \ | ||
| --git-sha 354a5ef6b337bf690b7a12fefe2c984e2139b029 \ | ||
| --version 1.2.0 \ |
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.
Could you clarify whether users are expected to pass 1.2.0 or 1.2.0-incubating? I do not see the incubating label anywhere in the example. And I think it is a requirement for the project to have all releases annotated with incubating. (I have not read the verify-release.sh script yet)
Assuming we want users to specify the version they want to verify, as opposed to letting the script discover the version from the git tag, it would be more user friendly to request a full version string like --version 1.2.0-incubating-rc2
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.
Agree. Updated the script to accept 9.8.7-incubating-rc42.
| # Nuke the directory listings (index.html from server) and robots.txt... | ||
| # (only wget2 downloads the robots.txt :( ) | ||
| find "${dir}" \( -name index.html -o -name robots.txt \) -exec rm {} + | ||
| find "${dir}" -name "*.prov" | while read -r helmProv; do |
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.
Could you clarify why this is necessary? helmProv makes me think that you want the helm chart related .prov files. But there are other such files in a release. And AFAIK they are not gzipped.
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, this is a workaround for wget2. A nasty one:
# The following is a hack for wget2, which behaves a bit different than wget.
# If the server returns `Content-Type: application/x-gzip`, the file is stored gzipped,
# although it's "plain text". Leaving it as gzip breaks signature + checksum tests.
Added the comment for clarification.
| log_info "Git commit from tag '${git_tag_full}': ${git_sha_from_tag}" | ||
| log_info "Git commit on tag '${git_tag_full}': ${git_sha_on_tag}" | ||
| if [[ "$git_sha_on_tag" != "$git_sha_from_tag" ]]; then | ||
| log_fatal "Git SHA ${git_sha_from_tag} on ${git_tag_full} is different from the current SHA ${git_sha_on_tag}" |
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.
In which case can this ever happen, considering L390 we run git checkout "${git_tag_full}"?
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.
Ah, left-over from a previous approach. The check's not necessary anymore.
cba3ed1 to
6a205fd
Compare
|
For 1.3.0-rc1, the output looks as follows. The reported errors are fixed with #3145 + #3146 + #3147 (on main, not in rc1). |
Performs a bunch of verifications against a proposed (staged) release candidate using the new `tools/verify-release/verify-release.sh` script against Maven artifacts, main distributions and Helm chart. Checks: * GPG signature and checksum verifications * All expected artifacts are present * Build artifacts are reproducible (minus known exceptions) * jar files * Main distribution zip/tarball * Helm chart * Build passes. * DISCLAIMER/LICENSE/NOTICE files are present in artifacts that require those More information in the added web site page. Fixes apache#2822
* UTF-8 error/warning signs * emit diff on tarball listing differences, if tarballs are different * dump tarball listings to disk, if tarballs are different * do not purge temp directory when errors were reported * fix usage of `find` for helm-chart checks
6a205fd to
9c9fbe9
Compare
Performs a bunch of verifications against a proposed (staged) release candidate using the new
tools/verify-release/verify-release.shscript against Maven artifacts, main distributions and Helm chart.Checks:
More information in the added web site page.
Fixes #2822