-
Notifications
You must be signed in to change notification settings - Fork 0
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
NH-82783: add jar signing using signpath.io for for GitHub release. #251
Conversation
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.
Thanks @cleverchuk! had a question and a comment :)
.github/workflows/release.yml
Outdated
echo "Status -> $status_state" | ||
|
||
sleep 5 | ||
done |
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.
I wonder if we should have a max retry in the above loop, so that any signing service issues/outages do not hold up release process, we just won't have the signed version of the jar.
And to be safe, how about we only download the jar when status is "Completed"
?
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.
I wonder if we should have a max retry in the above loop, so that any signing service issues/outages do not hold up release process, we just won't have the signed version of the jar.
Turned it into a separate job. Added the -f
flag which fails the job on server issue
And to be safe, how about we only download the jar when status is "Completed"?
It looks like it's not necessary because it's a separate job now?
.github/workflows/release.yml
Outdated
SIGN_PATH_SIGNING_POLICY: ${{ secrets.SIGN_PATH_SIGNING_POLICY }} | ||
SIGN_PATH_ORG_ID: ${{ secrets.SIGN_PATH_ORG_ID }} | ||
SIGN_PATH_ARTIFACT_SLUG: ${{ secrets.SIGN_PATH_ARTIFACT_SLUG }} | ||
|
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.
Were these set based on info in Keeper? Wondering if the Keeper entry is updated, or even still needed.
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.
Yes, they were excluding SIGN_PATH_ARTIFACT_SLUG
which doesn't exist there(or I missed it 🤷♂️ ).
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.
LGTM, thanks for the revisit @cleverchuk!
We're integrating signpath.io jar signing feature into the build pipeline. This PR adds a job that sends a signing request to signpath.io then waits for the processing to complete by polling the signing request data endpoint every 5 seconds. Once the endpoint responds with final status of the signing request, we proceed to downloading the signed jar. We either failing to download the signed jar which will fail the job or we succeed which will cause the job to continue to the next step of uploading the signed jar to GitHub release. This job has an implicit dependency on the
github_release
job and should be run after thegithub_release
has completed. This implicit dependency will be turned into an explicit one in a follow-on PR.