Adds CustomHTTPErrors ingress annotation and test

Adds per-server/location error-catch functionality to nginx template

Adds documentation

Reduces template duplication with helper function for CUSTOM_ERRORS data

Updates documentation

Adds e2e test for customerrors

Removes AllCustomHTTPErrors, replaces with template function with deduplication and adds e2e test of deduplication

Fixes copy-paste error in test, adds additional test cases

Reverts noop change in controller.go (unused now)

docs/user-guide/nginx-configuration/annotations.md

@@ -31,6 +31,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/base-url-scheme](#rewrite)|string|
 |[nginx.ingress.kubernetes.io/client-body-buffer-size](#client-body-buffer-size)|string|
 |[nginx.ingress.kubernetes.io/configuration-snippet](#configuration-snippet)|string|
+|[nginx.ingress.kubernetes.io/custom-http-errors](#custom-http-errors)|[]int|
 |[nginx.ingress.kubernetes.io/default-backend](#default-backend)|string|
 |[nginx.ingress.kubernetes.io/enable-cors](#enable-cors)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/cors-allow-origin](#enable-cors)|string|
@@ -213,6 +214,17 @@ The ingress controller requires a [default backend](../default-backend.md).
 This service handles the response when the service in the Ingress rule does not have endpoints.
 This is a global configuration for the ingress controller. In some cases could be required to return a custom content or format. In this scenario we can use the annotation `nginx.ingress.kubernetes.io/default-backend: <svc name>` to specify a custom default backend.
 
+### Custom HTTP Errors
+
+Like the [`custom-http-errors`](../configmap.md#custom-http-errors) value in the ConfigMap, this annotation will set NGINX `proxy-intercept-errors`, but only for the NGINX location associated with this ingress.
+Different ingresses can specify different sets of error codes. Even if multiple ingress objects share the same hostname, this annotation can be used to intercept different error codes for each ingress (for example, different error codes to be intercepted for different paths on the same hostname, if each path is on a different ingress).
+If `custom-http-errors` is also specified globally, the error values specified in this annotation will override the global value for the given ingress' hostname and path.
+
+Example usage:
+```
+custom-http-errors: "404,415"
+```
+
 ### Enable CORS
 
 To enable Cross-Origin Resource Sharing (CORS) in an Ingress rule, add the annotation

internal/ingress/annotations/annotations.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/ingress-nginx/internal/ingress/annotations/clientbodybuffersize"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/connection"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/cors"
+	"k8s.io/ingress-nginx/internal/ingress/annotations/customhttperrors"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/defaultbackend"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/influxdb"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/ipwhitelist"
@@ -72,6 +73,7 @@ type Ingress struct {
 	ConfigurationSnippet string
 	Connection           connection.Config
 	CorsConfig           cors.Config
+	CustomHTTPErrors     []int
 	DefaultBackend       *apiv1.Service
 	Denied               error
 	ExternalAuth         authreq.Config
@@ -112,6 +114,7 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor {
 			"ConfigurationSnippet": snippet.NewParser(cfg),
 			"Connection":           connection.NewParser(cfg),
 			"CorsConfig":           cors.NewParser(cfg),
+			"CustomHTTPErrors":     customhttperrors.NewParser(cfg),
 			"DefaultBackend":       defaultbackend.NewParser(cfg),
 			"ExternalAuth":         authreq.NewParser(cfg),
 			"Proxy":                proxy.NewParser(cfg),

internal/ingress/annotations/annotations_test.go

@@ -43,6 +43,7 @@ var (
 	annotationAffinityCookieName   = parser.GetAnnotationWithPrefix("session-cookie-name")
 	annotationAffinityCookieHash   = parser.GetAnnotationWithPrefix("session-cookie-hash")
 	annotationUpstreamHashBy       = parser.GetAnnotationWithPrefix("upstream-hash-by")
+	annotationCustomHTTPErrors     = parser.GetAnnotationWithPrefix("custom-http-errors")
 )
 
 type mockCfg struct {
@@ -270,6 +271,41 @@ func TestCors(t *testing.T) {
 
 	}
 }
+func TestCustomHTTPErrors(t *testing.T) {
+	ec := NewAnnotationExtractor(mockCfg{})
+	ing := buildIngress()
+
+	fooAnns := []struct {
+		annotations map[string]string
+		er          []int
+	}{
+		{map[string]string{annotationCustomHTTPErrors: "404,415"}, []int{404, 415}},
+		{map[string]string{annotationCustomHTTPErrors: "404"}, []int{404}},
+		{map[string]string{annotationCustomHTTPErrors: ""}, []int{}},
+		{map[string]string{annotationCustomHTTPErrors + "_no": "404"}, []int{}},
+		{map[string]string{}, []int{}},
+		{nil, []int{}},
+	}
+
+	for _, foo := range fooAnns {
+		ing.SetAnnotations(foo.annotations)
+		r := ec.Extract(ing).CustomHTTPErrors
+
+		// Check that expected codes were created
+		for i := range foo.er {
+			if r[i] != foo.er[i] {
+				t.Errorf("Returned %v but expected %v", r, foo.er)
+			}
+		}
+
+		// Check that no unexpected codes were also created
+		for i := range r {
+			if r[i] != foo.er[i] {
+				t.Errorf("Returned %v but expected %v", r, foo.er)
+			}
+		}
+	}
+}
 
 /*
 func TestMergeLocationAnnotations(t *testing.T) {

internal/ingress/annotations/customhttperrors/main.go

@@ -0,0 +1,57 @@
+/*
+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 customhttperrors
+
+import (
+	"strconv"
+	"strings"
+
+	extensions "k8s.io/api/extensions/v1beta1"
+
+	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
+	"k8s.io/ingress-nginx/internal/ingress/resolver"
+)
+
+type customhttperrors struct {
+	r resolver.Resolver
+}
+
+// NewParser creates a new custom http errors annotation parser
+func NewParser(r resolver.Resolver) parser.IngressAnnotation {
+	return customhttperrors{r}
+}
+
+// Parse parses the annotations contained in the ingress to use
+// custom http errors
+func (e customhttperrors) Parse(ing *extensions.Ingress) (interface{}, error) {
+	c, err := parser.GetStringAnnotation("custom-http-errors", ing)
+	if err != nil {
+		return nil, err
+	}
+
+	cSplit := strings.Split(c, ",")
+	var codes []int
+	for _, i := range cSplit {
+		num, err := strconv.Atoi(i)
+		if err != nil {
+			return nil, err
+		}
+		codes = append(codes, num)
+	}
+
+	return codes, nil
+}

internal/ingress/controller/controller.go

@@ -351,6 +351,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
 						loc.InfluxDB = anns.InfluxDB
 						loc.DefaultBackend = anns.DefaultBackend
 						loc.BackendProtocol = anns.BackendProtocol
+						loc.CustomHTTPErrors = anns.CustomHTTPErrors
 
 						if loc.Redirect.FromToWWW {
 							server.RedirectFromToWWW = true
@@ -391,6 +392,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
 						InfluxDB:             anns.InfluxDB,
 						DefaultBackend:       anns.DefaultBackend,
 						BackendProtocol:      anns.BackendProtocol,
+						CustomHTTPErrors:     anns.CustomHTTPErrors,
 					}
 
 					if loc.Redirect.FromToWWW {

internal/ingress/controller/template/template.go

@@ -151,14 +151,16 @@ var (
 		"serverConfig": func(all config.TemplateConfig, server *ingress.Server) interface{} {
 			return struct{ First, Second interface{} }{all, server}
 		},
-		"isValidClientBodyBufferSize": isValidClientBodyBufferSize,
-		"buildForwardedFor":           buildForwardedFor,
-		"buildAuthSignURL":            buildAuthSignURL,
-		"buildOpentracing":            buildOpentracing,
-		"proxySetHeader":              proxySetHeader,
-		"buildInfluxDB":               buildInfluxDB,
-		"enforceRegexModifier":        enforceRegexModifier,
-		"stripLocationModifer":        stripLocationModifer,
+		"isValidClientBodyBufferSize":  isValidClientBodyBufferSize,
+		"buildForwardedFor":            buildForwardedFor,
+		"buildAuthSignURL":             buildAuthSignURL,
+		"buildOpentracing":             buildOpentracing,
+		"proxySetHeader":               proxySetHeader,
+		"buildInfluxDB":                buildInfluxDB,
+		"enforceRegexModifier":         enforceRegexModifier,
+		"stripLocationModifer":         stripLocationModifer,
+		"buildCustomErrorDeps":         buildCustomErrorDeps,
+		"collectCustomErrorsPerServer": collectCustomErrorsPerServer,
 	}
 )
 
@@ -941,3 +943,40 @@ func proxySetHeader(loc interface{}) string {
 
 	return "proxy_set_header"
 }
+
+// buildCustomErrorDeps is a utility function returning a struct wrapper with
+// the data required to build the 'CUSTOM_ERRORS' template
+func buildCustomErrorDeps(proxySetHeaders map[string]string, errorCodes []int) interface{} {
+	return struct {
+		ProxySetHeaders map[string]string
+		ErrorCodes      []int
+	}{
+		ProxySetHeaders: proxySetHeaders,
+		ErrorCodes:      errorCodes,
+	}
+}
+
+// collectCustomErrorsPerServer is a utility function which will collect all
+// custom error codes for all locations of a server block, deduplicates them,
+// and returns a unique set (for the template to create @custom_xxx locations)
+func collectCustomErrorsPerServer(input interface{}) []int {
+	server, ok := input.(*ingress.Server)
+	if !ok {
+		glog.Errorf("expected a '*ingress.Server' type but %T was returned", input)
+		return nil
+	}
+
+	codesMap := make(map[int]bool)
+	for _, loc := range server.Locations {
+		for _, code := range loc.CustomHTTPErrors {
+			codesMap[code] = true
+		}
+	}
+
+	uniqueCodes := make([]int, 0, len(codesMap))
+	for key := range codesMap {
+		uniqueCodes = append(uniqueCodes, key)
+	}
+
+	return uniqueCodes
+}

internal/ingress/types.go

@@ -253,6 +253,9 @@ type Location struct {
 	// BackendProtocol indicates which protocol should be used to communicate with the service
 	// By default this is HTTP
 	BackendProtocol string `json:"backend-protocol"`
+	// CustomHTTPErrors specifies the error codes that should be intercepted.
+	// +optional
+	CustomHTTPErrors []int `json:"custom-http-errors"`
 }
 
 // SSLPassthroughBackend describes a SSL upstream server configured

rootfs/etc/nginx/template/nginx.tmpl

@@ -575,7 +575,7 @@ http {
         {{ $cfg.ServerSnippet }}
         {{ end }}
 
-        {{ template "CUSTOM_ERRORS" $all }}
+        {{ template "CUSTOM_ERRORS" (buildCustomErrorDeps $all.ProxySetHeaders $cfg.CustomHTTPErrors) }}
     }
     ## end server {{ $server.Hostname }}
 
@@ -677,7 +677,7 @@ http {
             proxy_pass          http://upstream_balancer;
         }
 
-        {{ template "CUSTOM_ERRORS" $all }}
+        {{ template "CUSTOM_ERRORS" (buildCustomErrorDeps $all.ProxySetHeaders $cfg.CustomHTTPErrors) }}
     }
 }
 
@@ -696,7 +696,7 @@ stream {
 {{/* definition of templates to avoid repetitions */}}
 {{ define "CUSTOM_ERRORS" }}
         {{ $proxySetHeaders := .ProxySetHeaders }}
-        {{ range $errCode := .Cfg.CustomHTTPErrors }}
+        {{ range $errCode := .ErrorCodes }}
         location @custom_{{ $errCode }} {
             internal;
 
@@ -818,6 +818,8 @@ stream {
         {{ $server.ServerSnippet }}
         {{ end }}
 
+        {{ template "CUSTOM_ERRORS" (buildCustomErrorDeps $all.ProxySetHeaders (collectCustomErrorsPerServer $server)) }}
+
         {{ $enforceRegex := enforceRegexModifier $server.Locations }}
         {{ range $location := $server.Locations }}
         {{ $path := buildLocation $location $enforceRegex }}
@@ -1173,6 +1175,16 @@ stream {
             proxy_set_header       X-Service-Port     $service_port;
             {{ end }}
 
+            {{/* if a location-specific error override is set, add the proxy_intercept here */}}
+            {{ if $location.CustomHTTPErrors }}
+            # Custom error pages per ingress
+            proxy_intercept_errors on;
+            {{ end }}
+
+            {{ range $errCode := $location.CustomHTTPErrors }}
+            error_page {{ $errCode }} = @custom_{{ $errCode }};{{ end }}
+
+
             {{ buildProxyPass $server.Hostname $all.Backends $location }}
             {{ if (or (eq $location.Proxy.ProxyRedirectFrom "default") (eq $location.Proxy.ProxyRedirectFrom "off")) }}
             proxy_redirect                          {{ $location.Proxy.ProxyRedirectFrom }};

test/e2e/annotations/customhttperrors.go

@@ -0,0 +1,119 @@
+/*
+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 annotations
+
+import (
+	"fmt"
+	"strings"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"k8s.io/ingress-nginx/test/e2e/framework"
+)
+
+var _ = framework.IngressNginxDescribe("Annotations - custom-http-errors", func() {
+	f := framework.NewDefaultFramework("custom-http-errors")
+
+	BeforeEach(func() {
+		f.NewEchoDeploymentWithReplicas(2)
+	})
+
+	AfterEach(func() {
+	})
+
+	It("should set proxy_intercept_errors", func() {
+		host := "customerrors.foo.com"
+
+		errorCodes := []string{"404", "500"}
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(errorCodes, ","),
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return Expect(server).Should(ContainSubstring("proxy_intercept_errors on;"))
+			})
+	})
+
+	It("should create error routes", func() {
+		host := "customerrors.foo.com"
+		errorCodes := []string{"404", "500"}
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(errorCodes, ","),
+		}
+
+		ing := framework.NewSingleIngress(host, "/test", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
+		f.EnsureIngress(ing)
+
+		for _, code := range errorCodes {
+			f.WaitForNginxServer(host,
+				func(server string) bool {
+					return Expect(server).Should(ContainSubstring(fmt.Sprintf("@custom_%s", code)))
+				})
+		}
+	})
+
+	It("should set up error_page routing", func() {
+		host := "customerrors.foo.com"
+		errorCodes := []string{"404", "500"}
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(errorCodes, ","),
+		}
+
+		ing := framework.NewSingleIngress(host, "/test", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
+		f.EnsureIngress(ing)
+
+		for _, code := range errorCodes {
+			f.WaitForNginxServer(host,
+				func(server string) bool {
+					return Expect(server).Should(ContainSubstring(fmt.Sprintf("error_page %s = @custom_%s", code, code)))
+				})
+		}
+	})
+
+	It("should create only one of each error route", func() {
+		host := "customerrors.foo.com"
+		errorCodes := [][]string{{"404", "500"}, {"400", "404"}}
+
+		for i, codeSet := range errorCodes {
+			annotations := map[string]string{
+				"nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(codeSet, ","),
+			}
+
+			ing := framework.NewSingleIngress(
+				fmt.Sprintf("%s-%d", host, i), fmt.Sprintf("/test-%d", i),
+				host, f.IngressController.Namespace, "http-svc", 80, &annotations)
+			f.EnsureIngress(ing)
+		}
+
+		for _, codeSet := range errorCodes {
+			for _, code := range codeSet {
+				f.WaitForNginxServer(host,
+					func(server string) bool {
+						count := strings.Count(server, fmt.Sprintf("location @custom_%s", code))
+						return Expect(count).Should(Equal(1))
+					})
+			}
+		}
+	})
+})