test: improve https_renew_cert.sh script - #42343
Merged
nodejs-github-bot merged 1 commit intoMar 17, 2022
Merged
nodejs-github-bot merged 1 commit into
nodejs-github-bot merged 1 commit into
Conversation
tniessen
force-pushed
the
test-improve-https-renew-cert-script
branch
from
March 15, 2022 11:27
1dd83f9 to
090583b
Compare
Collaborator
Member
|
Some suggestions to improve it further:
|
richardlau
approved these changes
Mar 15, 2022
tniessen
force-pushed
the
test-improve-https-renew-cert-script
branch
from
March 15, 2022 13:59
090583b to
509afd4
Compare
Member
Author
|
@lpinca Done, for whoever is going to run it in 10 years 😄 |
lpinca
approved these changes
Mar 15, 2022
aduh95
reviewed
Mar 15, 2022
aduh95
reviewed
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
force-pushed
the
test-improve-https-renew-cert-script
branch
from
March 15, 2022 16:38
509afd4 to
1a2391d
Compare
commented
Mar 15, 2022
Collaborator
aduh95
approved these changes
Mar 15, 2022
Comment on lines
+7
to
+9
| if [ ! -f key.pem ]; then | ||
| openssl genrsa -out key.pem 2048 | ||
| fi |
Contributor
There was a problem hiding this comment.
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 |
Mesteery
approved these changes
Mar 15, 2022
This was referenced Mar 16, 2022
commented
Mar 17, 2022
Collaborator
|
Landed in adbc94c |
This was referenced Mar 18, 2022
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
openssl rsa.#!/usr/bin/env bashinstead of#!/bin/bash.Refs: #42342
Refs: #37990