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 support to enable/disable proxy buffering #1998

Merged
merged 2 commits into from
Jan 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions docs/user-guide/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ The following annotations are supported:
|[nginx.ingress.kubernetes.io/upstream-hash-by](#custom-nginx-upstream-hashing)|string|
|[nginx.ingress.kubernetes.io/upstream-vhost](#custom-nginx-upstream-vhost)|string|
|[nginx.ingress.kubernetes.io/whitelist-source-range](#whitelist-source-range)|CIDR|
|[nginx.ingress.kubernetes.io/proxy-buffering](#proxy-buffering)|string|

**Note:** all the values must be a string. In case of booleans or number it must be quoted.

Expand Down Expand Up @@ -406,3 +407,15 @@ To use custom values in an Ingress rule define these annotation:
```yaml
nginx.ingress.kubernetes.io/proxy-body-size: 8m
```

### Proxy buffering

Enable or disable proxy buffering [`proxy_buffering`](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffering).
By default proxy buffering is disabled in the nginx config.

To configure this setting globally for all Ingress rules, the `proxy-buffering` value may be set in the NGINX ConfigMap.
To use custom values in an Ingress rule define these annotation:

```yaml
nginx.ingress.kubernetes.io/proxy-buffering: "on"
```
5 changes: 5 additions & 0 deletions docs/user-guide/configmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ The following table shows a configuration option's name, type, and the default v
|[limit‑rate](#limit-rate)|int|0|
|[limit‑rate‑after](#limit-rate-after)|int|0|
|[http‑redirect‑code](#http-redirect-code)|int|308|
|[proxy‑buffering](#proxy-buffering)|string|"off"|

## add-headers

Expand Down Expand Up @@ -698,3 +699,7 @@ Default code is 308.
Why the default code is 308?

[RFC 7238](https://tools.ietf.org/html/rfc7238) was created to define the 308 (Permanent Redirect) status code that is similar to 301 (Moved Permanently) but it keeps the payload in the redirect. This is important if the we send a redirect in methods like POST.

## proxy-buffering

Enables or disables [buffering of responses from the proxied server](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffering).
12 changes: 11 additions & 1 deletion internal/ingress/annotations/proxy/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type Config struct {
ProxyRedirectFrom string `json:"proxyRedirectFrom"`
ProxyRedirectTo string `json:"proxyRedirectTo"`
RequestBuffering string `json:"requestBuffering"`
ProxyBuffering string `json:"proxyBuffering"`
}

// Equal tests for equality between two Configuration types
Expand Down Expand Up @@ -83,6 +84,9 @@ func (l1 *Config) Equal(l2 *Config) bool {
if l1.ProxyRedirectTo != l2.ProxyRedirectTo {
return false
}
if l1.ProxyBuffering != l2.ProxyBuffering {
return false
}

return true
}
Expand All @@ -99,6 +103,7 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
// ParseAnnotations parses the annotations contained in the ingress
// rule used to configure upstream check parameters
func (a proxy) Parse(ing *extensions.Ingress) (interface{}, error) {

defBackend := a.r.GetDefaultBackend()
ct, err := parser.GetIntAnnotation("proxy-connect-timeout", ing)
if err != nil {
Expand Down Expand Up @@ -160,5 +165,10 @@ func (a proxy) Parse(ing *extensions.Ingress) (interface{}, error) {
prt = defBackend.ProxyRedirectTo
}

return &Config{bs, ct, st, rt, bufs, cd, cp, nu, pp, prf, prt, rb}, nil
pb, err := parser.GetStringAnnotation("proxy-buffering", ing)
if err != nil || pb == "" {
pb = defBackend.ProxyBuffering
}

return &Config{bs, ct, st, rt, bufs, cd, cp, nu, pp, prf, prt, rb, pb}, nil
}
5 changes: 5 additions & 0 deletions internal/ingress/annotations/proxy/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func (m mockBackend) GetDefaultBackend() defaults.Backend {
ProxyNextUpstream: "error",
ProxyPassParams: "nocanon keepalive=On",
ProxyRequestBuffering: "on",
ProxyBuffering: "off",
}
}

Expand All @@ -94,6 +95,7 @@ func TestProxy(t *testing.T) {
data[parser.GetAnnotationWithPrefix("proxy-next-upstream")] = "off"
data[parser.GetAnnotationWithPrefix("proxy-pass-params")] = "smax=5 max=10"
data[parser.GetAnnotationWithPrefix("proxy-request-buffering")] = "off"
data[parser.GetAnnotationWithPrefix("proxy-buffering")] = "on"
ing.SetAnnotations(data)

i, err := NewParser(mockBackend{}).Parse(ing)
Expand Down Expand Up @@ -128,6 +130,9 @@ func TestProxy(t *testing.T) {
if p.RequestBuffering != "off" {
t.Errorf("expected off as request-buffering but returned %v", p.RequestBuffering)
}
if p.ProxyBuffering != "on" {
t.Errorf("expected on as proxy-buffering but returned %v", p.ProxyBuffering)
}
}

func TestProxyWithNoAnnotation(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions internal/ingress/controller/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ func NewDefault() Configuration {
SkipAccessLogURLs: []string{},
LimitRate: 0,
LimitRateAfter: 0,
ProxyBuffering: "off",
},
UpstreamKeepaliveConnections: 32,
LimitConnZoneVariable: defaultLimitConnZoneVariable,
Expand Down
1 change: 1 addition & 0 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,7 @@ func (n *NGINXController) createServers(data []*extensions.Ingress,
NextUpstream: bdef.ProxyNextUpstream,
RequestBuffering: bdef.ProxyRequestBuffering,
ProxyRedirectFrom: bdef.ProxyRedirectFrom,
ProxyBuffering: bdef.ProxyBuffering,
}

// generated on Start() with createDefaultSSLCertificate()
Expand Down
4 changes: 4 additions & 0 deletions internal/ingress/defaults/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,8 @@ type Backend struct {
// Sets the initial amount after which the further transmission of a response to a client will be rate limited.
// http://nginx.org/en/docs/http/ngx_http_core_module.html#limit_rate_after
LimitRateAfter int `json:"limit-rate-after"`

// Enables or disables buffering of responses from the proxied server.
// http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffering
ProxyBuffering string `json:"proxy-buffering"`
}
2 changes: 1 addition & 1 deletion rootfs/etc/nginx/template/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ stream {
proxy_send_timeout {{ $location.Proxy.SendTimeout }}s;
proxy_read_timeout {{ $location.Proxy.ReadTimeout }}s;

proxy_buffering off;
proxy_buffering "{{ $location.Proxy.ProxyBuffering }}";
proxy_buffer_size "{{ $location.Proxy.BufferSize }}";
proxy_buffers 4 "{{ $location.Proxy.BufferSize }}";
proxy_request_buffering "{{ $location.Proxy.RequestBuffering }}";
Expand Down