Merge pull request #926 from aledbf/configure-errors

[nginx-ingress-controller] Custom errors should be optional

controllers/nginx/nginx.tmpl

@@ -123,18 +123,14 @@ http {
     ssl_dhparam {{ .sslDHParam }};
     {{ end }}
 
+    {{- if .customErrors }}
     # Custom error pages
     proxy_intercept_errors on;
+    {{ end -}}
 
-    error_page 403 = @custom_403;
-    error_page 404 = @custom_404;
-    error_page 405 = @custom_405;
-    error_page 408 = @custom_408;
-    error_page 413 = @custom_413;
-    error_page 501 = @custom_501;
-    error_page 502 = @custom_502;
-    error_page 503 = @custom_503;
-    error_page 504 = @custom_504;
+    {{- range $errCode := $cfg.customHttpErrors }}
+    error_page {{ $errCode }} = @custom_{{ $errCode }};
+    {{ end }}
 
     # In case of errors try the next upstream server before returning an error
     proxy_next_upstream                     error timeout invalid_header http_502 http_503 http_504 {{ if $cfg.retryNonIdempotent }}non_idempotent{{ end }};
@@ -153,6 +149,7 @@ http {
 
     {{ range $server := .servers }}
     server {
+        server_name {{ $server.Name }};
         listen 80{{ if $cfg.useProxyProtocol }} proxy_protocol{{ end }};
         {{ if $server.SSL }}listen 443 {{ if $cfg.useProxyProtocol }}proxy_protocol{{ end }} ssl {{ if $cfg.useHttp2 }}http2{{ end }};
         {{/* comment PEM sha is required to detect changes in the generated configuration and force a reload */}}
@@ -160,8 +157,6 @@ http {
         ssl_certificate {{ $server.SSLCertificate }};
         ssl_certificate_key {{ $server.SSLCertificateKey }};{{ end }}
 
-        server_name {{ $server.Name }};
-
         {{ if (and $server.SSL $cfg.hsts) }}
         if ($scheme = http) {
             return 301 https://$host$request_uri;
@@ -236,7 +231,7 @@ http {
         location / {
             proxy_pass             http://upstream-default-backend;
         }
-        {{ template "CUSTOM_ERRORS" $cfg }}
+        {{- template "CUSTOM_ERRORS" $cfg }}
     }
 
     # default server for services without endpoints
@@ -244,9 +239,13 @@ http {
         listen 8181;
 
         location / {
+            {{ if .customErrors }}
             content_by_lua_block {
                 openURL(503)
             }
+            {{ else }}
+            return 503;
+            {{ end }}
         }        
     }    
 }
@@ -285,59 +284,12 @@ stream {
 
 {{/* definition of templates to avoid repetitions */}}
 {{ define "CUSTOM_ERRORS" }}
-        location @custom_403 {
+        {{ range $errCode := .customHttpErrors }}
+        location @custom_{{ $errCode }} {
             internal;
             content_by_lua_block {
-                openURL(403)
+                openURL({{ $errCode }})
             }
-        }
-
-        location @custom_404 {
-            internal;
-            content_by_lua_block {
-                openURL(404)
-            }
-        }
-
-        location @custom_405 {
-            internal;
-            content_by_lua_block {
-                openURL(405)
-            }
-        }
-
-        location @custom_408 {
-            internal;
-            content_by_lua_block {
-                openURL(408)
-            }
-        }
-
-        location @custom_413 {
-            internal;
-            content_by_lua_block {
-                openURL(413)
-            }
-        }
-
-        location @custom_502 {
-            internal;
-            content_by_lua_block {
-                openURL(502)
-            }
-        }
-
-        location @custom_503 {
-            internal;
-            content_by_lua_block {
-                openURL(503)
-            }
-        }
-
-        location @custom_504 {
-            internal;
-            content_by_lua_block {
-                openURL(504)
-            }
-        }
+        }    
+        {{ end }}
 {{ end }}

controllers/nginx/nginx/main.go

@@ -124,6 +124,12 @@ type nginxConfiguration struct {
 	// accessed using HTTPS.
 	HSTSMaxAge string `structs:"hsts-max-age,omitempty"`
 
+	// enables which HTTP codes should be passed for processing with the error_page directive
+	// http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors
+	// http://nginx.org/en/docs/http/ngx_http_core_module.html#error_page
+	// By default this is disabled
+	CustomHTTPErrors []int
+
 	// Time during which a keep-alive client connection will stay open on the server side.
 	// The zero value disables keep-alive client connections
 	// http://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_timeout
@@ -276,6 +282,7 @@ func newDefaultNginxCfg() nginxConfiguration {
 		WorkerProcesses:          strconv.Itoa(runtime.NumCPU()),
 		VtsStatusZoneSize:        "10m",
 		UseHTTP2:                 true,
+		CustomHTTPErrors:         []int{},
 	}
 
 	if glog.V(5) {

controllers/nginx/nginx/template.go

@@ -56,15 +56,9 @@ func (ngx *Manager) writeCfg(cfg nginxConfiguration, ingressCfg IngressConfig) (
 	conf["udpUpstreams"] = ingressCfg.UDPUpstreams
 	conf["defResolver"] = ngx.defResolver
 	conf["sslDHParam"] = ngx.sslDHParam
+	conf["customErrors"] = len(cfg.CustomHTTPErrors) > 0
 	conf["cfg"] = fixKeyNames(structs.Map(cfg))
 
-	buffer := new(bytes.Buffer)
-	err := ngx.template.Execute(buffer, conf)
-	if err != nil {
-		glog.Infof("NGINX error: %v", err)
-		return false, err
-	}
-
 	if glog.V(3) {
 		b, err := json.Marshal(conf)
 		if err != nil {
@@ -73,6 +67,13 @@ func (ngx *Manager) writeCfg(cfg nginxConfiguration, ingressCfg IngressConfig) (
 		glog.Infof("NGINX configuration: %v", string(b))
 	}
 
+	buffer := new(bytes.Buffer)
+	err := ngx.template.Execute(buffer, conf)
+	if err != nil {
+		glog.Infof("NGINX error: %v", err)
+		return false, err
+	}
+
 	changed, err := ngx.needsReload(buffer)
 	if err != nil {
 		return false, err

controllers/nginx/nginx/utils.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"os/exec"
 	"reflect"
+	"strconv"
 	"strings"
 
 	"github.com/golang/glog"
@@ -30,6 +31,10 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 )
 
+const (
+	customHTTPErrors = "custom-http-errors"
+)
+
 // getDNSServers returns the list of nameservers located in the file /etc/resolv.conf
 func getDNSServers() []string {
 	file, err := ioutil.ReadFile("/etc/resolv.conf")
@@ -91,6 +96,19 @@ func (ngx *Manager) ReadConfig(config *api.ConfigMap) nginxConfiguration {
 		Metadata:         metadata,
 	})
 
+	cErrors := make([]int, 0)
+	if val, ok := config.Data[customHTTPErrors]; ok {
+		delete(config.Data, customHTTPErrors)
+		for _, i := range strings.Split(val, ",") {
+			j, err := strconv.Atoi(i)
+			if err != nil {
+				glog.Warningf("%v is not a valid http code", i)
+			} else {
+				cErrors = append(cErrors, j)
+			}
+		}
+	}
+
 	err = decoder.Decode(config.Data)
 	if err != nil {
 		glog.Infof("%v", err)
@@ -115,9 +133,23 @@ func (ngx *Manager) ReadConfig(config *api.ConfigMap) nginxConfiguration {
 		}
 	}
 
+	cfgDefault.CustomHTTPErrors = ngx.filterErrors(cErrors)
 	return cfgDefault
 }
 
+func (ngx *Manager) filterErrors(errCodes []int) []int {
+	fa := make([]int, 0)
+	for _, errCode := range errCodes {
+		if errCode > 299 && errCode < 600 {
+			fa = append(fa, errCode)
+		} else {
+			glog.Warningf("error code %v is not valid for custom error pages", errCode)
+		}
+	}
+
+	return fa
+}
+
 func (ngx *Manager) needsReload(data *bytes.Buffer) (bool, error) {
 	filename := ngx.ConfigFile
 	in, err := os.Open(filename)
@@ -179,17 +211,3 @@ func diff(b1, b2 []byte) (data []byte, err error) {
 	}
 	return
 }
-
-func toMap(iface interface{}) (map[string]interface{}, bool) {
-	value := reflect.ValueOf(iface)
-	if value.Kind() == reflect.Map {
-		m := map[string]interface{}{}
-		for _, k := range value.MapKeys() {
-			m[k.String()] = value.MapIndex(k).Interface()
-		}
-
-		return m, true
-	}
-
-	return map[string]interface{}{}, false
-}