reversed logic to disable-proxy-intercept-errors

This commit is contained in:
chriss-de 2023-01-25 11:50:27 +01:00
parent 0c86dace95
commit 23dac1d95d
12 changed files with 112 additions and 136 deletions

View file

@ -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

View file

@ -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

View file

@ -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),
},
}
}

View file

@ -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 {

View file

@ -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")
}
}

View file

@ -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",

View file

@ -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

View file

@ -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

View file

@ -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"`

View file

@ -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
}

View file

@ -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 }}

View file

@ -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;")
})