Skip to content

Commit

Permalink
Add override for proxy_intercept_errors when using Custom HTTP Errors (
Browse files Browse the repository at this point in the history
…#9497)

* added proxy-intercept-errors config option

* fixed error when comparing locations

* fixed missing location config from annotation
added e2e test

* reversed logic for proxy-intercept-errors to disable-proxy-intercept-errors

* reversed logic to disable-proxy-intercept-errors

* reversed logic

* default has to be false

* put comment in same line as return

* run gofmt

* fixing wrong Boilerplate header

* updated code to new IngressAnnotation interface

* fixes to satisfy PR comments

* synced with upstream; fixed typo

* gofumpt disableproxyintercepterrors.go

* gofumpt
  • Loading branch information
chriss-de authored Nov 17, 2023
1 parent e0446d7 commit ad406b6
Show file tree
Hide file tree
Showing 12 changed files with 395 additions and 108 deletions.
12 changes: 12 additions & 0 deletions docs/user-guide/nginx-configuration/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,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/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|
Expand Down Expand Up @@ -330,6 +331,17 @@ Example usage:
nginx.ingress.kubernetes.io/custom-http-errors: "404,415"
```

## Disable Proxy intercept Errors

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 `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/disable-proxy-intercept-errors: "false"
```

### Default Backend

This annotation is of the form `nginx.ingress.kubernetes.io/default-backend: <svc name>` to specify a custom default backend. This `<svc name>` 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.
Expand Down
11 changes: 10 additions & 1 deletion docs/user-guide/nginx-configuration/configmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,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{}||
|[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||
Expand Down Expand Up @@ -1128,10 +1129,18 @@ 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 [disable-proxy-intercept-errors](#disable-proxy-intercept-errors).

Example usage: `custom-http-errors: 404,415`

## disable-proxy-intercept-errors

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

## proxy-body-size

Sets the maximum allowed size of the client request body.
Expand Down
161 changes: 82 additions & 79 deletions internal/ingress/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"dario.cat/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/opentelemetry"
"k8s.io/ingress-nginx/internal/ingress/annotations/proxyssl"
Expand Down Expand Up @@ -75,46 +76,47 @@ 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
DefaultBackend *apiv1.Service
FastCGI fastcgi.Config
Denied *string
ExternalAuth authreq.Config
EnableGlobalAuth bool
HTTP2PushPreload bool
Opentelemetry opentelemetry.Config
Proxy proxy.Config
ProxySSL proxyssl.Config
RateLimit ratelimit.Config
GlobalRateLimit globalratelimit.Config
Redirect redirect.Config
Rewrite rewrite.Config
Satisfy string
ServerSnippet string
ServiceUpstream bool
SessionAffinity sessionaffinity.Config
SSLPassthrough bool
UsePortInRedirects bool
UpstreamHashBy upstreamhashby.Config
LoadBalancing string
UpstreamVhost string
Denylist ipdenylist.SourceRange
XForwardedPrefix string
SSLCipher sslcipher.Config
Logs log.Config
ModSecurity modsecurity.Config
Mirror mirror.Config
StreamSnippet string
Allowlist ipallowlist.SourceRange
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
FastCGI fastcgi.Config
Denied *string
ExternalAuth authreq.Config
EnableGlobalAuth bool
HTTP2PushPreload bool
Opentelemetry opentelemetry.Config
Proxy proxy.Config
ProxySSL proxyssl.Config
RateLimit ratelimit.Config
GlobalRateLimit globalratelimit.Config
Redirect redirect.Config
Rewrite rewrite.Config
Satisfy string
ServerSnippet string
ServiceUpstream bool
SessionAffinity sessionaffinity.Config
SSLPassthrough bool
UsePortInRedirects bool
UpstreamHashBy upstreamhashby.Config
LoadBalancing string
UpstreamVhost string
Denylist ipdenylist.SourceRange
XForwardedPrefix string
SSLCipher sslcipher.Config
Logs log.Config
ModSecurity modsecurity.Config
Mirror mirror.Config
StreamSnippet string
Allowlist ipallowlist.SourceRange
}

// Extractor defines the annotation parsers to be used in the extraction of annotations
Expand All @@ -126,45 +128,46 @@ 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),
"DefaultBackend": defaultbackend.NewParser(cfg),
"FastCGI": fastcgi.NewParser(cfg),
"ExternalAuth": authreq.NewParser(cfg),
"EnableGlobalAuth": authreqglobal.NewParser(cfg),
"HTTP2PushPreload": http2pushpreload.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),
"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),
"Allowlist": ipallowlist.NewParser(cfg),
"Denylist": ipdenylist.NewParser(cfg),
"XForwardedPrefix": xforwardedprefix.NewParser(cfg),
"SSLCipher": sslcipher.NewParser(cfg),
"Logs": log.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),
"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),
"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),
"Allowlist": ipallowlist.NewParser(cfg),
"Denylist": ipdenylist.NewParser(cfg),
"XForwardedPrefix": xforwardedprefix.NewParser(cfg),
"SSLCipher": sslcipher.NewParser(cfg),
"Logs": log.NewParser(cfg),
"BackendProtocol": backendprotocol.NewParser(cfg),
"ModSecurity": modsecurity.NewParser(cfg),
"Mirror": mirror.NewParser(cfg),
"StreamSnippet": streamsnippet.NewParser(cfg),
},
}
}
Expand Down
76 changes: 76 additions & 0 deletions internal/ingress/annotations/disableproxyintercepterrors/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
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.
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 disableproxyintercepterrors

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

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
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: r,
annotationConfig: disableProxyInterceptErrorsAnnotations,
}
}

func (pie disableProxyInterceptErrors) Parse(ing *networking.Ingress) (interface{}, error) {
val, err := parser.GetBoolAnnotation(disableProxyInterceptErrorsAnnotation, ing, pie.annotationConfig.Annotations)

// A missing annotation is not a problem, just use the default
if err == errors.ErrMissingAnnotations {
return false, nil // default is false
}

return val, nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
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.
See the License for the specific language governing permissions and
limitations under the License.
*/

package disableproxyintercepterrors

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("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 disable-proxy-intercept-errors")
}
}
Loading

0 comments on commit ad406b6

Please sign in to comment.