Refactor E2E test suite + add RSA tests #60
Merged
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 tests have been refactored to use a dedicated struct for the private and public keys, which contains the key itself and the path to it. This will allow a bigger refactoring of the E2E tests, so that each test case can be run independently of what type of key is used for signing & validation Signed-off-by: Bruno Bressi <[email protected]>
Instead of hardcoding the path in all tests, the value is derived from the previously unused private key variable returned. This way, the tests can now be refactored to run by only passing the key creation function Signed-off-by: Bruno Bressi <[email protected]>
The framework struct has been refactored to abstract the golang testing framework. This allows the E2E test cases to be written without having to create a new framework for each test. The framework functions now do not have to do a lot of micromanagement and cleanup; they just check whether an error has happened and they return. This allows for new functions to be written without having to think about whether to fail the test or not. The cleanup function takes care of the final step; cleaning up everything and then deciding whether the test failed or passed. Additionally, a new type is introduced, which will be used to wrap the tests cases, so they can be run used t.Run.
The test cases are now refactored to accept a signing function, so that the same test can be run regardless of RSA/ECDSA key without having to write too much duplicate code. The new fuction type is used for the signing function and each test case must now return the set of actions required for the use case to be tested, wrapped in a func which returns testing.T, so it may be run by the t.Run method.
Added variable so that the additional E2E test is also executed. This test must be refactored in a future commit/ removed, as it depends on an image already being present on the machine running the test.
Each case tests for ECDSA keys is now also tested for RSA keys. The tests were also accelerated by reducing the delay between checks from 5s to 500m Signed-off-by: Bruno Bressi <[email protected]>
eumel8
pushed a commit
that referenced
this pull request
Sep 20, 2024
* test: added rsa generation This allows the test framework to generate RSA keys Signed-off-by: Bruno Bressi <[email protected]> * chore: use another port for k3d registry The port 5000 is used in mac for some other server. * feat: new rsa E2E test Additionally bumped dependencies & code to go 1.23 * feat: added RSA key support in verification process Signed-off-by: Bruno Bressi <[email protected]> * chore: formatting Signed-off-by: Bruno Bressi <[email protected]> * feat: new test case for RSA Also moved port back to 5000 Signed-off-by: Bruno Bressi <[email protected]> * refactor: use port variable To make the tests easier to maintain, a variable was introduced for the port used in the ephemeral private registry used. * refactor: use constants for images This makes the tests somewhat easier to maintain and read. * chore: more resilient cleanup The cleanup method can now be called always when a test is run using the framework, as it cleans up whatever is there and ignores the rest. * fix: rsa keys now properly generated The keys had to be also imported to the cosign format to be usable for signing containers. Additionally, this commit refactors the signing method to use the CLI directly and not the cobra command, which was kind of unintuitive. An additional test, which doesn't run per default was added to test whether the sign method really works. * chore: formatting Signed-off-by: Bruno Bressi <[email protected]> * fix: signing and RSA public key fixes Since the switch to the `sign` module, the signatures of the ephemeral images being used in tests were not uploaded to the repository. This resulted in test failure, as the public key had no signature to verify. Additionally, the errors with the RSA private key not being suited for image signing and verification are also solved in this commit. The proper encoding algorithms are now used and the the correct values are returned. The imported public key and the generated one are now the same, and the signing private key has the correct header now. WIP. * test: added signImage test for RSA A simple test locke behind an env variable to test whether an RSA key can be used to sign a container image. In the future, this test should be an autonomous integration test and not be connected to the busybox image created during the E2E preparation * chore: fixed E2E test Housekeeping commit to refactor the tests so they use the new keypath argument, which allows them more flexibility and opens up for a future refactoring to simplify the test suite and allow to run the same test suite for multiple input keys (ECDSA, RSA). * chore: removed double @@ This was a typo Signed-off-by: Bruno Bressi <[email protected]> * docs: explanation of dns flag in e2e tests [skip ci] Signed-off-by: Bruno Bressi <[email protected]> * Refactor E2E test suite + add RSA tests (#60) * refactor: own struct for keys The tests have been refactored to use a dedicated struct for the private and public keys, which contains the key itself and the path to it. This will allow a bigger refactoring of the E2E tests, so that each test case can be run independently of what type of key is used for signing & validation Signed-off-by: Bruno Bressi <[email protected]> * refactor: use private key variable Instead of hardcoding the path in all tests, the value is derived from the previously unused private key variable returned. This way, the tests can now be refactored to run by only passing the key creation function Signed-off-by: Bruno Bressi <[email protected]> * refactor: [WIP] framework wraps testing.T The framework struct has been refactored to abstract the golang testing framework. This allows the E2E test cases to be written without having to create a new framework for each test. The framework functions now do not have to do a lot of micromanagement and cleanup; they just check whether an error has happened and they return. This allows for new functions to be written without having to think about whether to fail the test or not. The cleanup function takes care of the final step; cleaning up everything and then deciding whether the test failed or passed. Additionally, a new type is introduced, which will be used to wrap the tests cases, so they can be run used t.Run. * refactor: use new testing schema The test cases are now refactored to accept a signing function, so that the same test can be run regardless of RSA/ECDSA key without having to write too much duplicate code. The new fuction type is used for the signing function and each test case must now return the set of actions required for the use case to be tested, wrapped in a func which returns testing.T, so it may be run by the t.Run method. * chore: added E2E variable Added variable so that the additional E2E test is also executed. This test must be refactored in a future commit/ removed, as it depends on an image already being present on the machine running the test. * test: added rsa tests cases Each case tests for ECDSA keys is now also tested for RSA keys. The tests were also accelerated by reducing the delay between checks from 5s to 500m Signed-off-by: Bruno Bressi <[email protected]> --------- Signed-off-by: Bruno Bressi <[email protected]> --------- Signed-off-by: Bruno Bressi <[email protected]>
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.
Please merge before #59, as this is part of testing the feature introduced in the PR to solve #57. Once it's merged we can also merge 59 into main and do a release.
Motivation
Changes done
The E2E test cases were refactored to allow passing a signing function and the framework itself, to avoid having to create the framework in each test. Since the tests cases now don't just accept
testing.Tas an argument, they were refactored to return a function, so each case can still be run by callingt.Run(testCase).The next logical step seemed to be to wrap the
testing.Ttype in our ownFramework, as this is the role it principally plays: the testing framework for the E2E test cases. We let theFrameworkcentrally take care of handling errors & checking whether a test passed or failed and skip micromanaging in each function of the Framework (each function did its own cleanup and calledt.Fataleach time). We simplify theFrameworkby introducing graceful error handling (let everything fail and show first error) and letting each test case follow the same pattern:The
Frameworkwill do the rest. New tests just need to adhere to this pattern.Tests done