From 6f54aecbee33261fc349d4321a8c55c51a0308da Mon Sep 17 00:00:00 2001 From: ampant <52200332+ampant@users.noreply.github.com> Date: Thu, 22 Aug 2019 14:45:44 +0530 Subject: [PATCH] Update pkg/apis/configuration/v1alpha1/types.go Co-Authored-By: Dean Coakley --- docs/virtualserver-and-virtualserverroute.md | 7 +++ internal/configs/virtualserver.go | 4 +- internal/configs/virtualserver_test.go | 24 ++++++++++ pkg/apis/configuration/v1alpha1/types.go | 46 +++++++++---------- .../v1alpha1/zz_generated.deepcopy.go | 38 ++++++++------- .../configuration/validation/validation.go | 35 +++++++++----- .../validation/validation_test.go | 32 ++++++------- 7 files changed, 116 insertions(+), 70 deletions(-) diff --git a/docs/virtualserver-and-virtualserverroute.md b/docs/virtualserver-and-virtualserverroute.md index 9454b9fb62..43ec403ec8 100644 --- 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 index 97833159ae..71fe49cc36 100644 --- 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 index ab9cb8a0de..aa52b1c98b 100644 --- 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 index ee1e81fed2..212b8317f8 100644 --- 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 index 50b05c5c41..65909e0ae8 100644 --- a/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go @@ -8,22 +8,6 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Buffers) DeepCopyInto(out *Buffers) { - *out = *in - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Buffers. -func (in *Buffers) DeepCopy() *Buffers { - if in == nil { - return nil - } - out := new(Buffers) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Condition) DeepCopyInto(out *Condition) { *out = *in @@ -212,7 +196,11 @@ func (in *Upstream) DeepCopyInto(out *Upstream) { *out = new(bool) **out = **in } - out.ProxyBuffers = in.ProxyBuffers + if in.ProxyBuffers != nil { + in, out := &in.ProxyBuffers, &out.ProxyBuffers + *out = new(UpstreamBuffers) + **out = **in + } out.TLS = in.TLS if in.HealthCheck != nil { in, out := &in.HealthCheck, &out.HealthCheck @@ -232,6 +220,22 @@ func (in *Upstream) DeepCopy() *Upstream { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *UpstreamBuffers) DeepCopyInto(out *UpstreamBuffers) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UpstreamBuffers. +func (in *UpstreamBuffers) DeepCopy() *UpstreamBuffers { + if in == nil { + return nil + } + out := new(UpstreamBuffers) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpstreamTLS) DeepCopyInto(out *UpstreamTLS) { *out = *in diff --git a/pkg/apis/configuration/validation/validation.go b/pkg/apis/configuration/validation/validation.go index 33752d8802..b3ff0d1321 100644 --- 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 index 34516a30f5..0bfa4f33a7 100644 --- 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) } } }