Skip to content

Commit e9664c2

Browse files
FseroFsero
committed
Add disable path overlap validation flag
 ## What this PR does / why we need it: In #5651 there was a request to throw an error when there are two ingresses defining the same host and path, which was implemented as part of the validation webhook. Despite of this there are clear rules on the ingress controller that describes what the controller would do in [such situation (the oldest rule wins)](https://github.com/kubernetes/ingress-nginx/blob/main/docs/how-it-works.md?plain=1#L27) Some users are relying on this validation behaviour to prevent misconfigurations, but there are use cases where allowing it, maybe temporarily, is helpful. Those use cases includes: - Splitting large ingresses objects in smaller ones #10820 - Moving ingress objects between namespaces without downtime (like when you transfer an ingress from team A that owns namespace A to team B that owns namespace B) #10090 <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] CVE Report (Scanner found CVE and adding report) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation only ## Which issue/s this PR fixes It might help with #10820 <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## How Has This Been Tested? building an image and testing it in a local cluster, will update later with some real life scenarios <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> ## Checklist: - [X] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [X] I've read the [CONTRIBUTION](https://github.com/kubernetes/ingress-nginx/blob/main/CONTRIBUTING.md) guide - [X] I have added unit and/or e2e tests to cover my changes. - [X] All new and existing tests passed. Change-Id: I9d4124d1c36876b06d63100cd10988eaf2f41db9
1 parent 2088e37 commit e9664c2

File tree

4 files changed

+177
-46
lines changed

4 files changed

+177
-46
lines changed

‎internal/ingress/controller/controller.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,11 @@ type Configuration struct {
122122

123123
IngressClassConfiguration *ingressclass.Configuration
124124

125-
ValidationWebhook string
126-
ValidationWebhookCertPath string
127-
ValidationWebhookKeyPath string
128-
DisableFullValidationTest bool
125+
ValidationWebhook string
126+
ValidationWebhookCertPath string
127+
ValidationWebhookKeyPath string
128+
DisableFullValidationTest bool
129+
DisablePathOverlapValidation bool
129130

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

406-
err = checkOverlap(ing, servers)
407-
if err != nil {
408-
n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name)
409-
return err
407+
if n.cfg.DisablePathOverlapValidation {
408+
klog.Warningf("ingress %v in namespace %v not checked for path overlap since --disable-path-overlap-validation is enabled", ing.Name, ing.ObjectMeta.Namespace)
409+
} else {
410+
err = checkOverlap(ing, servers)
411+
if err != nil {
412+
n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name)
413+
return err
414+
}
410415
}
411416
testedSize := len(ings)
412417
if n.cfg.DisableFullValidationTest {

‎internal/ingress/controller/controller_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,83 @@ func TestCheckIngress(t *testing.T) {
357357
t.Errorf("with a new ingress without error, no error should be returned")
358358
}
359359
})
360+
361+
t.Run("When there is a duplicated ingress with same host and path it should error", func(t *testing.T) {
362+
ing := &networking.Ingress{
363+
ObjectMeta: metav1.ObjectMeta{
364+
Name: "test-ingress",
365+
Namespace: "test-namespace",
366+
Annotations: map[string]string{
367+
"kubernetes.io/ingress.class": "nginx",
368+
},
369+
},
370+
Spec: networking.IngressSpec{
371+
Rules: []networking.IngressRule{
372+
{
373+
Host: "example.com",
374+
IngressRuleValue: networking.IngressRuleValue{
375+
HTTP: &networking.HTTPIngressRuleValue{
376+
Paths: []networking.HTTPIngressPath{
377+
{
378+
Path: "/",
379+
PathType: &pathTypePrefix,
380+
Backend: networking.IngressBackend{
381+
Service: &networking.IngressServiceBackend{
382+
Name: "http-svc",
383+
Port: networking.ServiceBackendPort{
384+
Number: 80,
385+
Name: "http",
386+
},
387+
},
388+
},
389+
},
390+
},
391+
},
392+
},
393+
},
394+
},
395+
},
396+
}
397+
398+
nginx.store = &fakeIngressStore{
399+
ingresses: []*ingress.Ingress{
400+
{
401+
Ingress: *ing,
402+
ParsedAnnotations: &annotations.Ingress{},
403+
},
404+
},
405+
}
406+
duplicatedIngress := ing.DeepCopy()
407+
duplicatedIngress.ObjectMeta.Name = "duplicated-ingress"
408+
409+
nginx.cfg.DisablePathOverlapValidation = false
410+
nginx.command = testNginxTestCommand{
411+
t: t,
412+
err: nil,
413+
expected: "_,example.com",
414+
}
415+
416+
err = nginx.CheckIngress(duplicatedIngress)
417+
if err == nil {
418+
t.Errorf("expected errors but noone occurred")
419+
}
420+
t.Run("if disablePathOverlap is enabled should not throw any error", func(t *testing.T) {
421+
duplicatedIngress := ing.DeepCopy()
422+
duplicatedIngress.ObjectMeta.Name = "duplicated-ingress"
423+
424+
nginx.cfg.DisablePathOverlapValidation = true
425+
nginx.command = testNginxTestCommand{
426+
t: t,
427+
err: nil,
428+
expected: "_,example.com",
429+
}
430+
431+
err = nginx.CheckIngress(duplicatedIngress)
432+
if err != nil {
433+
t.Errorf("expected no errors but one %+v occurred", err)
434+
}
435+
})
436+
})
360437
})
361438

362439
t.Run("When the ingress is marked as deleted", func(t *testing.T) {

‎pkg/flags/flags.go

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,11 @@ Takes the form "<host>:port". If not provided, no admission controller is starte
209209
`The path of the validating webhook certificate PEM.`)
210210
validationWebhookKey = flags.String("validating-webhook-key", "",
211211
`The path of the validating webhook key PEM.`)
212+
disablePathOverlapValidation = flags.Bool("disable-path-overlap-validation", false,
213+
`Disable path overlap validation. Validation webhook blocks creating ingresses with the same hostname and path in the same ingressClass.
214+
These flags turn off this validation as it helps split ingresses or move them between namespaces as it relies on the existing model rules:
215+
'If the same path for the same host is defined in more than one Ingress, the oldest rule wins'`)
216+
212217
disableFullValidationTest = flags.Bool("disable-full-test", false,
213218
`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).`)
214219

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

341346
config := &controller.Configuration{
342-
APIServerHost: *apiserverHost,
343-
KubeConfigFile: *kubeConfigFile,
344-
UpdateStatus: *updateStatus,
345-
ElectionID: *electionID,
346-
ElectionTTL: *electionTTL,
347-
EnableProfiling: *profiling,
348-
EnableMetrics: *enableMetrics,
349-
MetricsPerHost: *metricsPerHost,
350-
MetricsPerUndefinedHost: *metricsPerUndefinedHost,
351-
MetricsBuckets: histogramBuckets,
352-
MetricsBucketFactor: *bucketFactor,
353-
MetricsMaxBuckets: *maxBuckets,
354-
ReportStatusClasses: *reportStatusClasses,
355-
ExcludeSocketMetrics: *excludeSocketMetrics,
356-
MonitorMaxBatchSize: *monitorMaxBatchSize,
357-
DisableServiceExternalName: *disableServiceExternalName,
358-
EnableSSLPassthrough: *enableSSLPassthrough,
359-
DisableLeaderElection: *disableLeaderElection,
360-
ResyncPeriod: *resyncPeriod,
361-
DefaultService: *defaultSvc,
362-
Namespace: *watchNamespace,
363-
WatchNamespaceSelector: namespaceSelector,
364-
ConfigMapName: *configMap,
365-
TCPConfigMapName: *tcpConfigMapName,
366-
UDPConfigMapName: *udpConfigMapName,
367-
DisableFullValidationTest: *disableFullValidationTest,
368-
DefaultSSLCertificate: *defSSLCertificate,
369-
DeepInspector: *deepInspector,
370-
PublishService: *publishSvc,
371-
PublishStatusAddress: *publishStatusAddress,
372-
UpdateStatusOnShutdown: *updateStatusOnShutdown,
373-
ShutdownGracePeriod: *shutdownGracePeriod,
374-
PostShutdownGracePeriod: *postShutdownGracePeriod,
375-
UseNodeInternalIP: *useNodeInternalIP,
376-
SyncRateLimit: *syncRateLimit,
377-
HealthCheckHost: *healthzHost,
378-
DynamicConfigurationRetries: *dynamicConfigurationRetries,
379-
EnableTopologyAwareRouting: *enableTopologyAwareRouting,
347+
APIServerHost: *apiserverHost,
348+
KubeConfigFile: *kubeConfigFile,
349+
UpdateStatus: *updateStatus,
350+
ElectionID: *electionID,
351+
ElectionTTL: *electionTTL,
352+
EnableProfiling: *profiling,
353+
EnableMetrics: *enableMetrics,
354+
MetricsPerHost: *metricsPerHost,
355+
MetricsPerUndefinedHost: *metricsPerUndefinedHost,
356+
MetricsBuckets: histogramBuckets,
357+
MetricsBucketFactor: *bucketFactor,
358+
MetricsMaxBuckets: *maxBuckets,
359+
ReportStatusClasses: *reportStatusClasses,
360+
ExcludeSocketMetrics: *excludeSocketMetrics,
361+
MonitorMaxBatchSize: *monitorMaxBatchSize,
362+
DisableServiceExternalName: *disableServiceExternalName,
363+
EnableSSLPassthrough: *enableSSLPassthrough,
364+
DisableLeaderElection: *disableLeaderElection,
365+
ResyncPeriod: *resyncPeriod,
366+
DefaultService: *defaultSvc,
367+
Namespace: *watchNamespace,
368+
WatchNamespaceSelector: namespaceSelector,
369+
ConfigMapName: *configMap,
370+
TCPConfigMapName: *tcpConfigMapName,
371+
UDPConfigMapName: *udpConfigMapName,
372+
DisableFullValidationTest: *disableFullValidationTest,
373+
DisablePathOverlapValidation: *disablePathOverlapValidation,
374+
DefaultSSLCertificate: *defSSLCertificate,
375+
DeepInspector: *deepInspector,
376+
PublishService: *publishSvc,
377+
PublishStatusAddress: *publishStatusAddress,
378+
UpdateStatusOnShutdown: *updateStatusOnShutdown,
379+
ShutdownGracePeriod: *shutdownGracePeriod,
380+
PostShutdownGracePeriod: *postShutdownGracePeriod,
381+
UseNodeInternalIP: *useNodeInternalIP,
382+
SyncRateLimit: *syncRateLimit,
383+
HealthCheckHost: *healthzHost,
384+
DynamicConfigurationRetries: *dynamicConfigurationRetries,
385+
EnableTopologyAwareRouting: *enableTopologyAwareRouting,
380386
ListenPorts: &ngx_config.ListenPorts{
381387
Default: *defServerPort,
382388
Health: *healthzPort,

‎test/e2e/admission/admission.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

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

@@ -61,6 +62,48 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
6162
assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should return an error")
6263
})
6364

65+
ginkgo.It("should allow path overlaps if disable-path-overlap-validation is enabled", func() {
66+
host := admissionTestHost
67+
68+
err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error {
69+
args := deployment.Spec.Template.Spec.Containers[0].Args
70+
args = append(args, "--disable-path-overlap-validation")
71+
deployment.Spec.Template.Spec.Containers[0].Args = args
72+
_, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{})
73+
return err
74+
})
75+
assert.Nil(ginkgo.GinkgoT(), err, "updating deployment")
76+
defer func() {
77+
err = f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error {
78+
args := []string{}
79+
80+
for _, v := range deployment.Spec.Template.Spec.Containers[0].Args {
81+
if strings.Contains(v, "--disable-path-overlap-validation") {
82+
continue
83+
}
84+
85+
args = append(args, v)
86+
}
87+
deployment.Spec.Template.Spec.Containers[0].Args = args
88+
_, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{})
89+
return err
90+
})
91+
assert.Nil(ginkgo.GinkgoT(), err, "restoring deployment")
92+
}()
93+
oneIngress := framework.NewSingleIngress("a-ingress", "/", host, f.Namespace, framework.EchoService, 80, nil)
94+
_, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), oneIngress, metav1.CreateOptions{})
95+
assert.Nil(ginkgo.GinkgoT(), err, "creating ingress")
96+
97+
f.WaitForNginxServer(host,
98+
func(server string) bool {
99+
return strings.Contains(server, fmt.Sprintf("server_name %v", host))
100+
})
101+
102+
anotherIngress := framework.NewSingleIngress("another-ingress", "/", host, f.Namespace, framework.EchoService, 80, nil)
103+
_, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), anotherIngress, metav1.CreateOptions{})
104+
assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should be allowed when disable-path-overlap-validation is true ")
105+
})
106+
64107
ginkgo.It("should allow overlaps of host and paths with canary annotation", func() {
65108
host := admissionTestHost
66109

0 commit comments

Comments
 (0)