Configure buffering for vs/vsr

Author: ampant

docs/virtualserver-and-virtualserverroute.md

@@ -18,6 +18,7 @@ This document is the reference documentation for the resources. To see additiona
     - [VirtualServerRoute.Subroute](#virtualserverroutesubroute)
   - [Common Parts of the VirtualServer and VirtualServerRoute](#common-parts-of-the-virtualserver-and-virtualserverroute)
     - [Upstream](#upstream)
+    - [Upstream.Buffers](#upstreambuffers)
     - [Upstream.TLS](#upstreamtls)
     - [Upstream.Healthcheck](#upstreamhealthcheck)
     - [Header](#header)
@@ -218,6 +219,23 @@ tls:
 | `tls` | The TLS configuration for the Upstream. | [`tls`](#UpstreamTLS) | No |
 | `healthCheck` | The health check configuration for the Upstream. See the [health_check](http://nginx.org/en/docs/http/ngx_http_upstream_hc_module.html#health_check) directive. Note: this feature is supported only in NGINX Plus. | [`healthcheck`](#UpstreamHealthcheck) | No |
 | `slow-start` | The slow start allows an upstream server to gradually recover its weight from 0 to its nominal value after it has been recovered or became available or when the server becomes available after a period of time it was considered unavailable. By default, the slow start is disabled. See the [slow_start](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#slow_start) parameter of the server directive. Note: The parameter cannot be used along with the `random`, `hash` or `ip_hash` load balancing methods and will be ignored. | `string` | No |
+| `buffering` | Enables buffering of responses from the upstream server. See the [proxy_buffering](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffering) directive. The default is set in the `proxy-buffering` ConfigMap key. | `boolean` | No |
+| `buffers` | Configures the buffers used for reading a response from the upstream server for a single connection. | [`buffers`](#UpstreamBuffers) | No |
+| `buffer-size` | Sets the size of the buffer used for reading the first part of a response received from the upstream server. See the [proxy_buffer_size](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffer_size) directive. The default is set in the `proxy-buffer-size` ConfigMap key. | `string` | No |
+
+### Upstream.Buffers
+The buffers field configures the buffers used for reading a response from the upstream server for a single connection:
+
+```yaml
+number: 4
+size: 8K   
+```
+See the [proxy_buffers](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffers) directive for additional information.
+
+| Field | Description | Type | Required |
+| ----- | ----------- | ---- | -------- |
+| `number` | Configures the number of buffers. The default is set in the `proxy-buffers` ConfigMap key. | `int` | Yes |
+| `size` | Configures the size of a buffer. The default is set in the `proxy-buffers` ConfigMap key. | `string` | Yes |
 
 ### Upstream.TLS
 

internal/configs/virtualserver.go

@@ -428,6 +428,20 @@ func generateString(s string, defaultS string) string {
 	return s
 }
 
+func generateBuffers(s *conf_v1alpha1.UpstreamBuffers, defaultS string) string {
+	if s == nil {
+		return defaultS
+	}
+	return fmt.Sprintf("%v %v", s.Number, s.Size)
+}
+
+func generateBool(s *bool, defaultS bool) bool {
+	if s != nil {
+		return *s
+	}
+	return defaultS
+}
+
 func generateLocation(path string, upstreamName string, upstream conf_v1alpha1.Upstream, cfgParams *ConfigParams) version2.Location {
 	return version2.Location{
 		Path:                     path,
@@ -437,9 +451,9 @@ func generateLocation(path string, upstreamName string, upstream conf_v1alpha1.U
 		ProxySendTimeout:         generateString(upstream.ProxySendTimeout, cfgParams.ProxySendTimeout),
 		ClientMaxBodySize:        generateString(upstream.ClientMaxBodySize, cfgParams.ClientMaxBodySize),
 		ProxyMaxTempFileSize:     cfgParams.ProxyMaxTempFileSize,
-		ProxyBuffering:           cfgParams.ProxyBuffering,
-		ProxyBuffers:             cfgParams.ProxyBuffers,
-		ProxyBufferSize:          cfgParams.ProxyBufferSize,
+		ProxyBuffering:           generateBool(upstream.ProxyBuffering, cfgParams.ProxyBuffering),
+		ProxyBuffers:             generateBuffers(upstream.ProxyBuffers, cfgParams.ProxyBuffers),
+		ProxyBufferSize:          generateString(upstream.ProxyBufferSize, cfgParams.ProxyBufferSize),
 		ProxyPass:                fmt.Sprintf("%v://%v", generateProxyPassProtocol(upstream.TLS.Enable), upstreamName),
 		ProxyNextUpstream:        generateString(upstream.ProxyNextUpstream, "error timeout"),
 		ProxyNextUpstreamTimeout: generateString(upstream.ProxyNextUpstreamTimeout, "0s"),

internal/configs/virtualserver_test.go

@@ -940,6 +940,29 @@ func TestGenerateString(t *testing.T) {
 	}
 }
 
+func TestGenerateBuffer(t *testing.T) {
+	tests := []struct {
+		inputS   *conf_v1alpha1.UpstreamBuffers
+		expected string
+	}{
+		{
+			inputS:   nil,
+			expected: "8 4k",
+		},
+		{
+			inputS:   &conf_v1alpha1.UpstreamBuffers{Number: 8, Size: "16K"},
+			expected: "8 16K",
+		},
+	}
+
+	for _, test := range tests {
+		result := generateBuffers(test.inputS, "8 4k")
+		if result != test.expected {
+			t.Errorf("generateBuffer() return %v but expected %v", result, test.expected)
+		}
+	}
+}
+
 func TestGenerateLocation(t *testing.T) {
 	cfgParams := ConfigParams{
 		ProxyConnectTimeout:  "30s",

pkg/apis/configuration/v1alpha1/types.go

@@ -25,24 +25,33 @@ type VirtualServerSpec struct {
 
 // Upstream defines an upstream.
 type Upstream struct {
-	Name                     string       `json:"name"`
-	Service                  string       `json:"service"`
-	Port                     uint16       `json:"port"`
-	LBMethod                 string       `json:"lb-method"`
-	FailTimeout              string       `json:"fail-timeout"`
-	MaxFails                 *int         `json:"max-fails"`
-	MaxConns                 *int         `json:"max-conns"`
-	Keepalive                *int         `json:"keepalive"`
-	ProxyConnectTimeout      string       `json:"connect-timeout"`
-	ProxyReadTimeout         string       `json:"read-timeout"`
-	ProxySendTimeout         string       `json:"send-timeout"`
-	ProxyNextUpstream        string       `json:"next-upstream"`
-	ProxyNextUpstreamTimeout string       `json:"next-upstream-timeout"`
-	ProxyNextUpstreamTries   int          `json:"next-upstream-tries"`
-	ClientMaxBodySize        string       `json:"client-max-body-size"`
-	TLS                      UpstreamTLS  `json:"tls"`
-	HealthCheck              *HealthCheck `json:"healthCheck"`
-	SlowStart                string       `json:"slow-start"`
+	Name                     string           `json:"name"`
+	Service                  string           `json:"service"`
+	Port                     uint16           `json:"port"`
+	LBMethod                 string           `json:"lb-method"`
+	FailTimeout              string           `json:"fail-timeout"`
+	MaxFails                 *int             `json:"max-fails"`
+	MaxConns                 *int             `json:"max-conns"`
+	Keepalive                *int             `json:"keepalive"`
+	ProxyConnectTimeout      string           `json:"connect-timeout"`
+	ProxyReadTimeout         string           `json:"read-timeout"`
+	ProxySendTimeout         string           `json:"send-timeout"`
+	ProxyNextUpstream        string           `json:"next-upstream"`
+	ProxyNextUpstreamTimeout string           `json:"next-upstream-timeout"`
+	ProxyNextUpstreamTries   int              `json:"next-upstream-tries"`
+	ProxyBuffering           *bool            `json:"buffering"`
+	ProxyBuffers             *UpstreamBuffers `json:"buffers"`
+	ProxyBufferSize          string           `json:"buffer-size"`
+	ClientMaxBodySize        string           `json:"client-max-body-size"`
+	TLS                      UpstreamTLS      `json:"tls"`
+	HealthCheck              *HealthCheck     `json:"healthCheck"`
+	SlowStart                string           `json:"slow-start"`
+}
+
+// UpstreamBuffers defines Buffer Configuration for an Upstream
+type UpstreamBuffers struct {
+	Number int    `json:"number"`
+	Size   string `json:"size"`
 }
 
 // UpstreamTLS defines a TLS configuration for an Upstream.

pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go

@@ -96,8 +96,28 @@ func validateTime(time string, fieldPath *field.Path) field.ErrorList {
 }
 
 // http://nginx.org/en/docs/syntax.html
-const sizeFmt = `\d+[kKmMgG]?`
-const sizeErrMsg = "must consist of numeric characters followed by a valid size suffix. 'k|K|m|M|g|G"
+const offsetFmt = `\d+[kKmMgG]?`
+const offsetErrMsg = "must consist of numeric characters followed by a valid size suffix. 'k|K|m|M|g|G"
+
+var offsetRegexp = regexp.MustCompile("^" + offsetFmt + "$")
+
+func validateOffset(offset string, fieldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	if offset == "" {
+		return allErrs
+	}
+
+	if !offsetRegexp.MatchString(offset) {
+		msg := validation.RegexError(offsetErrMsg, offsetFmt, "16", "32k", "64M")
+		return append(allErrs, field.Invalid(fieldPath, offset, msg))
+	}
+
+	return allErrs
+}
+
+const sizeFmt = `\d+[kKmM]?`
+const sizeErrMsg = "must consist of numeric characters followed by a valid size suffix. 'k|K|m|M"
 
 var sizeRegexp = regexp.MustCompile("^" + sizeFmt + "$")
 
@@ -112,6 +132,25 @@ func validateSize(size string, fieldPath *field.Path) field.ErrorList {
 		msg := validation.RegexError(sizeErrMsg, sizeFmt, "16", "32k", "64M")
 		return append(allErrs, field.Invalid(fieldPath, size, msg))
 	}
+	return allErrs
+}
+
+func validateBuffer(buff *v1alpha1.UpstreamBuffers, fieldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	if buff == nil {
+		return allErrs
+	}
+
+	if buff.Number <= 0 {
+		allErrs = append(allErrs, field.Invalid(fieldPath.Child("number"), buff.Number, "must be positive"))
+	}
+
+	if buff.Size == "" {
+		allErrs = append(allErrs, field.Required(fieldPath.Child("size"), "cannot be empty"))
+	} else {
+		allErrs = append(allErrs, validateSize(buff.Size, fieldPath.Child("size"))...)
+	}
 
 	return allErrs
 }
@@ -329,9 +368,11 @@ func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path, isP
 		allErrs = append(allErrs, validatePositiveIntOrZeroFromPointer(u.MaxFails, idxPath.Child("max-fails"))...)
 		allErrs = append(allErrs, validatePositiveIntOrZeroFromPointer(u.Keepalive, idxPath.Child("keepalive"))...)
 		allErrs = append(allErrs, validatePositiveIntOrZeroFromPointer(u.MaxConns, idxPath.Child("max-conns"))...)
-		allErrs = append(allErrs, validateSize(u.ClientMaxBodySize, idxPath.Child("client-max-body-size"))...)
+		allErrs = append(allErrs, validateOffset(u.ClientMaxBodySize, idxPath.Child("client-max-body-size"))...)
 		allErrs = append(allErrs, validateUpstreamHealthCheck(u.HealthCheck, idxPath.Child("healthCheck"))...)
 		allErrs = append(allErrs, validateTime(u.SlowStart, idxPath.Child("slow-start"))...)
+		allErrs = append(allErrs, validateBuffer(u.ProxyBuffers, idxPath.Child("buffers"))...)
+		allErrs = append(allErrs, validateSize(u.ProxyBufferSize, idxPath.Child("buffer-size"))...)
 
 		for _, msg := range validation.IsValidPortNum(int(u.Port)) {
 			allErrs = append(allErrs, field.Invalid(idxPath.Child("port"), u.Port, msg))

pkg/apis/configuration/validation/validation.go

@@ -322,6 +322,37 @@ func TestValidateUpstreamsFails(t *testing.T) {
 			},
 			msg: "invalid value for ClientMaxBodySize",
 		},
+		{
+			upstreams: []v1alpha1.Upstream{
+				{
+					Name:    "upstream1",
+					Service: "test-1",
+					Port:    80,
+					ProxyBuffers: &v1alpha1.UpstreamBuffers{
+						Number: -1,
+						Size:   "1G",
+					},
+				},
+			},
+			expectedUpstreamNames: map[string]sets.Empty{
+				"upstream1": {},
+			},
+			msg: "invalid value for ProxyBuffers",
+		},
+		{
+			upstreams: []v1alpha1.Upstream{
+				{
+					Name:            "upstream1",
+					Service:         "test-1",
+					Port:            80,
+					ProxyBufferSize: "1G",
+				},
+			},
+			expectedUpstreamNames: map[string]sets.Empty{
+				"upstream1": {},
+			},
+			msg: "invalid value for ProxyBufferSize",
+		},
 	}
 
 	isPlus := false
@@ -1658,16 +1689,64 @@ func TestValidateTime(t *testing.T) {
 	}
 }
 
+func TestValidateOffset(t *testing.T) {
+	var validInput = []string{"", "1", "10k", "11m", "1K", "100M", "5G"}
+	for _, test := range validInput {
+		allErrs := validateOffset(test, field.NewPath("offset-field"))
+		if len(allErrs) != 0 {
+			t.Errorf("validateOffset(%q) returned an error for valid input", test)
+		}
+	}
+
+	var invalidInput = []string{"55mm", "2mG", "6kb", "-5k", "1L", "5Gb"}
+	for _, test := range invalidInput {
+		allErrs := validateOffset(test, field.NewPath("offset-field"))
+		if len(allErrs) == 0 {
+			t.Errorf("validateOffset(%q) didn't return error for invalid input.", test)
+		}
+	}
+}
+
+func TestValidateBuffer(t *testing.T) {
+	validbuff := &v1alpha1.UpstreamBuffers{Number: 8, Size: "8k"}
+	allErrs := validateBuffer(validbuff, field.NewPath("buffers-field"))
+
+	if len(allErrs) != 0 {
+		t.Errorf("validateBuffer returned errors %v valid input %v", allErrs, validbuff)
+	}
+
+	invalidbuff := []*v1alpha1.UpstreamBuffers{
+		{
+			Number: -8,
+			Size:   "15m",
+		},
+		{
+			Number: 8,
+			Size:   "15G",
+		},
+		{
+			Number: 8,
+			Size:   "",
+		},
+	}
+	for _, test := range invalidbuff {
+		allErrs = validateBuffer(test, field.NewPath("buffers-field"))
+		if len(allErrs) == 0 {
+			t.Errorf("validateBuffer didn't return error for invalid input %v.", test)
+		}
+	}
+}
+
 func TestValidateSize(t *testing.T) {
-	var validInput = []string{"", "1", "10k", "11m", "1K", "100M"}
+	var validInput = []string{"", "4k", "8K", "16m", "32M"}
 	for _, test := range validInput {
 		allErrs := validateSize(test, field.NewPath("size-field"))
 		if len(allErrs) != 0 {
 			t.Errorf("validateSize(%q) returned an error for valid input", test)
 		}
 	}
 
-	var invalidInput = []string{"55mm", "2mG", "6kb", "-5k", "1L"}
+	var invalidInput = []string{"55mm", "2mG", "6kb", "-5k", "1L", "5G"}
 	for _, test := range invalidInput {
 		allErrs := validateSize(test, field.NewPath("size-field"))
 		if len(allErrs) == 0 {