diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index a29b0b14b..3f9aab8ee 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -224,6 +224,10 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error { return nil } + if n.cfg.DisableCatchAll && ing.Spec.Backend != nil { + return fmt.Errorf("This deployment is trying to create a catch-all ingress while DisableCatchAll flag is set to true. Remove '.spec.backend' or set DisableCatchAll flag to false.") + } + if parser.AnnotationsPrefix != parser.DefaultAnnotationsPrefix { for key := range ing.ObjectMeta.GetAnnotations() { if strings.HasPrefix(key, fmt.Sprintf("%s/", parser.DefaultAnnotationsPrefix)) { diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index 1542c7abb..a7560c3bf 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -256,6 +256,32 @@ func TestCheckIngress(t *testing.T) { } }) + t.Run("When a new catch-all ingress is being created despite catch-alls being disabled ", func(t *testing.T) { + backendBefore := ing.Spec.Backend + disableCatchAllBefore := nginx.cfg.DisableCatchAll + + nginx.command = testNginxTestCommand{ + t: t, + err: nil, + } + nginx.cfg.DisableCatchAll = true + + ing.Spec.Backend = &networking.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.IntOrString{ + IntVal: 80, + }, + } + + if nginx.CheckIngress(ing) == nil { + t.Errorf("with a new catch-all ingress and catch-alls disable, should return error") + } + + // reset backend and catch-all flag + ing.Spec.Backend = backendBefore + nginx.cfg.DisableCatchAll = disableCatchAllBefore + }) + t.Run("When the ingress is in a different namespace than the watched one", func(t *testing.T) { nginx.command = testNginxTestCommand{ t: t, diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index ad3aef5ad..0088c3b01 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -328,7 +328,7 @@ func New( return } - if isCatchAllIngress(ing.Spec) && disableCatchAll { + if hasCatchAllIngressRule(ing.Spec) && disableCatchAll { klog.InfoS("Ignoring delete for catch-all because of --disable-catch-all", "ingress", klog.KObj(ing)) return } @@ -353,7 +353,7 @@ func New( return } - if isCatchAllIngress(ing.Spec) && disableCatchAll { + if hasCatchAllIngressRule(ing.Spec) && disableCatchAll { klog.InfoS("Ignoring add for catch-all ingress because of --disable-catch-all", "ingress", klog.KObj(ing)) return } @@ -377,7 +377,7 @@ func New( validOld := class.IsValid(oldIng) validCur := class.IsValid(curIng) if !validOld && validCur { - if isCatchAllIngress(curIng.Spec) && disableCatchAll { + if hasCatchAllIngressRule(curIng.Spec) && disableCatchAll { klog.InfoS("ignoring update for catch-all ingress because of --disable-catch-all", "ingress", klog.KObj(curIng)) return } @@ -389,7 +389,7 @@ func New( ingDeleteHandler(old) return } else if validCur && !reflect.DeepEqual(old, cur) { - if isCatchAllIngress(curIng.Spec) && disableCatchAll { + if hasCatchAllIngressRule(curIng.Spec) && disableCatchAll { klog.InfoS("ignoring update for catch-all ingress and delete old one because of --disable-catch-all", "ingress", klog.KObj(curIng)) ingDeleteHandler(old) return @@ -624,10 +624,10 @@ func New( return store } -// isCatchAllIngress returns whether or not an ingress produces a +// hasCatchAllIngressRule returns whether or not an ingress produces a // catch-all server, and so should be ignored when --disable-catch-all is set -func isCatchAllIngress(spec networkingv1beta1.IngressSpec) bool { - return spec.Backend != nil && len(spec.Rules) == 0 +func hasCatchAllIngressRule(spec networkingv1beta1.IngressSpec) bool { + return spec.Backend != nil } // syncIngress parses ingress annotations converting the value of the diff --git a/test/e2e/settings/disable_catch_all.go b/test/e2e/settings/disable_catch_all.go index 71374ee39..1a8791d80 100644 --- a/test/e2e/settings/disable_catch_all.go +++ b/test/e2e/settings/disable_catch_all.go @@ -48,7 +48,7 @@ var _ = framework.IngressNginxDescribe("[Flag] disable-catch-all", func() { assert.Nil(ginkgo.GinkgoT(), err, "updating ingress controller deployment flags") }) - ginkgo.It("should ignore catch all Ingress", func() { + ginkgo.It("should ignore catch all Ingress with backend", func() { host := "foo" ing := framework.NewSingleCatchAllIngress("catch-all", f.Namespace, framework.EchoService, 80, nil) @@ -67,6 +67,18 @@ var _ = framework.IngressNginxDescribe("[Flag] disable-catch-all", func() { }) }) + ginkgo.It("should ignore catch all Ingress with backend and rules", func() { + host := "foo" + + ing := framework.NewSingleIngressWithBackendAndRules(host, "/", host, f.Namespace, framework.EchoService, 80, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer("_", func(cfg string) bool { + return strings.Contains(cfg, `set $ingress_name ""`) && + strings.Contains(cfg, `set $proxy_upstream_name "upstream-default-backend"`) + }) + }) + ginkgo.It("should delete Ingress updated to catch-all", func() { host := "foo" @@ -105,10 +117,10 @@ var _ = framework.IngressNginxDescribe("[Flag] disable-catch-all", func() { Status(http.StatusNotFound) }) - ginkgo.It("should allow Ingress with both a default backend and rules", func() { + ginkgo.It("should allow Ingress with rules", func() { host := "foo" - ing := framework.NewSingleIngressWithBackendAndRules("not-catch-all", "/rulepath", host, f.Namespace, framework.EchoService, 80, framework.EchoService, 80, nil) + ing := framework.NewSingleIngress("not-catch-all", "/", host, f.Namespace, framework.EchoService, 80, nil) f.EnsureIngress(ing) f.WaitForNginxServer(host, func(cfg string) bool {