-
Notifications
You must be signed in to change notification settings - Fork 52
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
tests: further cleanup/generalization. #65
Conversation
Also switch to using named arguments and sensible defaults when omitted.
Previously there was a lot of duplicated path constants (e.g. "name_constraints/"). We were also doing path manipulation in both Python and Rust code. This commit removes the duplication by introducing more variables. It also lifts all of the path manipulation to the Python code and switches to `os.path.join` instead of raw string bashing.
This allows choosing which test cases are generated, whether to run cargo fmt, and whether to run cargo test. By default all tests are generated and both cargo commands are run, matching pre-existing behaviour. These flags are helpful when developing new test cases as it allows iterating on a subset without as much churn in the tree.
The top header comment for generate.py was overly specific to the name constraint test generation. This commit updates it to reflect the current state of the script.
In future tests we will be generating intermediate issuers and so it's helpful to differentiate the root trust anchor's private/public key from other issuers in a certificate chain.
This commit adds two optional arguments to `ca_cert` to allow specifying an issuer name and private key as opposed to always generating an issuer certificate that is self-issued. This will allow the `ca_cert` helper to be used to make longer certificate chains. Similarly, a new argument to optionally provide an `x509.KeyUsage` is added. This will allow generating issuer certificates that assert different usage bits (e.g. including/excluding cRLSign).
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.
Seems okay.
The continued proliferation of Python code makes me wish we'd converted to Rust code sooner though. Also for some of these refactoring steps, making smaller commits that change one thing at a time would have helped make review easier.
It might be doable using
Noted. Will try to break things up even further in the future. |
This branch follows #64 and introduces a few more helpful generalizations/cleanups that I made while writing CRL focused test cases.
Some highlights:
cargo fmt
andcargo test
invocations are run post-generation.These changes do not affect the generated test cases so I haven't committed updated test files. We know the generator script + the test cases it spits out still work thanks to the CI improvements that landed in #64 :-)