feat(annotations): introduce enable-custom-http-errors annotation

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

@@ -49,6 +49,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/canary-weight-total](#canary)|number|
 |[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/enable-custom-http-errors](#custom-http-errors)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/custom-http-errors](#custom-http-errors)|[]int|
 |[nginx.ingress.kubernetes.io/custom-headers](#custom-headers)|string|
 |[nginx.ingress.kubernetes.io/default-backend](#default-backend)|string|
@@ -332,6 +333,10 @@ Like the [`custom-http-errors`](./configmap.md#custom-http-errors) value in the
 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.
 
+If you want to disable this behavior for a specific ingress, you can use the annotation `nginx.ingress.kubernetes.io/enable-custom-http-errors: "false"`.
+`nginx.ingress.kubernetes.io/enable-custom-http-errors`:
+   indicates if the custom http errors feature should be enabled or not to this Ingress rule. The default value is `"true"`.
+
 Example usage:
 ```
 nginx.ingress.kubernetes.io/custom-http-errors: "404,415"

internal/ingress/annotations/annotations_test.go

@@ -29,21 +29,22 @@ import (
 )
 
 var (
-	annotationPassthrough            = parser.GetAnnotationWithPrefix("ssl-passthrough")
-	annotationAffinityType           = parser.GetAnnotationWithPrefix("affinity")
-	annotationAffinityMode           = parser.GetAnnotationWithPrefix("affinity-mode")
-	annotationAffinityCanaryBehavior = parser.GetAnnotationWithPrefix("affinity-canary-behavior")
-	annotationCorsEnabled            = parser.GetAnnotationWithPrefix("enable-cors")
-	annotationCorsAllowMethods       = parser.GetAnnotationWithPrefix("cors-allow-methods")
-	annotationCorsAllowHeaders       = parser.GetAnnotationWithPrefix("cors-allow-headers")
-	annotationCorsExposeHeaders      = parser.GetAnnotationWithPrefix("cors-expose-headers")
-	annotationCorsAllowCredentials   = parser.GetAnnotationWithPrefix("cors-allow-credentials")
-	defaultCorsMethods               = "GET, PUT, POST, DELETE, PATCH, OPTIONS"
-	defaultCorsHeaders               = "DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization"
-	annotationAffinityCookieName     = parser.GetAnnotationWithPrefix("session-cookie-name")
-	annotationUpstreamHashBy         = parser.GetAnnotationWithPrefix("upstream-hash-by")
-	annotationCustomHTTPErrors       = parser.GetAnnotationWithPrefix("custom-http-errors")
-	annotationCustomHeaders          = parser.GetAnnotationWithPrefix("custom-headers")
+	annotationPassthrough             = parser.GetAnnotationWithPrefix("ssl-passthrough")
+	annotationAffinityType            = parser.GetAnnotationWithPrefix("affinity")
+	annotationAffinityMode            = parser.GetAnnotationWithPrefix("affinity-mode")
+	annotationAffinityCanaryBehavior  = parser.GetAnnotationWithPrefix("affinity-canary-behavior")
+	annotationCorsEnabled             = parser.GetAnnotationWithPrefix("enable-cors")
+	annotationCorsAllowMethods        = parser.GetAnnotationWithPrefix("cors-allow-methods")
+	annotationCorsAllowHeaders        = parser.GetAnnotationWithPrefix("cors-allow-headers")
+	annotationCorsExposeHeaders       = parser.GetAnnotationWithPrefix("cors-expose-headers")
+	annotationCorsAllowCredentials    = parser.GetAnnotationWithPrefix("cors-allow-credentials")
+	defaultCorsMethods                = "GET, PUT, POST, DELETE, PATCH, OPTIONS"
+	defaultCorsHeaders                = "DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization"
+	annotationAffinityCookieName      = parser.GetAnnotationWithPrefix("session-cookie-name")
+	annotationUpstreamHashBy          = parser.GetAnnotationWithPrefix("upstream-hash-by")
+	annotationCustomHTTPErrorsEnabled = parser.GetAnnotationWithPrefix("enable-custom-http-errors")
+	annotationCustomHTTPErrors        = parser.GetAnnotationWithPrefix("custom-http-errors")
+	annotationCustomHeaders           = parser.GetAnnotationWithPrefix("custom-headers")
 )
 
 type mockCfg struct {
@@ -298,6 +299,14 @@ func TestCustomHTTPErrors(t *testing.T) {
 		{map[string]string{annotationCustomHTTPErrors: "404"}, []int{404}},
 		{map[string]string{annotationCustomHTTPErrors: ""}, []int{}},
 		{map[string]string{annotationCustomHTTPErrors + "_no": "404"}, []int{}},
+		{map[string]string{annotationCustomHTTPErrorsEnabled: "true", annotationCustomHTTPErrors: "404,415"}, []int{404, 415}},
+		{map[string]string{annotationCustomHTTPErrorsEnabled: "true", annotationCustomHTTPErrors: "404"}, []int{404}},
+		{map[string]string{annotationCustomHTTPErrorsEnabled: "true", annotationCustomHTTPErrors: ""}, []int{}},
+		{map[string]string{annotationCustomHTTPErrorsEnabled: "true", annotationCustomHTTPErrors + "_no": "404"}, []int{}},
+		{map[string]string{annotationCustomHTTPErrorsEnabled: "false", annotationCustomHTTPErrors: "404,415"}, []int{}},
+		{map[string]string{annotationCustomHTTPErrorsEnabled: "false", annotationCustomHTTPErrors: "404"}, []int{}},
+		{map[string]string{annotationCustomHTTPErrorsEnabled: "false", annotationCustomHTTPErrors: ""}, []int{}},
+		{map[string]string{annotationCustomHTTPErrorsEnabled: "false", annotationCustomHTTPErrors + "_no": "404"}, []int{}},
 		{map[string]string{}, []int{}},
 		{nil, []int{}},
 	}

internal/ingress/annotations/customhttperrors/main.go

@@ -22,13 +22,16 @@ import (
 	"strings"
 
 	networking "k8s.io/api/networking/v1"
+	"k8s.io/klog/v2"
 
 	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
+	"k8s.io/ingress-nginx/internal/ingress/errors"
 	"k8s.io/ingress-nginx/internal/ingress/resolver"
 )
 
 const (
-	customHTTPErrorsAnnotation = "custom-http-errors"
+	customHTTPErrorsAnnotation        = "custom-http-errors"
+	customHTTPErrorsEnabledAnnotation = "enable-custom-http-errors"
 )
 
 // We accept anything between 400 and 599, on a comma separated.
@@ -37,13 +40,20 @@ var arrayOfHTTPErrors = regexp.MustCompile(`^(?:[4,5]\d{2},?)*$`)
 var customHTTPErrorsAnnotations = parser.Annotation{
 	Group: "backend",
 	Annotations: parser.AnnotationFields{
+		customHTTPErrorsEnabledAnnotation: {
+			Validator:     parser.ValidateBool,
+			Scope:         parser.AnnotationScopeLocation,
+			Risk:          parser.AnnotationRiskLow,
+			Documentation: `Indicates if the custom http errors feature should be enabled or not for this Ingress rule. The default value is "true".`,
+		},
+
 		customHTTPErrorsAnnotation: {
 			Validator: parser.ValidateRegex(arrayOfHTTPErrors, true),
 			Scope:     parser.AnnotationScopeLocation,
 			Risk:      parser.AnnotationRiskLow,
 			Documentation: `If a default backend annotation is specified on the ingress, the errors code specified on this annotation 
 			will be routed to that annotation's default backend service. Otherwise they will be routed to the global default backend.
-			A comma-separated list of error codes is accepted (anything between 400 and 599, like 403, 503)`,
+			A comma-separated list of error codes is accepted (anything between 400 and 599, like 403, 503).`,
 		},
 	},
 }
@@ -64,6 +74,18 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
 // Parse parses the annotations contained in the ingress to use
 // custom http errors
 func (e customhttperrors) Parse(ing *networking.Ingress) (interface{}, error) {
+	enabled, err := parser.GetBoolAnnotation(customHTTPErrorsEnabledAnnotation, ing, e.annotationConfig.Annotations)
+	if err != nil {
+		if errors.IsValidationError(err) {
+			klog.Warningf("%s is invalid, defaulting to 'true'", customHTTPErrorsEnabledAnnotation)
+		}
+		enabled = true
+	}
+
+	if !enabled {
+		return []int{}, nil
+	}
+
 	c, err := parser.GetStringAnnotation(customHTTPErrorsAnnotation, ing, e.annotationConfig.Annotations)
 	if err != nil {
 		return nil, err

internal/ingress/annotations/customhttperrors/main_test.go

@@ -18,7 +18,6 @@ package customhttperrors
 
 import (
 	"reflect"
-	"sort"
 	"testing"
 
 	api "k8s.io/api/core/v1"
@@ -28,6 +27,10 @@ import (
 	"k8s.io/ingress-nginx/internal/ingress/resolver"
 )
 
+const DefaultHTTPErrorsString = "400,404,500,502"
+
+var DefaultHTTPErrorsSlice = []int{400, 404, 500, 502}
+
 func buildIngress() *networking.Ingress {
 	return &networking.Ingress{
 		ObjectMeta: meta_v1.ObjectMeta{
@@ -47,32 +50,57 @@ func buildIngress() *networking.Ingress {
 	}
 }
 
-func TestParseInvalidAnnotations(t *testing.T) {
+func TestParseCodes(t *testing.T) {
 	ing := buildIngress()
 
-	_, err := NewParser(&resolver.Mock{}).Parse(ing)
-	if err == nil {
-		t.Errorf("expected error parsing ingress with custom-http-errors")
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix("custom-http-errors")] = DefaultHTTPErrorsString
+	ing.SetAnnotations(data)
+
+	i, err := NewParser(&resolver.Mock{}).Parse(ing)
+	if err != nil {
+		t.Errorf("unexpected error parsing ingress with custom-http-errors")
+	}
+	val, ok := i.([]int)
+	if !ok {
+		t.Errorf("expected a []int type")
 	}
 
+	expected := DefaultHTTPErrorsSlice
+	if !reflect.DeepEqual(expected, val) {
+		t.Errorf("expected %v but got %v", expected, val)
+	}
+}
+
+func TestEnabledSwitch(t *testing.T) {
+	ing := buildIngress()
+
 	data := map[string]string{}
-	data[parser.GetAnnotationWithPrefix("custom-http-errors")] = "400,404,abc,502"
+	data[parser.GetAnnotationWithPrefix("enable-custom-http-errors")] = "true"
+	data[parser.GetAnnotationWithPrefix("custom-http-errors")] = DefaultHTTPErrorsString
 	ing.SetAnnotations(data)
 
 	i, err := NewParser(&resolver.Mock{}).Parse(ing)
-	if err == nil {
-		t.Errorf("expected error parsing ingress with custom-http-errors")
+	if err != nil {
+		t.Errorf("unexpected error parsing ingress with custom-http-errors")
 	}
-	if i != nil {
-		t.Errorf("expected %v but got %v", nil, i)
+	val, ok := i.([]int)
+	if !ok {
+		t.Errorf("expected a []int type")
+	}
+
+	expected := DefaultHTTPErrorsSlice
+	if !reflect.DeepEqual(expected, val) {
+		t.Errorf("expected %v but got %v", expected, val)
 	}
 }
 
-func TestParseAnnotations(t *testing.T) {
+func TestDisabledSwitch(t *testing.T) {
 	ing := buildIngress()
 
 	data := map[string]string{}
-	data[parser.GetAnnotationWithPrefix("custom-http-errors")] = "400,404,500,502"
+	data[parser.GetAnnotationWithPrefix("enable-custom-http-errors")] = "false"
+	data[parser.GetAnnotationWithPrefix("custom-http-errors")] = DefaultHTTPErrorsString
 	ing.SetAnnotations(data)
 
 	i, err := NewParser(&resolver.Mock{}).Parse(ing)
@@ -84,10 +112,48 @@ func TestParseAnnotations(t *testing.T) {
 		t.Errorf("expected a []int type")
 	}
 
-	expected := []int{400, 404, 500, 502}
-	sort.Ints(val)
+	expected := []int{}
+	if !reflect.DeepEqual(expected, val) {
+		t.Errorf("expected %v but got %v", expected, val)
+	}
+}
 
+func TestEnabledByDefault(t *testing.T) {
+	ing := buildIngress()
+
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix("enable-custom-http-errors")] = "fakebool"
+	data[parser.GetAnnotationWithPrefix("custom-http-errors")] = DefaultHTTPErrorsString
+	ing.SetAnnotations(data)
+
+	i, err := NewParser(&resolver.Mock{}).Parse(ing)
+	if err != nil {
+		t.Errorf("unexpected error parsing ingress with custom-http-errors")
+	}
+	val, ok := i.([]int)
+	if !ok {
+		t.Errorf("expected a []int type")
+	}
+
+	expected := DefaultHTTPErrorsSlice
 	if !reflect.DeepEqual(expected, val) {
 		t.Errorf("expected %v but got %v", expected, val)
 	}
 }
+
+func TestErrorOnInvalidCode(t *testing.T) {
+	ing := buildIngress()
+
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix("enable-custom-http-errors")] = "true"
+	data[parser.GetAnnotationWithPrefix("custom-http-errors")] = "400,404,fakeint,502"
+	ing.SetAnnotations(data)
+
+	i, err := NewParser(&resolver.Mock{}).Parse(ing)
+	if err == nil {
+		t.Errorf("expected error parsing ingress with custom-http-errors")
+	}
+	if i != nil {
+		t.Errorf("expected %v but got %v", nil, i)
+	}
+}

test/e2e/annotations/customhttperrors.go

@@ -38,7 +38,7 @@ var _ = framework.DescribeAnnotation("custom-http-errors", func() {
 		f.NewEchoDeployment()
 	})
 
-	ginkgo.It("configures Nginx correctly", func() {
+	ginkgo.It("configures Nginx correctly when enabled", func() {
 		host := "customerrors.foo.com"
 
 		errorCodes := []string{"404", "500"}
@@ -119,4 +119,30 @@ var _ = framework.DescribeAnnotation("custom-http-errors", func() {
 		assert.Contains(ginkgo.GinkgoT(), serverConfig, errorBlockName(fmt.Sprintf("custom-default-backend-%s-%s", f.Namespace, customDefaultBackend), "503"))
 		assert.Contains(ginkgo.GinkgoT(), serverConfig, fmt.Sprintf("error_page %s = %s", "503", errorBlockName(fmt.Sprintf("custom-default-backend-%s-%s", f.Namespace, customDefaultBackend), "503")))
 	})
+
+	ginkgo.It("configures Nginx correctly when disabled", func() {
+		host := "customerrors.foo.com"
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/enable-custom-http-errors": "false",
+			"nginx.ingress.kubernetes.io/custom-http-errors":        "404, 503",
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		var serverConfig string
+		f.WaitForNginxServer(host, func(sc string) bool {
+			serverConfig = sc
+			return strings.Contains(serverConfig, fmt.Sprintf("server_name %s", host))
+		})
+
+		f.UpdateNginxConfigMapData("custom-http-errors", "404, 503")
+
+		ginkgo.By("not turning on proxy_intercept_errors directive")
+		assert.NotContains(ginkgo.GinkgoT(), serverConfig, "proxy_intercept_errors on;")
+
+		ginkgo.By("not creating error locations")
+		assert.NotContains(ginkgo.GinkgoT(), serverConfig, fmt.Sprintf("location %s", errorBlockName("upstream-default-backend", "off")))
+	})
 })