diff --git a/cmd/nginx/flags.go b/cmd/nginx/flags.go index b2e491cf55..ed8029ba7e 100644 --- a/cmd/nginx/flags.go +++ b/cmd/nginx/flags.go @@ -156,6 +156,9 @@ Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not en sslProxyPort = flags.Int("ssl-passthrough-proxy-port", 442, `Port to use internally for SSL Passthrough.`) defServerPort = flags.Int("default-server-port", 8181, `Port to use for exposing the default server (catch-all).`) healthzPort = flags.Int("healthz-port", 10254, "Port to use for the healthz endpoint.") + + disableCatchAll = flags.Bool("disable-catch-all", false, + `Disable support for catch-all Ingresses`) ) flag.Set("logtostderr", "true") @@ -254,6 +257,7 @@ Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not en SSLProxy: *sslProxyPort, Status: *statusPort, }, + DisableCatchAll: *disableCatchAll, } return false, config, nil diff --git a/docs/user-guide/cli-arguments.md b/docs/user-guide/cli-arguments.md index ac1fa29093..a593fd4146 100644 --- a/docs/user-guide/cli-arguments.md +++ b/docs/user-guide/cli-arguments.md @@ -46,3 +46,4 @@ They are set in the container spec of the `nginx-ingress-controller` Deployment | `--version` | Show release information about the NGINX Ingress controller and exit. | | `--vmodule moduleSpec` | comma-separated list of pattern=N settings for file-filtered logging | | `--watch-namespace string` | Namespace the controller watches for updates to Kubernetes objects. This includes Ingresses, Services and all configuration resources. All namespaces are watched if this parameter is left empty. | +| `--disable-catch-all` | Disable support for catch-all Ingresses. | diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index cd970ee140..df06174304 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -97,6 +97,8 @@ type Configuration struct { SyncRateLimit float32 DynamicCertificatesEnabled bool + + DisableCatchAll bool } // GetPublishService returns the Service used to set the load-balancer status of Ingresses. diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index fe67b9823f..35fbd7384c 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -915,7 +915,8 @@ func newNGINXController(t *testing.T) *NGINXController { fs, channels.NewRingChannel(10), false, - pod) + pod, + false) config := &Configuration{ ListenPorts: &ngx_config.ListenPorts{ diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 22065aa401..d70253b9e7 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -127,7 +127,8 @@ func NewNGINXController(config *Configuration, mc metric.Collector, fs file.File fs, n.updateCh, config.DynamicCertificatesEnabled, - pod) + pod, + config.DisableCatchAll) n.syncQueue = task.NewTaskQueue(n.syncIngress) diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index c6ace805eb..1fe6f9bdde 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -232,7 +232,8 @@ func New(checkOCSP bool, fs file.Filesystem, updateCh *channels.RingChannel, isDynamicCertificatesEnabled bool, - pod *k8s.PodInfo) Storer { + pod *k8s.PodInfo, + disableCatchAll bool) Storer { store := &k8sStore{ isOCSPCheckEnabled: checkOCSP, @@ -310,6 +311,10 @@ func New(checkOCSP bool, klog.Infof("ignoring add for ingress %v based on annotation %v with value %v", ing.Name, class.IngressKey, a) return } + if ing.Spec.Backend != nil && disableCatchAll { + klog.Infof("ignoring add for catch-all ingress %v/%v because of --disable-catch-all", ing.Namespace, ing.Name) + return + } recorder.Eventf(ing, corev1.EventTypeNormal, "CREATE", fmt.Sprintf("Ingress %s/%s", ing.Namespace, ing.Name)) store.syncIngress(ing) @@ -340,6 +345,10 @@ func New(checkOCSP bool, klog.Infof("ignoring delete for ingress %v based on annotation %v", ing.Name, class.IngressKey) return } + if ing.Spec.Backend != nil && disableCatchAll { + klog.Infof("ignoring delete for catch-all ingress %v/%v because of --disable-catch-all", ing.Namespace, ing.Name) + return + } recorder.Eventf(ing, corev1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", ing.Namespace, ing.Name)) store.listers.IngressWithAnnotation.Delete(ing) @@ -358,13 +367,27 @@ func New(checkOCSP bool, validOld := class.IsValid(oldIng) validCur := class.IsValid(curIng) if !validOld && validCur { + if curIng.Spec.Backend != nil && disableCatchAll { + klog.Infof("ignoring update for catch-all ingress %v/%v because of --disable-catch-all", curIng.Namespace, curIng.Name) + return + } + klog.Infof("creating ingress %v based on annotation %v", curIng.Name, class.IngressKey) recorder.Eventf(curIng, corev1.EventTypeNormal, "CREATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) } else if validOld && !validCur { klog.Infof("removing ingress %v based on annotation %v", curIng.Name, class.IngressKey) + // FIXME: this does not actually delete the Ingress resource. + // The existing one will be updated with latest changes and invalid ingress.class will be missed. recorder.Eventf(curIng, corev1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) } else if validCur && !reflect.DeepEqual(old, cur) { - recorder.Eventf(curIng, corev1.EventTypeNormal, "UPDATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) + if curIng.Spec.Backend != nil && disableCatchAll { + klog.Infof("ignoring update for catch-all ingress %v/%v because of --disable-catch-all", curIng.Namespace, curIng.Name) + // FIXME: this does not actually delete the Ingress resource. + // The existing one will be updated with latest changes. + recorder.Eventf(curIng, corev1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) + } else { + recorder.Eventf(curIng, corev1.EventTypeNormal, "UPDATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) + } } else { klog.Infof("ignoring ingress %v based on annotation %v", curIng.Name, class.IngressKey) return diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index e3cf086960..24789e9a8e 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -82,7 +82,8 @@ func TestStore(t *testing.T) { fs, updateCh, false, - pod) + pod, + false) storer.Run(stopCh) @@ -163,7 +164,8 @@ func TestStore(t *testing.T) { fs, updateCh, false, - pod) + pod, + false) storer.Run(stopCh) @@ -316,7 +318,8 @@ func TestStore(t *testing.T) { fs, updateCh, false, - pod) + pod, + false) storer.Run(stopCh) @@ -424,7 +427,8 @@ func TestStore(t *testing.T) { fs, updateCh, false, - pod) + pod, + false) storer.Run(stopCh) @@ -514,7 +518,8 @@ func TestStore(t *testing.T) { fs, updateCh, false, - pod) + pod, + false) storer.Run(stopCh) @@ -627,7 +632,8 @@ func TestStore(t *testing.T) { fs, updateCh, false, - pod) + pod, + false) storer.Run(stopCh) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index d45197b0ee..0525818b3b 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -352,6 +352,21 @@ func UpdateDeployment(kubeClientSet kubernetes.Interface, namespace string, name return nil } +// UpdateIngress runs the given updateFunc on the ingress +func UpdateIngress(kubeClientSet kubernetes.Interface, namespace string, name string, updateFunc func(d *extensions.Ingress) error) error { + ingress, err := kubeClientSet.ExtensionsV1beta1().Ingresses(namespace).Get(name, metav1.GetOptions{}) + if err != nil { + return err + } + + if err := updateFunc(ingress); err != nil { + return err + } + + _, err = kubeClientSet.ExtensionsV1beta1().Ingresses(namespace).Update(ingress) + return err +} + // NewSingleIngressWithTLS creates a simple ingress rule with TLS spec included func NewSingleIngressWithTLS(name, path, host, ns, service string, port int, annotations *map[string]string) *extensions.Ingress { return newSingleIngressWithRules(name, path, host, ns, service, port, annotations, true) diff --git a/test/e2e/settings/disable_catch_all.go b/test/e2e/settings/disable_catch_all.go new file mode 100644 index 0000000000..4c58c28346 --- /dev/null +++ b/test/e2e/settings/disable_catch_all.go @@ -0,0 +1,109 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package settings + +import ( + "strings" + + . "github.com/onsi/ginkgo" + appsv1beta1 "k8s.io/api/apps/v1beta1" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.IngressNginxDescribe("Disabled catch-all", func() { + f := framework.NewDefaultFramework("disabled-catch-all") + + BeforeEach(func() { + f.NewEchoDeploymentWithReplicas(1) + + framework.UpdateDeployment(f.KubeClientSet, f.IngressController.Namespace, "nginx-ingress-controller", 1, + func(deployment *appsv1beta1.Deployment) error { + args := deployment.Spec.Template.Spec.Containers[0].Args + args = append(args, "--disable-catch-all=true") + deployment.Spec.Template.Spec.Containers[0].Args = args + _, err := f.KubeClientSet.AppsV1beta1().Deployments(f.IngressController.Namespace).Update(deployment) + + return err + }) + }) + + AfterEach(func() { + }) + + It("should ignore catch all Ingress", func() { + host := "foo" + + ing := framework.NewSingleCatchAllIngress("catch-all", f.IngressController.Namespace, "http-svc", 80, nil) + f.EnsureIngress(ing) + + ing = framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, func(cfg string) bool { + return strings.Contains(cfg, "server_name foo") + }) + + f.WaitForNginxServer("_", func(cfg string) bool { + return strings.Contains(cfg, `set $ingress_name ""`) && + strings.Contains(cfg, `set $proxy_upstream_name "upstream-default-backend"`) + }) + }) + + // FIXME: This test doesn't work because of a bug in Ingress update handle in store package. + // It("should delete Ingress updated to catch-all", func() { + // host := "foo" + + // ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, nil) + // f.EnsureIngress(ing) + + // f.WaitForNginxServer(host, + // func(server string) bool { + // return strings.Contains(server, "server_name foo") + // }) + + // resp, _, errs := gorequest.New(). + // Get(f.IngressController.HTTPURL). + // Set("Host", host). + // End() + // Expect(errs).To(BeNil()) + // Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + + // err := framework.UpdateIngress(f.KubeClientSet, f.IngressController.Namespace, host, func(ingress *extensions.Ingress) error { + // ingress.Spec.Rules = nil + // ingress.Spec.Backend = &extensions.IngressBackend{ + // ServiceName: "http-svc", + // ServicePort: intstr.FromInt(80), + // } + // return nil + // }) + // Expect(err).ToNot(HaveOccurred()) + + // f.WaitForNginxConfiguration(func(cfg string) bool { + // return !strings.Contains(cfg, "server_name foo") && + // !strings.Contains(cfg, `set $ingress_name "foo"`) && + // !strings.Contains(cfg, `set $service_name "http-svc"`) + // }) + + // resp, _, errs = gorequest.New(). + // Get(f.IngressController.HTTPURL). + // Set("Host", host). + // End() + // Expect(errs).To(BeNil()) + // Expect(resp.StatusCode).Should(Equal(http.StatusNotFound)) + // }) +})