Add redirect support in VS/VSR

internal/configs/virtualserver_test.go

@@ -1348,7 +1348,7 @@ func TestGenerateActionRedirectConfig(t *testing.T) {
 	for _, test := range tests {
 		result := generateActionRedirectConfig(test.redirect)
 		if !reflect.DeepEqual(result, test.expected) {
-			t.Errorf("generateActionRedirectConfig() = %v, want %v", result, test.expected)
+			t.Errorf("generateActionRedirectConfig() returned %v, but expected %v", result, test.expected)
 		}
 	}
 }

pkg/apis/configuration/validation/validation.go

@@ -627,10 +627,18 @@ func validateActionRedirect(redirect *v1.ActionRedirect, fieldPath *field.Path)
 	return allErrs
 }
 
-// Returns a slice of vars enclosed in ${}. For example ${scheme} would return scheme.
-func captureVariables(s string) [][]string {
-	regex := regexp.MustCompile(`\$\{([^}]+)\}`)
-	return regex.FindAllStringSubmatch(s, -1)
+var regexFmtRegexp = regexp.MustCompile(`\$\{([^}]*)\}`)
+
+// captureVariables returns a slice of vars enclosed in ${}. For example "${a} ${b}" would return ["a", "b"].
+func captureVariables(s string) []string {
+	var nVars []string
+
+	res := regexFmtRegexp.FindAllStringSubmatch(s, -1)
+	for _, n := range res {
+		nVars = append(nVars, n[1])
+	}
+
+	return nVars
 }
 
 // validRedirectVariableNames includes NGINX variables allowed to be used in redirects.
@@ -650,14 +658,9 @@ func validateRedirectURL(redirectURL string, fieldPath *field.Path) field.ErrorL
 
 	allErrs = append(allErrs, validateRedirectURLFmt(redirectURL, fieldPath)...)
 
-	if strings.Contains(redirectURL, "$") {
-		nginxVars := captureVariables(redirectURL)
-		if nginxVars != nil {
-			for _, v := range nginxVars {
-				nVar := v[1]
-				allErrs = append(allErrs, validateVariable(nVar, validRedirectVariableNames, fieldPath)...)
-			}
-		}
+	nginxVars := captureVariables(redirectURL)
+	for _, nVar := range nginxVars {
+		allErrs = append(allErrs, validateVariable(nVar, validRedirectVariableNames, fieldPath)...)
 	}
 
 	return allErrs
@@ -676,16 +679,12 @@ func validateRedirectURLFmt(str string, fieldPath *field.Path) field.ErrorList {
 		return append(allErrs, field.Invalid(fieldPath, str, msg))
 	}
 
-	if strings.Contains(str, "${}") {
-		return append(allErrs, field.Invalid(fieldPath, str, "must not contain any empty variable ${}"))
-	}
-
 	if strings.HasSuffix(str, "$") {
 		return append(allErrs, field.Invalid(fieldPath, str, "must not end with $"))
 	}
 
 	for i, c := range str {
-		if c == '$' && i+1 < len(str) {
+		if c == '$' {
 			msg := "variables must be enclosed in curly braces, for example ${host}"
 
 			if str[i+1] != '{' {
@@ -704,16 +703,8 @@ func validateRedirectURLFmt(str string, fieldPath *field.Path) field.ErrorList {
 func validateVariable(nVar string, validVars map[string]bool, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if nVar == "" {
-		return append(allErrs, field.Invalid(fieldPath, nVar, "$ must be followed by a set of curly braces to define a nginx variable"))
-	}
-
-	if strings.Contains(nVar, "{") || strings.Contains(nVar, "}") || strings.Contains(nVar, "$") {
-		return append(allErrs, field.Invalid(fieldPath, nVar, "nested nginx variables are not valid. Any nested '{', '}', or '$' should be removed"))
-	}
-
 	if !validVars[nVar] {
-		msg := fmt.Sprintf("'%v' contains an invalid nginx variable. Accepted variables are: %v", nVar, mapToPrettyString(validVars))
+		msg := fmt.Sprintf("'%v' contains an invalid NGINX variable. Accepted variables are: %v", nVar, mapToPrettyString(validVars))
 		allErrs = append(allErrs, field.Invalid(fieldPath, nVar, msg))
 	}
 

pkg/apis/configuration/validation/validation_test.go

@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"reflect"
 	"testing"
 
 	v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
@@ -901,6 +902,32 @@ func TestValidateActionFails(t *testing.T) {
 	}
 }
 
+func TestCaptureVariables(t *testing.T) {
+	tests := []struct {
+		s        string
+		expected []string
+	}{
+		{
+			"${scheme}://${host}",
+			[]string{"scheme", "host"},
+		},
+		{
+			"http://www.nginx.org",
+			nil,
+		},
+		{
+			"${}",
+			[]string{""},
+		},
+	}
+	for _, test := range tests {
+		result := captureVariables(test.s)
+		if !reflect.DeepEqual(result, test.expected) {
+			t.Errorf("captureVariables(%s) returned %v but expected %v", test.s, result, test.expected)
+		}
+	}
+}
+
 func TestValidateRedirectURL(t *testing.T) {
 	tests := []struct {
 		redirectURL string
@@ -998,6 +1025,10 @@ func TestValidateRedirectURLFails(t *testing.T) {
 			redirectURL: `http://${}`,
 			msg:         "url containing blank var",
 		},
+		{
+			redirectURL: `http://${abca`,
+			msg:         "url containing a var without ending }",
+		},
 	}
 
 	for _, test := range tests {