From 9ebd4ee92fd0b9ecb34cfc0cbd0fead397a9ca6a Mon Sep 17 00:00:00 2001 From: Lorcan McVeigh Date: Fri, 26 Jul 2019 15:34:13 +0100 Subject: [PATCH] Add retries support for vs/vsr --- docs/virtualserver-and-virtualserverroute.md | 14 +- internal/configs/version2/config.go | 27 +-- .../version2/nginx-plus.virtualserver.tmpl | 3 + .../configs/version2/nginx.virtualserver.tmpl | 3 + internal/configs/virtualserver.go | 45 ++--- internal/configs/virtualserver_test.go | 157 +++++++++++++----- pkg/apis/configuration/v1alpha1/types.go | 25 +-- .../configuration/validation/validation.go | 40 +++++ .../validation/validation_test.go | 149 ++++++++++++++--- 9 files changed, 351 insertions(+), 112 deletions(-) diff --git a/docs/virtualserver-and-virtualserverroute.md b/docs/virtualserver-and-virtualserverroute.md index 846b09a769..732ecc1643 100644 --- a/docs/virtualserver-and-virtualserverroute.md +++ b/docs/virtualserver-and-virtualserverroute.md @@ -184,6 +184,9 @@ keepalive: 32 connect-timeout: 30s read-timeout: 30s send-timeout: 30s +next-upstream: "error timeout non_idempotent" +next-upstream-timeout: 5s +next-upstreeam-tries: 10 tls: enable: True ``` @@ -196,10 +199,13 @@ tls: | `lb-method` | The load [balancing method](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#choosing-a-load-balancing-method). To use the round-robin method, specify `round_robin`. The default is specified in the `lb-method` ConfigMap key. | `string` | No | | `fail-timeout` | The time during which the specified number of unsuccessful attempts to communicate with an upstream server should happen to consider the server unavailable. See the [fail_timeout](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#fail_timeout) parameter of the server directive. The default is set in the `fail-timeout` ConfigMap key. | `string` | No | | `max-fails` | The number of unsuccessful attempts to communicate with an upstream server that should happen in the duration set by the `fail-timeout` to consider the server unavailable. See the [max_fails](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#max_fails) parameter of the server directive. The default is set in the `max-fails` ConfgMap key. | `int` | No | -| `keepalive` | Configures the cache for connections to upstream servers. The value `0` disables the cache. See the [keepalive](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive) directive. The default is set in the `keepalive` ConfigMap key. | `int` | No -`connect-timeout` | The timeout for establishing a connection with an upstream server. See the [proxy_connect_timeout](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_connect_timeout) directive. The default is specified in the `proxy-connect-timeout` ConfigMap key. | `string` | No -`read-timeout` | The timeout for reading a response from an upstream server. See the [proxy_read_timeout](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_read_timeout) directive. The default is specified in the `proxy-read-timeout` ConfigMap key. | `string` | No -`send-timeout` | The timeout for transmitting a request to an upstream server. See the [proxy_send_timeout](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_send_timeout) directive. The default is specified in the `proxy-send-timeout` ConfigMap key. | `string` | No +| `keepalive` | Configures the cache for connections to upstream servers. The value `0` disables the cache. See the [keepalive](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive) directive. The default is set in the `keepalive` ConfigMap key. | `int` | No | +| `connect-timeout` | The timeout for establishing a connection with an upstream server. See the [proxy_connect_timeout](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_connect_timeout) directive. The default is specified in the `proxy-connect-timeout` ConfigMap key. | `string` | No | +| `read-timeout` | The timeout for reading a response from an upstream server. See the [proxy_read_timeout](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_read_timeout) directive. The default is specified in the `proxy-read-timeout` ConfigMap key. | `string` | No | +| `send-timeout` | The timeout for transmitting a request to an upstream server. See the [proxy_send_timeout](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_send_timeout) directive. The default is specified in the `proxy-send-timeout` ConfigMap key. | `string` | No | +| `next-upstream` | Specifies in which cases a request should be passed to the next upstream server. See the [proxy_next_upstream](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream) directive. The default is `error timeout`. | `string` | No | +| `next-upstream-timeout` | The time during which a request can be passed to the next upstream server. See the [proxy_next_upstream_timeout](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream_timeout) directive. The `0` value turns off the time limit. The default is `0`. | `string` | No | +| `next-upstream-tries` | The number of possible tries for passing a request to the next upstream server. See the [proxy_next_upstream_tries](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream_tries) directive. The `0` value turns off this limit. The default is `0`. | `int` | No | | `tls` | The TLS configuration for the Upstream. | [`tls`](#UpstreamTLS) | No | ### Upstream.TLS diff --git a/internal/configs/version2/config.go b/internal/configs/version2/config.go index e0c1f324db..ad4c7aebb1 100644 --- a/internal/configs/version2/config.go +++ b/internal/configs/version2/config.go @@ -49,18 +49,21 @@ type SSL struct { // Location defines a location. type Location struct { - Path string - Snippets []string - ProxyConnectTimeout string - ProxyReadTimeout string - ProxySendTimeout string - ClientMaxBodySize string - ProxyMaxTempFileSize string - ProxyBuffering bool - ProxyBuffers string - ProxyBufferSize string - ProxyPass string - HasKeepalive bool + Path string + Snippets []string + ProxyConnectTimeout string + ProxyReadTimeout string + ProxySendTimeout string + ClientMaxBodySize string + ProxyMaxTempFileSize string + ProxyBuffering bool + ProxyBuffers string + ProxyBufferSize string + ProxyPass string + ProxyNextUpstream string + ProxyNextUpstreamTimeout string + ProxyNextUpstreamTries int + HasKeepalive bool } // SplitClient defines a split_clients. diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 9cb108e25f..fcff628645 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -119,6 +119,9 @@ server { proxy_set_header X-Forwarded-Proto $scheme; proxy_pass {{ $l.ProxyPass }}; + proxy_next_upstream {{ $l.ProxyNextUpstream }}; + proxy_next_upstream_timeout {{ $l.ProxyNextUpstreamTimeout }}; + proxy_next_upstream_tries {{ $l.ProxyNextUpstreamTries }}; } {{ end }} } \ No newline at end of file diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 9cb108e25f..fcff628645 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -119,6 +119,9 @@ server { proxy_set_header X-Forwarded-Proto $scheme; proxy_pass {{ $l.ProxyPass }}; + proxy_next_upstream {{ $l.ProxyNextUpstream }}; + proxy_next_upstream_timeout {{ $l.ProxyNextUpstreamTimeout }}; + proxy_next_upstream_tries {{ $l.ProxyNextUpstreamTries }}; } {{ end }} } \ No newline at end of file diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 0aa2c77da3..298bb7f3ab 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -205,7 +205,7 @@ func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, endp s := version2.UpstreamServer{ Address: e, MaxFails: generateIntFromPointer(upstream.MaxFails, cfgParams.MaxFails), - FailTimeout: generateTime(upstream.FailTimeout, cfgParams.FailTimeout), + FailTimeout: generateString(upstream.FailTimeout, cfgParams.FailTimeout), } upsServers = append(upsServers, s) } @@ -214,7 +214,7 @@ func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, endp s := version2.UpstreamServer{ Address: nginx502Server, MaxFails: generateIntFromPointer(upstream.MaxFails, cfgParams.MaxFails), - FailTimeout: generateTime(upstream.FailTimeout, cfgParams.FailTimeout), + FailTimeout: generateString(upstream.FailTimeout, cfgParams.FailTimeout), } upsServers = append(upsServers, s) } @@ -236,13 +236,6 @@ func generateLBMethod(method string, defaultMethod string) string { return method } -func generateTime(time string, defaultTime string) string { - if time == "" { - return defaultTime - } - return time -} - func generateIntFromPointer(n *int, defaultN int) int { if n == nil { return defaultN @@ -264,20 +257,30 @@ func generateProxyPassProtocol(upstream conf_v1alpha1.Upstream) string { return "http" } +func generateString(s string, defaultS string) string { + if s == "" { + return defaultS + } + return s +} + func generateLocation(path string, upstreamName string, upstream conf_v1alpha1.Upstream, cfgParams *ConfigParams) version2.Location { return version2.Location{ - Path: path, - Snippets: cfgParams.LocationSnippets, - ProxyConnectTimeout: generateTime(upstream.ProxyConnectTimeout, cfgParams.ProxyConnectTimeout), - ProxyReadTimeout: generateTime(upstream.ProxyReadTimeout, cfgParams.ProxyReadTimeout), - ProxySendTimeout: generateTime(upstream.ProxySendTimeout, cfgParams.ProxySendTimeout), - ClientMaxBodySize: cfgParams.ClientMaxBodySize, - ProxyMaxTempFileSize: cfgParams.ProxyMaxTempFileSize, - ProxyBuffering: cfgParams.ProxyBuffering, - ProxyBuffers: cfgParams.ProxyBuffers, - ProxyBufferSize: cfgParams.ProxyBufferSize, - ProxyPass: fmt.Sprintf("%v://%v", generateProxyPassProtocol(upstream), upstreamName), - HasKeepalive: upstreamHasKeepalive(upstream, cfgParams), + Path: path, + Snippets: cfgParams.LocationSnippets, + ProxyConnectTimeout: generateString(upstream.ProxyConnectTimeout, cfgParams.ProxyConnectTimeout), + ProxyReadTimeout: generateString(upstream.ProxyReadTimeout, cfgParams.ProxyReadTimeout), + ProxySendTimeout: generateString(upstream.ProxySendTimeout, cfgParams.ProxySendTimeout), + ClientMaxBodySize: cfgParams.ClientMaxBodySize, + ProxyMaxTempFileSize: cfgParams.ProxyMaxTempFileSize, + ProxyBuffering: cfgParams.ProxyBuffering, + ProxyBuffers: cfgParams.ProxyBuffers, + ProxyBufferSize: cfgParams.ProxyBufferSize, + ProxyPass: fmt.Sprintf("%v://%v", generateProxyPassProtocol(upstream), upstreamName), + ProxyNextUpstream: generateString(upstream.ProxyNextUpstream, "error timeout"), + ProxyNextUpstreamTimeout: generateString(upstream.ProxyNextUpstreamTimeout, "0s"), + ProxyNextUpstreamTries: upstream.ProxyNextUpstreamTries, + HasKeepalive: upstreamHasKeepalive(upstream, cfgParams), } } diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index d1431957c1..93c7dc3395 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -263,14 +263,20 @@ func TestGenerateVirtualServerConfig(t *testing.T) { Snippets: []string{"# server snippet"}, Locations: []version2.Location{ { - Path: "/tea", - ProxyPass: "http://vs_default_cafe_tea", - HasKeepalive: true, + Path: "/tea", + ProxyPass: "http://vs_default_cafe_tea", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + HasKeepalive: true, }, { - Path: "/coffee", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee", - HasKeepalive: true, + Path: "/coffee", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + HasKeepalive: true, }, }, }, @@ -460,20 +466,32 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { }, Locations: []version2.Location{ { - Path: "@splits_0_split_0", - ProxyPass: "http://vs_default_cafe_tea-v1", + Path: "@splits_0_split_0", + ProxyPass: "http://vs_default_cafe_tea-v1", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, { - Path: "@splits_0_split_1", - ProxyPass: "http://vs_default_cafe_tea-v2", + Path: "@splits_0_split_1", + ProxyPass: "http://vs_default_cafe_tea-v2", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, { - Path: "@splits_1_split_0", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1", + Path: "@splits_1_split_0", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, { - Path: "@splits_1_split_1", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2", + Path: "@splits_1_split_1", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, }, }, @@ -704,20 +722,32 @@ func TestGenerateVirtualServerConfigForVirtualServerWithRules(t *testing.T) { }, Locations: []version2.Location{ { - Path: "@rules_0_match_0", - ProxyPass: "http://vs_default_cafe_tea-v2", + Path: "@rules_0_match_0", + ProxyPass: "http://vs_default_cafe_tea-v2", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, { - Path: "@rules_0_default", - ProxyPass: "http://vs_default_cafe_tea-v1", + Path: "@rules_0_default", + ProxyPass: "http://vs_default_cafe_tea-v1", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, { - Path: "@rules_1_match_0", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2", + Path: "@rules_1_match_0", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, { - Path: "@rules_1_default", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1", + Path: "@rules_1_default", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, }, }, @@ -895,6 +925,29 @@ func TestGenerateProxyPassProtocol(t *testing.T) { } } +func TestGenerateString(t *testing.T) { + tests := []struct { + inputS string + expected string + }{ + { + inputS: "http_404", + expected: "http_404", + }, + { + inputS: "", + expected: "error timeout", + }, + } + + for _, test := range tests { + result := generateString(test.inputS, "error timeout") + if result != test.expected { + t.Errorf("generateString() return %v but expected %v", result, test.expected) + } + } +} + func TestGenerateLocation(t *testing.T) { cfgParams := ConfigParams{ ProxyConnectTimeout: "30s", @@ -911,17 +964,20 @@ func TestGenerateLocation(t *testing.T) { upstreamName := "test-upstream" expected := version2.Location{ - Path: "/", - Snippets: []string{"# location snippet"}, - ProxyConnectTimeout: "30s", - ProxyReadTimeout: "31s", - ProxySendTimeout: "32s", - ClientMaxBodySize: "1m", - ProxyMaxTempFileSize: "1024m", - ProxyBuffering: true, - ProxyBuffers: "8 4k", - ProxyBufferSize: "4k", - ProxyPass: "http://test-upstream", + Path: "/", + Snippets: []string{"# location snippet"}, + ProxyConnectTimeout: "30s", + ProxyReadTimeout: "31s", + ProxySendTimeout: "32s", + ClientMaxBodySize: "1m", + ProxyMaxTempFileSize: "1024m", + ProxyBuffering: true, + ProxyBuffers: "8 4k", + ProxyBufferSize: "4k", + ProxyPass: "http://test-upstream", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, } result := generateLocation(path, upstreamName, conf_v1alpha1.Upstream{}, &cfgParams) @@ -1156,12 +1212,18 @@ func TestGenerateSplitRouteConfig(t *testing.T) { }, Locations: []version2.Location{ { - Path: "@splits_1_split_0", - ProxyPass: "http://vs_default_cafe_coffee-v1", + Path: "@splits_1_split_0", + ProxyPass: "http://vs_default_cafe_coffee-v1", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, { - Path: "@splits_1_split_1", - ProxyPass: "http://vs_default_cafe_coffee-v2", + Path: "@splits_1_split_1", + ProxyPass: "http://vs_default_cafe_coffee-v2", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, }, InternalRedirectLocation: version2.InternalRedirectLocation{ @@ -1364,16 +1426,25 @@ func TestGenerateRulesRouteConfig(t *testing.T) { }, Locations: []version2.Location{ { - Path: "@rules_1_match_0", - ProxyPass: "http://vs_default_cafe_coffee-v1", + Path: "@rules_1_match_0", + ProxyPass: "http://vs_default_cafe_coffee-v1", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, { - Path: "@rules_1_match_1", - ProxyPass: "http://vs_default_cafe_coffee-v2", + Path: "@rules_1_match_1", + ProxyPass: "http://vs_default_cafe_coffee-v2", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, { - Path: "@rules_1_default", - ProxyPass: "http://vs_default_cafe_tea", + Path: "@rules_1_default", + ProxyPass: "http://vs_default_cafe_tea", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, }, }, InternalRedirectLocation: version2.InternalRedirectLocation{ diff --git a/pkg/apis/configuration/v1alpha1/types.go b/pkg/apis/configuration/v1alpha1/types.go index a4a60279eb..d1237e8c2e 100644 --- a/pkg/apis/configuration/v1alpha1/types.go +++ b/pkg/apis/configuration/v1alpha1/types.go @@ -25,17 +25,20 @@ 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"` - Keepalive *int `json:"keepalive"` - ProxyConnectTimeout string `json:"connect-timeout"` - ProxyReadTimeout string `json:"read-timeout"` - ProxySendTimeout string `json:"send-timeout"` - TLS UpstreamTLS `json:"tls"` + 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"` + 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"` + TLS UpstreamTLS `json:"tls"` } // UpstreamTLS defines a TLS configuration for an Upstream. diff --git a/pkg/apis/configuration/validation/validation.go b/pkg/apis/configuration/validation/validation.go index 1a004702fb..47cb7bfeb2 100644 --- a/pkg/apis/configuration/validation/validation.go +++ b/pkg/apis/configuration/validation/validation.go @@ -142,6 +142,9 @@ func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path, isP allErrs = append(allErrs, validateTime(u.ProxyConnectTimeout, idxPath.Child("connect-timeout"))...) allErrs = append(allErrs, validateTime(u.ProxyReadTimeout, idxPath.Child("read-timeout"))...) allErrs = append(allErrs, validateTime(u.ProxySendTimeout, idxPath.Child("send-timeout"))...) + allErrs = append(allErrs, validateNextUpstream(u.ProxyNextUpstream, idxPath.Child("next-upstream"))...) + allErrs = append(allErrs, validateTime(u.ProxyNextUpstreamTimeout, idxPath.Child("next-upstream-timeout"))...) + allErrs = append(allErrs, validatePositiveIntOrZero(&u.ProxyNextUpstreamTries, idxPath.Child("next-upstream-tries"))...) allErrs = append(allErrs, validateUpstreamLBMethod(u.LBMethod, idxPath.Child("lb-method"), isPlus)...) allErrs = append(allErrs, validateTime(u.FailTimeout, idxPath.Child("fail-timeout"))...) @@ -156,6 +159,43 @@ func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path, isP return allErrs, upstreamNames } +var validNextUpstreamParams = map[string]bool{ + "error": true, + "timeout": true, + "invalid_header": true, + "http_500": true, + "http_502": true, + "http_503": true, + "http_504": true, + "http_403": true, + "http_404": true, + "http_429": true, + "non_idempotent": true, + "off": true, + "": true, +} + +// validateNextUpstream checks the values given for passing queries to a upstream +func validateNextUpstream(nextUpstream string, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + allParams := sets.String{} + if nextUpstream == "" { + return allErrs + } + params := strings.Fields(nextUpstream) + for _, para := range params { + if !validNextUpstreamParams[para] { + allErrs = append(allErrs, field.Invalid(fieldPath, para, "not a valid parameter")) + } + if allParams.Has(para) { + allErrs = append(allErrs, field.Invalid(fieldPath, para, "can not have duplicate parameters")) + } else { + allParams.Insert(para) + } + } + return allErrs +} + // validateUpstreamName checks is an upstream name is valid. // The rules for NGINX upstream names are less strict than IsDNS1035Label. // However, it is convenient to enforce IsDNS1035Label in the yaml for diff --git a/pkg/apis/configuration/validation/validation_test.go b/pkg/apis/configuration/validation/validation_test.go index 8846382f06..626e20100b 100644 --- a/pkg/apis/configuration/validation/validation_test.go +++ b/pkg/apis/configuration/validation/validation_test.go @@ -133,14 +133,20 @@ func TestValidateUpstreams(t *testing.T) { { upstreams: []v1alpha1.Upstream{ { - Name: "upstream1", - Service: "test-1", - Port: 80, + Name: "upstream1", + Service: "test-1", + Port: 80, + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: 5, }, { - Name: "upstream2", - Service: "test-2", - Port: 80, + Name: "upstream2", + Service: "test-2", + Port: 80, + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: 5, }, }, expectedUpstreamNames: map[string]sets.Empty{ @@ -171,9 +177,12 @@ func TestValidateUpstreamsFails(t *testing.T) { { upstreams: []v1alpha1.Upstream{ { - Name: "@upstream1", - Service: "test-1", - Port: 80, + Name: "@upstream1", + Service: "test-1", + Port: 80, + ProxyNextUpstream: "http_502", + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: 5, }, }, expectedUpstreamNames: sets.String{}, @@ -182,9 +191,12 @@ func TestValidateUpstreamsFails(t *testing.T) { { upstreams: []v1alpha1.Upstream{ { - Name: "upstream1", - Service: "@test-1", - Port: 80, + Name: "upstream1", + Service: "@test-1", + Port: 80, + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: 5, }, }, expectedUpstreamNames: map[string]sets.Empty{ @@ -195,9 +207,12 @@ func TestValidateUpstreamsFails(t *testing.T) { { upstreams: []v1alpha1.Upstream{ { - Name: "upstream1", - Service: "test-1", - Port: 0, + Name: "upstream1", + Service: "test-1", + Port: 0, + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: 5, }, }, expectedUpstreamNames: map[string]sets.Empty{ @@ -208,14 +223,20 @@ func TestValidateUpstreamsFails(t *testing.T) { { upstreams: []v1alpha1.Upstream{ { - Name: "upstream1", - Service: "test-1", - Port: 80, + Name: "upstream1", + Service: "test-1", + Port: 80, + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: 5, }, { - Name: "upstream1", - Service: "test-2", - Port: 80, + Name: "upstream1", + Service: "test-2", + Port: 80, + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: 5, }, }, expectedUpstreamNames: map[string]sets.Empty{ @@ -223,6 +244,54 @@ func TestValidateUpstreamsFails(t *testing.T) { }, msg: "duplicated upstreams", }, + { + upstreams: []v1alpha1.Upstream{ + { + Name: "upstream1", + Service: "test-1", + Port: 80, + ProxyNextUpstream: "https_504", + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: 5, + }, + }, + expectedUpstreamNames: map[string]sets.Empty{ + "upstream1": sets.Empty{}, + }, + msg: "invalid next upstream syntax", + }, + { + upstreams: []v1alpha1.Upstream{ + { + Name: "upstream1", + Service: "test-1", + Port: 80, + ProxyNextUpstream: "http_504", + ProxyNextUpstreamTimeout: "-2s", + ProxyNextUpstreamTries: 5, + }, + }, + expectedUpstreamNames: map[string]sets.Empty{ + "upstream1": sets.Empty{}, + }, + msg: "invalid upstream timeout value", + }, + { + upstreams: []v1alpha1.Upstream{ + { + Name: "upstream1", + Service: "test-1", + Port: 80, + ProxyNextUpstream: "https_504", + ProxyNextUpstreamTimeout: "10s", + ProxyNextUpstreamTries: -1, + }, + }, + expectedUpstreamNames: map[string]sets.Empty{ + "upstream1": sets.Empty{}, + }, + msg: "invalid upstream tries value", + }, } isPlus := false @@ -237,6 +306,44 @@ func TestValidateUpstreamsFails(t *testing.T) { } } +func TestValidateNextUpstream(t *testing.T) { + tests := []struct { + inputS string + }{ + { + inputS: "error timeout", + }, + { + inputS: "http_404 timeout", + }, + } + for _, test := range tests { + allErrs := validateNextUpstream(test.inputS, field.NewPath("next-upstreams")) + if len(allErrs) > 0 { + t.Errorf("validateNextUpstream(%q) returned errors %v for valid input.", test.inputS, allErrs) + } + } +} + +func TestValidateNextUpstreamFails(t *testing.T) { + tests := []struct { + inputS string + }{ + { + inputS: "error error", + }, + { + inputS: "https_404", + }, + } + for _, test := range tests { + allErrs := validateNextUpstream(test.inputS, field.NewPath("next-upstreams")) + if len(allErrs) < 0 { + t.Errorf("validateNextUpstream(%q) didn't return errors %v for invalid input.", test.inputS, allErrs) + } + } +} + func TestValidateDNS1035Label(t *testing.T) { validNames := []string{ "test",