From 5d4c3e1288ccfa840027d12c028ca3651c18de9d Mon Sep 17 00:00:00 2001 From: aydosman Date: Sun, 15 Mar 2026 14:27:42 +0000 Subject: [PATCH] Fix ingress stuck in deletion when IngressClass is removed first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/kubernetes-sigs/aws-load-balancer-controller/issues/4341 --- webhooks/networking/ingress_validator.go | 4 + webhooks/networking/ingress_validator_test.go | 81 +++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/webhooks/networking/ingress_validator.go b/webhooks/networking/ingress_validator.go index 52bfeec90b..426743afe5 100644 --- a/webhooks/networking/ingress_validator.go +++ b/webhooks/networking/ingress_validator.go @@ -83,6 +83,10 @@ func (v *ingressValidator) ValidateCreate(ctx context.Context, obj runtime.Objec 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. + if !ing.DeletionTimestamp.IsZero() { + return nil + } oldIng := oldObj.(*networking.Ingress) if skip, err := v.checkIngressClass(ctx, ing); skip || err != nil { v.metricsCollector.ObserveWebhookValidationError(apiPathValidateNetworkingIngress, "checkIngressClass") diff --git a/webhooks/networking/ingress_validator_test.go b/webhooks/networking/ingress_validator_test.go index 4fc5edb8dc..1eb0dbf528 100644 --- a/webhooks/networking/ingress_validator_test.go +++ b/webhooks/networking/ingress_validator_test.go @@ -1171,3 +1171,84 @@ func Test_ingressValidator_checkIngressAnnotationConditions(t *testing.T) { }) } } + +func Test_ingressValidator_ValidateUpdate_deletingIngress(t *testing.T) { + now := metav1.Now() + tests := []struct { + name string + ing *networking.Ingress + oldIng *networking.Ingress + wantErr bool + }{ + { + name: "ingress being deleted with missing IngressClass should be allowed", + ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns-1", + Name: "ing-1", + DeletionTimestamp: &now, + Finalizers: []string{"ingress.k8s.aws/resources"}, + }, + Spec: networking.IngressSpec{ + IngressClassName: awssdk.String("deleted-ingress-class"), + }, + }, + oldIng: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns-1", + Name: "ing-1", + }, + Spec: networking.IngressSpec{ + IngressClassName: awssdk.String("deleted-ingress-class"), + }, + }, + wantErr: false, + }, + { + name: "ingress not being deleted with missing IngressClass should be denied", + ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns-1", + Name: "ing-1", + }, + Spec: networking.IngressSpec{ + IngressClassName: awssdk.String("deleted-ingress-class"), + }, + }, + oldIng: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns-1", + Name: "ing-1", + }, + Spec: networking.IngressSpec{ + IngressClassName: awssdk.String("other-class"), + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + k8sSchema := runtime.NewScheme() + clientgoscheme.AddToScheme(k8sSchema) + elbv2api.AddToScheme(k8sSchema) + k8sClient := testclient.NewClientBuilder().WithScheme(k8sSchema).Build() + mockMetricsCollector := lbcmetrics.NewMockCollector() + v := &ingressValidator{ + annotationParser: annotations.NewSuffixAnnotationParser(annotations.AnnotationPrefixIngress), + classAnnotationMatcher: ingress.NewDefaultClassAnnotationMatcher("alb"), + classLoader: ingress.NewDefaultClassLoader(k8sClient, false), + manageIngressesWithoutIngressClass: false, + logger: logr.New(&log.NullLogSink{}), + metricsCollector: mockMetricsCollector, + } + err := v.ValidateUpdate(ctx, tt.ing, tt.oldIng) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +}