From ad406b64d8b0fc92b703acad24170a4aeb119d84 Mon Sep 17 00:00:00 2001 From: chriss-de <54666227+chriss-de@users.noreply.github.com> Date: Fri, 17 Nov 2023 05:43:54 +0100 Subject: [PATCH] Add override for proxy_intercept_errors when using Custom HTTP Errors (#9497) * added proxy-intercept-errors config option * fixed error when comparing locations * fixed missing location config from annotation added e2e test * reversed logic for proxy-intercept-errors to disable-proxy-intercept-errors * reversed logic to disable-proxy-intercept-errors * reversed logic * default has to be false * put comment in same line as return * run gofmt * fixing wrong Boilerplate header * updated code to new IngressAnnotation interface * fixes to satisfy PR comments * synced with upstream; fixed typo * gofumpt disableproxyintercepterrors.go * gofumpt --- .../nginx-configuration/annotations.md | 12 ++ .../nginx-configuration/configmap.md | 11 +- internal/ingress/annotations/annotations.go | 161 +++++++++--------- .../disableproxyintercepterrors/main.go | 76 +++++++++ .../disableproxyintercepterrors/main_test.go | 64 +++++++ internal/ingress/controller/config/config.go | 58 ++++--- internal/ingress/controller/controller.go | 1 + internal/ingress/defaults/main.go | 7 + pkg/apis/ingress/types.go | 5 + pkg/apis/ingress/types_equals.go | 4 + rootfs/etc/nginx/template/nginx.tmpl | 4 +- .../disableproxyintercepterrors.go | 100 +++++++++++ 12 files changed, 395 insertions(+), 108 deletions(-) create mode 100644 internal/ingress/annotations/disableproxyintercepterrors/main.go create mode 100644 internal/ingress/annotations/disableproxyintercepterrors/main_test.go create mode 100644 test/e2e/annotations/disableproxyintercepterrors.go diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 7cd34d344f..eda51a435a 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -50,6 +50,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz |[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/disable-proxy-intercept-errors](#disable-proxy-intercept-errors)|"true" or "false"| |[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| @@ -330,6 +331,17 @@ Example usage: nginx.ingress.kubernetes.io/custom-http-errors: "404,415" ``` +## Disable Proxy intercept Errors + +Like the [`disable-proxy-intercept-errors`](./configmap.md#disable-proxy-intercept-errors) value in the ConfigMap, this annotation allows to disable NGINX `proxy-intercept-errors` when `custom-http-errors` are set, but only for the NGINX location associated with this ingress. If a [default backend annotation](#default-backend) is specified on the ingress, the errors will be routed to that annotation's default backend service (instead of the global default backend). +Different ingresses can specify different sets of errors codes and there are UseCases where NGINX shall not intercept all errors returned from upstream. +If `disable-proxy-intercept-errors` is also specified globally, the annotation will override the global value for the given ingress' hostname and path. + +Example usage: +``` +nginx.ingress.kubernetes.io/disable-proxy-intercept-errors: "false" +``` + ### Default Backend This annotation is of the form `nginx.ingress.kubernetes.io/default-backend: ` to specify a custom default backend. This `` is a reference to a service inside of the same namespace in which you are applying this annotation. This annotation overrides the global default backend. In case the service has [multiple ports](https://kubernetes.io/docs/concepts/services-networking/service/#multi-port-services), the first one is the one which will receive the backend traffic. diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 92ac39ee6f..2e3dbf012b 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -180,6 +180,7 @@ The following table shows a configuration option's name, type, and the default v |[stream-snippet](#stream-snippet)|string|""|| |[location-snippet](#location-snippet)|string|""|| |[custom-http-errors](#custom-http-errors)|[]int|[]int{}|| +|[disable-proxy-intercept-errors](#disable-proxy-intercept-errors)|bool|"false"| |[proxy-body-size](#proxy-body-size)|string|"1m"|| |[proxy-connect-timeout](#proxy-connect-timeout)|int|5|| |[proxy-read-timeout](#proxy-read-timeout)|int|60|| @@ -1128,10 +1129,18 @@ You can not use this to add new locations that proxy to the Kubernetes pods, as Enables which HTTP codes should be passed for processing with the [error_page directive](https://nginx.org/en/docs/http/ngx_http_core_module.html#error_page) -Setting at least one code also enables [proxy_intercept_errors](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors) which are required to process error_page. +Setting at least one code also enables [proxy_intercept_errors](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors) if not disabled with [disable-proxy-intercept-errors](#disable-proxy-intercept-errors). Example usage: `custom-http-errors: 404,415` +## disable-proxy-intercept-errors + +Allows to disable [proxy-intercept-errors](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors). + +Disabling [proxy_intercept_errors](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors) allows to pass upstream errors to client even if [custom-http-errors](#custom-http-errors) are set. + +Example usage: `disable-proxy-intercept-errors: "true"` + ## proxy-body-size Sets the maximum allowed size of the client request body. diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index 44fabba4ee..302f0b4b11 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -20,6 +20,7 @@ import ( "dario.cat/mergo" "k8s.io/ingress-nginx/internal/ingress/annotations/canary" + "k8s.io/ingress-nginx/internal/ingress/annotations/disableproxyintercepterrors" "k8s.io/ingress-nginx/internal/ingress/annotations/modsecurity" "k8s.io/ingress-nginx/internal/ingress/annotations/opentelemetry" "k8s.io/ingress-nginx/internal/ingress/annotations/proxyssl" @@ -75,46 +76,47 @@ const DeniedKeyName = "Denied" // Ingress defines the valid annotations present in one NGINX Ingress rule type Ingress struct { metav1.ObjectMeta - BackendProtocol string - Aliases []string - BasicDigestAuth auth.Config - Canary canary.Config - CertificateAuth authtls.Config - ClientBodyBufferSize string - ConfigurationSnippet string - Connection connection.Config - CorsConfig cors.Config - CustomHTTPErrors []int - DefaultBackend *apiv1.Service - FastCGI fastcgi.Config - Denied *string - ExternalAuth authreq.Config - EnableGlobalAuth bool - HTTP2PushPreload bool - Opentelemetry opentelemetry.Config - Proxy proxy.Config - ProxySSL proxyssl.Config - RateLimit ratelimit.Config - GlobalRateLimit globalratelimit.Config - Redirect redirect.Config - Rewrite rewrite.Config - Satisfy string - ServerSnippet string - ServiceUpstream bool - SessionAffinity sessionaffinity.Config - SSLPassthrough bool - UsePortInRedirects bool - UpstreamHashBy upstreamhashby.Config - LoadBalancing string - UpstreamVhost string - Denylist ipdenylist.SourceRange - XForwardedPrefix string - SSLCipher sslcipher.Config - Logs log.Config - ModSecurity modsecurity.Config - Mirror mirror.Config - StreamSnippet string - Allowlist ipallowlist.SourceRange + BackendProtocol string + Aliases []string + BasicDigestAuth auth.Config + Canary canary.Config + CertificateAuth authtls.Config + ClientBodyBufferSize string + ConfigurationSnippet string + Connection connection.Config + CorsConfig cors.Config + CustomHTTPErrors []int + DisableProxyInterceptErrors bool + DefaultBackend *apiv1.Service + FastCGI fastcgi.Config + Denied *string + ExternalAuth authreq.Config + EnableGlobalAuth bool + HTTP2PushPreload bool + Opentelemetry opentelemetry.Config + Proxy proxy.Config + ProxySSL proxyssl.Config + RateLimit ratelimit.Config + GlobalRateLimit globalratelimit.Config + Redirect redirect.Config + Rewrite rewrite.Config + Satisfy string + ServerSnippet string + ServiceUpstream bool + SessionAffinity sessionaffinity.Config + SSLPassthrough bool + UsePortInRedirects bool + UpstreamHashBy upstreamhashby.Config + LoadBalancing string + UpstreamVhost string + Denylist ipdenylist.SourceRange + XForwardedPrefix string + SSLCipher sslcipher.Config + Logs log.Config + ModSecurity modsecurity.Config + Mirror mirror.Config + StreamSnippet string + Allowlist ipallowlist.SourceRange } // Extractor defines the annotation parsers to be used in the extraction of annotations @@ -126,45 +128,46 @@ type Extractor struct { func NewAnnotationExtractor(cfg resolver.Resolver) Extractor { return Extractor{ map[string]parser.IngressAnnotation{ - "Aliases": alias.NewParser(cfg), - "BasicDigestAuth": auth.NewParser(auth.AuthDirectory, cfg), - "Canary": canary.NewParser(cfg), - "CertificateAuth": authtls.NewParser(cfg), - "ClientBodyBufferSize": clientbodybuffersize.NewParser(cfg), - "ConfigurationSnippet": snippet.NewParser(cfg), - "Connection": connection.NewParser(cfg), - "CorsConfig": cors.NewParser(cfg), - "CustomHTTPErrors": customhttperrors.NewParser(cfg), - "DefaultBackend": defaultbackend.NewParser(cfg), - "FastCGI": fastcgi.NewParser(cfg), - "ExternalAuth": authreq.NewParser(cfg), - "EnableGlobalAuth": authreqglobal.NewParser(cfg), - "HTTP2PushPreload": http2pushpreload.NewParser(cfg), - "Opentelemetry": opentelemetry.NewParser(cfg), - "Proxy": proxy.NewParser(cfg), - "ProxySSL": proxyssl.NewParser(cfg), - "RateLimit": ratelimit.NewParser(cfg), - "GlobalRateLimit": globalratelimit.NewParser(cfg), - "Redirect": redirect.NewParser(cfg), - "Rewrite": rewrite.NewParser(cfg), - "Satisfy": satisfy.NewParser(cfg), - "ServerSnippet": serversnippet.NewParser(cfg), - "ServiceUpstream": serviceupstream.NewParser(cfg), - "SessionAffinity": sessionaffinity.NewParser(cfg), - "SSLPassthrough": sslpassthrough.NewParser(cfg), - "UsePortInRedirects": portinredirect.NewParser(cfg), - "UpstreamHashBy": upstreamhashby.NewParser(cfg), - "LoadBalancing": loadbalancing.NewParser(cfg), - "UpstreamVhost": upstreamvhost.NewParser(cfg), - "Allowlist": ipallowlist.NewParser(cfg), - "Denylist": ipdenylist.NewParser(cfg), - "XForwardedPrefix": xforwardedprefix.NewParser(cfg), - "SSLCipher": sslcipher.NewParser(cfg), - "Logs": log.NewParser(cfg), - "BackendProtocol": backendprotocol.NewParser(cfg), - "ModSecurity": modsecurity.NewParser(cfg), - "Mirror": mirror.NewParser(cfg), - "StreamSnippet": streamsnippet.NewParser(cfg), + "Aliases": alias.NewParser(cfg), + "BasicDigestAuth": auth.NewParser(auth.AuthDirectory, cfg), + "Canary": canary.NewParser(cfg), + "CertificateAuth": authtls.NewParser(cfg), + "ClientBodyBufferSize": clientbodybuffersize.NewParser(cfg), + "ConfigurationSnippet": snippet.NewParser(cfg), + "Connection": connection.NewParser(cfg), + "CorsConfig": cors.NewParser(cfg), + "CustomHTTPErrors": customhttperrors.NewParser(cfg), + "DisableProxyInterceptErrors": disableproxyintercepterrors.NewParser(cfg), + "DefaultBackend": defaultbackend.NewParser(cfg), + "FastCGI": fastcgi.NewParser(cfg), + "ExternalAuth": authreq.NewParser(cfg), + "EnableGlobalAuth": authreqglobal.NewParser(cfg), + "HTTP2PushPreload": http2pushpreload.NewParser(cfg), + "Opentelemetry": opentelemetry.NewParser(cfg), + "Proxy": proxy.NewParser(cfg), + "ProxySSL": proxyssl.NewParser(cfg), + "RateLimit": ratelimit.NewParser(cfg), + "GlobalRateLimit": globalratelimit.NewParser(cfg), + "Redirect": redirect.NewParser(cfg), + "Rewrite": rewrite.NewParser(cfg), + "Satisfy": satisfy.NewParser(cfg), + "ServerSnippet": serversnippet.NewParser(cfg), + "ServiceUpstream": serviceupstream.NewParser(cfg), + "SessionAffinity": sessionaffinity.NewParser(cfg), + "SSLPassthrough": sslpassthrough.NewParser(cfg), + "UsePortInRedirects": portinredirect.NewParser(cfg), + "UpstreamHashBy": upstreamhashby.NewParser(cfg), + "LoadBalancing": loadbalancing.NewParser(cfg), + "UpstreamVhost": upstreamvhost.NewParser(cfg), + "Allowlist": ipallowlist.NewParser(cfg), + "Denylist": ipdenylist.NewParser(cfg), + "XForwardedPrefix": xforwardedprefix.NewParser(cfg), + "SSLCipher": sslcipher.NewParser(cfg), + "Logs": log.NewParser(cfg), + "BackendProtocol": backendprotocol.NewParser(cfg), + "ModSecurity": modsecurity.NewParser(cfg), + "Mirror": mirror.NewParser(cfg), + "StreamSnippet": streamsnippet.NewParser(cfg), }, } } diff --git a/internal/ingress/annotations/disableproxyintercepterrors/main.go b/internal/ingress/annotations/disableproxyintercepterrors/main.go new file mode 100644 index 0000000000..650d29707b --- /dev/null +++ b/internal/ingress/annotations/disableproxyintercepterrors/main.go @@ -0,0 +1,76 @@ +/* +Copyright 2023 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 disableproxyintercepterrors + +import ( + networking "k8s.io/api/networking/v1" + + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/errors" + "k8s.io/ingress-nginx/internal/ingress/resolver" +) + +const ( + disableProxyInterceptErrorsAnnotation = "disable-proxy-intercept-errors" +) + +var disableProxyInterceptErrorsAnnotations = parser.Annotation{ + Group: "backend", + Annotations: parser.AnnotationFields{ + disableProxyInterceptErrorsAnnotation: { + Validator: parser.ValidateBool, + Scope: parser.AnnotationScopeLocation, + Risk: parser.AnnotationRiskLow, + Documentation: `This annotation allows to disable NGINX proxy-intercept-errors when custom-http-errors are set. + If a default backend annotation is specified on the ingress, the errors will be routed to that annotation's default backend service (instead of the global default backend). + Different ingresses can specify different sets of errors codes and there are UseCases where NGINX shall not intercept all errors returned from upstream.`, + }, + }, +} + +type disableProxyInterceptErrors struct { + r resolver.Resolver + annotationConfig parser.Annotation +} + +func (pie disableProxyInterceptErrors) GetDocumentation() parser.AnnotationFields { + return pie.annotationConfig.Annotations +} + +func (pie disableProxyInterceptErrors) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(pie.r.GetSecurityConfiguration().AnnotationsRiskLevel) + return parser.CheckAnnotationRisk(anns, maxrisk, disableProxyInterceptErrorsAnnotations.Annotations) +} + +// NewParser creates a new disableProxyInterceptErrors annotation parser +func NewParser(r resolver.Resolver) parser.IngressAnnotation { + return disableProxyInterceptErrors{ + r: r, + annotationConfig: disableProxyInterceptErrorsAnnotations, + } +} + +func (pie disableProxyInterceptErrors) Parse(ing *networking.Ingress) (interface{}, error) { + val, err := parser.GetBoolAnnotation(disableProxyInterceptErrorsAnnotation, ing, pie.annotationConfig.Annotations) + + // A missing annotation is not a problem, just use the default + if err == errors.ErrMissingAnnotations { + return false, nil // default is false + } + + return val, nil +} diff --git a/internal/ingress/annotations/disableproxyintercepterrors/main_test.go b/internal/ingress/annotations/disableproxyintercepterrors/main_test.go new file mode 100644 index 0000000000..1238bb4e07 --- /dev/null +++ b/internal/ingress/annotations/disableproxyintercepterrors/main_test.go @@ -0,0 +1,64 @@ +/* +Copyright 2022 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 disableproxyintercepterrors + +import ( + "testing" + + api "k8s.io/api/core/v1" + networking "k8s.io/api/networking/v1" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/resolver" +) + +func buildIngress() *networking.Ingress { + return &networking.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + }, + Spec: networking.IngressSpec{ + DefaultBackend: &networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "default-backend", + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + } +} + +func TestParseAnnotations(t *testing.T) { + ing := buildIngress() + + _, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + data := map[string]string{} + data[parser.GetAnnotationWithPrefix("disable-proxy-intercept-errors")] = "true" + ing.SetAnnotations(data) + // test ingress using the annotation without a TLS section + _, err = NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error parsing ingress with disable-proxy-intercept-errors") + } +} diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 67878f185a..bad82b8b0b 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -540,6 +540,10 @@ type Configuration struct { // http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_http_version ProxyHTTPVersion string `json:"proxy-http-version"` + // Disables NGINX proxy-intercept-errors when error_page/custom-http-errors are set + // https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors + DisableProxyInterceptErrors bool `json:"disable-proxy-intercept-errors,omitempty"` + // Sets the ipv4 addresses on which the server will accept requests. BindAddressIpv4 []string `json:"bind-address-ipv4,omitempty"` @@ -842,37 +846,39 @@ func NewDefault() Configuration { VariablesHashBucketSize: 256, VariablesHashMaxSize: 2048, UseHTTP2: true, + DisableProxyInterceptErrors: false, ProxyStreamTimeout: "600s", ProxyStreamNextUpstream: true, ProxyStreamNextUpstreamTimeout: "600s", ProxyStreamNextUpstreamTries: 3, Backend: defaults.Backend{ - ProxyBodySize: bodySize, - ProxyConnectTimeout: 5, - ProxyReadTimeout: 60, - ProxySendTimeout: 60, - ProxyBuffersNumber: 4, - ProxyBufferSize: "4k", - ProxyCookieDomain: "off", - ProxyCookiePath: "off", - ProxyNextUpstream: "error timeout", - ProxyNextUpstreamTimeout: 0, - ProxyNextUpstreamTries: 3, - ProxyRequestBuffering: "on", - ProxyRedirectFrom: "off", - ProxyRedirectTo: "off", - PreserveTrailingSlash: false, - SSLRedirect: true, - CustomHTTPErrors: []int{}, - DenylistSourceRange: []string{}, - WhitelistSourceRange: []string{}, - SkipAccessLogURLs: []string{}, - LimitRate: 0, - LimitRateAfter: 0, - ProxyBuffering: "off", - ProxyHTTPVersion: "1.1", - ProxyMaxTempFileSize: "1024m", - ServiceUpstream: false, + ProxyBodySize: bodySize, + ProxyConnectTimeout: 5, + ProxyReadTimeout: 60, + ProxySendTimeout: 60, + ProxyBuffersNumber: 4, + ProxyBufferSize: "4k", + ProxyCookieDomain: "off", + ProxyCookiePath: "off", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: 0, + ProxyNextUpstreamTries: 3, + ProxyRequestBuffering: "on", + ProxyRedirectFrom: "off", + ProxyRedirectTo: "off", + PreserveTrailingSlash: false, + SSLRedirect: true, + CustomHTTPErrors: []int{}, + DisableProxyInterceptErrors: false, + DenylistSourceRange: []string{}, + WhitelistSourceRange: []string{}, + SkipAccessLogURLs: []string{}, + LimitRate: 0, + LimitRateAfter: 0, + ProxyBuffering: "off", + ProxyHTTPVersion: "1.1", + ProxyMaxTempFileSize: "1024m", + ServiceUpstream: false, }, UpstreamKeepaliveConnections: 320, UpstreamKeepaliveTime: "1h", diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 701a747fa5..cb8d3712cd 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -1526,6 +1526,7 @@ func locationApplyAnnotations(loc *ingress.Location, anns *annotations.Ingress) loc.BackendProtocol = anns.BackendProtocol loc.FastCGI = anns.FastCGI loc.CustomHTTPErrors = anns.CustomHTTPErrors + loc.DisableProxyInterceptErrors = anns.DisableProxyInterceptErrors loc.ModSecurity = anns.ModSecurity loc.Satisfy = anns.Satisfy loc.Mirror = anns.Mirror diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go index 0526d443e6..2bb58c858f 100644 --- a/internal/ingress/defaults/main.go +++ b/internal/ingress/defaults/main.go @@ -34,6 +34,13 @@ type Backend struct { // toggles whether or not to remove trailing slashes during TLS redirects PreserveTrailingSlash bool `json:"preserve-trailing-slash"` + // allows usage of CustomHTTPErrors without intercepting service errors + // e.g. custom 404 and 503 when service-a does not exist or is not available + // but service-a can return 404 and 503 error codes without intercept + // http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors + // By default this is false + DisableProxyInterceptErrors bool `json:"disable-proxy-intercept-errors"` + // http://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size // Sets the maximum allowed size of the client request body ProxyBodySize string `json:"proxy-body-size"` diff --git a/pkg/apis/ingress/types.go b/pkg/apis/ingress/types.go index 6deba659e2..2ad17ec3dd 100644 --- a/pkg/apis/ingress/types.go +++ b/pkg/apis/ingress/types.go @@ -345,6 +345,11 @@ type Location struct { // CustomHTTPErrors specifies the error codes that should be intercepted. // +optional CustomHTTPErrors []int `json:"custom-http-errors"` + // ProxyInterceptErrors disables error intecepting when using CustomHTTPErrors + // e.g. custom 404 and 503 when service-a does not exist or is not available + // but service-a can return 404 and 503 error codes without intercept + // +optional + DisableProxyInterceptErrors bool `json:"disable-proxy-intercept-errors"` // ModSecurity allows to enable and configure modsecurity // +optional ModSecurity modsecurity.Config `json:"modsecurity"` diff --git a/pkg/apis/ingress/types_equals.go b/pkg/apis/ingress/types_equals.go index 2cde77e836..eeed9a06e4 100644 --- a/pkg/apis/ingress/types_equals.go +++ b/pkg/apis/ingress/types_equals.go @@ -466,6 +466,10 @@ func (l1 *Location) Equal(l2 *Location) bool { return false } + if l1.DisableProxyInterceptErrors != l2.DisableProxyInterceptErrors { + return false + } + return true } diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 4454a47108..94dc12412e 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -487,7 +487,7 @@ http { ssl_certificate {{ $cfg.DefaultSSLCertificate.PemFileName }}; ssl_certificate_key {{ $cfg.DefaultSSLCertificate.PemFileName }}; - {{ if gt (len $cfg.CustomHTTPErrors) 0 }} + {{ if and $cfg.CustomHTTPErrors (not $cfg.DisableProxyInterceptErrors) }} proxy_intercept_errors on; {{ end }} @@ -1473,7 +1473,7 @@ stream { {{ end }} {{/* if a location-specific error override is set, add the proxy_intercept here */}} - {{ if $location.CustomHTTPErrors }} + {{ if and $location.CustomHTTPErrors (not $location.DisableProxyInterceptErrors) }} # Custom error pages per ingress proxy_intercept_errors on; {{ end }} diff --git a/test/e2e/annotations/disableproxyintercepterrors.go b/test/e2e/annotations/disableproxyintercepterrors.go new file mode 100644 index 0000000000..17efa45884 --- /dev/null +++ b/test/e2e/annotations/disableproxyintercepterrors.go @@ -0,0 +1,100 @@ +/* +Copyright 2023 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" + "net/http" + "strings" + + networking "k8s.io/api/networking/v1" + + "github.com/onsi/ginkgo/v2" + "github.com/stretchr/testify/assert" + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.DescribeAnnotation("disable-proxy-intercept-errors", func() { + f := framework.NewDefaultFramework("disable-proxy-intercept-errors") + + ginkgo.BeforeEach(func() { + f.NewHttpbunDeployment() + f.NewEchoDeployment() + }) + + ginkgo.It("configures Nginx correctly", func() { + host := "pie.foo.com" + + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/custom-http-errors": "404", + "nginx.ingress.kubernetes.io/disable-proxy-intercept-errors": "true", + "nginx.ingress.kubernetes.io/default-backend": framework.EchoService, + } + + ingHTTPBunService := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.HTTPBunService, 80, annotations) + f.EnsureIngress(ingHTTPBunService) + + var serverConfig string + f.WaitForNginxServer(host, func(sc string) bool { + serverConfig = sc + return strings.Contains(serverConfig, fmt.Sprintf("server_name %s", host)) + }) + + ginkgo.By("turning off proxy_intercept_errors directive") + assert.NotContains(ginkgo.GinkgoT(), serverConfig, "proxy_intercept_errors on;") + + // the plan for client side testing + // create ingress where we disable intercept for code 404 - that error should get to the client + // the same ingress should intercept any other error (>300 and not 404) where we will get intercepted error + ginkgo.By("client test to check response - with intercept disabled") + requestID := "proxy_intercept_errors" + + f.HTTPTestClient(). + GET("/status/404"). + WithHeader("Host", host). + WithHeader("x-request-id", requestID). + Expect(). + Status(http.StatusNotFound). + Body().Empty() + + ginkgo.By("client test to check response - with intercept enabled") + err := framework.UpdateIngress(f.KubeClientSet, f.Namespace, host, func(ingress *networking.Ingress) error { + ingress.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/disable-proxy-intercept-errors"] = "false" + return nil + }) + assert.Nil(ginkgo.GinkgoT(), err) + + f.WaitForNginxServer(host, func(sc string) bool { + if serverConfig != sc { + serverConfig = sc + return true + } + return false + }) + + f.HTTPTestClient(). + GET("/status/404"). + WithHeader("Host", host). + WithHeader("x-request-id", requestID). + Expect(). + Status(http.StatusOK). + Body().Contains("x-code=404"). + Contains(fmt.Sprintf("x-ingress-name=%s", host)). + Contains(fmt.Sprintf("x-service-name=%s", framework.HTTPBunService)). + Contains(fmt.Sprintf("x-request-id=%s", requestID)) + }) +})