Add new flag to watch ingressclass by name instead of spec (#7609)

charts/ingress-nginx/templates/controller-daemonset.yaml

@@ -114,6 +114,9 @@ spec:
           {{- if .Values.controller.healthCheckHost }}
             - --healthz-host={{ .Values.controller.healthCheckHost }}
           {{- end }}
+          {{- if .Values.controller.ingressClassByName }}
+            - --ingress-class-by-name=true
+          {{- end }}
           {{- if .Values.controller.watchIngressWithoutClass }}
             - --watch-ingress-without-class=true
           {{- end }}

charts/ingress-nginx/templates/controller-deployment.yaml

@@ -115,6 +115,9 @@ spec:
           {{- if not (eq .Values.controller.healthCheckPath "/healthz") }}
             - --health-check-path={{ .Values.controller.healthCheckPath }}
           {{- end }}
+          {{- if .Values.controller.ingressClassByName }}
+            - --ingress-class-by-name=true
+          {{- end }}
           {{- if .Values.controller.watchIngressWithoutClass }}
             - --watch-ingress-without-class=true
           {{- end }}

charts/ingress-nginx/values.yaml

@@ -66,6 +66,9 @@ controller:
   # Defaults to false
   watchIngressWithoutClass: false
 
+  # Process IngressClass per name (additionally as per spec.controller)
+  ingressClassByName: false
+
   # Required for use with CNI based kubernetes installations (such as ones set up by kubeadm),
   # since CNI and hostport don't mix yet. Can be deprecated once https://github.com/kubernetes/kubernetes/issues/23920
   # is merged

cmd/nginx/flags.go

@@ -68,6 +68,9 @@ referenced in an Ingress Object should be the same value specified here to make
 		watchWithoutClass = flags.Bool("watch-ingress-without-class", false,
 			`Define if Ingress Controller should also watch for Ingresses without an IngressClass or the annotation specified`)
 
+		ingressClassByName = flags.Bool("ingress-class-by-name", false,
+			`Define if Ingress Controller should watch for Ingress Class by Name together with Controller Class`)
+
 		configMap = flags.String("configmap", "",
 			`Name of the ConfigMap containing custom global configurations for the controller.`)
 
@@ -302,6 +305,7 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g
 			Controller:         *ingressClassController,
 			AnnotationValue:    *ingressClassAnnotation,
 			WatchWithoutClass:  *watchWithoutClass,
+			IngressClassByName: *ingressClassByName,
 		},
 		DisableCatchAll:           *disableCatchAll,
 		ValidationWebhook:         *validationWebhook,

internal/ingress/controller/ingressclass/ingressclass.go

@@ -42,4 +42,8 @@ type IngressClassConfiguration struct {
 	// WatchWithoutClass defines if Controller should watch to Ingress Objects that does
 	// not contain an IngressClass configuration
 	WatchWithoutClass bool
+
+	//IngressClassByName defines if the Controller should watch for Ingress Classes by
+	// .metadata.name together with .spec.Controller
+	IngressClassByName bool
 }

internal/ingress/controller/store/store.go

@@ -426,7 +426,12 @@ func New(
 	ingressClassEventHandler := cache.ResourceEventHandlerFuncs{
 		AddFunc: func(obj interface{}) {
 			ingressclass := obj.(*networkingv1.IngressClass)
-			if ingressclass.Spec.Controller != icConfig.Controller {
+			foundClassByName := false
+			if icConfig.IngressClassByName && ingressclass.Name == icConfig.AnnotationValue {
+				klog.InfoS("adding ingressclass as ingress-class-by-name is configured", "ingressclass", klog.KObj(ingressclass))
+				foundClassByName = true
+			}
+			if !foundClassByName && ingressclass.Spec.Controller != icConfig.Controller {
 				klog.InfoS("ignoring ingressclass as the spec.controller is not the same of this ingress", "ingressclass", klog.KObj(ingressclass))
 				return
 			}

internal/ingress/controller/store/store_test.go

@@ -316,7 +316,7 @@ func TestStore(t *testing.T) {
 
 		err := framework.WaitForIngressInNamespace(clientSet, ns, ing.Name)
 		if err != nil {
-			t.Errorf("error waiting for secret: %v", err)
+			t.Errorf("error waiting for ingress: %v", err)
 		}
 		time.Sleep(1 * time.Second)
 
@@ -486,6 +486,112 @@ func TestStore(t *testing.T) {
 		}
 	})
 
+	t.Run("should return two events for add and delete and one for update of ingress and watch-ingress-by-name", func(t *testing.T) {
+		ns := createNamespace(clientSet, t)
+		defer deleteNamespace(ns, clientSet, t)
+		ic := createIngressClass(clientSet, t, "not-k8s.io/by-name")
+		defer deleteIngressClass(ic, clientSet, t)
+
+		createConfigMap(clientSet, ns, t)
+
+		stopCh := make(chan struct{})
+		updateCh := channels.NewRingChannel(1024)
+
+		var add uint64
+		var upd uint64
+		var del uint64
+
+		go func(ch *channels.RingChannel) {
+			for {
+				evt, ok := <-ch.Out()
+				if !ok {
+					return
+				}
+
+				e := evt.(Event)
+				if e.Obj == nil {
+					continue
+				}
+				if _, ok := e.Obj.(*networking.Ingress); !ok {
+					continue
+				}
+
+				switch e.Type {
+				case CreateEvent:
+					atomic.AddUint64(&add, 1)
+				case UpdateEvent:
+					atomic.AddUint64(&upd, 1)
+				case DeleteEvent:
+					atomic.AddUint64(&del, 1)
+				}
+			}
+		}(updateCh)
+
+		ingressClassconfig := &ingressclass.IngressClassConfiguration{
+			Controller:         ingressclass.DefaultControllerName,
+			AnnotationValue:    ic,
+			IngressClassByName: true,
+		}
+
+		storer := New(
+			ns,
+			fmt.Sprintf("%v/config", ns),
+			fmt.Sprintf("%v/tcp", ns),
+			fmt.Sprintf("%v/udp", ns),
+			"",
+			10*time.Minute,
+			clientSet,
+			updateCh,
+			false,
+			ingressClassconfig)
+
+		storer.Run(stopCh)
+		validSpec := commonIngressSpec
+		validSpec.IngressClassName = &ic
+		ing := ensureIngress(&networking.Ingress{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "ingclass-by-name",
+				Namespace: ns,
+			},
+			Spec: validSpec,
+		}, clientSet, t)
+
+		err := framework.WaitForIngressInNamespace(clientSet, ns, ing.Name)
+		if err != nil {
+			t.Errorf("error waiting for ingress: %v", err)
+		}
+		time.Sleep(1 * time.Second)
+
+		ingressUpdated := ing.DeepCopy()
+		ingressUpdated.Spec.Rules[0].Host = "update-dummy"
+		_ = ensureIngress(ingressUpdated, clientSet, t)
+		if err != nil {
+			t.Errorf("error updating ingress: %v", err)
+		}
+		// Secret takes a bit to update
+		time.Sleep(3 * time.Second)
+
+		err = clientSet.NetworkingV1().Ingresses(ingressUpdated.Namespace).Delete(context.TODO(), ingressUpdated.Name, metav1.DeleteOptions{})
+		if err != nil {
+			t.Errorf("error deleting ingress: %v", err)
+		}
+
+		err = framework.WaitForNoIngressInNamespace(clientSet, ingressUpdated.Namespace, ingressUpdated.Name)
+		if err != nil {
+			t.Errorf("error waiting for ingress deletion: %v", err)
+		}
+
+		if atomic.LoadUint64(&add) != 1 {
+			t.Errorf("expected 1 event of type Create but %v occurred", add)
+		}
+		if atomic.LoadUint64(&upd) != 1 {
+			t.Errorf("expected 1 event of type Update but %v occurred", upd)
+		}
+		if atomic.LoadUint64(&del) != 1 {
+			t.Errorf("expected 1 event of type Delete but %v occurred", del)
+		}
+	})
+
 	t.Run("should not receive updates for ingress with invalid class annotation", func(t *testing.T) {
 		ns := createNamespace(clientSet, t)
 		defer deleteNamespace(ns, clientSet, t)

test/e2e/settings/ingress_class.go

@@ -510,4 +510,72 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() {
 		})
 
 	})
+
+	ginkgo.Context("With ingress-class-by-name flag", func() {
+		ginkgo.BeforeEach(func() {
+			err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error {
+				args := []string{}
+				for _, v := range deployment.Spec.Template.Spec.Containers[0].Args {
+					if strings.Contains(v, "--ingress-class-by-name") &&
+						strings.Contains(v, "--ingress-class=test-new-ingress-class") {
+						continue
+					}
+
+					args = append(args, v)
+				}
+				args = append(args, "--ingress-class=test-new-ingress-class")
+				args = append(args, "--ingress-class-by-name")
+				deployment.Spec.Template.Spec.Containers[0].Args = args
+				_, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{})
+
+				return err
+			})
+			assert.Nil(ginkgo.GinkgoT(), err, "updating ingress controller deployment flags")
+		})
+
+		ginkgo.It("should watch Ingress that uses the class name even if spec is different", func() {
+			validHostClassName := "validhostclassname"
+
+			ing := framework.NewSingleIngress(validHostClassName, "/", validHostClassName, f.Namespace, framework.EchoService, 80, nil)
+			ing.Spec.IngressClassName = &otherIngressClassName
+			f.EnsureIngress(ing)
+
+			validHostClass := "validhostclassspec"
+			ing = framework.NewSingleIngress(validHostClass, "/", validHostClass, f.Namespace, framework.EchoService, 80, nil)
+			f.EnsureIngress(ing)
+
+			invalidHost := "invalidannotation"
+			annotations := map[string]string{
+				ingressclass.IngressKey: "testclass123",
+			}
+			ing = framework.NewSingleIngress(invalidHost, "/", invalidHost, f.Namespace, framework.EchoService, 80, annotations)
+			ing.Spec.IngressClassName = nil
+			f.EnsureIngress(ing)
+
+			f.WaitForNginxConfiguration(func(cfg string) bool {
+				return strings.Contains(cfg, "server_name validhostclassname") &&
+					strings.Contains(cfg, "server_name validhostclassspec") &&
+					!strings.Contains(cfg, "server_name invalidannotation")
+			})
+
+			f.HTTPTestClient().
+				GET("/").
+				WithHeader("Host", validHostClass).
+				Expect().
+				Status(http.StatusOK)
+
+			f.HTTPTestClient().
+				GET("/").
+				WithHeader("Host", validHostClassName).
+				Expect().
+				Status(http.StatusOK)
+
+			f.HTTPTestClient().
+				GET("/").
+				WithHeader("Host", invalidHost).
+				Expect().
+				Status(http.StatusNotFound)
+		})
+
+	})
 })