From 0e6f0bb88d33dfcf3cb2cfdc01d4b62a265a2fde Mon Sep 17 00:00:00 2001
From: Zenara Daley <zenara.daley@shopify.com>
Date: Thu, 13 Sep 2018 10:39:52 -0400
Subject: [PATCH] enforce ^~ location modifier when rewrite-target annotation
 is set

---
 .../ingress/controller/template/template.go   | 34 +++++++++++++++++--
 .../controller/template/template_test.go      |  2 +-
 rootfs/etc/nginx/template/nginx.tmpl          |  6 ++--
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go
index 39cfa5ddb..0b9044110 100644
--- a/internal/ingress/controller/template/template.go
+++ b/internal/ingress/controller/template/template.go
@@ -155,6 +155,7 @@ var (
 		"buildOpentracing":            buildOpentracing,
 		"proxySetHeader":              proxySetHeader,
 		"buildInfluxDB":               buildInfluxDB,
+		"atLeastOneNeedsRewrite":      atLeastOneNeedsRewrite,
 	}
 )
 
@@ -287,9 +288,32 @@ func buildResolvers(res interface{}, disableIpv6 interface{}) string {
 	return strings.Join(r, " ") + ";"
 }
 
+func needsRewrite(location *ingress.Location) bool {
+	if len(location.Rewrite.Target) > 0 && location.Rewrite.Target != location.Path {
+		return true
+	}
+	return false
+}
+
+// atLeastOneNeedsRewrite checks if the nginx.ingress.kubernetes.io/rewrite-target annotation is used on the '/' path
+func atLeastOneNeedsRewrite(input interface{}) bool {
+	locations, ok := input.([]*ingress.Location)
+	if !ok {
+		glog.Errorf("expected an '[]*ingress.Location' type but %T was returned", input)
+		return false
+	}
+
+	for _, location := range locations {
+		if needsRewrite(location) {
+			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{}, rewrite bool) string {
 	location, ok := input.(*ingress.Location)
 	if !ok {
 		glog.Errorf("expected an '*ingress.Location' type but %T was returned", input)
@@ -297,7 +321,7 @@ func buildLocation(input interface{}) string {
 	}
 
 	path := location.Path
-	if len(location.Rewrite.Target) > 0 && location.Rewrite.Target != path {
+	if needsRewrite(location) {
 		if path == slash {
 			return fmt.Sprintf("~* %s", path)
 		}
@@ -310,6 +334,12 @@ func buildLocation(input interface{}) string {
 		return fmt.Sprintf(`~* ^%s%s`, path, baseuri)
 	}
 
+	if rewrite == true {
+		if path == slash {
+			return path
+		}
+		return fmt.Sprintf(`^~ %s`, path)
+	}
 	return path
 }
 
diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go
index 7068d295e..7f75689f5 100644
--- a/internal/ingress/controller/template/template_test.go
+++ b/internal/ingress/controller/template/template_test.go
@@ -382,7 +382,7 @@ func TestBuildLocation(t *testing.T) {
 			Rewrite: rewrite.Config{Target: tc.Target, AddBaseURL: tc.AddBaseURL},
 		}
 
-		newLoc := buildLocation(loc)
+		newLoc := buildLocation(loc, tc.Path != tc.Target)
 		if tc.Location != newLoc {
 			t.Errorf("%s: expected '%v' but returned %v", k, tc.Location, newLoc)
 		}
diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl
index 027010882..915ce66a7 100644
--- a/rootfs/etc/nginx/template/nginx.tmpl
+++ b/rootfs/etc/nginx/template/nginx.tmpl
@@ -452,8 +452,9 @@ http {
 
     {{/* build the maps that will be use to validate the Whitelist */}}
     {{ range $server := $servers }}
+    {{ $usesRewrite := atLeastOneNeedsRewrite $server.Locations }}
     {{ range $location := $server.Locations }}
-    {{ $path := buildLocation $location }}
+    {{ $path := buildLocation $location $usesRewrite }}
 
     {{ if isLocationAllowed $location }}
     {{ if gt (len $location.Whitelist.CIDR) 0 }}
@@ -818,8 +819,9 @@ stream {
         {{ $server.ServerSnippet }}
         {{ end }}
 
+        {{ $usesRewrite := atLeastOneNeedsRewrite $server.Locations }}
         {{ range $location := $server.Locations }}
-        {{ $path := buildLocation $location }}
+        {{ $path := buildLocation $location $usesRewrite }}
         {{ $proxySetHeader := proxySetHeader $location }}
         {{ $authPath := buildAuthLocation $location }}