-
Notifications
You must be signed in to change notification settings - Fork 103
Cleanup orphan certificates when nodes are removed from NodeSet #1792
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?
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rabi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| } | ||
| if _, exists := allHostnames[hostname]; !exists { | ||
| util.LogForObject(helper, fmt.Sprintf("Deleting stale certificate %s for removed node %s", cert.Name, hostname), instance) | ||
| if err := helper.GetClient().Delete(ctx, &cert); client.IgnoreNotFound(err) != nil { |
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 think we should also delete the secret holding the certs after the cert CR was removed. In cert-manager there is only a global switch to cleanup the secrets holding the cert when the cert CR gets deleted, which is per default false and not happening. If we'd leave the cert secret and you add a node with the same hostname it would re-use the older certs, instead of replacing them this new ones.
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, let me add that. Thanks.
When a node is removed from a DataPlane NodeSet, the TLS certificates created for that node were not being cleaned up, leaving orphan Certificate resources in the cluster. This adds a cleanupStaleCertificates function that runs at the beginning of EnsureTLSCerts to delete certificates for nodes that no longer exist in the NodeSet. It uses the allHostnames map (which reflects current nodes) to identify stale certificates by comparing against the hostname label on existing Certificate CRs. Jira: OSPRH-26077 Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: rabi <ramishra@redhat.com>
|
/test functional |
|
@rabi: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/a92c233d21144e86ad9d6b51329980ab ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 19m 38s |
When a node is removed from a DataPlane NodeSet, the TLS certificates created for that node were not being cleaned up, leaving orphan Certificate resources in the cluster.
This adds a cleanupStaleCertificates function that runs at the beginning of EnsureTLSCerts to delete certificates for nodes that no longer exist in the NodeSet. It uses the allHostnames map (which reflects current nodes) to identify stale certificates by comparing against the hostname label on existing Certificate CRs.
Jira: OSPRH-26077