From cc0872cfab5724fb9bd287b3cb797877f73b4df4 Mon Sep 17 00:00:00 2001 From: chriss-de Date: Tue, 10 Jan 2023 16:36:56 +0100 Subject: [PATCH 01/15] 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 8cc6f4c168..48765cc8b2 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 77cee05078..1824027861 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 14415a4f7b..9b15f6561e 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 0000000000..4f7584ed35 --- /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 0000000000..1bb548568e --- /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 f064dad53e..112c46eba9 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 0aab2ff478..034e2b4202 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 9395683ece..a9df696120 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 0941e09560..3100273355 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 fc48de9128..dce50eb6f2 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 }} From ae2d6d422014165d77afc1deb186c071603f18a0 Mon Sep 17 00:00:00 2001 From: chriss-de Date: Wed, 11 Jan 2023 09:12:15 +0100 Subject: [PATCH 02/15] fixed error when comparing locations --- pkg/apis/ingress/types_equals.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/ingress/types_equals.go b/pkg/apis/ingress/types_equals.go index 3100273355..fe01311c91 100644 --- a/pkg/apis/ingress/types_equals.go +++ b/pkg/apis/ingress/types_equals.go @@ -472,7 +472,7 @@ func (l1 *Location) Equal(l2 *Location) bool { return false } - if !l1.ProxyInterceptErrors != l2.ProxyInterceptErrors { + if l1.ProxyInterceptErrors != l2.ProxyInterceptErrors { return false } From 0a5708bdebcb73734e9bd2efb15650db802336ae Mon Sep 17 00:00:00 2001 From: chriss-de Date: Wed, 11 Jan 2023 14:49:48 +0100 Subject: [PATCH 03/15] fixed missing location config from annotation added e2e test --- .../annotations/proxyintercepterrors/main.go | 8 +- internal/ingress/controller/controller.go | 1 + test/e2e/annotations/proxyintercepterrors.go | 82 +++++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 test/e2e/annotations/proxyintercepterrors.go diff --git a/internal/ingress/annotations/proxyintercepterrors/main.go b/internal/ingress/annotations/proxyintercepterrors/main.go index 4f7584ed35..881f7c7ef3 100644 --- a/internal/ingress/annotations/proxyintercepterrors/main.go +++ b/internal/ingress/annotations/proxyintercepterrors/main.go @@ -30,13 +30,13 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { return proxyInterceptErrors{r} } -func (s proxyInterceptErrors) Parse(ing *networking.Ingress) (interface{}, error) { - defBackend := s.r.GetDefaultBackend() - +func (pie proxyInterceptErrors) Parse(ing *networking.Ingress) (interface{}, error) { 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 true, nil + // default is true and only matters when "custom-http-errors" is also set } return val, nil diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 010eba162b..da41a21603 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -1426,6 +1426,7 @@ func locationApplyAnnotations(loc *ingress.Location, anns *annotations.Ingress) loc.BackendProtocol = anns.BackendProtocol loc.FastCGI = anns.FastCGI loc.CustomHTTPErrors = anns.CustomHTTPErrors + loc.ProxyInterceptErrors = anns.ProxyInterceptErrors loc.ModSecurity = anns.ModSecurity loc.Satisfy = anns.Satisfy loc.Mirror = anns.Mirror diff --git a/test/e2e/annotations/proxyintercepterrors.go b/test/e2e/annotations/proxyintercepterrors.go new file mode 100644 index 0000000000..b1f1aca946 --- /dev/null +++ b/test/e2e/annotations/proxyintercepterrors.go @@ -0,0 +1,82 @@ +/* +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/v2" + "github.com/stretchr/testify/assert" + networking "k8s.io/api/networking/v1" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.DescribeAnnotation("proxy-intercept-errors", func() { + f := framework.NewDefaultFramework("proxy-intercept-errors") + + ginkgo.BeforeEach(func() { + f.NewEchoDeployment() + }) + + ginkgo.It("configures Nginx correctly", func() { + pieState := "true" + host := "pie.foo.com" + + errorCodes := []string{"404"} + + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(errorCodes, ","), + "nginx.ingress.kubernetes.io/proxy-intercept-errors": pieState, + } + + 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)) + }) + + ginkgo.By("turning on proxy_intercept_errors directive") + assert.Contains(ginkgo.GinkgoT(), serverConfig, "proxy_intercept_errors on;") + + // -- + pieState = "false" + + ginkgo.By("updating configuration when only proxy-intercept-errors value changes") + err := framework.UpdateIngress(f.KubeClientSet, f.Namespace, host, func(ingress *networking.Ingress) error { + ingress.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/proxy-intercept-errors"] = pieState + return nil + }) + assert.Nil(ginkgo.GinkgoT(), err) + + f.WaitForNginxServer(host, func(sc string) bool { + if serverConfig != sc { + serverConfig = sc + return true + } + return false + }) + + ginkgo.By("turning off proxy_intercept_errors directive") + assert.NotContains(ginkgo.GinkgoT(), serverConfig, "proxy_intercept_errors on;") + + }) +}) From 0c86dace95cfdfddcf4520982ece76df4be032dd Mon Sep 17 00:00:00 2001 From: chriss-de Date: Wed, 25 Jan 2023 11:48:25 +0100 Subject: [PATCH 04/15] reversed logic for proxy-intercept-errors to disable-proxy-intercept-errors --- .../{proxyintercepterrors => disableproxyintercepterrors}/main.go | 0 .../main_test.go | 0 .../{proxyintercepterrors.go => disableproxyintercepterrors.go} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename internal/ingress/annotations/{proxyintercepterrors => disableproxyintercepterrors}/main.go (100%) rename internal/ingress/annotations/{proxyintercepterrors => disableproxyintercepterrors}/main_test.go (100%) rename test/e2e/annotations/{proxyintercepterrors.go => disableproxyintercepterrors.go} (100%) diff --git a/internal/ingress/annotations/proxyintercepterrors/main.go b/internal/ingress/annotations/disableproxyintercepterrors/main.go similarity index 100% rename from internal/ingress/annotations/proxyintercepterrors/main.go rename to internal/ingress/annotations/disableproxyintercepterrors/main.go diff --git a/internal/ingress/annotations/proxyintercepterrors/main_test.go b/internal/ingress/annotations/disableproxyintercepterrors/main_test.go similarity index 100% rename from internal/ingress/annotations/proxyintercepterrors/main_test.go rename to internal/ingress/annotations/disableproxyintercepterrors/main_test.go diff --git a/test/e2e/annotations/proxyintercepterrors.go b/test/e2e/annotations/disableproxyintercepterrors.go similarity index 100% rename from test/e2e/annotations/proxyintercepterrors.go rename to test/e2e/annotations/disableproxyintercepterrors.go From 23dac1d95d9d3168a99dcf56e14bf73b7cd9a659 Mon Sep 17 00:00:00 2001 From: chriss-de Date: Wed, 25 Jan 2023 11:50:27 +0100 Subject: [PATCH 05/15] reversed logic to disable-proxy-intercept-errors --- .../nginx-configuration/annotations.md | 10 +- .../nginx-configuration/configmap.md | 8 +- internal/ingress/annotations/annotations.go | 110 +++++++++--------- .../disableproxyintercepterrors/main.go | 12 +- .../disableproxyintercepterrors/main_test.go | 6 +- internal/ingress/controller/config/config.go | 58 ++++----- internal/ingress/controller/controller.go | 2 +- internal/ingress/defaults/main.go | 2 +- pkg/apis/ingress/types.go | 2 +- pkg/apis/ingress/types_equals.go | 2 +- rootfs/etc/nginx/template/nginx.tmpl | 4 +- .../disableproxyintercepterrors.go | 32 +---- 12 files changed, 112 insertions(+), 136 deletions(-) diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 48765cc8b2..aff55fffca 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -49,7 +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/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| @@ -332,15 +332,15 @@ Example usage: nginx.ingress.kubernetes.io/custom-http-errors: "404,415" ``` -## Proxy intercept Errors +## Disable 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). +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 `proxy-intercept-errors` is also specified globally, the annotation will override the global value for the given ingress' hostname and path. +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/proxy-intercept-errors: "false" +nginx.ingress.kubernetes.io/disable-proxy-intercept-errors: "false" ``` ### Default Backend diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 520e9c8308..5dab8881a4 100755 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -163,7 +163,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"| +|[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| @@ -1038,15 +1038,15 @@ 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) if not disabled with [proxy-intercept-errors](#proxy-intercept-errors). +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` -## proxy-intercept-errors +## disable-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"` +Example usage: `disable-proxy-intercept-errors: "true"` ## proxy-body-size diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index 9b15f6561e..8ec1d4a590 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -19,8 +19,8 @@ package annotations import ( "github.com/imdario/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/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" @@ -77,18 +77,18 @@ 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 - ProxyInterceptErrors bool - DefaultBackend *apiv1.Service + 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 //TODO: Change this back into an error when https://github.com/imdario/mergo/issues/100 is resolved FastCGI fastcgi.Config Denied *string @@ -132,48 +132,48 @@ 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), - "ProxyInterceptErrors": proxyintercepterrors.NewParser(cfg), - "DefaultBackend": defaultbackend.NewParser(cfg), - "FastCGI": fastcgi.NewParser(cfg), - "ExternalAuth": authreq.NewParser(cfg), - "EnableGlobalAuth": authreqglobal.NewParser(cfg), - "HTTP2PushPreload": http2pushpreload.NewParser(cfg), - "Opentracing": opentracing.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), - "SecureUpstream": secureupstream.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), - "Whitelist": ipwhitelist.NewParser(cfg), - "Denylist": ipdenylist.NewParser(cfg), - "XForwardedPrefix": xforwardedprefix.NewParser(cfg), - "SSLCipher": sslcipher.NewParser(cfg), - "Logs": log.NewParser(cfg), - "InfluxDB": influxdb.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), + "Opentracing": opentracing.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), + "SecureUpstream": secureupstream.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), + "Whitelist": ipwhitelist.NewParser(cfg), + "Denylist": ipdenylist.NewParser(cfg), + "XForwardedPrefix": xforwardedprefix.NewParser(cfg), + "SSLCipher": sslcipher.NewParser(cfg), + "Logs": log.NewParser(cfg), + "InfluxDB": influxdb.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 index 881f7c7ef3..b71bb0c2c3 100644 --- a/internal/ingress/annotations/disableproxyintercepterrors/main.go +++ b/internal/ingress/annotations/disableproxyintercepterrors/main.go @@ -11,7 +11,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package proxyintercepterrors +package disableproxyintercepterrors import ( networking "k8s.io/api/networking/v1" @@ -21,17 +21,17 @@ import ( "k8s.io/ingress-nginx/internal/ingress/resolver" ) -type proxyInterceptErrors struct { +type disableProxyInterceptErrors struct { r resolver.Resolver } -// NewParser creates a new proxyintercepterrors annotation parser +// NewParser creates a new disableProxyInterceptErrors annotation parser func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return proxyInterceptErrors{r} + return disableProxyInterceptErrors{r} } -func (pie proxyInterceptErrors) Parse(ing *networking.Ingress) (interface{}, error) { - val, err := parser.GetBoolAnnotation("proxy-intercept-errors", ing) +func (pie disableProxyInterceptErrors) Parse(ing *networking.Ingress) (interface{}, error) { + val, err := parser.GetBoolAnnotation("disable-proxy-intercept-errors", ing) // A missing annotation is not a problem, just use the default if err == errors.ErrMissingAnnotations { diff --git a/internal/ingress/annotations/disableproxyintercepterrors/main_test.go b/internal/ingress/annotations/disableproxyintercepterrors/main_test.go index 1bb548568e..ec708fc223 100644 --- a/internal/ingress/annotations/disableproxyintercepterrors/main_test.go +++ b/internal/ingress/annotations/disableproxyintercepterrors/main_test.go @@ -11,7 +11,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package proxyintercepterrors +package disableproxyintercepterrors import ( "testing" @@ -51,11 +51,11 @@ func TestParseAnnotations(t *testing.T) { } data := map[string]string{} - data[parser.GetAnnotationWithPrefix("proxy-intercept-errors")] = "true" + 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 proxy intercept errors") + 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 a2a3eaa8f7..cfd224bdbc 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -526,7 +526,7 @@ type Configuration struct { // 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"` + 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"` @@ -894,39 +894,39 @@ func NewDefault() Configuration { VariablesHashBucketSize: 256, VariablesHashMaxSize: 2048, UseHTTP2: true, - ProxyInterceptErrors: 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{}, - ProxyInterceptErrors: true, - 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 bae3c47f97..5b063cd21a 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -1479,7 +1479,7 @@ func locationApplyAnnotations(loc *ingress.Location, anns *annotations.Ingress) loc.BackendProtocol = anns.BackendProtocol loc.FastCGI = anns.FastCGI loc.CustomHTTPErrors = anns.CustomHTTPErrors - loc.ProxyInterceptErrors = anns.ProxyInterceptErrors + 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 034e2b4202..a82fc42223 100644 --- a/internal/ingress/defaults/main.go +++ b/internal/ingress/defaults/main.go @@ -39,7 +39,7 @@ type Backend struct { // 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"` + 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 diff --git a/pkg/apis/ingress/types.go b/pkg/apis/ingress/types.go index a9df696120..982b43b0ea 100644 --- a/pkg/apis/ingress/types.go +++ b/pkg/apis/ingress/types.go @@ -353,7 +353,7 @@ type Location struct { // 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"` + 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 fe01311c91..bbfe1b2704 100644 --- a/pkg/apis/ingress/types_equals.go +++ b/pkg/apis/ingress/types_equals.go @@ -472,7 +472,7 @@ func (l1 *Location) Equal(l2 *Location) bool { return false } - if l1.ProxyInterceptErrors != l2.ProxyInterceptErrors { + if l1.DisableProxyInterceptErrors != l2.DisableProxyInterceptErrors { return false } diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 112c7ae041..739748c7ac 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -473,7 +473,7 @@ http { ssl_certificate {{ $cfg.DefaultSSLCertificate.PemFileName }}; ssl_certificate_key {{ $cfg.DefaultSSLCertificate.PemFileName }}; - {{ if and (gt (len $cfg.CustomHTTPErrors) 0) $cfg.ProxyInterceptErrors }} + {{ if and (gt (len $cfg.CustomHTTPErrors) 0) (not $cfg.ProxyInterceptErrors) }} proxy_intercept_errors on; {{ end }} @@ -1457,7 +1457,7 @@ stream { {{ end }} {{/* if a location-specific error override is set, add the proxy_intercept here */}} - {{ if and $location.CustomHTTPErrors $location.ProxyInterceptErrors }} + {{ 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 index b1f1aca946..1ed16136bd 100644 --- a/test/e2e/annotations/disableproxyintercepterrors.go +++ b/test/e2e/annotations/disableproxyintercepterrors.go @@ -22,27 +22,24 @@ import ( "github.com/onsi/ginkgo/v2" "github.com/stretchr/testify/assert" - networking "k8s.io/api/networking/v1" - "k8s.io/ingress-nginx/test/e2e/framework" ) -var _ = framework.DescribeAnnotation("proxy-intercept-errors", func() { - f := framework.NewDefaultFramework("proxy-intercept-errors") +var _ = framework.DescribeAnnotation("disable-proxy-intercept-errors", func() { + f := framework.NewDefaultFramework("disable-proxy-intercept-errors") ginkgo.BeforeEach(func() { f.NewEchoDeployment() }) ginkgo.It("configures Nginx correctly", func() { - pieState := "true" host := "pie.foo.com" errorCodes := []string{"404"} annotations := map[string]string{ - "nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(errorCodes, ","), - "nginx.ingress.kubernetes.io/proxy-intercept-errors": pieState, + "nginx.ingress.kubernetes.io/custom-http-errors": strings.Join(errorCodes, ","), + "nginx.ingress.kubernetes.io/disable-proxy-intercept-errors": "true", } ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations) @@ -55,27 +52,6 @@ var _ = framework.DescribeAnnotation("proxy-intercept-errors", func() { }) ginkgo.By("turning on proxy_intercept_errors directive") - assert.Contains(ginkgo.GinkgoT(), serverConfig, "proxy_intercept_errors on;") - - // -- - pieState = "false" - - ginkgo.By("updating configuration when only proxy-intercept-errors value changes") - err := framework.UpdateIngress(f.KubeClientSet, f.Namespace, host, func(ingress *networking.Ingress) error { - ingress.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/proxy-intercept-errors"] = pieState - return nil - }) - assert.Nil(ginkgo.GinkgoT(), err) - - f.WaitForNginxServer(host, func(sc string) bool { - if serverConfig != sc { - serverConfig = sc - return true - } - return false - }) - - ginkgo.By("turning off proxy_intercept_errors directive") assert.NotContains(ginkgo.GinkgoT(), serverConfig, "proxy_intercept_errors on;") }) From 621f8923ca782c64f1e9a00309d1e872c6ce4f0f Mon Sep 17 00:00:00 2001 From: chriss-de Date: Thu, 26 Jan 2023 16:12:07 +0100 Subject: [PATCH 06/15] reversed logic --- rootfs/etc/nginx/template/nginx.tmpl | 2 +- test/e2e/annotations/disableproxyintercepterrors.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 739748c7ac..388d8f5d0e 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -473,7 +473,7 @@ http { ssl_certificate {{ $cfg.DefaultSSLCertificate.PemFileName }}; ssl_certificate_key {{ $cfg.DefaultSSLCertificate.PemFileName }}; - {{ if and (gt (len $cfg.CustomHTTPErrors) 0) (not $cfg.ProxyInterceptErrors) }} + {{ if and $cfg.CustomHTTPErrors (not $cfg.DisableProxyInterceptErrors) }} proxy_intercept_errors on; {{ end }} diff --git a/test/e2e/annotations/disableproxyintercepterrors.go b/test/e2e/annotations/disableproxyintercepterrors.go index 1ed16136bd..77c89e95e5 100644 --- a/test/e2e/annotations/disableproxyintercepterrors.go +++ b/test/e2e/annotations/disableproxyintercepterrors.go @@ -51,7 +51,7 @@ var _ = framework.DescribeAnnotation("disable-proxy-intercept-errors", func() { return strings.Contains(serverConfig, fmt.Sprintf("server_name %s", host)) }) - ginkgo.By("turning on proxy_intercept_errors directive") + ginkgo.By("turning off proxy_intercept_errors directive") assert.NotContains(ginkgo.GinkgoT(), serverConfig, "proxy_intercept_errors on;") }) From de016868319805bae38a13598c1cd5fb4ef85b82 Mon Sep 17 00:00:00 2001 From: chriss-de Date: Thu, 26 Jan 2023 16:45:48 +0100 Subject: [PATCH 07/15] default has to be false --- .../ingress/annotations/disableproxyintercepterrors/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/ingress/annotations/disableproxyintercepterrors/main.go b/internal/ingress/annotations/disableproxyintercepterrors/main.go index b71bb0c2c3..245b1cb93c 100644 --- a/internal/ingress/annotations/disableproxyintercepterrors/main.go +++ b/internal/ingress/annotations/disableproxyintercepterrors/main.go @@ -35,8 +35,8 @@ func (pie disableProxyInterceptErrors) Parse(ing *networking.Ingress) (interface // A missing annotation is not a problem, just use the default if err == errors.ErrMissingAnnotations { - return true, nil - // default is true and only matters when "custom-http-errors" is also set + return false, nil + // default is false } return val, nil From 4ab1ab232c5033083bfc952b1c0584ec4896cd83 Mon Sep 17 00:00:00 2001 From: chriss-de Date: Wed, 1 Feb 2023 14:51:45 +0100 Subject: [PATCH 08/15] put comment in same line as return --- .../ingress/annotations/disableproxyintercepterrors/main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/ingress/annotations/disableproxyintercepterrors/main.go b/internal/ingress/annotations/disableproxyintercepterrors/main.go index 245b1cb93c..4f15a395be 100644 --- a/internal/ingress/annotations/disableproxyintercepterrors/main.go +++ b/internal/ingress/annotations/disableproxyintercepterrors/main.go @@ -35,8 +35,7 @@ func (pie disableProxyInterceptErrors) Parse(ing *networking.Ingress) (interface // A missing annotation is not a problem, just use the default if err == errors.ErrMissingAnnotations { - return false, nil - // default is false + return false, nil // default is false } return val, nil From 1ea61f1ab270a43a72681134300a33425356cc88 Mon Sep 17 00:00:00 2001 From: chriss-de Date: Thu, 23 Mar 2023 15:16:07 +0100 Subject: [PATCH 09/15] run gofmt --- internal/ingress/annotations/annotations.go | 84 ++++++++++----------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index dcf7b1ae8b..cecadef356 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -134,49 +134,49 @@ 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), + "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), - "Opentracing": opentracing.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), - "SecureUpstream": secureupstream.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), - "Whitelist": ipwhitelist.NewParser(cfg), - "Denylist": ipdenylist.NewParser(cfg), - "XForwardedPrefix": xforwardedprefix.NewParser(cfg), - "SSLCipher": sslcipher.NewParser(cfg), - "Logs": log.NewParser(cfg), - "InfluxDB": influxdb.NewParser(cfg), - "BackendProtocol": backendprotocol.NewParser(cfg), - "ModSecurity": modsecurity.NewParser(cfg), - "Mirror": mirror.NewParser(cfg), - "StreamSnippet": streamsnippet.NewParser(cfg), + "DefaultBackend": defaultbackend.NewParser(cfg), + "FastCGI": fastcgi.NewParser(cfg), + "ExternalAuth": authreq.NewParser(cfg), + "EnableGlobalAuth": authreqglobal.NewParser(cfg), + "HTTP2PushPreload": http2pushpreload.NewParser(cfg), + "Opentracing": opentracing.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), + "SecureUpstream": secureupstream.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), + "Whitelist": ipwhitelist.NewParser(cfg), + "Denylist": ipdenylist.NewParser(cfg), + "XForwardedPrefix": xforwardedprefix.NewParser(cfg), + "SSLCipher": sslcipher.NewParser(cfg), + "Logs": log.NewParser(cfg), + "InfluxDB": influxdb.NewParser(cfg), + "BackendProtocol": backendprotocol.NewParser(cfg), + "ModSecurity": modsecurity.NewParser(cfg), + "Mirror": mirror.NewParser(cfg), + "StreamSnippet": streamsnippet.NewParser(cfg), }, } } From ed67cbdacbb663ab8b7af26764c685ac3c8731fd Mon Sep 17 00:00:00 2001 From: Christoph Hartmann Date: Tue, 18 Apr 2023 10:32:06 +0200 Subject: [PATCH 10/15] fixing wrong Boilerplate header --- .../ingress/annotations/disableproxyintercepterrors/main.go | 5 ++++- .../annotations/disableproxyintercepterrors/main_test.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/ingress/annotations/disableproxyintercepterrors/main.go b/internal/ingress/annotations/disableproxyintercepterrors/main.go index 4f15a395be..818fdf1427 100644 --- a/internal/ingress/annotations/disableproxyintercepterrors/main.go +++ b/internal/ingress/annotations/disableproxyintercepterrors/main.go @@ -1,9 +1,12 @@ /* -Copyright 2017 The Kubernetes Authors. +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. diff --git a/internal/ingress/annotations/disableproxyintercepterrors/main_test.go b/internal/ingress/annotations/disableproxyintercepterrors/main_test.go index ec708fc223..1238bb4e07 100644 --- a/internal/ingress/annotations/disableproxyintercepterrors/main_test.go +++ b/internal/ingress/annotations/disableproxyintercepterrors/main_test.go @@ -1,9 +1,12 @@ /* -Copyright 2017 The Kubernetes Authors. +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. From f05d795650399e9ce1f79f52bbea02c045bb7646 Mon Sep 17 00:00:00 2001 From: Christoph Hartmann Date: Wed, 26 Jul 2023 09:05:06 +0200 Subject: [PATCH 11/15] updated code to new IngressAnnotation interface --- .../disableproxyintercepterrors/main.go | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/internal/ingress/annotations/disableproxyintercepterrors/main.go b/internal/ingress/annotations/disableproxyintercepterrors/main.go index 818fdf1427..ac86110870 100644 --- a/internal/ingress/annotations/disableproxyintercepterrors/main.go +++ b/internal/ingress/annotations/disableproxyintercepterrors/main.go @@ -24,17 +24,48 @@ import ( "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 + 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} + return disableProxyInterceptErrors{ + r: r, + annotationConfig: disableProxyInterceptErrorsAnnotations, + } } func (pie disableProxyInterceptErrors) Parse(ing *networking.Ingress) (interface{}, error) { - val, err := parser.GetBoolAnnotation("disable-proxy-intercept-errors", ing) + val, err := parser.GetBoolAnnotation(disableProxyInterceptErrorsAnnotation, ing, pie.annotationConfig.Annotations) // A missing annotation is not a problem, just use the default if err == errors.ErrMissingAnnotations { From 37ea3d6b22326a1c348dcb2bea913d8e402c1629 Mon Sep 17 00:00:00 2001 From: Christoph Hartmann Date: Mon, 4 Sep 2023 20:22:59 +0200 Subject: [PATCH 12/15] fixes to satisfy PR comments --- docs/user-guide/nginx-configuration/configmap.md | 4 +++- .../ingress/annotations/disableproxyintercepterrors/main.go | 2 +- internal/ingress/defaults/main.go | 4 ++-- test/e2e/annotations/disableproxyintercepterrors.go | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 9b91b6122b..5396468f92 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -1122,7 +1122,9 @@ Example usage: `custom-http-errors: 404,415` ## disable-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. +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"` diff --git a/internal/ingress/annotations/disableproxyintercepterrors/main.go b/internal/ingress/annotations/disableproxyintercepterrors/main.go index ac86110870..650d29707b 100644 --- a/internal/ingress/annotations/disableproxyintercepterrors/main.go +++ b/internal/ingress/annotations/disableproxyintercepterrors/main.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Kubernetes Authors. +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. diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go index 2f3be97fea..c04f7ffb7d 100644 --- a/internal/ingress/defaults/main.go +++ b/internal/ingress/defaults/main.go @@ -34,11 +34,11 @@ 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 + // allows usage of 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 + // By default this is disabled DisableProxyInterceptErrors bool `json:"disable-proxy-intercept-errors"` // http://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size diff --git a/test/e2e/annotations/disableproxyintercepterrors.go b/test/e2e/annotations/disableproxyintercepterrors.go index 77c89e95e5..82598e968f 100644 --- a/test/e2e/annotations/disableproxyintercepterrors.go +++ b/test/e2e/annotations/disableproxyintercepterrors.go @@ -1,5 +1,5 @@ /* -Copyright 2018 The Kubernetes Authors. +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. From 05f600150ed726aa05d87f84e4393ff2d2578fa2 Mon Sep 17 00:00:00 2001 From: Christoph Hartmann Date: Mon, 25 Sep 2023 10:10:41 +0200 Subject: [PATCH 13/15] synced with upstream; fixed typo --- internal/ingress/defaults/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go index c04f7ffb7d..0288977e7f 100644 --- a/internal/ingress/defaults/main.go +++ b/internal/ingress/defaults/main.go @@ -34,7 +34,7 @@ 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 intecepting service errors + // 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 From 476306903af6a267d3402d1871a3ab9feca55b49 Mon Sep 17 00:00:00 2001 From: Christoph Hartmann Date: Fri, 13 Oct 2023 10:44:19 +0200 Subject: [PATCH 14/15] gofumpt disableproxyintercepterrors.go --- test/e2e/annotations/disableproxyintercepterrors.go | 1 - 1 file changed, 1 deletion(-) diff --git a/test/e2e/annotations/disableproxyintercepterrors.go b/test/e2e/annotations/disableproxyintercepterrors.go index 82598e968f..1559b3c99b 100644 --- a/test/e2e/annotations/disableproxyintercepterrors.go +++ b/test/e2e/annotations/disableproxyintercepterrors.go @@ -53,6 +53,5 @@ var _ = framework.DescribeAnnotation("disable-proxy-intercept-errors", func() { ginkgo.By("turning off proxy_intercept_errors directive") assert.NotContains(ginkgo.GinkgoT(), serverConfig, "proxy_intercept_errors on;") - }) }) From 874e89cc972a4ebf615339a681279dd8ce5e2f09 Mon Sep 17 00:00:00 2001 From: Christoph Hartmann Date: Fri, 10 Nov 2023 22:13:35 +0100 Subject: [PATCH 15/15] gofumpt --- test/e2e/annotations/disableproxyintercepterrors.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/annotations/disableproxyintercepterrors.go b/test/e2e/annotations/disableproxyintercepterrors.go index 891903c309..17efa45884 100644 --- a/test/e2e/annotations/disableproxyintercepterrors.go +++ b/test/e2e/annotations/disableproxyintercepterrors.go @@ -18,10 +18,11 @@ package annotations import ( "fmt" - networking "k8s.io/api/networking/v1" "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" @@ -95,6 +96,5 @@ var _ = framework.DescribeAnnotation("disable-proxy-intercept-errors", func() { 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)) - }) })