util: rewrite test/prepend_testcerts_openssl.sh, update testdata #421
Conversation
This file contains 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
The old version needed to be run from a specific directory, didn't pass `shellcheck`, and would unconditionally prepend the OpenSSL text output to all certs in the testdata dir (even if they already had it). This version should be safer and suitable to be integrated with CI in a later step. It also supports taking a glob as the first argument and only prepending certs that need it and have a filename that matches the glob.
This test file does not parse successfully with OpenSSL 1.1.1d on my dev machine. Adding a small text note about this before the PEM content avoids the `v2/test/prepend_testcerts_openssl.sh` script emitting a warning.
This updates CI to run the `test/prepend_testcerts_openssl.sh` script and fail if there are any diffs to the `testdata/` directory. This would indicate there was a `.pem` file that didn't have text prepended to it.
…lint into cpu-improve-openssl-prepend-script
sleevi
reviewed
Mar 17, 2020
|
Since this is test-only I'm going to go ahead and merge it. Thanks! |
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.
The old version of
test/prepend_testcerts_openssl.shneeded to be run from a specific directory, didn't passshellcheck, and would unconditionally prepend the OpenSSL text output to all certs in the testdata dir (even if they already had it).The version in 04c9d99 should be safer and suitable to be integrated with CI. It also supports taking a glob as the first argument and only prepending certs that need it and have a filename that matches the glob. I kept it in BASH rather than rewriting it in Go since we're shelling out to
openssland I find that's fiddly work in Go and the overall complexity of the script is low.b250164 contains the results of running the script against the full
testdatadirectory. Many testcerts were missing OpenSSL text output and this commit fixes them across the board.06f8b73 is the one exception where
opensslfails to parse the certificate. For now I've added a note about this to the.pemfile such that theprepend_testcerts_openssl.shscript no longer emits a warning. I haven't done any investigation work into the testcert/testcase and why it fails to parse.db871ab updates CI to run the
test/prepend_testcerts_openssl.shscript for each branch's build and exit with an error code if there are diffs. This will make sure we don't introduce any new test certs after b250164 that forget to include some text before the PEM contents.