From b3a22f7fc0c64fc608b1768300598e1a594405c8 Mon Sep 17 00:00:00 2001 From: Elvin Efendi Date: Mon, 24 Sep 2018 23:33:13 -0400 Subject: [PATCH 1/2] do not require --default-backend-service --- cmd/nginx/flag_test.go | 6 +- cmd/nginx/flags.go | 4 -- cmd/nginx/main.go | 26 ++++---- deploy/mandatory.yaml | 62 ------------------- internal/ingress/controller/controller.go | 6 ++ rootfs/etc/nginx/template/nginx.tmpl | 11 ++++ test/e2e/annotations/alias.go | 2 +- .../defaultbackend/custom_default_backend.go | 62 +++++++++++++++++++ .../ingress-controller/mandatory.yaml | 61 ------------------ 9 files changed, 97 insertions(+), 143 deletions(-) create mode 100644 test/e2e/defaultbackend/custom_default_backend.go diff --git a/cmd/nginx/flag_test.go b/cmd/nginx/flag_test.go index 8215bfd18..77c2504d4 100644 --- a/cmd/nginx/flag_test.go +++ b/cmd/nginx/flag_test.go @@ -31,10 +31,10 @@ func resetForTesting(usage func()) { flag.Usage = usage } -func TestMandatoryFlag(t *testing.T) { +func TestNoMandatoryFlag(t *testing.T) { _, _, err := parseFlags() - if err == nil { - t.Fatalf("Expected an error about default backend service") + if err != nil { + t.Fatalf("Expected no error but got: %s", err) } } diff --git a/cmd/nginx/flags.go b/cmd/nginx/flags.go index 384f6ff12..f2fcd2012 100644 --- a/cmd/nginx/flags.go +++ b/cmd/nginx/flags.go @@ -176,10 +176,6 @@ Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not en return true, nil, nil } - if *defaultSvc == "" { - return false, nil, fmt.Errorf("Please specify --default-backend-service") - } - if *ingressClass != "" { glog.Infof("Watching for Ingress class: %s", *ingressClass) diff --git a/cmd/nginx/main.go b/cmd/nginx/main.go index 58efb26c7..ff16b6361 100644 --- a/cmd/nginx/main.go +++ b/cmd/nginx/main.go @@ -84,20 +84,22 @@ func main() { handleFatalInitError(err) } - defSvcNs, defSvcName, err := k8s.ParseNameNS(conf.DefaultService) - if err != nil { - glog.Fatal(err) - } - - _, err = kubeClient.CoreV1().Services(defSvcNs).Get(defSvcName, metav1.GetOptions{}) - if err != nil { - // TODO (antoineco): compare with error types from k8s.io/apimachinery/pkg/api/errors - if strings.Contains(err.Error(), "cannot get services in the namespace") { - glog.Fatalf("✖ The cluster seems to be running with a restrictive Authorization mode and the Ingress controller does not have the required permissions to operate normally.") + if len(conf.DefaultService) > 0 { + defSvcNs, defSvcName, err := k8s.ParseNameNS(conf.DefaultService) + if err != nil { + glog.Fatal(err) } - glog.Fatalf("No service with name %v found: %v", conf.DefaultService, err) + + _, err = kubeClient.CoreV1().Services(defSvcNs).Get(defSvcName, metav1.GetOptions{}) + if err != nil { + // TODO (antoineco): compare with error types from k8s.io/apimachinery/pkg/api/errors + if strings.Contains(err.Error(), "cannot get services in the namespace") { + glog.Fatalf("✖ The cluster seems to be running with a restrictive Authorization mode and the Ingress controller does not have the required permissions to operate normally.") + } + glog.Fatalf("No service with name %v found: %v", conf.DefaultService, err) + } + glog.Infof("Validated %v as the default backend.", conf.DefaultService) } - glog.Infof("Validated %v as the default backend.", conf.DefaultService) if conf.Namespace != "" { _, err = kubeClient.CoreV1().Namespaces().Get(conf.Namespace, metav1.GetOptions{}) diff --git a/deploy/mandatory.yaml b/deploy/mandatory.yaml index 6906fb956..daaa0fe6f 100644 --- a/deploy/mandatory.yaml +++ b/deploy/mandatory.yaml @@ -6,67 +6,6 @@ metadata: name: ingress-nginx --- -apiVersion: extensions/v1beta1 -kind: Deployment -metadata: - name: default-http-backend - labels: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx - namespace: ingress-nginx -spec: - replicas: 1 - selector: - matchLabels: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx - template: - metadata: - labels: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx - spec: - terminationGracePeriodSeconds: 60 - containers: - - name: default-http-backend - # Any image is permissible as long as: - # 1. It serves a 404 page at / - # 2. It serves 200 on a /healthz endpoint - image: gcr.io/google_containers/defaultbackend:1.4 - livenessProbe: - httpGet: - path: /healthz - port: 8080 - scheme: HTTP - initialDelaySeconds: 30 - timeoutSeconds: 5 - ports: - - containerPort: 8080 - resources: - limits: - cpu: 10m - memory: 20Mi - requests: - cpu: 10m - memory: 20Mi ---- - -apiVersion: v1 -kind: Service -metadata: - name: default-http-backend - namespace: ingress-nginx - labels: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx -spec: - ports: - - port: 80 - targetPort: 8080 - selector: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx ---- kind: ConfigMap apiVersion: v1 @@ -277,7 +216,6 @@ spec: image: quay.io/kubernetes-ingress-controller/nginx-ingress-controller:0.19.0 args: - /nginx-ingress-controller - - --default-backend-service=$(POD_NAMESPACE)/default-http-backend - --configmap=$(POD_NAMESPACE)/nginx-configuration - --tcp-services-configmap=$(POD_NAMESPACE)/tcp-services - --udp-services-configmap=$(POD_NAMESPACE)/udp-services diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index af59bf7f8..15467c227 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -362,6 +362,12 @@ func (n *NGINXController) getDefaultUpstream() *ingress.Backend { Name: defUpstreamName, } svcKey := n.cfg.DefaultService + + if len(svcKey) == 0 { + upstream.Endpoints = append(upstream.Endpoints, n.DefaultEndpoint()) + return upstream + } + svc, err := n.store.GetService(svcKey) if err != nil { glog.Warningf("Error getting default backend %q: %v", svcKey, err) diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index afbb394f3..d50b08af7 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -584,6 +584,17 @@ http { {{ end }} + # backend for when default-backend-service is not configured or it does not have endpoints + server { + listen {{ $all.ListenPorts.Default }} default_server {{ if $all.Cfg.ReusePort }}reuseport{{ end }} backlog={{ $all.BacklogSize }}; + {{ if $IsIPV6Enabled }}listen [::]:{{ $all.ListenPorts.Default }} default_server {{ if $all.Cfg.ReusePort }}reuseport{{ end }} backlog={{ $all.BacklogSize }};{{ end }} + set $proxy_upstream_name "-"; + + location / { + return 404; + } + } + # default server, used for NGINX healthcheck and access to nginx stats server { listen {{ $all.ListenPorts.Status }} default_server {{ if $all.Cfg.ReusePort }}reuseport{{ end }} backlog={{ $all.BacklogSize }}; diff --git a/test/e2e/annotations/alias.go b/test/e2e/annotations/alias.go index dc8196fb6..8fe9303f6 100644 --- a/test/e2e/annotations/alias.go +++ b/test/e2e/annotations/alias.go @@ -98,7 +98,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Alias", func() { Expect(len(errs)).Should(BeNumerically("==", 0)) Expect(resp.StatusCode).Should(Equal(http.StatusNotFound)) - Expect(body).Should(ContainSubstring("default backend - 404")) + Expect(body).Should(ContainSubstring("404 Not Found")) }) It("should return status code 200 for host 'foo' and 'bar'", func() { diff --git a/test/e2e/defaultbackend/custom_default_backend.go b/test/e2e/defaultbackend/custom_default_backend.go new file mode 100644 index 000000000..ea9e9f5dd --- /dev/null +++ b/test/e2e/defaultbackend/custom_default_backend.go @@ -0,0 +1,62 @@ +/* +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 defaultbackend + +import ( + "fmt" + "net/http" + "strings" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/parnurzeal/gorequest" + + appsv1beta1 "k8s.io/api/apps/v1beta1" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.IngressNginxDescribe("Dynamic Certificate", func() { + f := framework.NewDefaultFramework("custom-default-backend") + + BeforeEach(func() { + err := f.NewEchoDeploymentWithReplicas(1) + Expect(err).NotTo(HaveOccurred()) + + 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, fmt.Sprintf("--default-backend-service=%s/%s", f.IngressController.Namespace, "http-svc")) + deployment.Spec.Template.Spec.Containers[0].Args = args + _, err := f.KubeClientSet.AppsV1beta1().Deployments(f.IngressController.Namespace).Update(deployment) + + return err + }) + + err = f.WaitForNginxServer("_", + func(server string) bool { + return strings.Contains(server, "set $proxy_upstream_name \"upstream-default-backend\"") + }) + Expect(err).ToNot(HaveOccurred()) + }) + + It("uses custom default backend", func() { + resp, _, errs := gorequest.New().Get(f.IngressController.HTTPURL).End() + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + }) +}) diff --git a/test/manifests/ingress-controller/mandatory.yaml b/test/manifests/ingress-controller/mandatory.yaml index 349c01bd6..3c84ba722 100644 --- a/test/manifests/ingress-controller/mandatory.yaml +++ b/test/manifests/ingress-controller/mandatory.yaml @@ -1,65 +1,5 @@ --- -apiVersion: extensions/v1beta1 -kind: Deployment -metadata: - name: default-http-backend - labels: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx -spec: - replicas: 1 - selector: - matchLabels: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx - template: - metadata: - labels: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx - spec: - terminationGracePeriodSeconds: 60 - containers: - - name: default-http-backend - # Any image is permissible as long as: - # 1. It serves a 404 page at / - # 2. It serves 200 on a /healthz endpoint - image: gcr.io/google_containers/defaultbackend:1.4 - livenessProbe: - httpGet: - path: /healthz - port: 8080 - scheme: HTTP - initialDelaySeconds: 30 - timeoutSeconds: 5 - ports: - - containerPort: 8080 - resources: - limits: - cpu: 10m - memory: 20Mi - requests: - cpu: 10m - memory: 20Mi ---- - -apiVersion: v1 -kind: Service -metadata: - name: default-http-backend - labels: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx -spec: - ports: - - port: 80 - targetPort: 8080 - selector: - app.kubernetes.io/name: default-http-backend - app.kubernetes.io/part-of: ingress-nginx ---- - kind: ConfigMap apiVersion: v1 metadata: @@ -275,7 +215,6 @@ spec: image: ingress-controller/nginx-ingress-controller:dev args: - /nginx-ingress-controller - - --default-backend-service=$(POD_NAMESPACE)/default-http-backend - --configmap=$(POD_NAMESPACE)/nginx-configuration - --tcp-services-configmap=$(POD_NAMESPACE)/tcp-services - --udp-services-configmap=$(POD_NAMESPACE)/udp-services From 14815c546ce45ea7190b6be6e2a2077b46435ee2 Mon Sep 17 00:00:00 2001 From: Elvin Efendi Date: Tue, 25 Sep 2018 21:21:16 -0400 Subject: [PATCH 2/2] update docs --- docs/examples/static-ip/nginx-ingress-controller.yaml | 1 - docs/user-guide/cli-arguments.md | 4 ++-- docs/user-guide/multiple-ingress.md | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/examples/static-ip/nginx-ingress-controller.yaml b/docs/examples/static-ip/nginx-ingress-controller.yaml index 6664d0ddd..18ed2d467 100644 --- a/docs/examples/static-ip/nginx-ingress-controller.yaml +++ b/docs/examples/static-ip/nginx-ingress-controller.yaml @@ -54,5 +54,4 @@ spec: fieldPath: metadata.namespace args: - /nginx-ingress-controller - - --default-backend-service=$(POD_NAMESPACE)/default-http-backend - --publish-service=$(POD_NAMESPACE)/nginx-ingress-lb diff --git a/docs/user-guide/cli-arguments.md b/docs/user-guide/cli-arguments.md index 54c922fd2..6ef03108d 100644 --- a/docs/user-guide/cli-arguments.md +++ b/docs/user-guide/cli-arguments.md @@ -10,8 +10,8 @@ They are set in the container spec of the `nginx-ingress-controller` Deployment | `--annotations-prefix string` | Prefix of the Ingress annotations specific to the NGINX controller. (default "nginx.ingress.kubernetes.io") | | `--apiserver-host string` | Address of the Kubernetes API server. Takes the form "protocol://address:port". If not specified, it is assumed the program runs inside a Kubernetes cluster and local discovery is attempted. | | `--configmap string` | Name of the ConfigMap containing custom global configurations for the controller. | -| `--default-backend-service string` | Service used to serve HTTP requests not matching any known server name (catch-all). Takes the form "namespace/name". The controller configures NGINX to forward requests to the first port of this Service. | -| `--default-server-port int` | Port to use for exposing the default server (catch-all). (default 8181) | +| `--default-backend-service string` | Service used to serve HTTP requests not matching any known server name (catch-all). Takes the form "namespace/name". The controller configures NGINX to forward requests to the first port of this Service. If not specified, 404 page will be returned diretly from Nginx.| +| `--default-server-port int` | When `default-backend-service` is not specified or specified service does not have any endpoint, a local endpoint with this port will be used to serve 404 page from inside Nginx. | | `--default-ssl-certificate string` | Secret containing a SSL certificate to be used by the default HTTPS server (catch-all). Takes the form "namespace/name". | | `--election-id string` | Election id to use for Ingress status updates. (default "ingress-controller-leader") | | `--enable-dynamic-certificates` | Dynamically serves certificates instead of reloading NGINX when certificates are created, updated, or deleted. Currently does not support OCSP stapling, so --enable-ssl-chain-completion must be turned off. Assuming the certificate is generated with a 2048 bit RSA key/cert pair, this feature can store roughly 5000 certificates. This is an experiemental feature that currently is not ready for production use. Feature backed by OpenResty Lua libraries. (disabled by default) | diff --git a/docs/user-guide/multiple-ingress.md b/docs/user-guide/multiple-ingress.md index e83220177..7a5acf830 100644 --- a/docs/user-guide/multiple-ingress.md +++ b/docs/user-guide/multiple-ingress.md @@ -43,7 +43,6 @@ spec: - name: nginx-ingress-internal-controller args: - /nginx-ingress-controller - - '--default-backend-service=ingress/nginx-ingress-default-backend' - '--election-id=ingress-controller-leader-internal' - '--ingress-class=nginx-internal' - '--configmap=ingress/nginx-ingress-internal-controller'