-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat/rsa keys #59
Merged
Merged
Feat/rsa keys #59
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
This allows the test framework to generate RSA keys Signed-off-by: Bruno Bressi <[email protected]>
The port 5000 is used in mac for some other server.
Additionally bumped dependencies & code to go 1.23
Signed-off-by: Bruno Bressi <[email protected]>
Signed-off-by: Bruno Bressi <[email protected]>
Also moved port back to 5000 Signed-off-by: Bruno Bressi <[email protected]>
To make the tests easier to maintain, a variable was introduced for the port used in the ephemeral private registry used.
This makes the tests somewhat easier to maintain and read.
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.
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.
Signed-off-by: Bruno Bressi <[email protected]>
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.
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
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).
puffitos
added
enhancement
New feature or request
dependencies
Pull requests that update a dependency file
labels
Sep 16, 2024
This was a typo Signed-off-by: Bruno Bressi <[email protected]>
Signed-off-by: Bruno Bressi <[email protected]>
eumel8
approved these changes
Sep 20, 2024
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.
LGTM
* 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]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
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.
Motivation
Mostly resolves #57. Some additional E2E testing should be done to lock this feature down.
Changes
New features:
Housekeeping changes:
sign
package directly; it's simpler to use.Tests done
Two new E2E test cases were added to show that this does indeed work. Some manual testing was done to check how to import keys to cosing, sign and verify containers with those keys.