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

Co-Authored-By: Dean Coakley <[email protected]>
nginx · Aug 23, 2019 · 6f54aec · 6f54aec
1 parent 48ef449
commit 6f54aec
Show file tree

Hide file tree

Showing 7 changed files with 116 additions and 70 deletions.
diff --git a/docs/virtualserver-and-virtualserverroute.md b/docs/virtualserver-and-virtualserverroute.md
@@ -221,6 +221,13 @@ 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
+
+| 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
 
 | Field | Description | Type | Required |

diff --git a/internal/configs/virtualserver.go b/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)

diff --git a/internal/configs/virtualserver_test.go b/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",

diff --git a/pkg/apis/configuration/v1alpha1/types.go b/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"`

diff --git a/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/apis/configuration/validation/validation.go b/pkg/apis/configuration/validation/validation.go
@@ -101,7 +101,7 @@ const sizeErrMsg = "must consist of numeric characters followed by a valid size
 
 var sizeRegexp = regexp.MustCompile("^" + sizeFmt + "$")
 
-func validateSize(size string, fieldPath *field.Path) field.ErrorList {
+func validateOffset(size string, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	if size == "" {
@@ -116,10 +116,15 @@ func validateSize(size string, fieldPath *field.Path) field.ErrorList {
 	return allErrs
 }
 
-func validateBuffer(buff v1alpha1.Buffers, fieldPath *field.Path) field.ErrorList {
+const bufsizeFmt = `\d+[kKmM]?`
+const bufsizeErrMsg = "must consist of numeric characters followed by a valid size suffix. 'k|K|m|M"
+
+var bufsizeRegexp = regexp.MustCompile("^" + bufsizeFmt + "$")
+
+func validateBuffer(buff *v1alpha1.UpstreamBuffers, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if (v1alpha1.Buffers{}) == buff {
+	if buff == nil {
 		return allErrs
 	}
 
@@ -128,22 +133,28 @@ func validateBuffer(buff v1alpha1.Buffers, fieldPath *field.Path) field.ErrorLis
 	}
 
 	if buff.Size == "" {
-		return append(allErrs, field.Invalid(fieldPath, buff.Size, "cannot be empty"))
+		return append(allErrs, field.Required(fieldPath, "buffer size cannot be empty"))
 	}
 
-	if buff.Size == "4k" || buff.Size == "8k" {
-		return allErrs
+	if !bufsizeRegexp.MatchString(buff.Size) {
+		msg := validation.RegexError(bufsizeErrMsg, bufsizeFmt, "16", "32k", "64M")
+		return append(allErrs, field.Invalid(fieldPath, buff.Size, msg))
 	}
-	return append(allErrs, field.Invalid(fieldPath, buff.Size, "must be 4k or 8k"))
+	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 !bufsizeRegexp.MatchString(size) {
+		msg := validation.RegexError(bufsizeErrMsg, bufsizeFmt, "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 +370,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))

diff --git a/pkg/apis/configuration/validation/validation_test.go b/pkg/apis/configuration/validation/validation_test.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,53 @@ 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"}
+	validbuff := &v1alpha1.UpstreamBuffers{Number: 8, Size: "8k"}
 	allErrs := validateBuffer(validbuff, field.NewPath("proxy_buffers"))
 
 	if len(allErrs) != 0 {
 		t.Errorf("validateBuffer returned errors %v valid input %v", allErrs, validbuff)
 	}
 
-	invalidbuff := v1alpha1.Buffers{Number: -8, Size: "15k"}
+	invalidbuff := &v1alpha1.UpstreamBuffers{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)
 	}
 }
 
-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("proxy_buffer_size"))
 		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"}
 	for _, test := range invalidInput {
-		allErrs := validateBufSize(test, field.NewPath("proxy_buffer_size"))
+		allErrs := validateSize(test, field.NewPath("proxy_buffer_size"))
 		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)
 		}
 	}
 }