-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Remove Secret from ClusterCa generate cert methods #10915
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
Conversation
Signed-off-by: Katherine Stanley <[email protected]>
330c87c
to
93fa54c
Compare
/azp run regression |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
One nit. But it seems to be good otherwise. Thanks.
Map<String, CertAndKey> existingCertificates, | ||
Set<NodeRef> nodes, | ||
boolean isMaintenanceTimeWindowsSatisfied, | ||
boolean forceRenew |
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.
Here - as well as in the other methods below - you call it forceRenewe
. But you pass into it the value hasCaCertGenerationChanged
. Given force-renew is an annotation for force certificate renewal, I think we should rename this to better correspond to what indicates here? E.g. caCertGenerationChanged
?
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.
Yeah I see your point, I've renamed it
Signed-off-by: Katherine Stanley <[email protected]>
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.
Thanks @katheris
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!
Type of change
Description
Remove use of Secret object from ClusterCa generate certificate methods.
To support per-broker Secrets for certificates we need to separate the handling of the certificates from storing them in the Secret.
Support issue #7687
Checklist
Please go through this checklist and make sure all applicable tasks have been done