diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index a0746be8a..3666defbf 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -171,6 +171,7 @@ var ( "buildOpentracing": buildOpentracing, "proxySetHeader": proxySetHeader, "buildInfluxDB": buildInfluxDB, + "enforceRegexModifier": enforceRegexModifier, "buildCustomErrorDeps": buildCustomErrorDeps, "buildCustomErrorLocationsPerServer": buildCustomErrorLocationsPerServer, "shouldLoadModSecurityModule": shouldLoadModSecurityModule, @@ -372,9 +373,26 @@ 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{}) string { +func buildLocation(input interface{}, enforceRegex bool) string { location, ok := input.(*ingress.Location) if !ok { klog.Errorf("expected an '*ingress.Location' type but %T was returned", input) @@ -382,7 +400,7 @@ func buildLocation(input interface{}) string { } path := location.Path - if location.Rewrite.UseRegex { + if enforceRegex { return fmt.Sprintf(`~* "^%s"`, path) } diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index 3243dbda3..2f2dd670a 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 - UseRegex bool + enforceRegex 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) + actual := buildLocation(invalidType, true) 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, UseRegex: tc.UseRegex}, + Rewrite: rewrite.Config{Target: tc.Target}, } - newLoc := buildLocation(loc) + newLoc := buildLocation(loc, tc.enforceRegex) if tc.Location != newLoc { t.Errorf("%s: expected '%v' but returned %v", k, tc.Location, newLoc) } @@ -1195,6 +1195,32 @@ 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 0b6240d1d..834a331ae 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -903,8 +903,9 @@ stream { {{ buildMirrorLocations $server.Locations }} + {{ $enforceRegex := enforceRegexModifier $server.Locations }} {{ range $location := $server.Locations }} - {{ $path := buildLocation $location }} + {{ $path := buildLocation $location $enforceRegex }} {{ $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 b357ce940..37c5a75b9 100644 --- a/test/e2e/annotations/rewrite.go +++ b/test/e2e/annotations/rewrite.go @@ -95,8 +95,7 @@ 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") @@ -130,7 +129,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'") @@ -171,7 +170,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}" {`) })