Merge pull request #4450 from Shopify/proxy-max-temp-file-size

Add nginx proxy_max_temp_file_size configuration option
kubernetes · Aug 15, 2019 · b5fecd0 · b5fecd0
2 parents 2450e7d + d8bd8c5
commit b5fecd0
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 31 deletions.
diff --git a/internal/ingress/annotations/proxy/main.go b/internal/ingress/annotations/proxy/main.go
@@ -25,22 +25,23 @@ import (
 
 // Config returns the proxy timeout to use in the upstream server/s
 type Config struct {
-	BodySize            string `json:"bodySize"`
-	ConnectTimeout      int    `json:"connectTimeout"`
-	SendTimeout         int    `json:"sendTimeout"`
-	ReadTimeout         int    `json:"readTimeout"`
-	BuffersNumber       int    `json:"buffersNumber"`
-	BufferSize          string `json:"bufferSize"`
-	CookieDomain        string `json:"cookieDomain"`
-	CookiePath          string `json:"cookiePath"`
-	NextUpstream        string `json:"nextUpstream"`
-	NextUpstreamTimeout int    `json:"nextUpstreamTimeout"`
-	NextUpstreamTries   int    `json:"nextUpstreamTries"`
-	ProxyRedirectFrom   string `json:"proxyRedirectFrom"`
-	ProxyRedirectTo     string `json:"proxyRedirectTo"`
-	RequestBuffering    string `json:"requestBuffering"`
-	ProxyBuffering      string `json:"proxyBuffering"`
-	ProxyHTTPVersion    string `json:"proxyHTTPVersion"`
+	BodySize             string `json:"bodySize"`
+	ConnectTimeout       int    `json:"connectTimeout"`
+	SendTimeout          int    `json:"sendTimeout"`
+	ReadTimeout          int    `json:"readTimeout"`
+	BuffersNumber        int    `json:"buffersNumber"`
+	BufferSize           string `json:"bufferSize"`
+	CookieDomain         string `json:"cookieDomain"`
+	CookiePath           string `json:"cookiePath"`
+	NextUpstream         string `json:"nextUpstream"`
+	NextUpstreamTimeout  int    `json:"nextUpstreamTimeout"`
+	NextUpstreamTries    int    `json:"nextUpstreamTries"`
+	ProxyRedirectFrom    string `json:"proxyRedirectFrom"`
+	ProxyRedirectTo      string `json:"proxyRedirectTo"`
+	RequestBuffering     string `json:"requestBuffering"`
+	ProxyBuffering       string `json:"proxyBuffering"`
+	ProxyHTTPVersion     string `json:"proxyHTTPVersion"`
+	ProxyMaxTempFileSize string `json:"proxyMaxTempFileSize"`
 }
 
 // Equal tests for equality between two Configuration types
@@ -100,6 +101,10 @@ func (l1 *Config) Equal(l2 *Config) bool {
 		return false
 	}
 
+	if l1.ProxyMaxTempFileSize != l2.ProxyMaxTempFileSize {
+		return false
+	}
+
 	return true
 }
 
@@ -200,5 +205,10 @@ func (a proxy) Parse(ing *networking.Ingress) (interface{}, error) {
 		config.ProxyHTTPVersion = defBackend.ProxyHTTPVersion
 	}
 
+	config.ProxyMaxTempFileSize, err = parser.GetStringAnnotation("proxy-max-temp-file-size", ing)
+	if err != nil {
+		config.ProxyMaxTempFileSize = defBackend.ProxyMaxTempFileSize
+	}
+
 	return config, nil
 }
diff --git a/internal/ingress/annotations/proxy/main_test.go b/internal/ingress/annotations/proxy/main_test.go
@@ -82,6 +82,7 @@ func (m mockBackend) GetDefaultBackend() defaults.Backend {
 		ProxyRequestBuffering:    "on",
 		ProxyBuffering:           "off",
 		ProxyHTTPVersion:         "1.1",
+		ProxyMaxTempFileSize:     "1024m",
 	}
 }
 
@@ -101,6 +102,7 @@ func TestProxy(t *testing.T) {
 	data[parser.GetAnnotationWithPrefix("proxy-request-buffering")] = "off"
 	data[parser.GetAnnotationWithPrefix("proxy-buffering")] = "on"
 	data[parser.GetAnnotationWithPrefix("proxy-http-version")] = "1.0"
+	data[parser.GetAnnotationWithPrefix("proxy-max-temp-file-size")] = "128k"
 	ing.SetAnnotations(data)
 
 	i, err := NewParser(mockBackend{}).Parse(ing)
@@ -147,6 +149,9 @@ func TestProxy(t *testing.T) {
 	if p.ProxyHTTPVersion != "1.0" {
 		t.Errorf("expected 1.0 as proxy-http-version but returned %v", p.ProxyHTTPVersion)
 	}
+	if p.ProxyMaxTempFileSize != "128k" {
+		t.Errorf("expected 128k as proxy-max-temp-file-size but returned %v", p.ProxyMaxTempFileSize)
+	}
 }
 
 func TestProxyWithNoAnnotation(t *testing.T) {
@@ -196,4 +201,7 @@ func TestProxyWithNoAnnotation(t *testing.T) {
 	if p.ProxyHTTPVersion != "1.1" {
 		t.Errorf("expected 1.1 as proxy-http-version but returned %v", p.ProxyHTTPVersion)
 	}
+	if p.ProxyMaxTempFileSize != "1024m" {
+		t.Errorf("expected 1024m as proxy-max-temp-file-size but returned %v", p.ProxyMaxTempFileSize)
+	}
 }
diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go
@@ -728,6 +728,7 @@ func NewDefault() Configuration {
 			LimitRateAfter:           0,
 			ProxyBuffering:           "off",
 			ProxyHTTPVersion:         "1.1",
+			ProxyMaxTempFileSize:     "1024m",
 		},
 		UpstreamKeepaliveConnections: 32,
 		UpstreamKeepaliveTimeout:     60,

diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go
@@ -912,21 +912,22 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
 
 	bdef := n.store.GetDefaultBackend()
 	ngxProxy := proxy.Config{
-		BodySize:            bdef.ProxyBodySize,
-		ConnectTimeout:      bdef.ProxyConnectTimeout,
-		SendTimeout:         bdef.ProxySendTimeout,
-		ReadTimeout:         bdef.ProxyReadTimeout,
-		BuffersNumber:       bdef.ProxyBuffersNumber,
-		BufferSize:          bdef.ProxyBufferSize,
-		CookieDomain:        bdef.ProxyCookieDomain,
-		CookiePath:          bdef.ProxyCookiePath,
-		NextUpstream:        bdef.ProxyNextUpstream,
-		NextUpstreamTimeout: bdef.ProxyNextUpstreamTimeout,
-		NextUpstreamTries:   bdef.ProxyNextUpstreamTries,
-		RequestBuffering:    bdef.ProxyRequestBuffering,
-		ProxyRedirectFrom:   bdef.ProxyRedirectFrom,
-		ProxyBuffering:      bdef.ProxyBuffering,
-		ProxyHTTPVersion:    bdef.ProxyHTTPVersion,
+		BodySize:             bdef.ProxyBodySize,
+		ConnectTimeout:       bdef.ProxyConnectTimeout,
+		SendTimeout:          bdef.ProxySendTimeout,
+		ReadTimeout:          bdef.ProxyReadTimeout,
+		BuffersNumber:        bdef.ProxyBuffersNumber,
+		BufferSize:           bdef.ProxyBufferSize,
+		CookieDomain:         bdef.ProxyCookieDomain,
+		CookiePath:           bdef.ProxyCookiePath,
+		NextUpstream:         bdef.ProxyNextUpstream,
+		NextUpstreamTimeout:  bdef.ProxyNextUpstreamTimeout,
+		NextUpstreamTries:    bdef.ProxyNextUpstreamTries,
+		RequestBuffering:     bdef.ProxyRequestBuffering,
+		ProxyRedirectFrom:    bdef.ProxyRedirectFrom,
+		ProxyBuffering:       bdef.ProxyBuffering,
+		ProxyHTTPVersion:     bdef.ProxyHTTPVersion,
+		ProxyMaxTempFileSize: bdef.ProxyMaxTempFileSize,
 	}
 
 	// initialize default server and root location

diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go
@@ -154,4 +154,8 @@ type Backend struct {
 	// Modifies the HTTP version the proxy uses to interact with the backend.
 	// http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_http_version
 	ProxyHTTPVersion string `json:"proxy-http-version"`
+
+	// Sets the maximum temp file size when proxy-buffers capacity is exceeded.
+	// http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_max_temp_file_size
+	ProxyMaxTempFileSize string `json:"proxy-max-temp-file-size"`
 }
diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl
@@ -1201,6 +1201,9 @@ stream {
             proxy_buffering                         {{ $location.Proxy.ProxyBuffering }};
             proxy_buffer_size                       {{ $location.Proxy.BufferSize }};
             proxy_buffers                           {{ $location.Proxy.BuffersNumber }} {{ $location.Proxy.BufferSize }};
+            {{ if isValidByteSize $location.Proxy.ProxyMaxTempFileSize true }}
+            proxy_max_temp_file_size                {{ $location.Proxy.ProxyMaxTempFileSize }};
+            {{ end }}
             proxy_request_buffering                 {{ $location.Proxy.RequestBuffering }};
             proxy_http_version                      {{ $location.Proxy.ProxyHTTPVersion }};