Merge e9664c2566 into de1a4c463c

2025-02-17 09:50:33 -08:00 · 2025-02-17 09:50:33 -08:00 · 9940cbb571
commit 9940cbb571
parent de1a4c463c e9664c2566
4 changed files with 177 additions and 46 deletions
--- a/internal/ingress/controller/controller.go
+++ b/internal/ingress/controller/controller.go
@ -122,10 +122,11 @@ type Configuration struct {

 	IngressClassConfiguration *ingressclass.Configuration

-	ValidationWebhook         string
-	ValidationWebhookCertPath string
-	ValidationWebhookKeyPath  string
-	DisableFullValidationTest bool
+	ValidationWebhook            string
+	ValidationWebhookCertPath    string
+	ValidationWebhookKeyPath     string
+	DisableFullValidationTest    bool
+	DisablePathOverlapValidation bool

 	GlobalExternalAuth  *ngx_config.GlobalExternalAuth
 	MaxmindEditionFiles *[]string
@ -403,10 +404,14 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error {
 	startTest := time.Now().UnixNano() / 1000000
 	_, servers, pcfg := n.getConfiguration(ings)

-	err = checkOverlap(ing, servers)
-	if err != nil {
-		n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name)
-		return err
+	if n.cfg.DisablePathOverlapValidation {
+		klog.Warningf("ingress %v in namespace %v not checked for path overlap since --disable-path-overlap-validation is enabled", ing.Name, ing.ObjectMeta.Namespace)
+	} else {
+		err = checkOverlap(ing, servers)
+		if err != nil {
+			n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name)
+			return err
+		}
 	}
 	testedSize := len(ings)
 	if n.cfg.DisableFullValidationTest {
--- a/internal/ingress/controller/controller_test.go
+++ b/internal/ingress/controller/controller_test.go
@ -357,6 +357,83 @@ func TestCheckIngress(t *testing.T) {
 				t.Errorf("with a new ingress without error, no error should be returned")
 			}
 		})
+
+		t.Run("When there is a duplicated ingress with same host and path it should error", func(t *testing.T) {
+			ing := &networking.Ingress{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-ingress",
+					Namespace: "test-namespace",
+					Annotations: map[string]string{
+						"kubernetes.io/ingress.class": "nginx",
+					},
+				},
+				Spec: networking.IngressSpec{
+					Rules: []networking.IngressRule{
+						{
+							Host: "example.com",
+							IngressRuleValue: networking.IngressRuleValue{
+								HTTP: &networking.HTTPIngressRuleValue{
+									Paths: []networking.HTTPIngressPath{
+										{
+											Path:     "/",
+											PathType: &pathTypePrefix,
+											Backend: networking.IngressBackend{
+												Service: &networking.IngressServiceBackend{
+													Name: "http-svc",
+													Port: networking.ServiceBackendPort{
+														Number: 80,
+														Name:   "http",
+													},
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			}
+
+			nginx.store = &fakeIngressStore{
+				ingresses: []*ingress.Ingress{
+					{
+						Ingress:           *ing,
+						ParsedAnnotations: &annotations.Ingress{},
+					},
+				},
+			}
+			duplicatedIngress := ing.DeepCopy()
+			duplicatedIngress.ObjectMeta.Name = "duplicated-ingress"
+
+			nginx.cfg.DisablePathOverlapValidation = false
+			nginx.command = testNginxTestCommand{
+				t:        t,
+				err:      nil,
+				expected: "_,example.com",
+			}
+
+			err = nginx.CheckIngress(duplicatedIngress)
+			if err == nil {
+				t.Errorf("expected errors but noone occurred")
+			}
+			t.Run("if disablePathOverlap is enabled should not throw any error", func(t *testing.T) {
+				duplicatedIngress := ing.DeepCopy()
+				duplicatedIngress.ObjectMeta.Name = "duplicated-ingress"
+
+				nginx.cfg.DisablePathOverlapValidation = true
+				nginx.command = testNginxTestCommand{
+					t:        t,
+					err:      nil,
+					expected: "_,example.com",
+				}
+
+				err = nginx.CheckIngress(duplicatedIngress)
+				if err != nil {
+					t.Errorf("expected no errors but one %+v occurred", err)
+				}
+			})
+		})
 	})

 	t.Run("When the ingress is marked as deleted", func(t *testing.T) {
--- a/pkg/flags/flags.go
+++ b/pkg/flags/flags.go
@ -209,6 +209,11 @@ Takes the form "<host>:port". If not provided, no admission controller is starte
 			`The path of the validating webhook certificate PEM.`)
 		validationWebhookKey = flags.String("validating-webhook-key", "",
 			`The path of the validating webhook key PEM.`)
+		disablePathOverlapValidation = flags.Bool("disable-path-overlap-validation", false,
+			`Disable path overlap validation. Validation webhook blocks creating ingresses with the same hostname and path in the same ingressClass.
+        These flags turn off this validation as it helps split ingresses or move them between namespaces as it relies on the existing model rules:
+        'If the same path for the same host is defined in more than one Ingress, the oldest rule wins'`)
+
 		disableFullValidationTest = flags.Bool("disable-full-test", false,
 			`Disable full test of all merged ingresses at the admission stage and tests the template of the ingress being created or updated  (full test of all ingresses is enabled by default).`)

@ -339,44 +344,45 @@ https://blog.maxmind.com/2019/12/significant-changes-to-accessing-and-using-geol
 	ngx_config.EnableSSLChainCompletion = *enableSSLChainCompletion

 	config := &controller.Configuration{
-		APIServerHost:               *apiserverHost,
-		KubeConfigFile:              *kubeConfigFile,
-		UpdateStatus:                *updateStatus,
-		ElectionID:                  *electionID,
-		ElectionTTL:                 *electionTTL,
-		EnableProfiling:             *profiling,
-		EnableMetrics:               *enableMetrics,
-		MetricsPerHost:              *metricsPerHost,
-		MetricsPerUndefinedHost:     *metricsPerUndefinedHost,
-		MetricsBuckets:              histogramBuckets,
-		MetricsBucketFactor:         *bucketFactor,
-		MetricsMaxBuckets:           *maxBuckets,
-		ReportStatusClasses:         *reportStatusClasses,
-		ExcludeSocketMetrics:        *excludeSocketMetrics,
-		MonitorMaxBatchSize:         *monitorMaxBatchSize,
-		DisableServiceExternalName:  *disableServiceExternalName,
-		EnableSSLPassthrough:        *enableSSLPassthrough,
-		DisableLeaderElection:       *disableLeaderElection,
-		ResyncPeriod:                *resyncPeriod,
-		DefaultService:              *defaultSvc,
-		Namespace:                   *watchNamespace,
-		WatchNamespaceSelector:      namespaceSelector,
-		ConfigMapName:               *configMap,
-		TCPConfigMapName:            *tcpConfigMapName,
-		UDPConfigMapName:            *udpConfigMapName,
-		DisableFullValidationTest:   *disableFullValidationTest,
-		DefaultSSLCertificate:       *defSSLCertificate,
-		DeepInspector:               *deepInspector,
-		PublishService:              *publishSvc,
-		PublishStatusAddress:        *publishStatusAddress,
-		UpdateStatusOnShutdown:      *updateStatusOnShutdown,
-		ShutdownGracePeriod:         *shutdownGracePeriod,
-		PostShutdownGracePeriod:     *postShutdownGracePeriod,
-		UseNodeInternalIP:           *useNodeInternalIP,
-		SyncRateLimit:               *syncRateLimit,
-		HealthCheckHost:             *healthzHost,
-		DynamicConfigurationRetries: *dynamicConfigurationRetries,
-		EnableTopologyAwareRouting:  *enableTopologyAwareRouting,
+		APIServerHost:                *apiserverHost,
+		KubeConfigFile:               *kubeConfigFile,
+		UpdateStatus:                 *updateStatus,
+		ElectionID:                   *electionID,
+		ElectionTTL:                  *electionTTL,
+		EnableProfiling:              *profiling,
+		EnableMetrics:                *enableMetrics,
+		MetricsPerHost:               *metricsPerHost,
+		MetricsPerUndefinedHost:      *metricsPerUndefinedHost,
+		MetricsBuckets:               histogramBuckets,
+		MetricsBucketFactor:          *bucketFactor,
+		MetricsMaxBuckets:            *maxBuckets,
+		ReportStatusClasses:          *reportStatusClasses,
+		ExcludeSocketMetrics:         *excludeSocketMetrics,
+		MonitorMaxBatchSize:          *monitorMaxBatchSize,
+		DisableServiceExternalName:   *disableServiceExternalName,
+		EnableSSLPassthrough:         *enableSSLPassthrough,
+		DisableLeaderElection:        *disableLeaderElection,
+		ResyncPeriod:                 *resyncPeriod,
+		DefaultService:               *defaultSvc,
+		Namespace:                    *watchNamespace,
+		WatchNamespaceSelector:       namespaceSelector,
+		ConfigMapName:                *configMap,
+		TCPConfigMapName:             *tcpConfigMapName,
+		UDPConfigMapName:             *udpConfigMapName,
+		DisableFullValidationTest:    *disableFullValidationTest,
+		DisablePathOverlapValidation: *disablePathOverlapValidation,
+		DefaultSSLCertificate:        *defSSLCertificate,
+		DeepInspector:                *deepInspector,
+		PublishService:               *publishSvc,
+		PublishStatusAddress:         *publishStatusAddress,
+		UpdateStatusOnShutdown:       *updateStatusOnShutdown,
+		ShutdownGracePeriod:          *shutdownGracePeriod,
+		PostShutdownGracePeriod:      *postShutdownGracePeriod,
+		UseNodeInternalIP:            *useNodeInternalIP,
+		SyncRateLimit:                *syncRateLimit,
+		HealthCheckHost:              *healthzHost,
+		DynamicConfigurationRetries:  *dynamicConfigurationRetries,
+		EnableTopologyAwareRouting:   *enableTopologyAwareRouting,
 		ListenPorts: &ngx_config.ListenPorts{
 			Default:  *defServerPort,
 			Health:   *healthzPort,
--- a/test/e2e/admission/admission.go
+++ b/test/e2e/admission/admission.go
@ -26,6 +26,7 @@ import (

 	"github.com/onsi/ginkgo/v2"
 	"github.com/stretchr/testify/assert"
+	appsv1 "k8s.io/api/apps/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

@ -61,6 +62,48 @@ 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 allow path overlaps if disable-path-overlap-validation is enabled", func() {
+		host := admissionTestHost
+
+		err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error {
+			args := deployment.Spec.Template.Spec.Containers[0].Args
+			args = append(args, "--disable-path-overlap-validation")
+			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 deployment")
+		defer 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, "--disable-path-overlap-validation") {
+						continue
+					}
+
+					args = append(args, v)
+				}
+				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, "restoring deployment")
+		}()
+		oneIngress := framework.NewSingleIngress("a-ingress", "/", host, f.Namespace, framework.EchoService, 80, nil)
+		_, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), oneIngress, 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))
+			})
+
+		anotherIngress := framework.NewSingleIngress("another-ingress", "/", host, f.Namespace, framework.EchoService, 80, nil)
+		_, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), anotherIngress, metav1.CreateOptions{})
+		assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should  be allowed when disable-path-overlap-validation is true ")
+	})
+
 	ginkgo.It("should allow overlaps of host and paths with canary annotation", func() {
 		host := admissionTestHost