PR Changes

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 |

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.

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 }};

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,

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)) {

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"))