diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 3666defbf9..a0746be8ac 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -171,7 +171,6 @@ var ( "buildOpentracing": buildOpentracing, "proxySetHeader": proxySetHeader, "buildInfluxDB": buildInfluxDB, - "enforceRegexModifier": enforceRegexModifier, "buildCustomErrorDeps": buildCustomErrorDeps, "buildCustomErrorLocationsPerServer": buildCustomErrorLocationsPerServer, "shouldLoadModSecurityModule": shouldLoadModSecurityModule, @@ -373,26 +372,9 @@ func needsRewrite(location *ingress.Location) bool { return false } -// 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 { - klog.Errorf("expected an '[]*ingress.Location' type but %T was returned", input) - return false - } - - for _, location := range locations { - if needsRewrite(location) || location.Rewrite.UseRegex { - return true - } - } - return false -} - // buildLocation produces the location string, if the ingress has redirects // (specified through the nginx.ingress.kubernetes.io/rewrite-target annotation) -func buildLocation(input interface{}, enforceRegex bool) string { +func buildLocation(input interface{}) string { location, ok := input.(*ingress.Location) if !ok { klog.Errorf("expected an '*ingress.Location' type but %T was returned", input) @@ -400,7 +382,7 @@ func buildLocation(input interface{}, enforceRegex bool) string { } path := location.Path - if enforceRegex { + if location.Rewrite.UseRegex { return fmt.Sprintf(`~* "^%s"`, path) } diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index 2f2dd670a1..3243dbda36 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -64,7 +64,7 @@ var ( Sticky bool XForwardedPrefix string SecureBackend bool - enforceRegex bool + UseRegex bool }{ "when secure backend enabled": { "/", @@ -275,7 +275,7 @@ func TestQuote(t *testing.T) { func TestBuildLocation(t *testing.T) { invalidType := &ingress.Ingress{} expected := "/" - actual := buildLocation(invalidType, true) + actual := buildLocation(invalidType) if !reflect.DeepEqual(expected, actual) { t.Errorf("Expected '%v' but returned '%v'", expected, actual) @@ -284,10 +284,10 @@ func TestBuildLocation(t *testing.T) { for k, tc := range tmplFuncTestcases { loc := &ingress.Location{ Path: tc.Path, - Rewrite: rewrite.Config{Target: tc.Target}, + Rewrite: rewrite.Config{Target: tc.Target, UseRegex: tc.UseRegex}, } - newLoc := buildLocation(loc, tc.enforceRegex) + newLoc := buildLocation(loc) if tc.Location != newLoc { t.Errorf("%s: expected '%v' but returned %v", k, tc.Location, newLoc) } @@ -1195,32 +1195,6 @@ func TestBuildOpenTracing(t *testing.T) { } -func TestEnforceRegexModifier(t *testing.T) { - invalidType := &ingress.Ingress{} - expected := false - actual := enforceRegexModifier(invalidType) - - if expected != actual { - t.Errorf("Expected '%v' but returned '%v'", expected, actual) - } - - locs := []*ingress.Location{ - { - Rewrite: rewrite.Config{ - Target: "/alright", - UseRegex: true, - }, - Path: "/ok", - }, - } - expected = true - actual = enforceRegexModifier(locs) - - if expected != actual { - t.Errorf("Expected '%v' but returned '%v'", expected, actual) - } -} - func TestShouldLoadModSecurityModule(t *testing.T) { // ### Invalid argument type tests ### // The first tests should return false. diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 834a331ae0..0b6240d1d0 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -903,9 +903,8 @@ stream { {{ buildMirrorLocations $server.Locations }} - {{ $enforceRegex := enforceRegexModifier $server.Locations }} {{ range $location := $server.Locations }} - {{ $path := buildLocation $location $enforceRegex }} + {{ $path := buildLocation $location }} {{ $proxySetHeader := proxySetHeader $location }} {{ $authPath := buildAuthLocation $location $all.Cfg.GlobalExternalAuth.URL }} {{ $applyGlobalAuth := shouldApplyGlobalAuth $location $all.Cfg.GlobalExternalAuth.URL }} diff --git a/test/e2e/annotations/rewrite.go b/test/e2e/annotations/rewrite.go index 37c5a75b9c..b357ce9407 100644 --- a/test/e2e/annotations/rewrite.go +++ b/test/e2e/annotations/rewrite.go @@ -95,7 +95,8 @@ var _ = framework.DescribeAnnotation("rewrite-target use-regex enable-rewrite-lo f.WaitForNginxServer(host, func(server string) bool { - return strings.Contains(server, `location ~* "^/" {`) && strings.Contains(server, `location ~* "^/.well-known/acme/challenge" {`) + return strings.Contains(server, `location / {`) && + strings.Contains(server, `location /.well-known/acme/challenge {`) }) ginkgo.By("making a second request to the non-rewritten location") @@ -129,7 +130,7 @@ var _ = framework.DescribeAnnotation("rewrite-target use-regex enable-rewrite-lo f.WaitForNginxServer(host, func(server string) bool { - return strings.Contains(server, `location ~* "^/foo" {`) && strings.Contains(server, `location ~* "^/foo.+" {`) + return strings.Contains(server, `location /foo {`) && strings.Contains(server, `location ~* "^/foo.+" {`) }) ginkgo.By("ensuring '/foo' matches '~* ^/foo'") @@ -170,7 +171,7 @@ var _ = framework.DescribeAnnotation("rewrite-target use-regex enable-rewrite-lo f.WaitForNginxServer(host, func(server string) bool { - return strings.Contains(server, `location ~* "^/foo/bar/bar" {`) && + return strings.Contains(server, `location /foo/bar/bar {`) && strings.Contains(server, `location ~* "^/foo/bar/[a-z]{3}" {`) })