Update pkg/apis/configuration/v1alpha1/types.go

Co-Authored-By: Dean Coakley <[email protected]>

Author: ampant

docs/virtualserver-and-virtualserverroute.md

@@ -221,6 +221,25 @@ tls:
 | `buffers` | Configures the buffers used for reading a response from the upstream server for a single connection. The buffers field configures the buffers used for reading a response from the upstream server for a single connection: 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. | `buffers` | 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
+Buffers defines Buffer Configuration for an Upstream. The buffers field configures the buffers used for reading a response from the upstream server for a single connection. For example
+
+```yaml
+name: tea
+    service: tea-svc
+    port: 80
+    buffering: true
+    buffers:
+      number: 4
+      size: 8K
+    buffer-size: 16k
+```
+
+| Field | Description | Type | Required |
+| ----- | ----------- | ---- | -------- |
+| `number` | Configures the number of buffers. The default is set in the `proxy-buffers`  ConfigMap key. The default is 8 | `int` | Yes |
+| `size` | Configures the size of a buffer. The default is set in the `proxy-buffers`  ConfigMap key. Sizes can be specified in bytes, kilobytes (suffixes k and K) or megabytes (suffixes m and M), for example, “1024”, “8k”, “1m”. The default is 4k | `string` | Yes |
+
 ### Upstream.TLS
 
 | Field | Description | Type | Required |

internal/configs/virtualserver.go

@@ -391,8 +391,8 @@ func generateString(s string, defaultS string) string {
 	return s
 }
 
-func generateBuffers(s conf_v1alpha1.Buffers, defaultS string) string {
-	if (conf_v1alpha1.Buffers{}) == s {
+func generateBuffers(s *conf_v1alpha1.UpstreamBuffers, defaultS string) string {
+	if s == nil {
 		return defaultS
 	}
 	return fmt.Sprintf("%v %v", s.Number, s.Size)

internal/configs/virtualserver_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/nginxinc/kubernetes-ingress/internal/configs/version2"
 	"github.com/nginxinc/kubernetes-ingress/internal/nginx"
+	"github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1"
 	conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -937,6 +938,29 @@ func TestGenerateString(t *testing.T) {
 	}
 }
 
+func TestGenerateBuffer(t *testing.T) {
+	tests := []struct {
+		inputS   *v1alpha1.UpstreamBuffers
+		expected string
+	}{
+		{
+			inputS:   nil,
+			expected: "8 4k",
+		},
+		{
+			inputS:   &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,29 +25,29 @@ 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"`
-	ProxyBuffering           *bool        `json:"buffering"`
-	ProxyBuffers             Buffers      `json:"buffers"`
-	ProxyBufferSize          string       `json:"buffersize"`
-	ClientMaxBodySize        string       `json:"client-max-body-size"`
-	TLS                      UpstreamTLS  `json:"tls"`
-	HealthCheck              *HealthCheck `json:"healthCheck"`
-}
-
-//Buffers defines Buffer Configuration for an Upstream
+	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"`
+}
+
+//UpstreamBuffers defines Buffer Configuration for an Upstream
 type UpstreamBuffers struct {
 	Number int    `json:"number"`
 	Size   string `json:"size"`

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

@@ -96,54 +96,58 @@ 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 sizeRegexp = regexp.MustCompile("^" + sizeFmt + "$")
+var offsetRegexp = regexp.MustCompile("^" + offsetFmt + "$")
 
-func validateSize(size string, fieldPath *field.Path) field.ErrorList {
+func validateOffset(size string, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	if size == "" {
 		return allErrs
 	}
 
-	if !sizeRegexp.MatchString(size) {
-		msg := validation.RegexError(sizeErrMsg, sizeFmt, "16", "32k", "64M")
+	if !offsetRegexp.MatchString(size) {
+		msg := validation.RegexError(offsetErrMsg, offsetFmt, "16", "32k", "64M")
 		return append(allErrs, field.Invalid(fieldPath, size, msg))
 	}
 
 	return allErrs
 }
 
-func validateBuffer(buff v1alpha1.Buffers, fieldPath *field.Path) field.ErrorList {
+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 + "$")
+
+func validateBuffer(buff *v1alpha1.UpstreamBuffers, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if (v1alpha1.Buffers{}) == buff {
+	if buff == nil {
 		return allErrs
 	}
 
 	if buff.Number <= 0 {
 		return append(allErrs, field.Invalid(fieldPath, buff.Number, "must be positive"))
 	}
 
-	if buff.Size == "" {
-		return append(allErrs, field.Invalid(fieldPath, buff.Size, "cannot be empty"))
-	}
-
-	if buff.Size == "4k" || buff.Size == "8k" {
-		return allErrs
-	}
-	return append(allErrs, field.Invalid(fieldPath, buff.Size, "must be 4k or 8k"))
+	allErrs = validateSize(buff.Size, fieldPath)
+	return allErrs
 }
 
-func validateBufSize(size string, fieldPath *field.Path) field.ErrorList {
+func validateSize(size string, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if size == "" || size == "4k" || size == "8k" {
+	if size == "" {
 		return allErrs
 	}
-	return append(allErrs, field.Invalid(fieldPath, size, "must be 4k or 8k"))
+
+	if !sizeRegexp.MatchString(size) {
+		msg := validation.RegexError(sizeErrMsg, sizeFmt, "16", "32k", "64M")
+		return append(allErrs, field.Invalid(fieldPath, size, msg))
+	}
+	return allErrs
 }
 
 func validateUpstreamLBMethod(lBMethod string, fieldPath *field.Path, isPlus bool) field.ErrorList {
@@ -359,10 +363,10 @@ 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, validateBuffer(u.ProxyBuffers, idxPath.Child("buffers"))...)
-		allErrs = append(allErrs, validateBufSize(u.ProxyBufferSize, idxPath.Child("buffer-size"))...)
+		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

@@ -328,9 +328,9 @@ func TestValidateUpstreamsFails(t *testing.T) {
 					Name:    "upstream1",
 					Service: "test-1",
 					Port:    80,
-					ProxyBuffers: v1alpha1.Buffers{
+					ProxyBuffers: &v1alpha1.UpstreamBuffers{
 						Number: -1,
-						Size:   "1k",
+						Size:   "1G",
 					},
 				},
 			},
@@ -345,7 +345,7 @@ func TestValidateUpstreamsFails(t *testing.T) {
 					Name:            "upstream1",
 					Service:         "test-1",
 					Port:            80,
-					ProxyBufferSize: "1k",
+					ProxyBufferSize: "1G",
 				},
 			},
 			expectedUpstreamNames: map[string]sets.Empty{
@@ -1689,53 +1689,59 @@ func TestValidateTime(t *testing.T) {
 	}
 }
 
-func TestValidateSize(t *testing.T) {
+func TestValidateOffset(t *testing.T) {
 	var validInput = []string{"", "1", "10k", "11m", "1K", "100M"}
 	for _, test := range validInput {
-		allErrs := validateSize(test, field.NewPath("size-field"))
+		allErrs := validateOffset(test, field.NewPath("size-field"))
 		if len(allErrs) != 0 {
-			t.Errorf("validateSize(%q) returned an error for valid input", test)
+			t.Errorf("validateOffset(%q) returned an error for valid input", test)
 		}
 	}
 
 	var invalidInput = []string{"55mm", "2mG", "6kb", "-5k", "1L"}
 	for _, test := range invalidInput {
-		allErrs := validateSize(test, field.NewPath("size-field"))
+		allErrs := validateOffset(test, field.NewPath("size-field"))
 		if len(allErrs) == 0 {
-			t.Errorf("validateSize(%q) didn't return error for invalid input.", test)
+			t.Errorf("validateOffset(%q) didn't return error for invalid input.", test)
 		}
 	}
 }
 
 func TestValidateBuffer(t *testing.T) {
-	validbuff := v1alpha1.Buffers{Number: 8, Size: "8k"}
-	allErrs := validateBuffer(validbuff, field.NewPath("proxy_buffers"))
+	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.Buffers{Number: -8, Size: "15k"}
-	allErr := validateBuffer(invalidbuff, field.NewPath("proxy_buffers"))
-	if len(allErr) == 0 {
-		t.Errorf("validateBuffer(%q) didn't return error for invalid input.", invalidbuff)
+	invalidbuff := &v1alpha1.UpstreamBuffers{Number: -8, Size: "15G"}
+	allErrs = validateBuffer(invalidbuff, field.NewPath("buffers-field"))
+	if len(allErrs) == 0 {
+		t.Errorf("validateBuffer didn't return error %v for invalid input %v.", allErrs, invalidbuff)
+	}
+
+	invalidbuff = &v1alpha1.UpstreamBuffers{Number: 8, Size: "15G"}
+	allErrs = validateBuffer(invalidbuff, field.NewPath("buffers-field"))
+	if len(allErrs) == 0 {
+		t.Errorf("validateBuffer didn't return error %v for invalid input %v.", allErrs, invalidbuff)
 	}
 }
 
-func TestValidateBufSize(t *testing.T) {
-	var validInput = []string{"", "4k", "8k"}
+func TestValidateSize(t *testing.T) {
+	var validInput = []string{"", "4k", "8K", "16m", "32M"}
 	for _, test := range validInput {
-		allErrs := validateBufSize(test, field.NewPath("proxy_buffer_size"))
+		allErrs := validateSize(test, field.NewPath("size-field"))
 		if len(allErrs) != 0 {
-			t.Errorf("validateBufSize(%q) returned an error for valid input", test)
+			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 := validateBufSize(test, field.NewPath("proxy_buffer_size"))
+		allErrs := validateSize(test, field.NewPath("size-field"))
 		if len(allErrs) == 0 {
-			t.Errorf("validateBufSize(%q) didn't return error for invalid input.", test)
+			t.Errorf("validateSize(%q) didn't return error for invalid input.", test)
 		}
 	}
 }