Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow curly braces to be used in regex paths #3182

Merged
merged 1 commit into from
Oct 4, 2018
Merged

Allow curly braces to be used in regex paths #3182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/user-guide/ingress-path-matching.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Regular Expression Support

The ingress controller supports **case insensitive** regular expressions in the `spec.rules.http.paths.path` field. __Currently curly braces `{}` cannot be used in regular expressions due to a [known issue](https://github.com/kubernetes/ingress-nginx/issues/3155).__
The ingress controller supports **case insensitive** regular expressions in the `spec.rules.http.paths.path` field.


See the [description](./nginx-configuration/annotations.md#use-regex) of the `use-regex` annotation for more details.
Expand Down
14 changes: 6 additions & 8 deletions internal/ingress/controller/template/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,6 @@ func enforceRegexModifier(input interface{}) bool {

// buildLocation produces the location string, if the ingress has redirects
// (specified through the nginx.ingress.kubernetes.io/rewrite-target annotation)
// 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 {
Expand All @@ -340,11 +338,11 @@ func buildLocation(input interface{}, enforceRegex bool) string {
// Not treat the slash after "location path" as a part of baseuri
baseuri = fmt.Sprintf(`\/?%s`, baseuri)
}
return fmt.Sprintf(`~* ^%s%s`, path, baseuri)
return fmt.Sprintf(`~* "^%s%s"`, path, baseuri)
}

if enforceRegex {
return fmt.Sprintf(`~* ^%s`, path)
return fmt.Sprintf(`~* "^%s"`, path)
}
return path
}
Expand Down Expand Up @@ -528,15 +526,15 @@ subs_filter '%v' '$1<base href="%v://$http_host%v">' ro;
// special case redirect to /
// ie /something to /
return fmt.Sprintf(`
rewrite (?i)%s(.*) /$1 break;
rewrite (?i)%s$ / break;
rewrite "(?i)%s(.*)" /$1 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;
rewrite "(?i)%s(.*)" %s/$1 break;
rewrite "(?i)%s$" %s/ break;
%v%v %s%s;
%v`, path, location.Rewrite.Target, location.Path, location.Rewrite.Target, xForwardedPrefix, proxyPass, proto, upstreamName, abu)
}
Expand Down
66 changes: 33 additions & 33 deletions internal/ingress/controller/template/template_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ var (
"/jenkins",
"~* ^/",
`
rewrite (?i)/(.*) /jenkins/$1 break;
rewrite (?i)/$ /jenkins/ break;
rewrite "(?i)/(.*)" /jenkins/$1 break;
rewrite "(?i)/$" /jenkins/ break;
proxy_pass http://upstream-name;
`,
false,
Expand All @@ -143,10 +143,10 @@ proxy_pass http://upstream-name;
"redirect /something to /": {
"/something",
"/",
`~* ^/something\/?(?<baseuri>.*)`,
`~* "^/something\/?(?<baseuri>.*)"`,
`
rewrite (?i)/something/(.*) /$1 break;
rewrite (?i)/something$ / break;
rewrite "(?i)/something/(.*)" /$1 break;
rewrite "(?i)/something$" / break;
proxy_pass http://upstream-name;
`,
false,
Expand All @@ -159,10 +159,10 @@ proxy_pass http://upstream-name;
"redirect /end-with-slash/ to /not-root": {
"/end-with-slash/",
"/not-root",
"~* ^/end-with-slash/(?<baseuri>.*)",
`~* "^/end-with-slash/(?<baseuri>.*)"`,
`
rewrite (?i)/end-with-slash/(.*) /not-root/$1 break;
rewrite (?i)/end-with-slash/$ /not-root/ break;
rewrite "(?i)/end-with-slash/(.*)" /not-root/$1 break;
rewrite "(?i)/end-with-slash/$" /not-root/ break;
proxy_pass http://upstream-name;
`,
false,
Expand All @@ -175,10 +175,10 @@ proxy_pass http://upstream-name;
"redirect /something-complex to /not-root": {
"/something-complex",
"/not-root",
`~* ^/something-complex\/?(?<baseuri>.*)`,
`~* "^/something-complex\/?(?<baseuri>.*)"`,
`
rewrite (?i)/something-complex/(.*) /not-root/$1 break;
rewrite (?i)/something-complex$ /not-root/ break;
rewrite "(?i)/something-complex/(.*)" /not-root/$1 break;
rewrite "(?i)/something-complex$" /not-root/ break;
proxy_pass http://upstream-name;
`,
false,
Expand All @@ -193,8 +193,8 @@ proxy_pass http://upstream-name;
"/jenkins",
"~* ^/",
`
rewrite (?i)/(.*) /jenkins/$1 break;
rewrite (?i)/$ /jenkins/ break;
rewrite "(?i)/(.*)" /jenkins/$1 break;
rewrite "(?i)/$" /jenkins/ break;
proxy_pass http://upstream-name;

set_escape_uri $escaped_base_uri $baseuri;
Expand All @@ -210,10 +210,10 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
"redirect /something to / and rewrite": {
"/something",
"/",
`~* ^/something\/?(?<baseuri>.*)`,
`~* "^/something\/?(?<baseuri>.*)"`,
`
rewrite (?i)/something/(.*) /$1 break;
rewrite (?i)/something$ / break;
rewrite "(?i)/something/(.*)" /$1 break;
rewrite "(?i)/something$" / break;
proxy_pass http://upstream-name;

set_escape_uri $escaped_base_uri $baseuri;
Expand All @@ -229,10 +229,10 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
"redirect /end-with-slash/ to /not-root and rewrite": {
"/end-with-slash/",
"/not-root",
`~* ^/end-with-slash/(?<baseuri>.*)`,
`~* "^/end-with-slash/(?<baseuri>.*)"`,
`
rewrite (?i)/end-with-slash/(.*) /not-root/$1 break;
rewrite (?i)/end-with-slash/$ /not-root/ break;
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;
Expand All @@ -248,10 +248,10 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
"redirect /something-complex to /not-root and rewrite": {
"/something-complex",
"/not-root",
`~* ^/something-complex\/?(?<baseuri>.*)`,
`~* "^/something-complex\/?(?<baseuri>.*)"`,
`
rewrite (?i)/something-complex/(.*) /not-root/$1 break;
rewrite (?i)/something-complex$ /not-root/ break;
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;
Expand All @@ -267,10 +267,10 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
"redirect /something to / and rewrite with specific scheme": {
"/something",
"/",
`~* ^/something\/?(?<baseuri>.*)`,
`~* "^/something\/?(?<baseuri>.*)"`,
`
rewrite (?i)/something/(.*) /$1 break;
rewrite (?i)/something$ / break;
rewrite "(?i)/something/(.*)" /$1 break;
rewrite "(?i)/something$" / break;
proxy_pass http://upstream-name;

set_escape_uri $escaped_base_uri $baseuri;
Expand All @@ -288,8 +288,8 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="
"/something",
`~* ^/`,
`
rewrite (?i)/(.*) /something/$1 break;
rewrite (?i)/$ /something/ break;
rewrite "(?i)/(.*)" /something/$1 break;
rewrite "(?i)/$" /something/ break;
proxy_pass http://sticky-upstream-name;
`,
false,
Expand All @@ -304,8 +304,8 @@ proxy_pass http://sticky-upstream-name;
"/something",
`~* ^/`,
`
rewrite (?i)/(.*) /something/$1 break;
rewrite (?i)/$ /something/ break;
rewrite "(?i)/(.*)" /something/$1 break;
rewrite "(?i)/$" /something/ break;
proxy_pass http://upstream_balancer;
`,
false,
Expand All @@ -318,10 +318,10 @@ proxy_pass http://upstream_balancer;
"add the X-Forwarded-Prefix header": {
"/there",
"/something",
`~* ^/there\/?(?<baseuri>.*)`,
`~* "^/there\/?(?<baseuri>.*)"`,
`
rewrite (?i)/there/(.*) /something/$1 break;
rewrite (?i)/there$ /something/ break;
rewrite "(?i)/there/(.*)" /something/$1 break;
rewrite "(?i)/there$" /something/ break;
proxy_set_header X-Forwarded-Prefix "/there/";
proxy_pass http://sticky-upstream-name;
`,
Expand All @@ -335,7 +335,7 @@ proxy_pass http://sticky-upstream-name;
"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,
"",
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/annotations/rewrite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ 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;")
return strings.Contains(server, `rewrite "(?i)/something/(.*)" /$1 break;`) &&
strings.Contains(server, `rewrite "(?i)/something$" / break;`)
})
Expect(err).NotTo(HaveOccurred())

Expand Down Expand Up @@ -154,7 +154,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Rewrite", func() {

err = 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" {`)
})
Expect(err).NotTo(HaveOccurred())

Expand Down Expand Up @@ -195,7 +195,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Rewrite", func() {

err = f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, "location ~* ^/foo {") && strings.Contains(server, `location ~* ^/foo.+\/?(?<baseuri>.*) {`)
return strings.Contains(server, `location ~* "^/foo" {`) && strings.Contains(server, `location ~* "^/foo.+\/?(?<baseuri>.*)" {`)
})
Expect(err).NotTo(HaveOccurred())

Expand Down Expand Up @@ -234,14 +234,14 @@ var _ = framework.IngressNginxDescribe("Annotations - Rewrite", func() {
"nginx.ingress.kubernetes.io/use-regex": "true",
"nginx.ingress.kubernetes.io/rewrite-target": "/new/backend",
}
ing = framework.NewSingleIngress("regex", "/foo/bar/[a-z][a-z][a-z]", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
ing = framework.NewSingleIngress("regex", "/foo/bar/[a-z]{3}", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

_, err = f.EnsureIngress(ing)
Expect(err).NotTo(HaveOccurred())
Expect(ing).NotTo(BeNil())

err = f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, "location ~* ^/foo/bar/bar {") && strings.Contains(server, `location ~* ^/foo/bar/[a-z][a-z][a-z]\/?(?<baseuri>.*) {`)
return strings.Contains(server, `location ~* "^/foo/bar/bar" {`) && strings.Contains(server, `location ~* "^/foo/bar/[a-z]{3}\/?(?<baseuri>.*)" {`)
})
Expect(err).NotTo(HaveOccurred())

Expand Down