reversed logic to disable-proxy-intercept-errors

kubernetes · Jan 25, 2023 · 23dac1d · 23dac1d
1 parent 0c86dac
commit 23dac1d
Show file tree

Hide file tree

Showing 12 changed files with 112 additions and 136 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 }}