Merge pull request #1326 from aledbf/fix-headers

Pass headers from the custom error backend

controllers/nginx/pkg/cmd/controller/nginx.go

@@ -40,7 +40,6 @@ import (
 	extensions "k8s.io/api/extensions/v1beta1"
 
 	"k8s.io/ingress/controllers/nginx/pkg/config"
-	"k8s.io/ingress/controllers/nginx/pkg/fastcgi"
 	ngx_template "k8s.io/ingress/controllers/nginx/pkg/template"
 	"k8s.io/ingress/controllers/nginx/pkg/version"
 	"k8s.io/ingress/core/pkg/ingress"
@@ -59,8 +58,6 @@ const (
 	vtsStatusModule     statusModule = "vts"
 
 	defUpstreamName = "upstream-default-backend"
-
-	fastCGISocket = "/var/run/go-fastcgi.sock"
 )
 
 var (
@@ -93,23 +90,6 @@ func newNGINXController() *NGINXController {
 		backendDefaults: config.NewDefault().Backend,
 	}
 
-	fcgiListener, err := net.Listen("unix", fastCGISocket)
-	if err != nil {
-		glog.Fatalf("%v", err)
-	}
-
-	err = os.Chmod(fastCGISocket, 0777)
-	if err != nil {
-		glog.Fatalf("%v", err)
-	}
-
-	go func() {
-		err = fastcgi.ServeError(fcgiListener)
-		if err != nil {
-			glog.Fatalf("%v", err)
-		}
-	}()
-
 	var onChange func()
 	onChange = func() {
 		template, err := ngx_template.NewTemplate(tmplPath, onChange)
@@ -679,18 +659,6 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 		ListenPorts:             n.ports,
 	}
 
-	// We need to extract the endpoints to be used in the fastcgi error handler
-	for _, b := range ingressCfg.Backends {
-		if b.Name == defUpstreamName {
-			eps := []string{}
-			for _, e := range b.Endpoints {
-				eps = append(eps, fmt.Sprintf("%v:%v", e.Address, e.Port))
-			}
-			tc.DefaultBackendEndpoints = strings.Join(eps, ",")
-			break
-		}
-	}
-
 	content, err := n.t.Write(tc)
 
 	if err != nil {

controllers/nginx/pkg/config/config.go

@@ -455,7 +455,6 @@ func (cfg Configuration) BuildLogFormatUpstream() string {
 
 // TemplateConfig contains the nginx configuration to render the file nginx.conf
 type TemplateConfig struct {
-	DefaultBackendEndpoints string
 	ProxySetHeaders         map[string]string
 	AddHeaders              map[string]string
 	MaxOpenFiles            int

controllers/nginx/pkg/fastcgi/fastcgi.go

@@ -1,110 +0,0 @@
-/*
-Copyright 2015 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 fastcgi
-
-import (
-	"fmt"
-	"io/ioutil"
-	"math/rand"
-	"net"
-	"net/http"
-	"net/http/fcgi"
-	"strconv"
-	"strings"
-
-	"github.com/golang/glog"
-)
-
-const (
-	// CodeHeader name of the header that indicates the expected response
-	// status code
-	CodeHeader = "X-Code"
-	// FormatHeader name of the header with the expected Content-Type to be
-	// sent to the client
-	FormatHeader = "X-Format"
-	// EndpointsHeader comma separated header that contains the list of
-	// endpoints for the default backend
-	EndpointsHeader = "X-Endpoints"
-	// ContentTypeHeader returns information about the type of the returned body
-	ContentTypeHeader = "Content-Type"
-)
-
-// ServeError creates a fastcgi handler to serve the custom error pages
-func ServeError(l net.Listener) error {
-	return fcgi.Serve(l, handler())
-}
-
-func handler() http.Handler {
-	r := http.NewServeMux()
-	r.HandleFunc("/", serveError)
-	return r
-}
-
-func serveError(w http.ResponseWriter, req *http.Request) {
-	code := req.Header.Get(CodeHeader)
-	format := req.Header.Get(FormatHeader)
-
-	if format == "" || format == "*/*" {
-		format = "text/html"
-	}
-
-	httpCode, err := strconv.Atoi(code)
-	if err != nil {
-		httpCode = 404
-	}
-
-	de := []byte(fmt.Sprintf("default backend - %v", httpCode))
-
-	w.Header().Set(ContentTypeHeader, format)
-	w.WriteHeader(httpCode)
-
-	eh := req.Header.Get(EndpointsHeader)
-	if eh == "" {
-		glog.Error("no endpoints for default backend")
-		w.Write(de)
-		return
-	}
-
-	eps := strings.Split(eh, ",")
-
-	// TODO: add retries in case of errors
-	ep := eps[rand.Intn(len(eps))]
-	r, err := http.NewRequest("GET", fmt.Sprintf("http://%v/", ep), nil)
-	r.Header = req.Header
-	if err != nil {
-		glog.Errorf("unexpected error: %v", err)
-		w.Write(de)
-		return
-	}
-
-	client := &http.Client{}
-	resp, err := client.Do(r)
-	if err != nil {
-		glog.Errorf("unexpected error: %v", err)
-		w.Write(de)
-		return
-	}
-
-	b, err := ioutil.ReadAll(resp.Body)
-	if err != nil {
-		glog.Errorf("unexpected error: %v", err)
-		w.Write(de)
-		return
-	}
-	defer resp.Body.Close()
-	w.Write(b)
-}

controllers/nginx/pkg/fastcgi/fastcgi_test.go

@@ -1,104 +0,0 @@
-/*
-Copyright 2015 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 fastcgi
-
-import (
-	"fmt"
-	"io/ioutil"
-	"net/http"
-	"net/http/httptest"
-	"strconv"
-	"strings"
-	"testing"
-)
-
-type dummyHandler struct {
-}
-
-func (d *dummyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	code := req.Header.Get(CodeHeader)
-	format := req.Header.Get(FormatHeader)
-
-	if format == "" || format == "*/*" {
-		format = "text/html"
-	}
-
-	httpCode, err := strconv.Atoi(code)
-	if err != nil {
-		httpCode = 404
-	}
-
-	de := []byte(code)
-	w.Header().Set(ContentTypeHeader, format)
-	w.WriteHeader(httpCode)
-	w.Write(de)
-}
-
-func TestErrorHandler(t *testing.T) {
-	tt := []struct {
-		name      string
-		code      int
-		format    string
-		endpoints string
-	}{
-		{name: "404 text/html", code: 404, format: "text/html", endpoints: "127.0.0.1:80"},
-		{name: "503 text/html", code: 503, format: "text/html"},
-		{name: "404 application/json", code: 404, format: "application/json", endpoints: "127.0.0.1:80"},
-	}
-
-	server := httptest.NewServer(&dummyHandler{})
-	defer server.Close()
-	hp := strings.Replace(server.URL, "http://", "", -1)
-
-	for _, tc := range tt {
-		t.Run(tc.name, func(t *testing.T) {
-			req, err := http.NewRequest("GET", server.URL, nil)
-			req.Header.Add(CodeHeader, fmt.Sprintf("%v", tc.code))
-			req.Header.Add(FormatHeader, tc.format)
-			req.Header.Add(EndpointsHeader, hp)
-			if err != nil {
-				t.Fatalf("could not created request: %v", err)
-			}
-			w := httptest.NewRecorder()
-			serveError(w, req)
-
-			res := w.Result()
-			defer res.Body.Close()
-
-			b, err := ioutil.ReadAll(res.Body)
-			if err != nil {
-				t.Fatalf("could not read response: %v", err)
-			}
-
-			if res.StatusCode != tc.code {
-				t.Errorf("expected status %v; got %v", tc.code, res.StatusCode)
-			}
-
-			ct := res.Header.Get(ContentTypeHeader)
-			if ct != tc.format {
-				t.Errorf("expected content type %v; got %v", tc.format, ct)
-			}
-
-			if len(b) == 0 {
-				t.Fatalf("unexpected empty body")
-			}
-
-			if string(b) != strconv.Itoa(tc.code) {
-				t.Fatalf("body: %v", string(b))
-			}
-		})
-	}
-}

controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl

@@ -393,13 +393,10 @@ http {
 
         location / {
             {{ if .CustomErrors }}
-            include             /etc/nginx/fastcgi_params;
+            proxy_set_header    X-Code 404;
-            fastcgi_param       HTTP_X_Code 404;
-            fastcgi_pass        unix:/var/run/go-fastcgi.sock;
-            {{ else }}
-            set $proxy_upstream_name "upstream-default-backend";
-            proxy_pass               http://upstream-default-backend;
             {{ end }}
+            set $proxy_upstream_name "upstream-default-backend";
+            proxy_pass          http://upstream-default-backend;
         }
 
         {{ template "CUSTOM_ERRORS" $all }}
@@ -474,21 +471,19 @@ stream {
 
 {{/* definition of templates to avoid repetitions */}}
 {{ define "CUSTOM_ERRORS" }}
-        {{ $defaultBackendEndpoints := .DefaultBackendEndpoints }}
+        {{ $proxySetHeaders := .ProxySetHeaders }}
         {{ range $errCode := .Cfg.CustomHTTPErrors }}
         location @custom_{{ $errCode }} {
             internal;
-            include             /etc/nginx/fastcgi_params;
 
-            fastcgi_param       HTTP_X_Code             {{ $errCode }};
+            proxy_set_header       X-Code             {{ $errCode }};
-            fastcgi_param       HTTP_X_Format           $http_accept;
+            proxy_set_header       X-Format           $http_accept;
-            fastcgi_param       HTTP_X_Original_URI     $request_uri;
+            proxy_set_header       X-Original-URI     $request_uri;
-            fastcgi_param       HTTP_X_Namespace        $namespace;
+            proxy_set_header       X-Namespace        $namespace;
-            fastcgi_param       HTTP_X_Ingress_Name     $ingress_name;
+            proxy_set_header       X-Ingress-Name     $ingress_name;
-            fastcgi_param       HTTP_X_Service_Name     $service_name;
+            proxy_set_header       X-Service-Name     $service_name;
-            fastcgi_param       HTTP_X_Endpoints        "{{ $defaultBackendEndpoints }}";
 
-            fastcgi_pass        unix:/var/run/go-fastcgi.sock;
+            proxy_pass             http://upstream-default-backend;
         }
         {{ end }}
 {{ end }}