Merge pull request #2899 from jeroenvand/jvd-fix-rewrite

fixed rewrites for paths not ending in /
kubernetes · Aug 19, 2018 · c083599 · c083599
2 parents a982713 + e428095
commit c083599
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 8 deletions.
diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go
@@ -470,7 +470,6 @@ func buildProxyPass(host string, b interface{}, loc interface{}, dynamicConfigur
 		var xForwardedPrefix string
 
 		if location.Rewrite.AddBaseURL {
-			// path has a slash suffix, so that it can be connected with baseuri directly
 			bPath := fmt.Sprintf("%s$escaped_base_uri", path)
 			regex := `(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)`
 			scheme := "$scheme"
@@ -494,15 +493,16 @@ subs_filter '%v' '$1<base href="%v://$http_host%v">' ro;
 			// ie /something to /
 			return fmt.Sprintf(`
 rewrite (?i)%s(.*) /$1 break;
-rewrite (?i)%s / break;
+rewrite (?i)%s$ / break;
 %v%v %s%s;
 %v`, path, location.Path, xForwardedPrefix, proxyPass, proto, upstreamName, abu)
 		}
 
 		return fmt.Sprintf(`
 rewrite (?i)%s(.*) %s/$1 break;
+rewrite (?i)%s$ %s/ break;
 %v%v %s%s;
-%v`, path, location.Rewrite.Target, xForwardedPrefix, proxyPass, proto, upstreamName, abu)
+%v`, path, location.Rewrite.Target, location.Path, location.Rewrite.Target, xForwardedPrefix, proxyPass, proto, upstreamName, abu)
 	}
 
 	// default proxy_pass

diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go
@@ -123,6 +123,7 @@ var (
 			"~* /",
 			`
 rewrite (?i)/(.*) /jenkins/$1 break;
+rewrite (?i)/$ /jenkins/ break;
 proxy_pass http://upstream-name;
 `,
 			false,
@@ -137,7 +138,7 @@ proxy_pass http://upstream-name;
 			`~* ^/something\/?(?<baseuri>.*)`,
 			`
 rewrite (?i)/something/(.*) /$1 break;
-rewrite (?i)/something / break;
+rewrite (?i)/something$ / break;
 proxy_pass http://upstream-name;
 `,
 			false,
@@ -152,6 +153,7 @@ proxy_pass http://upstream-name;
 			"~* ^/end-with-slash/(?<baseuri>.*)",
 			`
 rewrite (?i)/end-with-slash/(.*) /not-root/$1 break;
+rewrite (?i)/end-with-slash/$ /not-root/ break;
 proxy_pass http://upstream-name;
 `,
 			false,
@@ -166,6 +168,7 @@ proxy_pass http://upstream-name;
 			`~* ^/something-complex\/?(?<baseuri>.*)`,
 			`
 rewrite (?i)/something-complex/(.*) /not-root/$1 break;
+rewrite (?i)/something-complex$ /not-root/ break;
 proxy_pass http://upstream-name;
 `,
 			false,
@@ -180,6 +183,7 @@ proxy_pass http://upstream-name;
 			"~* /",
 			`
 rewrite (?i)/(.*) /jenkins/$1 break;
+rewrite (?i)/$ /jenkins/ break;
 proxy_pass http://upstream-name;
 
 set_escape_uri $escaped_base_uri $baseuri;
@@ -197,7 +201,7 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
 			`~* ^/something\/?(?<baseuri>.*)`,
 			`
 rewrite (?i)/something/(.*) /$1 break;
-rewrite (?i)/something / break;
+rewrite (?i)/something$ / break;
 proxy_pass http://upstream-name;
 
 set_escape_uri $escaped_base_uri $baseuri;
@@ -215,6 +219,7 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
 			`~* ^/end-with-slash/(?<baseuri>.*)`,
 			`
 rewrite (?i)/end-with-slash/(.*) /not-root/$1 break;
+rewrite (?i)/end-with-slash/$ /not-root/ break;
 proxy_pass http://upstream-name;
 
 set_escape_uri $escaped_base_uri $baseuri;
@@ -232,6 +237,7 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
 			`~* ^/something-complex\/?(?<baseuri>.*)`,
 			`
 rewrite (?i)/something-complex/(.*) /not-root/$1 break;
+rewrite (?i)/something-complex$ /not-root/ break;
 proxy_pass http://upstream-name;
 
 set_escape_uri $escaped_base_uri $baseuri;
@@ -249,7 +255,7 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
 			`~* ^/something\/?(?<baseuri>.*)`,
 			`
 rewrite (?i)/something/(.*) /$1 break;
-rewrite (?i)/something / break;
+rewrite (?i)/something$ / break;
 proxy_pass http://upstream-name;
 
 set_escape_uri $escaped_base_uri $baseuri;
@@ -267,6 +273,7 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
 			`~* /`,
 			`
 rewrite (?i)/(.*) /something/$1 break;
+rewrite (?i)/$ /something/ break;
 proxy_pass http://sticky-upstream-name;
 `,
 			false,
@@ -281,6 +288,7 @@ proxy_pass http://sticky-upstream-name;
 			`~* /`,
 			`
 rewrite (?i)/(.*) /something/$1 break;
+rewrite (?i)/$ /something/ break;
 proxy_pass http://upstream_balancer;
 `,
 			false,
@@ -295,6 +303,7 @@ proxy_pass http://upstream_balancer;
 			`~* ^/there\/?(?<baseuri>.*)`,
 			`
 rewrite (?i)/there/(.*) /something/$1 break;
+rewrite (?i)/there$ /something/ break;
 proxy_set_header X-Forwarded-Prefix "/there/";
 proxy_pass http://sticky-upstream-name;
 `,

diff --git a/test/e2e/annotations/rewrite.go b/test/e2e/annotations/rewrite.go
@@ -56,7 +56,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Rewrite", func() {
 		err = f.WaitForNginxServer(host,
 			func(server string) bool {
 				return strings.Contains(server, "rewrite (?i)/something/(.*) /$1 break;") &&
-					strings.Contains(server, "rewrite (?i)/something / break;")
+					strings.Contains(server, "rewrite (?i)/something$ / break;")
 			})
 		Expect(err).NotTo(HaveOccurred())
 
@@ -114,7 +114,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Rewrite", func() {
 
 		logs, err := f.NginxLogs()
 		Expect(err).ToNot(HaveOccurred())
-		Expect(logs).To(ContainSubstring(`"(?i)/something" matches "/something", client:`))
+		Expect(logs).To(ContainSubstring(`"(?i)/something$" matches "/something", client:`))
 		Expect(logs).To(ContainSubstring(`rewritten data: "/", args: "",`))
 	})
 })