diff --git a/docs/virtualserver-and-virtualserverroute.md b/docs/virtualserver-and-virtualserverroute.md index 877fab6c01..f9b42684dc 100644 --- a/docs/virtualserver-and-virtualserverroute.md +++ b/docs/virtualserver-and-virtualserverroute.md @@ -220,7 +220,7 @@ tls: ### Upstream.Healthcheck -The Healthcheck defines an [active health check](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-health-check/). In the example below we enable a health check for an upstream and configure all the available parameters. +The Healthcheck defines an [active health check](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-health-check/). In the example below we enable a health check for an upstream and configure all the available parameters: ```yaml name: tea @@ -249,8 +249,8 @@ healthCheck: | ----- | ----------- | ---- | -------- | | `enable` | Enables a health check for an upstream server. The default is `false`. | `boolean` | No | | `path` | The path used for health check requests. The default is `/`. | `string` | No | -| `interval` | The interval between two consecutive health checks. The default is `5s`. | `time` | No | -| `jitter` | The time within which each health check will be randomly delayed. By default, there is no delay. | `time` | No | +| `interval` | The interval between two consecutive health checks. The default is `5s`. | `string` | No | +| `jitter` | The time within which each health check will be randomly delayed. By default, there is no delay. | `string` | No | | `fails` | The number of consecutive failed health checks of a particular upstream server after which this server will be considered unhealthy. The default is `1`. | `integer` | No | | `passes` | The number of consecutive passed health checks of a particular upstream server after which the server will be considered healthy. The default is `1`. | `integer` | No | | `port` | The port used for health check requests. By default, the port of the upstream is used. Note: in contrast with the port of the upstream, this port is not a service port, but a port of a pod. | `integer` | No | diff --git a/internal/configs/version2/config.go b/internal/configs/version2/config.go index 73dd47f088..c069736c43 100644 --- a/internal/configs/version2/config.go +++ b/internal/configs/version2/config.go @@ -2,10 +2,11 @@ package version2 // VirtualServerConfig holds NGINX configuration for a VirtualServer. type VirtualServerConfig struct { - Server Server - Upstreams []Upstream - SplitClients []SplitClient - Maps []Map + Server Server + Upstreams []Upstream + SplitClients []SplitClient + Maps []Map + StatusMatches []StatusMatch } // Upstream defines an upstream. diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index a3773aeeb3..9970cedf57 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -30,6 +30,12 @@ map {{ $m.Source }} {{ $m.Variable }} { } {{ end }} +{{ range $m := .StatusMatches }} +match {{ $m.Name }} { + status {{ $m.Code }}; +} +{{ end }} + {{ $s := .Server }} server { listen 80{{ if $s.ProxyProtocol }} proxy_protocol{{ end }}; @@ -85,7 +91,7 @@ server { {{ range $hc := $s.HealthChecks }} location @hc-{{ $hc.Name }} { {{ range $n, $v := $hc.Headers }} - proxy_set_header {{ $n }} {{ $v }}; + proxy_set_header {{ $n }} "{{ $v }}"; {{ end }} proxy_connect_timeout {{ $hc.ProxyConnectTimeout }}; proxy_read_timeout {{ $hc.ProxyReadTimeout }}; diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 24689868f8..697296e24e 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -284,6 +284,7 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam Upstreams: upstreams, SplitClients: splitClients, Maps: maps, + StatusMatches: statusMatches, Server: version2.Server{ ServerName: virtualServerEx.VirtualServer.Spec.Host, ProxyProtocol: baseCfgParams.ProxyProtocol, diff --git a/pkg/apis/configuration/validation/validation.go b/pkg/apis/configuration/validation/validation.go index 6847578efc..c3e75bfc1f 100644 --- a/pkg/apis/configuration/validation/validation.go +++ b/pkg/apis/configuration/validation/validation.go @@ -3,6 +3,7 @@ package validation import ( "fmt" "regexp" + "strconv" "strings" "github.com/nginxinc/kubernetes-ingress/internal/configs" @@ -57,7 +58,7 @@ func validateTLS(tls *v1alpha1.TLS, fieldPath *field.Path) field.ErrorList { return validateSecretName(tls.Secret, fieldPath.Child("secret")) } -func validatePositiveInt(n int, fieldPath *field.Path) field.ErrorList { +func validatePositiveIntOrZero(n int, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if n < 0 { @@ -67,7 +68,7 @@ func validatePositiveInt(n int, fieldPath *field.Path) field.ErrorList { return allErrs } -func validatePositiveIntOrZero(n *int, fieldPath *field.Path) field.ErrorList { +func validatePositiveIntOrZeroFromPointer(n *int, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if n == nil { return allErrs @@ -122,11 +123,14 @@ func validateUpstreamHealthCheck(hc *v1alpha1.HealthCheck, fieldPath *field.Path return allErrs } - allErrs = append(allErrs, validatePath(hc.Path, fieldPath.Child("path"))...) + if hc.Path != "" { + allErrs = append(allErrs, validatePath(hc.Path, fieldPath.Child("path"))...) + } + allErrs = append(allErrs, validateTime(hc.Interval, fieldPath.Child("interval"))...) allErrs = append(allErrs, validateTime(hc.Jitter, fieldPath.Child("jitter"))...) - allErrs = append(allErrs, validatePositiveInt(hc.Fails, fieldPath.Child("fails"))...) - allErrs = append(allErrs, validatePositiveInt(hc.Passes, fieldPath.Child("passes"))...) + allErrs = append(allErrs, validatePositiveIntOrZero(hc.Fails, fieldPath.Child("fails"))...) + allErrs = append(allErrs, validatePositiveIntOrZero(hc.Passes, fieldPath.Child("passes"))...) allErrs = append(allErrs, validateTime(hc.ConnectTimeout, fieldPath.Child("connect-timeout"))...) allErrs = append(allErrs, validateTime(hc.ReadTimeout, fieldPath.Child("read-timeout"))...) allErrs = append(allErrs, validateTime(hc.SendTimeout, fieldPath.Child("send-timeout"))...) @@ -146,20 +150,52 @@ func validateUpstreamHealthCheck(hc *v1alpha1.HealthCheck, fieldPath *field.Path return allErrs } -const statusMatchFmt = "(\\d*|\\s|\\-|\\!\\s)*" -const statusMatchFmtErrMsg string = "a valid statusMatch must consist of digits, spaces, `! ` or `-`" - -var statusMatchRegexp = regexp.MustCompile("^" + statusMatchFmt + "$") - func validateStatusMatch(s string, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if !statusMatchRegexp.MatchString(s) { - msg := validation.RegexError(statusMatchFmtErrMsg, statusMatchFmt, "200", "! 500", "200-399") - return append(allErrs, field.Invalid(fieldPath, s, msg)) + + if s == "" { + return allErrs + } + + if strings.HasPrefix(s, "!") { + if !strings.HasPrefix(s, "! ") { + allErrs = append(allErrs, field.Invalid(fieldPath, s, "must have an space character after the `!`")) + } + } + + statuses := strings.Split(s, " ") + for i, value := range statuses { + if value == "!" { + if i != 0 { + allErrs = append(allErrs, field.Invalid(fieldPath, s, "`!` can only appear once at the beginning")) + } + } else if strings.Contains(s, "-") { + statusRange := strings.Split(value, "-") + for _, code := range statusRange { + if msg := validateStatusCodeInRange(code); msg != "" { + allErrs = append(allErrs, field.Invalid(fieldPath, s, msg)) + } + } + } else if msg := validateStatusCodeInRange(value); msg != "" { + allErrs = append(allErrs, field.Invalid(fieldPath, s, msg)) + } } + return allErrs } +func validateStatusCodeInRange(status string) string { + code, err := strconv.ParseInt(status, 10, 64) + if err != nil { + return fmt.Sprintf("%v must be a valid integer", status) + } + + if code < 100 || code > 999 { + return validation.InclusiveRangeError(100, 999) + } + return "" +} + func validateHeader(h v1alpha1.Header, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -178,15 +214,14 @@ func validateHeader(h v1alpha1.Header, fieldPath *field.Path) field.ErrorList { return allErrs } -const headerValueFmt = "[^{};'\"]*" -const headerValueFmtErrMsg string = "a valid header value must not include `'`, `\"`, {`, `}` or `;`" +const headerValueFmt = `([^{};$"\\]|\\.)*` +const headerValueFmtErrMsg string = `a valid header value must not include '$', '{', '}' or ';', must have all '"' escaped and must not end with an unescaped '\'` var headerValueFmtRegexp = regexp.MustCompile("^" + headerValueFmt + "$") func isValidHeaderValue(s string) []string { if !headerValueFmtRegexp.MatchString(s) { - return []string{validation.RegexError(headerValueFmtErrMsg, headerValueFmt, "my.service", "service")} - + return []string{validation.RegexError(headerValueFmtErrMsg, headerValueFmt, "my.service", "\"my service\"")} } return nil } @@ -230,13 +265,12 @@ func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path, isP 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, validatePositiveIntOrZeroFromPointer(&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"))...) - allErrs = append(allErrs, validatePositiveIntOrZero(u.MaxFails, idxPath.Child("max-fails"))...) - allErrs = append(allErrs, validatePositiveIntOrZero(u.MaxConns, idxPath.Child("max-conns"))...) - allErrs = append(allErrs, validatePositiveIntOrZero(u.Keepalive, idxPath.Child("keepalive"))...) + 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, validateUpstreamHealthCheck(u.HealthCheck, idxPath.Child("healthCheck"))...) for _, msg := range validation.IsValidPortNum(int(u.Port)) { diff --git a/pkg/apis/configuration/validation/validation_test.go b/pkg/apis/configuration/validation/validation_test.go index 78489abcd0..4550e26184 100644 --- a/pkg/apis/configuration/validation/validation_test.go +++ b/pkg/apis/configuration/validation/validation_test.go @@ -1566,7 +1566,7 @@ func createPointerFromInt(n int) *int { return &n } -func TestValidatePositiveIntOrZero(t *testing.T) { +func TestValidatePositiveIntOrZeroFromPointer(t *testing.T) { tests := []struct { number *int msg string @@ -1585,21 +1585,53 @@ func TestValidatePositiveIntOrZero(t *testing.T) { }, } + for _, test := range tests { + allErrs := validatePositiveIntOrZeroFromPointer(test.number, field.NewPath("int-field")) + + if len(allErrs) != 0 { + t.Errorf("validatePositiveIntOrZeroFromPointer returned errors for case: %v", test.msg) + } + } +} + +func TestValidatePositiveIntOrZeroFromPointerFails(t *testing.T) { + number := createPointerFromInt(-1) + allErrs := validatePositiveIntOrZeroFromPointer(number, field.NewPath("int-field")) + + if len(allErrs) == 0 { + t.Error("validatePositiveIntOrZeroFromPointer returned no errors for case: invalid (-1)") + } +} + +func TestValidatePositiveIntOrZero(t *testing.T) { + tests := []struct { + number int + msg string + }{ + { + number: 0, + msg: "valid (0)", + }, + { + number: 1, + msg: "valid (1)", + }, + } + for _, test := range tests { allErrs := validatePositiveIntOrZero(test.number, field.NewPath("int-field")) if len(allErrs) != 0 { - t.Errorf("validatePositiveInt returned errors for case: %v", test.msg) + t.Errorf("validatePositiveIntOrZero returned errors for case: %v", test.msg) } } } func TestValidatePositiveIntOrZeroFails(t *testing.T) { - number := createPointerFromInt(-1) - allErrs := validatePositiveIntOrZero(number, field.NewPath("int-field")) + allErrs := validatePositiveIntOrZero(-1, field.NewPath("int-field")) if len(allErrs) == 0 { - t.Error("validatePositiveInt returned no errors for case: invalid (-1)") + t.Error("validatePositiveIntOrZero returned no errors for case: invalid (-1)") } } @@ -1709,10 +1741,30 @@ func TestValidateStatusMatchFails(t *testing.T) { status: "qwe", msg: "Invalid: no digits", }, + { + status: "!", + msg: "Invalid: `!` character only", + }, { status: "!500", msg: "Invalid: no space after !", }, + { + status: "0", + msg: "Invalid: status out of range (below 100)", + }, + { + status: "1000", + msg: "Invalid: status out of range (above 999)", + }, + { + status: "20-600", + msg: "Invalid: codes in range is out of range", + }, + { + status: "! 200 ! 500", + msg: "Invalid: 2 exclamation symbols", + }, } for _, test := range tests { allErrs := validateStatusMatch(test.status, field.NewPath("statusMatch")) @@ -1724,14 +1776,30 @@ func TestValidateStatusMatchFails(t *testing.T) { } func TestValidateHeader(t *testing.T) { - header := v1alpha1.Header{ - Name: "Host", - Value: "my.service", + tests := []struct { + header v1alpha1.Header + msg string + }{ + { + header: v1alpha1.Header{ + Name: "Host", + Value: "my.service", + }, + }, + { + header: v1alpha1.Header{ + Name: "Host", + Value: `\"my.service\"`, + }, + }, } - allErrs := validateHeader(header, field.NewPath("headers")) - if len(allErrs) != 0 { - t.Errorf("validateHeader returned errors %v for valid input %v", allErrs, header) + for _, test := range tests { + allErrs := validateHeader(test.header, field.NewPath("headers")) + + if len(allErrs) != 0 { + t.Errorf("validateHeader returned errors %v for valid input %v", allErrs, test.header) + } } } @@ -1768,6 +1836,20 @@ func TestValidateHeaderFails(t *testing.T) { }, msg: "Invalid value with `;`", }, + { + header: v1alpha1.Header{ + Name: "Host", + Value: `"my.service`, + }, + msg: `Invalid value with unescaped '"'`, + }, + { + header: v1alpha1.Header{ + Name: "Host", + Value: `my.service\`, + }, + msg: "Invalid value with ending '\\'", + }, } for _, test := range tests { allErrs := validateHeader(test.header, field.NewPath("headers"))