Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable path overlap validation flag #10943

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
77 changes: 77 additions & 0 deletions internal/ingress/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
82 changes: 44 additions & 38 deletions pkg/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).`)

Expand Down Expand Up @@ -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,
Expand Down
43 changes: 43 additions & 0 deletions test/e2e/admission/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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

Expand Down