Fix ingress stuck in deletion when IngressClass is removed first#4624
Fix ingress stuck in deletion when IngressClass is removed first#4624aydosman wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
When an IngressClass is deleted before the Ingresses that reference it, the reconciler correctly marks those Ingresses as inactive members and attempts to remove their group finalizers via a PATCH/UPDATE. That update triggers the ValidateUpdate admission webhook, which tries to look up the now-missing IngressClass and denies the request. The finalizer is never removed, leaving the Ingress permanently stuck with a DeletionTimestamp and no way to clean it up short of manually editing the spec. The fix is to skip all ValidateUpdate checks when the Ingress already has a DeletionTimestamp set. An ingress in that state has no active invariants to enforce — the only meaningful operation remaining is cleanup. This is consistent with ValidateDelete, which already returns nil unconditionally. Adds a test covering both the fixed path (deleting ingress with missing IngressClass is allowed) and the unchanged path (non-deleting ingress with missing IngressClass is still denied). Fixes: kubernetes-sigs#4341
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aydosman The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @aydosman! |
k8s-ci-robot
added
needs-ok-to-test
|
Hi @aydosman. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
k8s-ci-robot
added
size/M
|
Hi @zac-nixon @shraddhabang — this is a fix for #4341. The issue was that |
|
|
||
| func (v *ingressValidator) ValidateUpdate(ctx context.Context, obj runtime.Object, oldObj runtime.Object) error { | ||
| ing := obj.(*networking.Ingress) | ||
| // skip validation for ingresses being deleted to allow finalizer removal when IngressClass is already gone. |
There was a problem hiding this comment.
You're not really checking if the IngressClass is gone here. If the intended logic is to allow Ingress deletion to proceed when IngressClass is already gone, then adding a check for a NotFoundError is probably better.
https://github.com/aydosman/aws-load-balancer-controller/blob/5d4c3e1288ccfa840027d12c028ca3651c18de9d/webhooks/networking/ingress_validator.go#L90-L94
There was a problem hiding this comment.
@zac-nixon Thank you; I was going for the simplest solution. I will look at this at some point this week.
3 participants
Issue
#4341
Description
When an IngressClass is deleted before the Ingresses that reference it,
the reconciler correctly marks those Ingresses as inactive members and
attempts to remove their group finalizers via a PATCH/UPDATE. That update
triggers the ValidateUpdate admission webhook, which tries to look up the
now-missing IngressClass and denies the request. The finalizer is never
removed, leaving the Ingress permanently stuck with a DeletionTimestamp
and no way to clean it up short of manually editing the spec.
The fix is to skip all ValidateUpdate checks when the Ingress already has
a DeletionTimestamp set. An ingress in that state has no active invariants
to enforce — the only meaningful operation remaining is cleanup. This is
consistent with ValidateDelete, which already returns nil unconditionally.
Adds a test covering both the fixed path (deleting ingress with missing
IngressClass is allowed) and the unchanged path (non-deleting ingress with
missing IngressClass is still denied).
Checklist
README.md, or thedocsdirectory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯