From cc0872cfab5724fb9bd287b3cb797877f73b4df4 Mon Sep 17 00:00:00 2001 From: chriss-de Date: Tue, 10 Jan 2023 16:36:56 +0100 Subject: [PATCH] added proxy-intercept-errors config option --- .../nginx-configuration/annotations.md | 12 ++++ .../nginx-configuration/configmap.md | 9 ++- internal/ingress/annotations/annotations.go | 3 + .../annotations/proxyintercepterrors/main.go | 43 +++++++++++++ .../proxyintercepterrors/main_test.go | 61 +++++++++++++++++++ internal/ingress/controller/config/config.go | 6 ++ 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 +- 10 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 internal/ingress/annotations/proxyintercepterrors/main.go create mode 100644 internal/ingress/annotations/proxyintercepterrors/main_test.go diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 8cc6f4c16..48765cc8b 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/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/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/proxy-intercept-errors](#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| @@ -331,6 +332,17 @@ Example usage: nginx.ingress.kubernetes.io/custom-http-errors: "404,415" ``` +## Proxy intercept Errors + +Like the [`proxy-intercept-errors`](./configmap.md#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 `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/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 77cee0507..182402786 100755 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -161,6 +161,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{}| +|[proxy-intercept-errors](#proxy-intercept-errors)|bool|"true"| |[proxy-body-size](#proxy-body-size)|string|"1m"| |[proxy-connect-timeout](#proxy-connect-timeout)|int|5| |[proxy-read-timeout](#proxy-read-timeout)|int|60| @@ -1028,10 +1029,16 @@ 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 [proxy-intercept-errors](#proxy-intercept-errors). Example usage: `custom-http-errors: 404,415` +## proxy-intercept-errors + +Allows to disable proxy-intercept-errors if [custom-http-errors](#custom-http-errors) are set. 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: `proxy-intercept-errors: "false"` + ## 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 14415a4f7..9b15f6561 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -20,6 +20,7 @@ import ( "github.com/imdario/mergo" "k8s.io/ingress-nginx/internal/ingress/annotations/canary" "k8s.io/ingress-nginx/internal/ingress/annotations/modsecurity" + "k8s.io/ingress-nginx/internal/ingress/annotations/proxyintercepterrors" "k8s.io/ingress-nginx/internal/ingress/annotations/proxyssl" "k8s.io/ingress-nginx/internal/ingress/annotations/sslcipher" "k8s.io/ingress-nginx/internal/ingress/annotations/streamsnippet" @@ -86,6 +87,7 @@ type Ingress struct { Connection connection.Config CorsConfig cors.Config CustomHTTPErrors []int + ProxyInterceptErrors bool DefaultBackend *apiv1.Service //TODO: Change this back into an error when https://github.com/imdario/mergo/issues/100 is resolved FastCGI fastcgi.Config @@ -139,6 +141,7 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor { "Connection": connection.NewParser(cfg), "CorsConfig": cors.NewParser(cfg), "CustomHTTPErrors": customhttperrors.NewParser(cfg), + "ProxyInterceptErrors": proxyintercepterrors.NewParser(cfg), "DefaultBackend": defaultbackend.NewParser(cfg), "FastCGI": fastcgi.NewParser(cfg), "ExternalAuth": authreq.NewParser(cfg), diff --git a/internal/ingress/annotations/proxyintercepterrors/main.go b/internal/ingress/annotations/proxyintercepterrors/main.go new file mode 100644 index 000000000..4f7584ed3 --- /dev/null +++ b/internal/ingress/annotations/proxyintercepterrors/main.go @@ -0,0 +1,43 @@ +/* +Copyright 2017 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 proxyintercepterrors + +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" +) + +type proxyInterceptErrors struct { + r resolver.Resolver +} + +// NewParser creates a new proxyintercepterrors annotation parser +func NewParser(r resolver.Resolver) parser.IngressAnnotation { + return proxyInterceptErrors{r} +} + +func (s proxyInterceptErrors) Parse(ing *networking.Ingress) (interface{}, error) { + defBackend := s.r.GetDefaultBackend() + + val, err := parser.GetBoolAnnotation("proxy-intercept-errors", ing) + // A missing annotation is not a problem, just use the default + if err == errors.ErrMissingAnnotations { + return defBackend.ProxyInterceptErrors, nil + } + + return val, nil +} diff --git a/internal/ingress/annotations/proxyintercepterrors/main_test.go b/internal/ingress/annotations/proxyintercepterrors/main_test.go new file mode 100644 index 000000000..1bb548568 --- /dev/null +++ b/internal/ingress/annotations/proxyintercepterrors/main_test.go @@ -0,0 +1,61 @@ +/* +Copyright 2017 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 proxyintercepterrors + +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("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 proxy intercept errors") + } +} diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index f064dad53..112c46eba 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -519,6 +519,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 + ProxyInterceptErrors bool `json:"proxy-intercept-errors,omitempty"` + // Sets the ipv4 addresses on which the server will accept requests. BindAddressIpv4 []string `json:"bind-address-ipv4,omitempty"` @@ -873,6 +877,7 @@ func NewDefault() Configuration { VariablesHashBucketSize: 256, VariablesHashMaxSize: 2048, UseHTTP2: true, + ProxyInterceptErrors: true, ProxyStreamTimeout: "600s", ProxyStreamNextUpstream: true, ProxyStreamNextUpstreamTimeout: "600s", @@ -895,6 +900,7 @@ func NewDefault() Configuration { PreserveTrailingSlash: false, SSLRedirect: true, CustomHTTPErrors: []int{}, + ProxyInterceptErrors: true, DenylistSourceRange: []string{}, WhitelistSourceRange: []string{}, SkipAccessLogURLs: []string{}, diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go index 0aab2ff47..034e2b420 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"` + // for use when using CustomHTTPErrors without intecepting 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 enabled when CustomHTTPErrors is enabled + ProxyInterceptErrors bool `json:"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 9395683ec..a9df69612 100644 --- a/pkg/apis/ingress/types.go +++ b/pkg/apis/ingress/types.go @@ -349,6 +349,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 + ProxyInterceptErrors bool `json:"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 0941e0956..310027335 100644 --- a/pkg/apis/ingress/types_equals.go +++ b/pkg/apis/ingress/types_equals.go @@ -472,6 +472,10 @@ func (l1 *Location) Equal(l2 *Location) bool { return false } + if !l1.ProxyInterceptErrors != l2.ProxyInterceptErrors { + return false + } + return true } diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index fc48de912..dce50eb6f 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -470,7 +470,7 @@ http { ssl_certificate {{ $cfg.DefaultSSLCertificate.PemFileName }}; ssl_certificate_key {{ $cfg.DefaultSSLCertificate.PemFileName }}; - {{ if gt (len $cfg.CustomHTTPErrors) 0 }} + {{ if and (gt (len $cfg.CustomHTTPErrors) 0) $cfg.ProxyInterceptErrors }} proxy_intercept_errors on; {{ end }} @@ -1450,7 +1450,7 @@ stream { {{ end }} {{/* if a location-specific error override is set, add the proxy_intercept here */}} - {{ if $location.CustomHTTPErrors }} + {{ if and $location.CustomHTTPErrors $location.ProxyInterceptErrors }} # Custom error pages per ingress proxy_intercept_errors on; {{ end }}