Merge pull request #323 from krancour/feat/proxy_buffer_size

feat(proxyBuffers): Add router-level and app-level proxy buffer config options

README.md

@@ -262,6 +262,10 @@ _Note that Kubernetes annotation maps are all of Go type `map[string]string`.  A
 | <a name="ssl-hsts-max-age"></a>deis-router | deployment | [router.deis.io/nginx.ssl.hsts.maxAge](#ssl-hsts-max-age) | `"10886400"` | Maximum number of seconds user agents should observe HSTS rewrites. |
 | <a name="ssl-hsts-include-sub-domains"></a>deis-router | deployment | [router.deis.io/nginx.ssl.hsts.includeSubDomains](#ssl-hsts-include-sub-domains) | `"false"` | Whether to enforce HSTS for subsequent requests to all subdomains of the original request. |
 | <a name="ssl-hsts-preload"></a>deis-router | deployment | [router.deis.io/nginx.ssl.hsts.preload](#ssl-hsts-preload) | `"false"` | Whether to allow the domain to be included in the HSTS preload list. |
+| <a name="proxy-buffers-enabled"></a>deis-router | deployment | [router.deis.io/nginx.proxyBuffers.enabled](#proxy-buffers-enabled) | `"false"` | Whether to enabled proxy buffering for all applications (this can be overridden on an application basis). |
+| <a name="proxy-buffers-number"></a>deis-router | deployment | [router.deis.io/nginx.proxyBuffers.number](#proxy-buffers-number) | `"8"` | `number` argument to the nginx `proxy_buffers` directive for all applications (this can be overridden on an application basis). |
+| <a name="proxy-buffers-size"></a>deis-router | deployment | [router.deis.io/nginx.proxyBuffers.size](#proxy-buffers-size) | `"4k"` | `size` argument to the nginx `proxy_buffers` directive expressed in bytes (no suffix), kilobytes (suffixes `k` and `K`), or megabytes (suffixes `m` and `M`). This setting applies to all applications, but can be overridden on an application basis. |
+| <a name="proxy-buffers-busy-size"></a>deis-router | deployment | [router.deis.io/nginx.proxyBuffers.busySize](#proxy-buffers-busy-size) | `"8k"` | nginx `proxy_busy_buffers_size` expressed in bytes (no suffix), kilobytes (suffixes `k` and `K`), or megabytes (suffixes `m` and `M`). This setting applies to all applications, but can be overridden on an application basis. |
 | <a name="builder-connect-timeout"></a>deis-builder | service | [router.deis.io/nginx.connectTimeout](#builder-connect-timeout) | `"10s"` | nginx `proxy_connect_timeout` setting expressed in units `ms`, `s`, `m`, `h`, `d`, `w`, `M`, or `y`. |
 | <a name="builder-tcp-timeout"></a>deis-builder | service | [router.deis.io/nginx.tcpTimeout](#builder-tcp-timeout) | `"1200s"` | nginx `proxy_timeout` setting expressed in units `ms`, `s`, `m`, `h`, `d`, `w`, `M`, or `y`. |
 | <a name="app-domains"></a>routable application | service | [router.deis.io/domains](#app-domains) | N/A | Comma-delimited list of domains for which traffic should be routed to the application.  These may be fully qualified (e.g. `foo.example.com`) or, if not containing any `.` character, will be considered subdomains of the router's domain, if that is defined. |
@@ -271,6 +275,10 @@ _Note that Kubernetes annotation maps are all of Go type `map[string]string`.  A
 | <a name="app-tcp-timeout"></a>routable application | service | [router.deis.io/tcpTimeout](#app-tcp-timeout) | router's `defaultTimeout` | nginx `proxy_send_timeout` and `proxy_read_timeout` settings expressed in units `ms`, `s`, `m`, `h`, `d`, `w`, `M`, or `y`. |
 | <a name="app-maintenance"></a>routable application | service | [router.deis.io/maintenance](#app-maintenance) | `"false"` | Whether the app is under maintenance so that all traffic for this app is redirected to a static maintenance page with an error code of `503`. |
 | <a name="ssl-enforce"></a>routable application | service | [router.deis.io/ssl.enforce](#ssl-enforce) | `"false"` | Whether to respond with a 301 for all HTTP requests with a permanent redirect to the HTTPS equivalent address. |
+| <a name="app-nginx-proxy-buffers-enabled"></a>routable application | service | [router.deis.io/nginx.proxyBuffers.enabled](#app-nginx-proxy-buffers-enabled) | `"false"` | Whether to enabled proxy buffering. This can be used to override the same option set globally on the router. |
+| <a name="app-nginx-proxy-buffers-number"></a>routable application | service | [router.deis.io/nginx.proxyBuffers.number](#app-nginx-proxy-buffers-number) | `"8"` | `number` argument to the nginx `proxy_buffers` directive. This can be used to override the same option set globally on the router. |
+| <a name="app-nginx-proxy-buffers-size"></a>routable application | service | [router.deis.io/nginx.proxyBuffers.size](#app-nginx-proxy-buffers-size) | `"4k"` | `size` argument to the nginx `proxy_buffers` directive expressed in bytes (no suffix), kilobytes (suffixes `k` and `K`), or megabytes (suffixes `m` and `M`). This can be used to override the same option set globally on the router. |
+| <a name="app-nginx-proxy-buffers-busy-size"></a>routable application | service | [router.deis.io/nginx.proxyBuffers.busySize](#app-nginx-proxy-buffers-busy-size) | `"8k"` | nginx `proxy_busy_buffers_size` expressed in bytes (no suffix), kilobytes (suffixes `k` and `K`), or megabytes (suffixes `m` and `M`). This can be used to override the same option set globally on the router. |
 
 #### Annotations by example
 

model/model.go

@@ -1,6 +1,8 @@
 package model
 
 import (
+	"bytes"
+	"encoding/gob"
 	"fmt"
 	"log"
 	"strings"
@@ -60,11 +62,16 @@ type RouterConfig struct {
 	AppConfigs               []*AppConfig
 	BuilderConfig            *BuilderConfig
 	PlatformCertificate      *Certificate
-	HTTP2Enabled             bool   `key:"http2Enabled" constraint:"(?i)^(true|false)$"`
-	LogFormat                string `key:"logFormat"`
+	HTTP2Enabled             bool                `key:"http2Enabled" constraint:"(?i)^(true|false)$"`
+	LogFormat                string              `key:"logFormat"`
+	ProxyBuffersConfig       *ProxyBuffersConfig `key:"proxyBuffers"`
 }
 
-func newRouterConfig() *RouterConfig {
+func newRouterConfig() (*RouterConfig, error) {
+	proxyBuffersConfig, err := newProxyBuffersConfig(nil)
+	if err != nil {
+		return nil, err
+	}
 	return &RouterConfig{
 		WorkerProcesses:          "auto",
 		MaxWorkerConnections:     "768",
@@ -87,7 +94,8 @@ func newRouterConfig() *RouterConfig {
 		DefaultServiceIP:         "",
 		HTTP2Enabled:             true,
 		LogFormat:                `[$time_iso8601] - $app_name - $remote_addr - $remote_user - $status - "$request" - $bytes_sent - "$http_referer" - "$http_user_agent" - "$server_name" - $upstream_addr - $http_host - $upstream_response_time - $request_time`,
-	}
+		ProxyBuffersConfig:       proxyBuffersConfig,
+	}, nil
 }
 
 // GzipConfig encapsulates gzip configuration.
@@ -126,17 +134,23 @@ type AppConfig struct {
 	CertMappings   map[string]string `key:"certificates" constraint:"(?i)^((([a-z0-9]+(-*[a-z0-9]+)*)|((\\*\\.)?[a-z0-9]+(-*[a-z0-9]+)*\\.)+[a-z0-9]+(-*[a-z0-9]+)+):([a-z0-9]+(-*[a-z0-9]+)*)(\\s*,\\s*)?)+$"`
 	Certificates   map[string]*Certificate
 	Available      bool
-	Maintenance    bool       `key:"maintenance" constraint:"(?i)^(true|false)$"`
-	SSLConfig      *SSLConfig `key:"ssl"`
+	Maintenance    bool            `key:"maintenance" constraint:"(?i)^(true|false)$"`
+	SSLConfig      *SSLConfig      `key:"ssl"`
+	Nginx          *NginxAppConfig `key:"nginx"`
 }
 
-func newAppConfig(routerConfig *RouterConfig) *AppConfig {
+func newAppConfig(routerConfig *RouterConfig) (*AppConfig, error) {
+	nginxConfig, err := newNginxAppConfig(routerConfig)
+	if err != nil {
+		return nil, err
+	}
 	return &AppConfig{
 		ConnectTimeout: "30s",
 		TCPTimeout:     routerConfig.DefaultTimeout,
 		Certificates:   make(map[string]*Certificate, 0),
 		SSLConfig:      newSSLConfig(),
-	}
+		Nginx:          nginxConfig,
+	}, nil
 }
 
 // BuilderConfig encapsulates the configuration of the deis-builder-- if it's in use.
@@ -215,6 +229,55 @@ func newHSTSConfig() *HSTSConfig {
 	}
 }
 
+// NginxAppConfig is a wrapper for all Nginx-specific app configurations. These
+// options shouldn't be expected to be universally supported by alternative
+// router implementations.
+type NginxAppConfig struct {
+	ProxyBuffersConfig *ProxyBuffersConfig `key:"proxyBuffers"`
+}
+
+func newNginxAppConfig(routerConfig *RouterConfig) (*NginxAppConfig, error) {
+	proxyBuffersConfig, err := newProxyBuffersConfig(routerConfig.ProxyBuffersConfig)
+	if err != nil {
+		return nil, err
+	}
+	return &NginxAppConfig{
+		ProxyBuffersConfig: proxyBuffersConfig,
+	}, nil
+}
+
+// ProxyBuffersConfig represents configuration options having to do with Nginx
+// proxy buffers.
+type ProxyBuffersConfig struct {
+	Enabled  bool   `key:"enabled" constraint:"(?i)^(true|false)$"`
+	Number   int    `key:"number" constraint:"^[1-9]\\d*$"`
+	Size     string `key:"size" constraint:"^[1-9]\\d*[kKmM]?$"`
+	BusySize string `key:"busySize" constraint:"^[1-9]\\d*[kKmM]?$"`
+}
+
+func newProxyBuffersConfig(proxyBuffersConfig *ProxyBuffersConfig) (*ProxyBuffersConfig, error) {
+	if proxyBuffersConfig != nil {
+		var buf bytes.Buffer
+		enc := gob.NewEncoder(&buf)
+		dec := gob.NewDecoder(&buf)
+		err := enc.Encode(proxyBuffersConfig)
+		if err != nil {
+			return nil, err
+		}
+		var copy *ProxyBuffersConfig
+		err = dec.Decode(&copy)
+		if err != nil {
+			return nil, err
+		}
+		return copy, nil
+	}
+	return &ProxyBuffersConfig{
+		Number:   8,
+		Size:     "4k",
+		BusySize: "8k",
+	}, nil
+}
+
 // Build creates a RouterConfig configuration object by querying the k8s API for
 // relevant metadata concerning itself and all routable services.
 func Build(kubeClient *kubernetes.Clientset) (*RouterConfig, error) {
@@ -328,8 +391,11 @@ func build(kubeClient *kubernetes.Clientset, routerDeployment *v1beta1ext.Deploy
 }
 
 func buildRouterConfig(routerDeployment *v1beta1.Deployment, platformCertSecret *v1.Secret, dhParamSecret *v1.Secret) (*RouterConfig, error) {
-	routerConfig := newRouterConfig()
-	err := modeler.MapToModel(routerDeployment.Annotations, "nginx", routerConfig)
+	routerConfig, err := newRouterConfig()
+	if err != nil {
+		return nil, err
+	}
+	err = modeler.MapToModel(routerDeployment.Annotations, "nginx", routerConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -351,7 +417,10 @@ func buildRouterConfig(routerDeployment *v1beta1.Deployment, platformCertSecret
 }
 
 func buildAppConfig(kubeClient *kubernetes.Clientset, service v1.Service, routerConfig *RouterConfig) (*AppConfig, error) {
-	appConfig := newAppConfig(routerConfig)
+	appConfig, err := newAppConfig(routerConfig)
+	if err != nil {
+		return nil, err
+	}
 	appConfig.Name = service.Labels["app"]
 	// If we didn't get the app name from the app label, fall back to inferring the app name from
 	// the service's own name.
@@ -363,7 +432,7 @@ func buildAppConfig(kubeClient *kubernetes.Clientset, service v1.Service, router
 	if appConfig.Name != service.Namespace {
 		appConfig.Name = service.Namespace + "/" + appConfig.Name
 	}
-	err := modeler.MapToModel(service.Annotations, "", appConfig)
+	err = modeler.MapToModel(service.Annotations, "", appConfig)
 	if err != nil {
 		return nil, err
 	}

model/model_test.go

@@ -84,7 +84,10 @@ func TestBuildRouterConfig(t *testing.T) {
 		},
 	}
 
-	expectedConfig := newRouterConfig()
+	expectedConfig, err := newRouterConfig()
+	if err != nil {
+		t.Error(err)
+	}
 	sslConfig := newSSLConfig()
 	hstsConfig := newHSTSConfig()
 	platformCert := newCertificate("foo", "bar")

model/model_validation_test.go

@@ -343,51 +343,111 @@ func TestValidHSTSPreload(t *testing.T) {
 	testValidValues(t, newTestHSTSConfig, "Preload", "preload", []string{"true", "false", "TRUE", "FALSE"})
 }
 
-func testInvalidValues(t *testing.T, builder func() interface{}, fieldName string, key string, badValues []string) {
+func TestInvalidProxyBuffersEnabled(t *testing.T) {
+	testInvalidValues(t, newTestProxyBuffersConfig, "Enabled", "enabled", []string{"0", "-1", "foobar"})
+}
+
+func TestValidProxyBuffersEnabled(t *testing.T) {
+	testValidValues(t, newTestProxyBuffersConfig, "Enabled", "enabled", []string{"true", "false", "TRUE", "FALSE"})
+}
+
+func TestInvalidProxyBuffersNumber(t *testing.T) {
+	testInvalidValues(t, newTestProxyBuffersConfig, "Number", "number", []string{"0", "-1", "foobar"})
+}
+
+func TestValidProxyBuffersNumber(t *testing.T) {
+	testValidValues(t, newTestProxyBuffersConfig, "Number", "number", []string{"1", "2", "10"})
+}
+
+func TestInvalidProxyBuffersSize(t *testing.T) {
+	testInvalidValues(t, newTestProxyBuffersConfig, "Size", "size", []string{"0", "-1", "foobar"})
+}
+
+func TestValidProxyBuffersSize(t *testing.T) {
+	testValidValues(t, newTestProxyBuffersConfig, "Size", "size", []string{"1", "2", "20", "1k", "2k", "10m", "10M"})
+}
+
+func TestInvalidProxyBuffersBusySize(t *testing.T) {
+	testInvalidValues(t, newTestProxyBuffersConfig, "BusySize", "busySize", []string{"0", "-1", "foobar"})
+}
+
+func TestValidProxyBusyBuffersBusySize(t *testing.T) {
+	testValidValues(t, newTestProxyBuffersConfig, "BusySize", "busySize", []string{"1", "2", "20", "1k", "2k", "10m", "10M"})
+}
+
+func testInvalidValues(
+	t *testing.T,
+	builder func() (interface{}, error),
+	fieldName string,
+	key string,
+	badValues []string,
+) {
 	badMap := make(map[string]string, 1)
 	for _, badValue := range badValues {
 		badMap[key] = badValue
-		model := builder()
-		err := testModeler.MapToModel(badMap, "", model)
+		model, err := builder()
+		if err != nil {
+			t.Errorf("Unexpected error: %s", err)
+			t.FailNow()
+		}
+		err = testModeler.MapToModel(badMap, "", model)
 		checkError(t, badValue, err)
 	}
 }
 
-func testValidValues(t *testing.T, builder func() interface{}, fieldName string, key string, goodValues []string) {
+func testValidValues(
+	t *testing.T,
+	builder func() (interface{}, error),
+	fieldName string,
+	key string,
+	goodValues []string,
+) {
 	goodMap := make(map[string]string, 1)
 	for _, goodValue := range goodValues {
 		goodMap[key] = goodValue
-		model := builder()
-		err := testModeler.MapToModel(goodMap, "", model)
+		model, err := builder()
+		if err != nil {
+			t.Errorf("Unexpected error: %s", err)
+			t.FailNow()
+		}
+		err = testModeler.MapToModel(goodMap, "", model)
 		if err != nil {
 			t.Errorf("Using value \"%s\", received an unexpected error: %s", goodValue, err)
 			t.FailNow()
 		}
 	}
 }
 
-func newTestRouterConfig() interface{} {
+func newTestRouterConfig() (interface{}, error) {
 	return newRouterConfig()
 }
 
-func newTestGzipConfig() interface{} {
-	return newGzipConfig()
+func newTestGzipConfig() (interface{}, error) {
+	return newGzipConfig(), nil
+}
+
+func newTestAppConfig() (interface{}, error) {
+	routerConfig, err := newRouterConfig()
+	if err != nil {
+		return nil, err
+	}
+	return newAppConfig(routerConfig)
 }
 
-func newTestAppConfig() interface{} {
-	return newAppConfig(newRouterConfig())
+func newTestBuilderConfig() (interface{}, error) {
+	return newBuilderConfig(), nil
 }
 
-func newTestBuilderConfig() interface{} {
-	return newBuilderConfig()
+func newTestSSLConfig() (interface{}, error) {
+	return newSSLConfig(), nil
 }
 
-func newTestSSLConfig() interface{} {
-	return newSSLConfig()
+func newTestHSTSConfig() (interface{}, error) {
+	return newHSTSConfig(), nil
 }
 
-func newTestHSTSConfig() interface{} {
-	return newHSTSConfig()
+func newTestProxyBuffersConfig() (interface{}, error) {
+	return newProxyBuffersConfig(nil)
 }
 
 func checkError(t *testing.T, value string, err error) {

nginx/config.go

@@ -146,7 +146,10 @@ http {
 		}
 
 		location / {
-			proxy_buffering off;
+			proxy_buffering {{ if $routerConfig.ProxyBuffersConfig.Enabled }}on{{ else }}off{{ end }};
+			proxy_buffer_size {{ $routerConfig.ProxyBuffersConfig.Size }};
+			proxy_buffers {{ $routerConfig.ProxyBuffersConfig.Number }} {{ $routerConfig.ProxyBuffersConfig.Size }};
+			proxy_busy_buffers_size {{ $routerConfig.ProxyBuffersConfig.BusySize }};
 			proxy_set_header Host $host;
 			proxy_set_header X-Forwarded-For $remote_addr;
 			proxy_set_header X-Forwarded-Proto $access_scheme;
@@ -242,7 +245,11 @@ http {
 			add_header X-Correlation-Id $correlation_id always;
 			{{end}}
 
-			{{ if $appConfig.Maintenance }}return 503;{{ else if $appConfig.Available }}proxy_buffering off;
+			{{ if $appConfig.Maintenance }}return 503;{{ else if $appConfig.Available }}
+			proxy_buffering {{ if $appConfig.Nginx.ProxyBuffersConfig.Enabled }}on{{ else }}off{{ end }};
+			proxy_buffer_size {{ $appConfig.Nginx.ProxyBuffersConfig.Size }};
+			proxy_buffers {{ $appConfig.Nginx.ProxyBuffersConfig.Number }} {{ $appConfig.Nginx.ProxyBuffersConfig.Size }};
+			proxy_busy_buffers_size {{ $appConfig.Nginx.ProxyBuffersConfig.BusySize }};
 			proxy_set_header Host $host;
 			proxy_set_header X-Forwarded-For $remote_addr;
 			proxy_set_header X-Forwarded-Proto $access_scheme;