Refactor Encryption logic for improved testing#645
Conversation
| encryptionConfigFilepath := filepath.Join("testdata", "encryption-provider-config-wildcard.yaml") | ||
| configBytes, _ := os.ReadFile(encryptionConfigFilepath) |
There was a problem hiding this comment.
Instead of the filepath + os.ReadFile can use:
backup-restore-operator/e2e/test/test.go
Line 14 in 155b1a9
this way the tests aren't dependent on specific paths
can also probably move that into a pkg/test instead of e2e/test if you want
There was a problem hiding this comment.
Maybe as a follow up PR we move that to test, and consider making it more generic? To use:
func Data(testFs embed.FS, filename string) []byte {
So then we can still keep organizing each pkgs test stubs in place?
NVM, I forgot we need to pass the k8s apis we're re-using a file path. So I'm not sure the Data() trick works here, since the apiserver code reads the bytes out the config file itself.
Making a mental note that this area is also something we could potentially workaround in the future with expansions to BROs logic around encryption.
|
/backport release/v6.x |
|
Not creating port PR, there was an error running git am -3: |
|
Manual backporting it is... |
| // Open the file for reading (or other operations) and return the handle | ||
| file, err := os.Open(EncryptionProviderConfigKey) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
There was a problem hiding this comment.
I didn't notice this before, but you we should be careful about opening files inside a function and expecting others to close said file.
Can lead to concurrent closes or file descriptor leaks.
We should look into alternative return types instead of os.File. I also don't see this being used by anything other than the tests at the moment.
Will we need this inside the controller logic?
|
Part of rancher/rancher#48832 |
Before the change there are 2 files in
util; 0% and 22% respectively.Before this change, the
utilpackage is currently at 50% coverage.After the change there are 3 files (counting new sub package); 0%, 13% and 70.4% respectively.
After this change, the
utilpackage will be at 66.7% coverage.