Add 'use regex' annotation to toggle nginx regex location modifier

internal/ingress/annotations/rewrite/main.go

@@ -38,6 +38,8 @@ type Config struct {
 	ForceSSLRedirect bool `json:"forceSSLRedirect"`
 	// AppRoot defines the Application Root that the Controller must redirect if it's in '/' context
 	AppRoot string `json:"appRoot"`
+	// UseRegex indicates whether or not the locations use regex paths
+	UseRegex bool `json:useRegex`
 }
 
 // Equal tests for equality between two Redirect types
@@ -66,6 +68,9 @@ func (r1 *Config) Equal(r2 *Config) bool {
 	if r1.AppRoot != r2.AppRoot {
 		return false
 	}
+	if r1.UseRegex != r2.UseRegex {
+		return false
+	}
 
 	return true
 }
@@ -94,6 +99,7 @@ func (a rewrite) Parse(ing *extensions.Ingress) (interface{}, error) {
 	abu, _ := parser.GetBoolAnnotation("add-base-url", ing)
 	bus, _ := parser.GetStringAnnotation("base-url-scheme", ing)
 	ar, _ := parser.GetStringAnnotation("app-root", ing)
+	ur, _ := parser.GetBoolAnnotation("use-regex", ing)
 
 	return &Config{
 		Target:           rt,
@@ -102,5 +108,6 @@ func (a rewrite) Parse(ing *extensions.Ingress) (interface{}, error) {
 		SSLRedirect:      sslRe,
 		ForceSSLRedirect: fSslRe,
 		AppRoot:          ar,
+		UseRegex:         ur,
 	}, nil
 }

internal/ingress/annotations/rewrite/main_test.go

@@ -178,3 +178,20 @@ func TestAppRoot(t *testing.T) {
 		t.Errorf("Unexpected value got in AppRoot")
 	}
 }
+
+func TestUseRegex(t *testing.T) {
+	ing := buildIngress()
+
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix("use-regex")] = "true"
+	ing.SetAnnotations(data)
+
+	i, _ := NewParser(mockBackend{redirect: true}).Parse(ing)
+	redirect, ok := i.(*Config)
+	if !ok {
+		t.Errorf("expected a App Context")
+	}
+	if redirect.UseRegex != true {
+		t.Errorf("Unexpected value got in UseRegex")
+	}
+}

internal/ingress/controller/controller.go

@@ -620,6 +620,10 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
 		sort.SliceStable(value.Locations, func(i, j int) bool {
 			return value.Locations[i].Path > value.Locations[j].Path
 		})
+
+		sort.SliceStable(value.Locations, func(i, j int) bool {
+			return len(value.Locations[i].Path) > len(value.Locations[j].Path)
+		})
 		aServers = append(aServers, value)
 	}
 

internal/ingress/controller/template/template.go

@@ -156,7 +156,8 @@ var (
 		"buildOpentracing":            buildOpentracing,
 		"proxySetHeader":              proxySetHeader,
 		"buildInfluxDB":               buildInfluxDB,
-		"atLeastOneNeedsRewrite":      atLeastOneNeedsRewrite,
+		"enforceRegexModifier":        enforceRegexModifier,
+		"stripLocationModifer":        stripLocationModifer,
 	}
 )
 
@@ -296,8 +297,13 @@ func needsRewrite(location *ingress.Location) bool {
 	return false
 }
 
-// atLeastOneNeedsRewrite checks if the nginx.ingress.kubernetes.io/rewrite-target annotation is used on the '/' path
-func atLeastOneNeedsRewrite(input interface{}) bool {
+func stripLocationModifer(path string) string {
+	return strings.TrimLeft(path, "~* ")
+}
+
+// enforceRegexModifier checks if the "rewrite-target" or "use-regex" annotation
+// is used on any location path within a server
+func enforceRegexModifier(input interface{}) bool {
 	locations, ok := input.([]*ingress.Location)
 	if !ok {
 		glog.Errorf("expected an '[]*ingress.Location' type but %T was returned", input)
@@ -305,7 +311,7 @@ func atLeastOneNeedsRewrite(input interface{}) bool {
 	}
 
 	for _, location := range locations {
-		if needsRewrite(location) {
+		if needsRewrite(location) || location.Rewrite.UseRegex {
 			return true
 		}
 	}
@@ -314,7 +320,9 @@ func atLeastOneNeedsRewrite(input interface{}) bool {
 
 // buildLocation produces the location string, if the ingress has redirects
 // (specified through the nginx.ingress.kubernetes.io/rewrite-target annotation)
-func buildLocation(input interface{}, rewrite bool) string {
+// TODO: return quotes around returned location path to prevent regex from breaking under certain conditions, see:
+// https://github.com/kubernetes/ingress-nginx/issues/3155
+func buildLocation(input interface{}, enforceRegex bool) string {
 	location, ok := input.(*ingress.Location)
 	if !ok {
 		glog.Errorf("expected an '*ingress.Location' type but %T was returned", input)
@@ -324,7 +332,7 @@ func buildLocation(input interface{}, rewrite bool) string {
 	path := location.Path
 	if needsRewrite(location) {
 		if path == slash {
-			return fmt.Sprintf("~* %s", path)
+			return fmt.Sprintf("~* ^%s", path)
 		}
 		// baseuri regex will parse basename from the given location
 		baseuri := `(?<baseuri>.*)`
@@ -335,11 +343,8 @@ func buildLocation(input interface{}, rewrite bool) string {
 		return fmt.Sprintf(`~* ^%s%s`, path, baseuri)
 	}
 
-	if rewrite {
-		if path == slash {
-			return path
-		}
-		return fmt.Sprintf(`^~ %s`, path)
+	if enforceRegex {
+		return fmt.Sprintf(`~* ^%s`, path)
 	}
 	return path
 }

internal/ingress/controller/template/template_test.go

@@ -50,7 +50,7 @@ var (
 		XForwardedPrefix            bool
 		DynamicConfigurationEnabled bool
 		SecureBackend               bool
-		atLeastOneNeedsRewrite      bool
+		enforceRegex                bool
 	}{
 		"when secure backend enabled": {
 			"/",
@@ -127,7 +127,7 @@ var (
 		"redirect / to /jenkins": {
 			"/",
 			"/jenkins",
-			"~* /",
+			"~* ^/",
 			`
 rewrite (?i)/(.*) /jenkins/$1 break;
 rewrite (?i)/$ /jenkins/ break;
@@ -191,7 +191,7 @@ proxy_pass http://upstream-name;
 		"redirect / to /jenkins and rewrite": {
 			"/",
 			"/jenkins",
-			"~* /",
+			"~* ^/",
 			`
 rewrite (?i)/(.*) /jenkins/$1 break;
 rewrite (?i)/$ /jenkins/ break;
@@ -286,7 +286,7 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
 		"redirect / to /something with sticky enabled": {
 			"/",
 			"/something",
-			`~* /`,
+			`~* ^/`,
 			`
 rewrite (?i)/(.*) /something/$1 break;
 rewrite (?i)/$ /something/ break;
@@ -302,7 +302,7 @@ proxy_pass http://sticky-upstream-name;
 		"redirect / to /something with sticky and dynamic config enabled": {
 			"/",
 			"/something",
-			`~* /`,
+			`~* ^/`,
 			`
 rewrite (?i)/(.*) /something/$1 break;
 rewrite (?i)/$ /something/ break;
@@ -332,22 +332,10 @@ proxy_pass http://sticky-upstream-name;
 			false,
 			false,
 			true},
-		"do not use ^~ location modifier on index when ingress does not use rewrite target but at least one other ingress does": {
-			"/",
-			"/",
-			"/",
-			"proxy_pass http://upstream-name;",
-			false,
-			"",
-			false,
-			false,
-			false,
-			false,
-			true},
-		"use ^~ location modifier when ingress does not use rewrite target but at least one other ingress does": {
+		"use ~* location modifier when ingress does not use rewrite/regex target but at least one other ingress does": {
 			"/something",
 			"/something",
-			"^~ /something",
+			"~* ^/something",
 			"proxy_pass http://upstream-name;",
 			false,
 			"",
@@ -425,7 +413,7 @@ func TestBuildLocation(t *testing.T) {
 			Rewrite: rewrite.Config{Target: tc.Target, AddBaseURL: tc.AddBaseURL},
 		}
 
-		newLoc := buildLocation(loc, tc.atLeastOneNeedsRewrite)
+		newLoc := buildLocation(loc, tc.enforceRegex)
 		if tc.Location != newLoc {
 			t.Errorf("%s: expected '%v' but returned %v", k, tc.Location, newLoc)
 		}