diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 48765cc8b..aff55fffc 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 520e9c830..5dab8881a 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 9b15f6561..8ec1d4a59 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 881f7c7ef..b71bb0c2c 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 1bb548568..ec708fc22 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 a2a3eaa8f..cfd224bdb 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 bae3c47f9..5b063cd21 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 034e2b420..a82fc4222 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 a9df69612..982b43b0e 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 fe01311c9..bbfe1b270 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 112c7ae04..739748c7a 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 b1f1aca94..1ed16136b 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;") })