diff --git a/controllers/nginx/pkg/template/template.go b/controllers/nginx/pkg/template/template.go index b3762586e..00631db79 100644 --- a/controllers/nginx/pkg/template/template.go +++ b/controllers/nginx/pkg/template/template.go @@ -200,9 +200,15 @@ func buildLocation(input interface{}) string { } path := location.Path - if len(location.Rewrite.Target) > 0 && location.Rewrite.Target != path { + + if len(location.Rewrite.Target) > 0 { + if location.Rewrite.Target == path { + // This is an invalid rewrite case + return path + } + if path == slash { - return fmt.Sprintf("~* %s", path) + return fmt.Sprintf("%s %s", location.Rewrite.LocationModifier, path) } // baseuri regex will parse basename from the given location baseuri := `(?.*)` @@ -210,7 +216,11 @@ func buildLocation(input interface{}) 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%s", location.Rewrite.LocationModifier, path, baseuri) + } + + if len(location.Rewrite.LocationModifier) > 0 { + return fmt.Sprintf("%s %s", location.Rewrite.LocationModifier, path) } return path diff --git a/controllers/nginx/pkg/template/template_test.go b/controllers/nginx/pkg/template/template_test.go index 62d0342f4..2ee91abb5 100644 --- a/controllers/nginx/pkg/template/template_test.go +++ b/controllers/nginx/pkg/template/template_test.go @@ -32,61 +32,68 @@ import ( "k8s.io/ingress/core/pkg/ingress/annotations/rewrite" ) +const ( + emptyTarget = "" +) + var ( // TODO: add tests for secure endpoints tmplFuncTestcases = map[string]struct { - Path string - Target string - Location string - ProxyPass string - AddBaseURL bool - BaseURLScheme string + Path string + Target string + LocationModifier string + Location string + ProxyPass string + AddBaseURL bool + BaseURLScheme string }{ - "invalid redirect / to /": {"/", "/", "/", "proxy_pass http://upstream-name;", false, ""}, - "redirect / to /jenkins": {"/", "/jenkins", "~* /", + "case insensitive location modifier": {"/something", emptyTarget, "~*", "~* /something", "proxy_pass http://upstream-name;", false, ""}, + "case sensitive location modifier": {"/something", emptyTarget, "~", "~ /something", "proxy_pass http://upstream-name;", false, ""}, + "invalid redirect / to /": {"/", "/", "~*", "/", "proxy_pass http://upstream-name;", false, ""}, + "redirect / to /jenkins": {"/", "/jenkins", "~*", "~* /", ` rewrite /(.*) /jenkins/$1 break; proxy_pass http://upstream-name; `, false, ""}, - "redirect /something to /": {"/something", "/", `~* ^/something\/?(?.*)`, ` + "redirect /something to /": {"/something", "/", "~*", `~* ^/something\/?(?.*)`, ` rewrite /something/(.*) /$1 break; rewrite /something / break; proxy_pass http://upstream-name; `, false, ""}, - "redirect /end-with-slash/ to /not-root": {"/end-with-slash/", "/not-root", "~* ^/end-with-slash/(?.*)", ` + "redirect /end-with-slash/ to /not-root": {"/end-with-slash/", "/not-root", "~*", "~* ^/end-with-slash/(?.*)", ` rewrite /end-with-slash/(.*) /not-root/$1 break; proxy_pass http://upstream-name; `, false, ""}, - "redirect /something-complex to /not-root": {"/something-complex", "/not-root", `~* ^/something-complex\/?(?.*)`, ` + "redirect /something-complex to /not-root": {"/something-complex", "/not-root", "~*", `~* ^/something-complex\/?(?.*)`, ` rewrite /something-complex/(.*) /not-root/$1 break; proxy_pass http://upstream-name; `, false, ""}, - "redirect / to /jenkins and rewrite": {"/", "/jenkins", "~* /", ` + "redirect / to /jenkins and rewrite": {"/", "/jenkins", "~*", "~* /", ` rewrite /(.*) /jenkins/$1 break; proxy_pass http://upstream-name; subs_filter '' '' r; subs_filter '' '' r; `, true, ""}, - "redirect /something to / and rewrite": {"/something", "/", `~* ^/something\/?(?.*)`, ` + "redirect /something to / and rewrite": {"/something", "/", "~*", `~* ^/something\/?(?.*)`, ` rewrite /something/(.*) /$1 break; rewrite /something / break; proxy_pass http://upstream-name; subs_filter '' '' r; subs_filter '' '' r; `, true, ""}, - "redirect /end-with-slash/ to /not-root and rewrite": {"/end-with-slash/", "/not-root", `~* ^/end-with-slash/(?.*)`, ` + "redirect /end-with-slash/ to /not-root and rewrite": {"/end-with-slash/", "/not-root", "~*", `~* ^/end-with-slash/(?.*)`, ` rewrite /end-with-slash/(.*) /not-root/$1 break; proxy_pass http://upstream-name; subs_filter '' '' r; subs_filter '' '' r; `, true, ""}, - "redirect /something-complex to /not-root and rewrite": {"/something-complex", "/not-root", `~* ^/something-complex\/?(?.*)`, ` + "redirect /something-complex to /not-root and rewrite": {"/something-complex", "/not-root", "~*", `~* ^/something-complex\/?(?.*)`, ` rewrite /something-complex/(.*) /not-root/$1 break; proxy_pass http://upstream-name; subs_filter '' '' r; subs_filter '' '' r; `, true, ""}, - "redirect /something to / and rewrite with specific scheme": {"/something", "/", `~* ^/something\/?(?.*)`, ` + "redirect /something to / and rewrite with specific scheme": {"/something", "/", "~*", `~* ^/something\/?(?.*)`, ` rewrite /something/(.*) /$1 break; rewrite /something / break; proxy_pass http://upstream-name; @@ -118,13 +125,17 @@ func TestFormatIP(t *testing.T) { func TestBuildLocation(t *testing.T) { for k, tc := range tmplFuncTestcases { loc := &ingress.Location{ - Path: tc.Path, - Rewrite: rewrite.Redirect{Target: tc.Target, AddBaseURL: tc.AddBaseURL}, + Path: tc.Path, + Rewrite: rewrite.Redirect{ + Target: tc.Target, + AddBaseURL: tc.AddBaseURL, + LocationModifier: tc.LocationModifier, + }, } newLoc := buildLocation(loc) if tc.Location != newLoc { - t.Errorf("%s: expected '%v' but returned %v", k, tc.Location, newLoc) + t.Errorf("%s: expected '%v' but returned '%v'", k, tc.Location, newLoc) } } } @@ -132,8 +143,12 @@ func TestBuildLocation(t *testing.T) { func TestBuildProxyPass(t *testing.T) { for k, tc := range tmplFuncTestcases { loc := &ingress.Location{ - Path: tc.Path, - Rewrite: rewrite.Redirect{Target: tc.Target, AddBaseURL: tc.AddBaseURL, BaseURLScheme: tc.BaseURLScheme}, + Path: tc.Path, + Rewrite: rewrite.Redirect{ + Target: tc.Target, + AddBaseURL: tc.AddBaseURL, + BaseURLScheme: tc.BaseURLScheme, + }, Backend: "upstream-name", } diff --git a/core/pkg/ingress/annotations/rewrite/main.go b/core/pkg/ingress/annotations/rewrite/main.go index 32cc421ae..b54efd189 100644 --- a/core/pkg/ingress/annotations/rewrite/main.go +++ b/core/pkg/ingress/annotations/rewrite/main.go @@ -19,23 +19,28 @@ package rewrite import ( extensions "k8s.io/api/extensions/v1beta1" + "github.com/golang/glog" "k8s.io/ingress/core/pkg/ingress/annotations/parser" "k8s.io/ingress/core/pkg/ingress/resolver" ) const ( - rewriteTo = "ingress.kubernetes.io/rewrite-target" - addBaseURL = "ingress.kubernetes.io/add-base-url" - baseURLScheme = "ingress.kubernetes.io/base-url-scheme" - sslRedirect = "ingress.kubernetes.io/ssl-redirect" - forceSSLRedirect = "ingress.kubernetes.io/force-ssl-redirect" - appRoot = "ingress.kubernetes.io/app-root" + rewriteTo = "ingress.kubernetes.io/rewrite-target" + locationModifier = "ingress.kubernetes.io/location-modifier" + defaultRewriteLocationModifier = "~*" + addBaseURL = "ingress.kubernetes.io/add-base-url" + baseURLScheme = "ingress.kubernetes.io/base-url-scheme" + sslRedirect = "ingress.kubernetes.io/ssl-redirect" + forceSSLRedirect = "ingress.kubernetes.io/force-ssl-redirect" + appRoot = "ingress.kubernetes.io/app-root" ) // Redirect describes the per location redirect config type Redirect struct { // Target URI where the traffic must be redirected Target string `json:"target"` + // Location modifier + LocationModifier string `json:"locationModifier"` // AddBaseURL indicates if is required to add a base tag in the head // of the responses from the upstream servers AddBaseURL bool `json:"addBaseUrl"` @@ -92,6 +97,11 @@ func NewParser(br resolver.DefaultBackend) parser.IngressAnnotation { // rule used to rewrite the defined paths func (a rewrite) Parse(ing *extensions.Ingress) (interface{}, error) { rt, _ := parser.GetStringAnnotation(rewriteTo, ing) + locMod, _ := parser.GetStringAnnotation(locationModifier, ing) + + if rt != "" && locMod == "" { + locMod = defaultRewriteLocationModifier + } sslRe, err := parser.GetBoolAnnotation(sslRedirect, ing) if err != nil { sslRe = a.backendResolver.GetDefaultBackend().SSLRedirect @@ -103,12 +113,18 @@ func (a rewrite) Parse(ing *extensions.Ingress) (interface{}, error) { abu, _ := parser.GetBoolAnnotation(addBaseURL, ing) bus, _ := parser.GetStringAnnotation(baseURLScheme, ing) ar, _ := parser.GetStringAnnotation(appRoot, ing) - return &Redirect{ + + redirect := &Redirect{ Target: rt, + LocationModifier: locMod, AddBaseURL: abu, BaseURLScheme: bus, SSLRedirect: sslRe, ForceSSLRedirect: fSslRe, AppRoot: ar, - }, nil + } + + glog.V(5).Infof("created redirect %s", redirect) + + return redirect, nil } diff --git a/core/pkg/ingress/annotations/rewrite/main_test.go b/core/pkg/ingress/annotations/rewrite/main_test.go index 6529857f9..884c7ee4e 100644 --- a/core/pkg/ingress/annotations/rewrite/main_test.go +++ b/core/pkg/ingress/annotations/rewrite/main_test.go @@ -100,6 +100,31 @@ func TestRedirect(t *testing.T) { if redirect.Target != defRoute { t.Errorf("Expected %v as redirect but returned %s", defRoute, redirect.Target) } + + if redirect.LocationModifier != defaultRewriteLocationModifier { + t.Errorf("Expected %v as implicit location modifier but returned %s", defaultRewriteLocationModifier, redirect.LocationModifier) + } +} + +func TestRegex(t *testing.T) { + ing := buildIngress() + + data := map[string]string{} + modifier := "~" + data[locationModifier] = modifier + ing.SetAnnotations(data) + + i, err := NewParser(mockBackend{}).Parse(ing) + if err != nil { + t.Errorf("Unexpected error with ingress: %v", err) + } + redirect, ok := i.(*Redirect) + if !ok { + t.Errorf("expected a Redirect type") + } + if redirect.LocationModifier != modifier { + t.Errorf("Expected %v as location modifier but returned %s", modifier, redirect.LocationModifier) + } } func TestSSLRedirect(t *testing.T) {