Config/Annotations: Remove proxy-busy-buffers-size default value.
#13780
+7
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
triage/accepted
✅ Deploy Preview for kubernetes-ingress-nginx canceled.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Gacko The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
|
Here's a diff that combines the changes of #12433, #13610 and this PR: diff --git a/docs/user-guide/nginx-configuration/annotations-risk.md b/docs/user-guide/nginx-configuration/annotations-risk.md
index be24c09309..aff9357b88 100755
--- a/docs/user-guide/nginx-configuration/annotations-risk.md
+++ b/docs/user-guide/nginx-configuration/annotations-risk.md
@@ -73,6 +73,7 @@
| Proxy | proxy-buffer-size | Low | location |
| Proxy | proxy-buffering | Low | location |
| Proxy | proxy-buffers-number | Low | location |
+| Proxy | proxy-busy-buffers-size | Low | location |
| Proxy | proxy-connect-timeout | Low | location |
| Proxy | proxy-cookie-domain | Medium | location |
| Proxy | proxy-cookie-path | Medium | location |
diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md
index 8de8afe5ba..7d15cd8210 100755
--- a/docs/user-guide/nginx-configuration/annotations.md
+++ b/docs/user-guide/nginx-configuration/annotations.md
@@ -116,6 +116,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
|[nginx.ingress.kubernetes.io/proxy-buffering](#proxy-buffering)|string|
|[nginx.ingress.kubernetes.io/proxy-buffers-number](#proxy-buffers-number)|number|
|[nginx.ingress.kubernetes.io/proxy-buffer-size](#proxy-buffer-size)|string|
+|[nginx.ingress.kubernetes.io/proxy-busy-buffers-size](#proxy-busy-buffers-size)|string|
|[nginx.ingress.kubernetes.io/proxy-max-temp-file-size](#proxy-max-temp-file-size)|string|
|[nginx.ingress.kubernetes.io/ssl-ciphers](#ssl-ciphers)|string|
|[nginx.ingress.kubernetes.io/ssl-prefer-server-ciphers](#ssl-ciphers)|"true" or "false"|
@@ -728,7 +729,7 @@ To use custom values in an Ingress rule define these annotation:
nginx.ingress.kubernetes.io/proxy-buffering: "on"
-### Proxy buffers Number
+### Proxy buffers number
Sets the number of the buffers in [`proxy_buffers`](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffers) used for reading the first part of the response received from the proxied server.
By default proxy buffers number is set as 4
@@ -748,6 +749,16 @@ To configure this setting globally, set `proxy-buffer-size` in [NGINX ConfigMap]
nginx.ingress.kubernetes.io/proxy-buffer-size: "8k"
+### Proxy busy buffers size
+
+[Limits the total size of buffers that can be busy](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_busy_buffers_size) sending a response to the client while the response is not yet fully read.
+By default, size is limited by the size of two buffers set by the `proxy_buffer_size` and `proxy_buffers` directives.
+
+To configure this setting globally, set `proxy-busy-buffers-size` in the [ConfigMap](./configmap.md#proxy-busy-buffers-size). To use custom values in an Ingress rule, define this annotation:
+```yaml
+nginx.ingress.kubernetes.io/proxy-busy-buffers-size: "16k"
+```
+
### Proxy max temp file size
When [`buffering`](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffering) of responses from the proxied server is enabled, and the whole response does not fit into the buffers set by the [`proxy_buffer_size`](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffer_size) and [`proxy_buffers`](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffers) directives, a part of the response can be saved to a temporary file. This directive sets the maximum `size` of the temporary file setting the [`proxy_max_temp_file_size`](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_max_temp_file_size). The size of data written to the temporary file at a time is set by the [`proxy_temp_file_write_size`](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_temp_file_write_size) directive.
diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md
index 1f4e764ed6..1a26d95962 100644
--- a/docs/user-guide/nginx-configuration/configmap.md
+++ b/docs/user-guide/nginx-configuration/configmap.md
@@ -180,6 +180,7 @@ The following table shows a configuration option's name, type, and the default v
| [proxy-send-timeout](#proxy-send-timeout) | int | 60 | |
| [proxy-buffers-number](#proxy-buffers-number) | int | 4 | |
| [proxy-buffer-size](#proxy-buffer-size) | string | "4k" | |
+| [proxy-busy-buffers-size](#proxy-busy-buffers-size) | string | "" | |
| [proxy-cookie-path](#proxy-cookie-path) | string | "off" | |
| [proxy-cookie-domain](#proxy-cookie-domain) | string | "off" | |
| [proxy-next-upstream](#proxy-next-upstream) | string | "error timeout" | |
@@ -1119,6 +1120,10 @@ Sets the number of the buffer used for [reading the first part of the response](
Sets the size of the buffer used for [reading the first part of the response](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffer_size) received from the proxied server. This part usually contains a small response header.
+## proxy-busy-buffers-size
+
+[Limits the total size of buffers that can be busy](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_busy_buffers_size) sending a response to the client while the response is not yet fully read.
+
## proxy-cookie-path
Sets a text that [should be changed in the path attribute](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cookie_path) of the “Set-Cookie” header fields of a proxied server response.
diff --git a/internal/ingress/annotations/proxy/main.go b/internal/ingress/annotations/proxy/main.go
index 9d26462616..d3b496883e 100644
--- a/internal/ingress/annotations/proxy/main.go
+++ b/internal/ingress/annotations/proxy/main.go
@@ -31,6 +31,7 @@ const (
proxyReadTimeoutAnnotation = "proxy-read-timeout"
proxyBuffersNumberAnnotation = "proxy-buffers-number"
proxyBufferSizeAnnotation = "proxy-buffer-size"
+ proxyBusyBuffersSizeAnnotation = "proxy-busy-buffers-size"
proxyCookiePathAnnotation = "proxy-cookie-path"
proxyCookieDomainAnnotation = "proxy-cookie-domain"
proxyBodySizeAnnotation = "proxy-body-size"
@@ -82,6 +83,12 @@ var proxyAnnotations = parser.Annotation{
Documentation: `This annotation sets the size of the buffer proxy_buffer_size used for reading the first part of the response received from the proxied server.
By default proxy buffer size is set as "4k".`,
},
+ proxyBusyBuffersSizeAnnotation: {
+ Validator: parser.ValidateRegex(parser.SizeRegex, true),
+ Scope: parser.AnnotationScopeLocation,
+ Risk: parser.AnnotationRiskLow,
+ Documentation: `This annotation limits the total size of buffers that can be busy sending a response to the client while the response is not yet fully read.`,
+ },
proxyCookiePathAnnotation: {
Validator: parser.ValidateRegex(parser.URLIsValidRegex, true),
Scope: parser.AnnotationScopeLocation,
@@ -167,6 +174,7 @@ type Config struct {
ReadTimeout int `json:"readTimeout"`
BuffersNumber int `json:"buffersNumber"`
BufferSize string `json:"bufferSize"`
+ BusyBuffersSize string `json:"busyBuffersSize"`
CookieDomain string `json:"cookieDomain"`
CookiePath string `json:"cookiePath"`
NextUpstream string `json:"nextUpstream"`
@@ -206,6 +214,9 @@ func (l1 *Config) Equal(l2 *Config) bool {
if l1.BufferSize != l2.BufferSize {
return false
}
+ if l1.BusyBuffersSize != l2.BusyBuffersSize {
+ return false
+ }
if l1.CookieDomain != l2.CookieDomain {
return false
}
@@ -290,6 +301,11 @@ func (a proxy) Parse(ing *networking.Ingress) (interface{}, error) {
config.BufferSize = defBackend.ProxyBufferSize
}
+ config.BusyBuffersSize, err = parser.GetStringAnnotation(proxyBusyBuffersSizeAnnotation, ing, a.annotationConfig.Annotations)
+ if err != nil {
+ config.BusyBuffersSize = defBackend.ProxyBusyBuffersSize
+ }
+
config.CookiePath, err = parser.GetStringAnnotation(proxyCookiePathAnnotation, ing, a.annotationConfig.Annotations)
if err != nil {
config.CookiePath = defBackend.ProxyCookiePath
diff --git a/internal/ingress/annotations/proxy/main_test.go b/internal/ingress/annotations/proxy/main_test.go
index 9446ae9709..b666431cdb 100644
--- a/internal/ingress/annotations/proxy/main_test.go
+++ b/internal/ingress/annotations/proxy/main_test.go
@@ -88,6 +88,7 @@ func (m mockBackend) GetDefaultBackend() defaults.Backend {
ProxyReadTimeout: 20,
ProxyBuffersNumber: 4,
ProxyBufferSize: "10k",
+ ProxyBusyBuffersSize: "",
ProxyBodySize: "3k",
ProxyNextUpstream: "error",
ProxyNextUpstreamTimeout: 0,
@@ -108,6 +109,7 @@ func TestProxy(t *testing.T) {
data[parser.GetAnnotationWithPrefix("proxy-read-timeout")] = "3"
data[parser.GetAnnotationWithPrefix("proxy-buffers-number")] = "8"
data[parser.GetAnnotationWithPrefix("proxy-buffer-size")] = "1k"
+ data[parser.GetAnnotationWithPrefix("proxy-busy-buffers-size")] = "4k"
data[parser.GetAnnotationWithPrefix("proxy-body-size")] = "2k"
data[parser.GetAnnotationWithPrefix("proxy-next-upstream")] = off
data[parser.GetAnnotationWithPrefix("proxy-next-upstream-timeout")] = "5"
@@ -141,6 +143,9 @@ func TestProxy(t *testing.T) {
if p.BufferSize != "1k" {
t.Errorf("expected 1k as buffer-size but returned %v", p.BufferSize)
}
+ if p.BusyBuffersSize != "4k" {
+ t.Errorf("expected 4k as busy-buffers-size but returned %v", p.BusyBuffersSize)
+ }
if p.BodySize != "2k" {
t.Errorf("expected 2k as body-size but returned %v", p.BodySize)
}
@@ -176,6 +181,7 @@ func TestProxyComplex(t *testing.T) {
data[parser.GetAnnotationWithPrefix("proxy-read-timeout")] = "3"
data[parser.GetAnnotationWithPrefix("proxy-buffers-number")] = "8"
data[parser.GetAnnotationWithPrefix("proxy-buffer-size")] = "1k"
+ data[parser.GetAnnotationWithPrefix("proxy-busy-buffers-size")] = "4k"
data[parser.GetAnnotationWithPrefix("proxy-body-size")] = "2k"
data[parser.GetAnnotationWithPrefix("proxy-next-upstream")] = "error http_502"
data[parser.GetAnnotationWithPrefix("proxy-next-upstream-timeout")] = "5"
@@ -209,6 +215,9 @@ func TestProxyComplex(t *testing.T) {
if p.BufferSize != "1k" {
t.Errorf("expected 1k as buffer-size but returned %v", p.BufferSize)
}
+ if p.BusyBuffersSize != "4k" {
+ t.Errorf("expected 4k as buffer-size but returned %v", p.BusyBuffersSize)
+ }
if p.BodySize != "2k" {
t.Errorf("expected 2k as body-size but returned %v", p.BodySize)
}
@@ -249,6 +258,9 @@ func TestProxyWithNoAnnotation(t *testing.T) {
if !ok {
t.Fatalf("expected a Config type")
}
+ if p.BusyBuffersSize != "" {
+ t.Errorf("expected empty BusyBuffersSize but returned %v", p.BusyBuffersSize)
+ }
if p.ConnectTimeout != 10 {
t.Errorf("expected 10 as connect-timeout but returned %v", p.ConnectTimeout)
}
@@ -286,3 +298,21 @@ func TestProxyWithNoAnnotation(t *testing.T) {
t.Errorf("expected 1024m as proxy-max-temp-file-size but returned %v", p.ProxyMaxTempFileSize)
}
}
+
+func TestProxyWithBusyBuffersSizeAnnotation(t *testing.T) {
+ ing := buildIngress()
+ data := map[string]string{}
+ data[parser.GetAnnotationWithPrefix("proxy-busy-buffers-size")] = "4k"
+ ing.SetAnnotations(data)
+ i, err := NewParser(mockBackend{}).Parse(ing)
+ if err != nil {
+ t.Fatalf("unexpected error parsing a valid")
+ }
+ p, ok := i.(*Config)
+ if !ok {
+ t.Fatalf("expected a Config type")
+ }
+ if p.BusyBuffersSize != "4k" {
+ t.Errorf("expected 4k as BusyBuffersSize but returned %v", p.BusyBuffersSize)
+ }
+}
diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go
index 5bdcef669d..bf7025027a 100644
--- a/internal/ingress/controller/config/config.go
+++ b/internal/ingress/controller/config/config.go
@@ -860,6 +860,7 @@ func NewDefault() Configuration {
ProxySendTimeout: 60,
ProxyBuffersNumber: 4,
ProxyBufferSize: "4k",
+ ProxyBusyBuffersSize: "",
ProxyCookieDomain: "off",
ProxyCookiePath: "off",
ProxyNextUpstream: "error timeout",
diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go
index c6230f00ce..0894c0dfc1 100644
--- a/internal/ingress/controller/controller.go
+++ b/internal/ingress/controller/controller.go
@@ -1266,6 +1266,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
ReadTimeout: bdef.ProxyReadTimeout,
BuffersNumber: bdef.ProxyBuffersNumber,
BufferSize: bdef.ProxyBufferSize,
+ BusyBuffersSize: bdef.ProxyBusyBuffersSize,
CookieDomain: bdef.ProxyCookieDomain,
CookiePath: bdef.ProxyCookiePath,
NextUpstream: bdef.ProxyNextUpstream,
diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go
index 332f8d83f4..8aa3fb3171 100644
--- a/internal/ingress/defaults/main.go
+++ b/internal/ingress/defaults/main.go
@@ -69,6 +69,11 @@ type Backend struct {
// https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffer_size
ProxyBufferSize string `json:"proxy-buffer-size"`
+ // Limits the total size of buffers that can be busy sending a response to the client while
+ // the response is not yet fully read.
+ // http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_busy_buffers_size
+ ProxyBusyBuffersSize string `json:"proxy-busy-buffers-size"`
+
// Sets a text that should be changed in the path attribute of the “Set-Cookie” header fields of
// a proxied server response.
// https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cookie_path
diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl
index 85a9b2655c..c618bd2d4f 100644
--- a/rootfs/etc/nginx/template/nginx.tmpl
+++ b/rootfs/etc/nginx/template/nginx.tmpl
@@ -1076,6 +1076,9 @@ stream {
{{ end }}
proxy_buffer_size {{ $location.Proxy.BufferSize }};
proxy_buffers {{ $location.Proxy.BuffersNumber }} {{ $location.Proxy.BufferSize }};
+ {{ if $location.Proxy.BusyBuffersSize }}
+ proxy_busy_buffers_size {{ $location.Proxy.BusyBuffersSize }};
+ {{ end }}
proxy_request_buffering {{ $location.Proxy.RequestBuffering }};
proxy_ssl_server_name on;
@@ -1333,6 +1336,9 @@ stream {
proxy_buffering {{ $location.Proxy.ProxyBuffering }};
proxy_buffer_size {{ $location.Proxy.BufferSize }};
proxy_buffers {{ $location.Proxy.BuffersNumber }} {{ $location.Proxy.BufferSize }};
+ {{ if $location.Proxy.BusyBuffersSize }}
+ proxy_busy_buffers_size {{ $location.Proxy.BusyBuffersSize }};
+ {{ end }}
{{ if isValidByteSize $location.Proxy.ProxyMaxTempFileSize true }}
proxy_max_temp_file_size {{ $location.Proxy.ProxyMaxTempFileSize }};
{{ end }}
diff --git a/test/e2e/annotations/proxy.go b/test/e2e/annotations/proxy.go
index 235b828e7a..8e98660217 100644
--- a/test/e2e/annotations/proxy.go
+++ b/test/e2e/annotations/proxy.go
@@ -160,11 +160,13 @@ var _ = framework.DescribeAnnotation("proxy-*", func() {
proxyBuffering := "on"
proxyBuffersNumber := "8"
proxyBufferSize := "8k"
+ proxyBusyBuffersSize := "16k"
annotations := make(map[string]string)
annotations["nginx.ingress.kubernetes.io/proxy-buffering"] = proxyBuffering
annotations["nginx.ingress.kubernetes.io/proxy-buffers-number"] = proxyBuffersNumber
annotations["nginx.ingress.kubernetes.io/proxy-buffer-size"] = proxyBufferSize
+ annotations["nginx.ingress.kubernetes.io/proxy-busy-buffers-size"] = proxyBusyBuffersSize
ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
f.EnsureIngress(ing)
@@ -174,6 +176,7 @@ var _ = framework.DescribeAnnotation("proxy-*", func() {
return strings.Contains(server, fmt.Sprintf("proxy_buffering %s;", proxyBuffering)) &&
strings.Contains(server, fmt.Sprintf("proxy_buffer_size %s;", proxyBufferSize)) &&
strings.Contains(server, fmt.Sprintf("proxy_buffers %s %s;", proxyBuffersNumber, proxyBufferSize)) &&
+ strings.Contains(server, fmt.Sprintf("proxy_busy_buffers_size %s;", proxyBusyBuffersSize)) &&
strings.Contains(server, fmt.Sprintf("proxy_request_buffering %s;", proxyBuffering))
})
}) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This PR hopefully ultimately fixes #13598.
/triage accepted
/kind bug
/priority important-soon
Types of changes
Which issue/s this PR fixes
Fixes #13598.
How Has This Been Tested?
Checklist: