Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add override for proxy_intercept_errors when using Custom HTTP Errors #9497

Merged
merged 40 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
cc0872c
added proxy-intercept-errors config option
chriss-de Jan 10, 2023
ae2d6d4
fixed error when comparing locations
chriss-de Jan 11, 2023
0a5708b
fixed missing location config from annotation
chriss-de Jan 11, 2023
e87fd22
Merge branch 'kubernetes:main' into pie
chriss-de Jan 19, 2023
ef17381
Merge branch 'kubernetes:main' into pie
chriss-de Jan 24, 2023
0c86dac
reversed logic for proxy-intercept-errors to disable-proxy-intercept-…
chriss-de Jan 25, 2023
23dac1d
reversed logic to disable-proxy-intercept-errors
chriss-de Jan 25, 2023
621f892
reversed logic
chriss-de Jan 26, 2023
de01686
default has to be false
chriss-de Jan 26, 2023
892a3b6
Merge branch 'kubernetes:main' into pie
chriss-de Jan 26, 2023
d706f1e
synced with upstream
chriss-de Feb 1, 2023
4ab1ab2
put comment in same line as return
chriss-de Feb 1, 2023
4ecc6c4
Merge branch 'kubernetes:main' into pie
chriss-de Feb 6, 2023
bb1175e
synced with upstream
chriss-de Feb 14, 2023
2d134cd
synced with upstream
chriss-de Mar 23, 2023
1ea61f1
run gofmt
chriss-de Mar 23, 2023
b78728f
synced with upstream
chriss-de Apr 17, 2023
ed67cbd
fixing wrong Boilerplate header
chriss-de Apr 18, 2023
59c49c3
Merge branch 'kubernetes:main' into pie
chriss-de Apr 27, 2023
8f066f0
synced with upstream
chriss-de Jun 12, 2023
c252a1e
synced with upstream
chriss-de Jun 14, 2023
35deadb
synced with upstream
chriss-de Jun 19, 2023
a0d988f
Merge branch 'kubernetes:main' into pie
chriss-de Jun 21, 2023
04f52f2
Merge branch 'kubernetes:main' into pie
chriss-de Jun 27, 2023
4c28b9e
Merge branch 'kubernetes:main' into pie
chriss-de Jul 13, 2023
fc4d1f0
synced with upstream
chriss-de Jul 22, 2023
cd5452c
Merge branch 'kubernetes:main' into pie
chriss-de Jul 26, 2023
f05d795
updated code to new IngressAnnotation interface
chriss-de Jul 26, 2023
c18544a
synced with upstream
chriss-de Sep 1, 2023
37ea3d6
fixes to satisfy PR comments
chriss-de Sep 4, 2023
4c36f94
Merge remote-tracking branch 'upstream/main' into pie
chriss-de Sep 25, 2023
05f6001
synced with upstream; fixed typo
chriss-de Sep 25, 2023
ad923aa
Merge branch 'kubernetes:main' into pie
chriss-de Sep 27, 2023
4cfda9e
Merge branch 'kubernetes:main' into pie
chriss-de Oct 4, 2023
452436f
Merge branch 'kubernetes:main' into pie
chriss-de Oct 10, 2023
06c1bba
Merge branch 'kubernetes:main' into pie
chriss-de Oct 11, 2023
d1e34d2
Merge branch 'kubernetes:main' into pie
chriss-de Oct 13, 2023
4763069
gofumpt disableproxyintercepterrors.go
chriss-de Oct 13, 2023
0fd5b19
Merge remote-tracking branch 'upstream/main' into pie
chriss-de Nov 10, 2023
874e89c
gofumpt
chriss-de Nov 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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).
tao12345666333 marked this conversation as resolved.
Show resolved Hide resolved
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
Loading