Controller: Fix host/path overlap detection for multiple rules. (#13162)

Author: rkevin-arch

internal/ingress/controller/controller.go

@@ -1834,16 +1834,14 @@ func checkOverlap(ing *networking.Ingress, servers []*ingress.Server) error {
 				continue
 			}
 
-			// same ingress
+			// path overlap. Check if one of the ingresses has a canary annotation
+			isCanaryEnabled, annotationErr := parser.GetBoolAnnotation("canary", ing, canary.CanaryAnnotations.Annotations)
 			for _, existing := range existingIngresses {
 				if existing.ObjectMeta.Namespace == ing.ObjectMeta.Namespace && existing.ObjectMeta.Name == ing.ObjectMeta.Name {
-					return nil
+					// same ingress
+					continue
 				}
-			}
 
-			// path overlap. Check if one of the ingresses has a canary annotation
-			isCanaryEnabled, annotationErr := parser.GetBoolAnnotation("canary", ing, canary.CanaryAnnotations.Annotations)
-			for _, existing := range existingIngresses {
 				isExistingCanaryEnabled, existingAnnotationErr := parser.GetBoolAnnotation("canary", existing, canary.CanaryAnnotations.Annotations)
 
 				if isCanaryEnabled && isExistingCanaryEnabled {
@@ -1856,7 +1854,6 @@ func checkOverlap(ing *networking.Ingress, servers []*ingress.Server) error {
 			}
 
 			// no overlap
-			return nil
 		}
 	}
 

test/e2e/admission/admission.go

@@ -60,6 +60,37 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
 		assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should return an error")
 	})
 
+	ginkgo.It("should not allow overlaps of host and paths without canary annotations in any rule", func() {
+		host := admissionTestHost
+
+		firstIngress := framework.NewSingleIngressWithMultiplePaths(
+			"first-ingress",
+			[]string{"/safe-path-1", "/conflict-path"},
+			host,
+			f.Namespace,
+			framework.EchoService,
+			80,
+			nil)
+		_, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{})
+		assert.Nil(ginkgo.GinkgoT(), err, "creating ingress")
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, fmt.Sprintf("server_name %v", host))
+			})
+
+		secondIngress := framework.NewSingleIngressWithMultiplePaths(
+			"second-ingress",
+			[]string{"/safe-path-2", "/conflict-path"},
+			host,
+			f.Namespace,
+			framework.EchoService,
+			80,
+			nil)
+		_, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), secondIngress, metav1.CreateOptions{})
+		assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should return an error")
+	})
+
 	ginkgo.It("should allow overlaps of host and paths with canary annotation", func() {
 		host := admissionTestHost