From 5faa855e66650725f132bcbc1a10a66290978e90 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Wed, 4 May 2016 21:59:22 -0300 Subject: [PATCH 1/2] Custom errors should be optional --- controllers/nginx/nginx.tmpl | 61 ++++++++++++++++++++------------- controllers/nginx/nginx/main.go | 14 ++++++++ 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/controllers/nginx/nginx.tmpl b/controllers/nginx/nginx.tmpl index a57b770a0..50d200901 100644 --- a/controllers/nginx/nginx.tmpl +++ b/controllers/nginx/nginx.tmpl @@ -123,18 +123,23 @@ http { ssl_dhparam {{ .sslDHParam }}; {{ end }} + {{ $interceptHttpErrors := $cfg.interceptHttp403 || $cfg.interceptHttp404 || $cfg.interceptHttp405 || + $cfg.interceptHttp408 || $cfg.interceptHttp413 || $cfg.interceptHttp501 || + $cfg.interceptHttp502 || $cfg.interceptHttp503 || $cfg.interceptHttp504 }} + {{ if $interceptHttpErrors }} # 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; + {{ if $cfg.interceptHttp403 }}error_page 403 = @custom_403;{{ end }} + {{ if $cfg.interceptHttp404 }}error_page 404 = @custom_404;{{ end }} + {{ if $cfg.interceptHttp405 }}error_page 405 = @custom_405;{{ end }} + {{ if $cfg.interceptHttp408 }}error_page 408 = @custom_408;{{ end }} + {{ if $cfg.interceptHttp413 }}error_page 413 = @custom_413;{{ end }} + {{ if $cfg.interceptHttp501 }}error_page 501 = @custom_501;{{ end }} + {{ if $cfg.interceptHttp502 }}error_page 502 = @custom_502;{{ end }} + {{ if $cfg.interceptHttp503 }}error_page 503 = @custom_503;{{ end }} + {{ if $cfg.interceptHttp504 }}error_page 504 = @custom_504;{{ 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 }}; @@ -285,59 +290,67 @@ stream { {{/* definition of templates to avoid repetitions */}} {{ define "CUSTOM_ERRORS" }} + {{ if $cfg.interceptHttp403 }} location @custom_403 { internal; content_by_lua_block { openURL(403) } - } - + }{{ end }} + {{ if $cfg.interceptHttp404 }} location @custom_404 { internal; content_by_lua_block { openURL(404) } - } - + }{{ end }} + {{ if $cfg.interceptHttp405 }} location @custom_405 { internal; content_by_lua_block { openURL(405) } - } - + }{{ end }} + {{ if $cfg.interceptHttp408 }} location @custom_408 { internal; content_by_lua_block { openURL(408) } - } - + }{{ end }} + {{ if $cfg.interceptHttp413 }} location @custom_413 { internal; content_by_lua_block { openURL(413) } - } - + }{{ end }} + {{ if $cfg.interceptHttp501 }} + location @custom_501 { + internal; + content_by_lua_block { + openURL(501) + } + }{{ end }} + {{ if $cfg.interceptHttp502 }} location @custom_502 { internal; content_by_lua_block { openURL(502) } - } - + }{{ end }} + {{ if $cfg.interceptHttp503 }} location @custom_503 { internal; content_by_lua_block { openURL(503) } - } - + }{{ end }} + {{ if $cfg.interceptHttp504 }} location @custom_504 { internal; content_by_lua_block { openURL(504) } - } + }{{ end }} {{ end }} diff --git a/controllers/nginx/nginx/main.go b/controllers/nginx/nginx/main.go index 9c474cf87..c2f31e2be 100644 --- a/controllers/nginx/nginx/main.go +++ b/controllers/nginx/nginx/main.go @@ -124,6 +124,20 @@ type nginxConfiguration struct { // accessed using HTTPS. HSTSMaxAge string `structs:"hsts-max-age,omitempty"` + // enables or disable if HTTP codes should be passed to a client or be redirected to nginx for + // processing with the error_page directive + // http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors + // By default this is disabled + InterceptHTTP403 bool `structs:"intercept-error-403,omitempty"` + InterceptHTTP404 bool `structs:"intercept-error-404,omitempty"` + InterceptHTTP405 bool `structs:"intercept-error-405,omitempty"` + InterceptHTTP408 bool `structs:"intercept-error-408,omitempty"` + InterceptHTTP413 bool `structs:"intercept-error-413,omitempty"` + InterceptHTTP501 bool `structs:"intercept-error-502,omitempty"` + InterceptHTTP502 bool `structs:"intercept-error-502,omitempty"` + InterceptHTTP503 bool `structs:"intercept-error-503,omitempty"` + InterceptHTTP504 bool `structs:"intercept-error-504,omitempty"` + // 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 From 28f982845d0c4daf317406641588d8b1a1164ee9 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Mon, 23 May 2016 20:15:13 -0300 Subject: [PATCH 2/2] Change errors to a list of codes --- controllers/nginx/nginx.tmpl | 93 +++++------------------------ controllers/nginx/nginx/main.go | 15 ++--- controllers/nginx/nginx/template.go | 15 ++--- controllers/nginx/nginx/utils.go | 46 +++++++++----- 4 files changed, 60 insertions(+), 109 deletions(-) diff --git a/controllers/nginx/nginx.tmpl b/controllers/nginx/nginx.tmpl index 50d200901..418caf939 100644 --- a/controllers/nginx/nginx.tmpl +++ b/controllers/nginx/nginx.tmpl @@ -123,23 +123,14 @@ http { ssl_dhparam {{ .sslDHParam }}; {{ end }} - {{ $interceptHttpErrors := $cfg.interceptHttp403 || $cfg.interceptHttp404 || $cfg.interceptHttp405 || - $cfg.interceptHttp408 || $cfg.interceptHttp413 || $cfg.interceptHttp501 || - $cfg.interceptHttp502 || $cfg.interceptHttp503 || $cfg.interceptHttp504 }} - {{ if $interceptHttpErrors }} + {{- if .customErrors }} # Custom error pages proxy_intercept_errors on; - {{ end }} + {{ end -}} - {{ if $cfg.interceptHttp403 }}error_page 403 = @custom_403;{{ end }} - {{ if $cfg.interceptHttp404 }}error_page 404 = @custom_404;{{ end }} - {{ if $cfg.interceptHttp405 }}error_page 405 = @custom_405;{{ end }} - {{ if $cfg.interceptHttp408 }}error_page 408 = @custom_408;{{ end }} - {{ if $cfg.interceptHttp413 }}error_page 413 = @custom_413;{{ end }} - {{ if $cfg.interceptHttp501 }}error_page 501 = @custom_501;{{ end }} - {{ if $cfg.interceptHttp502 }}error_page 502 = @custom_502;{{ end }} - {{ if $cfg.interceptHttp503 }}error_page 503 = @custom_503;{{ end }} - {{ if $cfg.interceptHttp504 }}error_page 504 = @custom_504;{{ end }} + {{- 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 }}; @@ -158,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 */}} @@ -165,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; @@ -241,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 @@ -249,9 +239,13 @@ http { listen 8181; location / { + {{ if .customErrors }} content_by_lua_block { openURL(503) } + {{ else }} + return 503; + {{ end }} } } } @@ -290,67 +284,12 @@ stream { {{/* definition of templates to avoid repetitions */}} {{ define "CUSTOM_ERRORS" }} - {{ if $cfg.interceptHttp403 }} - location @custom_403 { + {{ range $errCode := .customHttpErrors }} + location @custom_{{ $errCode }} { internal; content_by_lua_block { - openURL(403) + openURL({{ $errCode }}) } - }{{ end }} - {{ if $cfg.interceptHttp404 }} - location @custom_404 { - internal; - content_by_lua_block { - openURL(404) - } - }{{ end }} - {{ if $cfg.interceptHttp405 }} - location @custom_405 { - internal; - content_by_lua_block { - openURL(405) - } - }{{ end }} - {{ if $cfg.interceptHttp408 }} - location @custom_408 { - internal; - content_by_lua_block { - openURL(408) - } - }{{ end }} - {{ if $cfg.interceptHttp413 }} - location @custom_413 { - internal; - content_by_lua_block { - openURL(413) - } - }{{ end }} - {{ if $cfg.interceptHttp501 }} - location @custom_501 { - internal; - content_by_lua_block { - openURL(501) - } - }{{ end }} - {{ if $cfg.interceptHttp502 }} - location @custom_502 { - internal; - content_by_lua_block { - openURL(502) - } - }{{ end }} - {{ if $cfg.interceptHttp503 }} - location @custom_503 { - internal; - content_by_lua_block { - openURL(503) - } - }{{ end }} - {{ if $cfg.interceptHttp504 }} - location @custom_504 { - internal; - content_by_lua_block { - openURL(504) - } - }{{ end }} + } + {{ end }} {{ end }} diff --git a/controllers/nginx/nginx/main.go b/controllers/nginx/nginx/main.go index c2f31e2be..e880e9bf7 100644 --- a/controllers/nginx/nginx/main.go +++ b/controllers/nginx/nginx/main.go @@ -124,19 +124,11 @@ type nginxConfiguration struct { // accessed using HTTPS. HSTSMaxAge string `structs:"hsts-max-age,omitempty"` - // enables or disable if HTTP codes should be passed to a client or be redirected to nginx for - // processing with the error_page directive + // 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 - InterceptHTTP403 bool `structs:"intercept-error-403,omitempty"` - InterceptHTTP404 bool `structs:"intercept-error-404,omitempty"` - InterceptHTTP405 bool `structs:"intercept-error-405,omitempty"` - InterceptHTTP408 bool `structs:"intercept-error-408,omitempty"` - InterceptHTTP413 bool `structs:"intercept-error-413,omitempty"` - InterceptHTTP501 bool `structs:"intercept-error-502,omitempty"` - InterceptHTTP502 bool `structs:"intercept-error-502,omitempty"` - InterceptHTTP503 bool `structs:"intercept-error-503,omitempty"` - InterceptHTTP504 bool `structs:"intercept-error-504,omitempty"` + 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 @@ -290,6 +282,7 @@ func newDefaultNginxCfg() nginxConfiguration { WorkerProcesses: strconv.Itoa(runtime.NumCPU()), VtsStatusZoneSize: "10m", UseHTTP2: true, + CustomHTTPErrors: []int{}, } if glog.V(5) { diff --git a/controllers/nginx/nginx/template.go b/controllers/nginx/nginx/template.go index 4d15e1db3..23ba743eb 100644 --- a/controllers/nginx/nginx/template.go +++ b/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 diff --git a/controllers/nginx/nginx/utils.go b/controllers/nginx/nginx/utils.go index c4bfa8d8c..7e70ab66c 100644 --- a/controllers/nginx/nginx/utils.go +++ b/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 -}