Skip to content
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

[main] Cannot predict encryption transformer results before running after k8s 1.32 updates #642

Open
mallardduck opened this issue Jan 13, 2025 · 2 comments

Comments

@mallardduck
Copy link
Member

This issue potentially affects and has chances of regressions: #599

This issue affects our ability to update and support for k8s 1.32. I found this issue in the process of making a draft PR to do this on main. In theory we can start that work because wrangler has an RC with 1.32 support. So I wanted us to be ahead of that task this time around.

In testing I found that our CI tests immediately start to fail when updating to the 0.32.0 apis that match k8s 1.32. Here is an example of the errors I'm seeing:

=== RUN   TestIsDefaultEncryptionTransformer_PartialWildcard
    util_test.go:78: 
        	Error Trace:	.../SUSE/backup-restore-operator/pkg/util/util_test.go:78
        	Error:      	Should be true
        	Test:       	TestIsDefaultEncryptionTransformer_PartialWildcard
--- FAIL: TestIsDefaultEncryptionTransformer_PartialWildcard (3.86s)

This test (and all other new failing tests) rely on our ability to check the transformer via:

func IsDefaultEncryptionTransformer(transformer value.Transformer) bool {
return transformer == identity.NewEncryptCheckTransformer()
}

This is due to a change in upstream k8s apiserver library which seems to no longer allow us to verify the transformer before it runs.

Granted because we are not the etcd/apiserver that is the target for this code we cannot be surprised. However we still need to find both a short term and long term solution for how we with to adapt to this type of issue. I think that for our current concern we should find a solution that works - but should open a tech-debt issue to consider the long-term solution if we want to maintain an internal version of this code instead of relying on potentially changing apiserver code.

@mallardduck
Copy link
Member Author

This is an initial PR for solving this issue: #644

While it resolves the issue we've encountered with the upgrade, there are a few distinct downsides:

  1. it's slightly less testable,
  2. it now relies on more of a "run then compare" methodology (compared to only running encrypt/decrypt when necessary), and
  3. it may continue to change in the future.

@mallardduck
Copy link
Member Author

I'm going to split this work up across two PRs:

  1. Tech-Debt focused refactor to improve tests: Refactor Encryption logic for improved testing #645
  2. Then the k8s 1.32 updates

This allows PR1 to be backported and PR2 to be one that only lands at main. Overall this strategy should keep our active branches more similar in structure for maintainability. This way all active branches take some of the necessary changes for 1.32 but not all of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant