-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add CA chains when missing in the broker Secrets #12403
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
base: main
Are you sure you want to change the base?
Add CA chains when missing in the broker Secrets #12403
Conversation
Signed-off-by: Jakub Scholz <[email protected]>
5e9efb8 to
aa36cee
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12403 +/- ##
============================================
- Coverage 75.02% 75.00% -0.02%
- Complexity 6660 6663 +3
============================================
Files 373 373
Lines 25385 25390 +5
Branches 3411 3412 +1
============================================
- Hits 19044 19043 -1
- Misses 4954 4957 +3
- Partials 1387 1390 +3
🚀 New features to boost your workflow:
|
|
/gha run pipeline=upgrade,regression |
|
⏳ System test verification started: link The following 10 job(s) will be executed:
Tests will start after successful build completion. |
|
🎉 System test verification passed: link |
cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ClusterCa.java
Outdated
Show resolved
Hide resolved
| // For more details, see https://github.com/strimzi/strimzi-kafka-operator/issues/12364. | ||
| // | ||
| // After some time - after multiple Strimzi releases, once the CA chains are added in all clusters, we | ||
| // should be able to remove this logic again. |
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.
should we have an issue open for this and referring to it in this comment? wdyt?
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.
I'm happy to open one if it is approved.
Signed-off-by: Jakub Scholz <[email protected]>
tinaselenge
left a comment
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, thanks Jakub.
ppatierno
left a comment
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
Type of change
Description
This PR continues on the work from #12390 and makes sure the CA chain is added during the upgrade of the operator. That makes sure that after the upgrade to Strimzi 0.51.0, the CA chain is added for existing clusters and not only for new ones.
Given the certificates are stored as byte arrays and the CA chain is at the end, the comparing is not completely straightforward. But hopefully I made it sufficiently simple and readable. After some time - once we are sure that everyone upgraded away from 0.48-0.50 - we should be able to remove this logic again.
(It also does minor refactoring of the
maybeCopyOrGenerateCertsmethod to avoid the negativeifnd make the code a bit more readable)This should (hopefully) resolve #12364
Checklist