Skip to content

test: improve https_renew_cert.sh script - #42343

Merged
nodejs-github-bot merged 1 commit into
nodejs:masterfrom
tniessen:test-improve-https-renew-cert-script
Mar 17, 2022
Merged

nodejs-github-bot merged 1 commit into
nodejs:masterfrom
tniessen:test-improve-https-renew-cert-script

Conversation

@tniessen

@tniessen tniessen commented Mar 15, 2022

Copy link
Copy Markdown
Member
  • To avoid unnecessarily large diffs, only generate a new private key if necessary. Otherwise, reuse the existing private key and only issue a new certificate.
  • Remove an unnecessary conversion step using openssl rsa.
  • Extend the certificate validity from 1 year to 10 years.
  • Show a text representation of the issued certificate upon completion such that the user can verify the validity.
  • Make the script executable.
  • Use #!/usr/bin/env bash instead of #!/bin/bash.
  • Allow the script to be called from any directory.

Refs: #42342
Refs: #37990

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure. labels Mar 15, 2022
@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 15, 2022
@tniessen
tniessen force-pushed the test-improve-https-renew-cert-script branch from 1dd83f9 to 090583b Compare March 15, 2022 11:27
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 15, 2022
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@lpinca

lpinca commented Mar 15, 2022

Copy link
Copy Markdown
Member

Some suggestions to improve it further:

  • Make the script executable.
  • Use /usr/bin/env bash instead of /bin/bash.
  • Allow it to be called from any path.

@tniessen tniessen added the blocked PRs that are blocked by other issues or PRs. label Mar 15, 2022
@tniessen
tniessen force-pushed the test-improve-https-renew-cert-script branch from 090583b to 509afd4 Compare March 15, 2022 13:59
@tniessen

Copy link
Copy Markdown
Member Author

@lpinca Done, for whoever is going to run it in 10 years 😄

Comment thread test/fixtures/keys/selfsigned-no-keycertsign/https_renew_cert.sh Outdated
Comment thread test/fixtures/keys/selfsigned-no-keycertsign/https_renew_cert.sh Outdated
Comment thread test/fixtures/keys/selfsigned-no-keycertsign/https_renew_cert.sh Outdated
Comment thread test/fixtures/keys/selfsigned-no-keycertsign/.gitignore Outdated
@tniessen tniessen removed the blocked PRs that are blocked by other issues or PRs. label Mar 15, 2022
- To avoid unnecessarily large diffs, only generate a new private key
  if necessary. Otherwise, reuse the existing private key and only
  issue a new certificate.
- Remove an unnecessary conversion step using openssl rsa and the
  intermediate rsa.pem and csr.pem files.
- Extend the certificate validity from 1 year to 10 years.
- Show a text representation of the issued certificate upon completion
  such that the user can verify the validity.
- Make the script executable.
- Use "#!/usr/bin/env bash" instead of "#!/bin/bash".
- Allow the script to be called from any directory.

Refs: nodejs#42342
Refs: nodejs#37990
@tniessen
tniessen force-pushed the test-improve-https-renew-cert-script branch from 509afd4 to 1a2391d Compare March 15, 2022 16:38
@tniessen tniessen added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Mar 15, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 15, 2022
@nodejs-github-bot

ghost commented Mar 15, 2022

Copy link
Copy Markdown
Collaborator

Comment on lines +7 to +9
if [ ! -f key.pem ]; then
openssl genrsa -out key.pem 2048
fi

ghost Mar 15, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: this could be simplified

Suggested change
if [ ! -f key.pem ]; then
openssl genrsa -out key.pem 2048
fi
[ -f key.pem ] || openssl genrsa -out key.pem 2048

@tniessen tniessen added the commit-queue PRs queued for automated landing through the Commit Queue. label Mar 17, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Mar 17, 2022
@nodejs-github-bot
nodejs-github-bot merged commit adbc94c into nodejs:master Mar 17, 2022
@nodejs-github-bot

ghost commented Mar 17, 2022

Copy link
Copy Markdown
Collaborator

Landed in adbc94c

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

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants