diff --git a/docs/virtualserver-and-virtualserverroute.md b/docs/virtualserver-and-virtualserverroute.md index ea4b1f1600..9c246f2f84 100644 --- a/docs/virtualserver-and-virtualserverroute.md +++ b/docs/virtualserver-and-virtualserverroute.md @@ -26,6 +26,7 @@ This document is the reference documentation for the resources. To see additiona - [Upstream.SessionCookie](#upstreamsessioncookie) - [Header](#header) - [Action](#action) + - [Action.Redirect](#actionredirect) - [Split](#split) - [Match](#match) - [Condition](#condition) @@ -399,7 +400,26 @@ In the example below, client requests are passed to an upstream `coffee`: | Field | Description | Type | Required | | ----- | ----------- | ---- | -------- | -| `pass` | Passes requests to an upstream. The upstream with that name must be defined in the resource. | `string` | Yes | +| `pass` | Passes requests to an upstream. The upstream with that name must be defined in the resource. | `string` | No* | +| `redirect` | Redirects requests to a provided URL. | [`action.redirect`](#ActionRedirect) | No* | + +\* -- an action must include exactly one of the following: `pass` or `redirect`. + +### Action.Redirect + +The redirect action defines a redirect to return for a request. + +In the example below, client requests are passed to a url `http://www.nginx.com`: +```yaml +redirect: + url: http://www.nginx.com + code: 301 +``` + +| Field | Description | Type | Required | +| ----- | ----------- | ---- | -------- | +| `url` | The URL to redirect the request to. Supported NGINX variables: `$scheme`, `$http_x_forwarded_proto`, `$request_uri`, `$host`. Variables must be inclosed in the curly braces. For example: `${host}${request_uri}`. | `string` | Yes | +| `code` | The status code of a redirect. The allowed values are: `301`, `302`, `307`, `308`. The default is `301`. | `int` | No | ### Split diff --git a/internal/configs/version2/config.go b/internal/configs/version2/config.go index 38d82132b1..58a3cbca96 100644 --- a/internal/configs/version2/config.go +++ b/internal/configs/version2/config.go @@ -72,6 +72,7 @@ type Location struct { ProxyNextUpstreamTimeout string ProxyNextUpstreamTries int HasKeepalive bool + Redirect *ActionRedirect } // SplitClient defines a split_clients. @@ -104,6 +105,12 @@ type TLSRedirect struct { BasedOn string } +// ActionRedirect defines a redirect in a location. +type ActionRedirect struct { + URL string + Code int +} + // SessionCookie defines a session cookie for an upstream. type SessionCookie struct { Enable bool diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 886b97e814..71093cc75e 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -113,22 +113,27 @@ server { {{ $snippet }} {{ end }} + {{ with $l.Redirect }} + return {{ .Code }} "{{ .URL }}"; + {{ end }} + + {{ if $l.ProxyPass }} proxy_connect_timeout {{ $l.ProxyConnectTimeout }}; proxy_read_timeout {{ $l.ProxyReadTimeout }}; proxy_send_timeout {{ $l.ProxySendTimeout }}; client_max_body_size {{ $l.ClientMaxBodySize }}; - {{ if $l.ProxyMaxTempFileSize }} + {{ if $l.ProxyMaxTempFileSize }} proxy_max_temp_file_size {{ $l.ProxyMaxTempFileSize }}; - {{ end }} + {{ end }} proxy_buffering {{ if $l.ProxyBuffering }}on{{ else }}off{{ end }}; - {{ if $l.ProxyBuffers }} + {{ if $l.ProxyBuffers }} proxy_buffers {{ $l.ProxyBuffers }}; - {{ end }} - {{ if $l.ProxyBufferSize }} + {{ end }} + {{ if $l.ProxyBufferSize }} proxy_buffer_size {{ $l.ProxyBufferSize }}; - {{ end }} + {{ end }} proxy_http_version 1.1; @@ -146,6 +151,7 @@ server { proxy_next_upstream {{ $l.ProxyNextUpstream }}; proxy_next_upstream_timeout {{ $l.ProxyNextUpstreamTimeout }}; proxy_next_upstream_tries {{ $l.ProxyNextUpstreamTries }}; + {{ end }} } {{ end }} } diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 442f5f2f88..c2202c9107 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -82,22 +82,27 @@ server { {{ $snippet }} {{ end }} + {{ with $l.Redirect }} + return {{ .Code }} "{{ .URL }}"; + {{ end }} + + {{ if $l.ProxyPass }} proxy_connect_timeout {{ $l.ProxyConnectTimeout }}; proxy_read_timeout {{ $l.ProxyReadTimeout }}; proxy_send_timeout {{ $l.ProxySendTimeout }}; client_max_body_size {{ $l.ClientMaxBodySize }}; - {{ if $l.ProxyMaxTempFileSize }} + {{ if $l.ProxyMaxTempFileSize }} proxy_max_temp_file_size {{ $l.ProxyMaxTempFileSize }}; - {{ end }} + {{ end }} proxy_buffering {{ if $l.ProxyBuffering }}on{{ else }}off{{ end }}; - {{ if $l.ProxyBuffers }} + {{ if $l.ProxyBuffers }} proxy_buffers {{ $l.ProxyBuffers }}; - {{ end }} - {{ if $l.ProxyBufferSize }} + {{ end }} + {{ if $l.ProxyBufferSize }} proxy_buffer_size {{ $l.ProxyBufferSize }}; - {{ end }} + {{ end }} proxy_http_version 1.1; @@ -115,6 +120,7 @@ server { proxy_next_upstream {{ $l.ProxyNextUpstream }}; proxy_next_upstream_timeout {{ $l.ProxyNextUpstreamTimeout }}; proxy_next_upstream_tries {{ $l.ProxyNextUpstreamTries }}; + {{ end }} } {{ end }} } diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index da53f0d041..591ce79fac 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -250,7 +250,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE } else { upstreamName := virtualServerUpstreamNamer.GetNameForUpstream(r.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(r.Path, upstreamName, upstream, vsc.cfgParams) + loc := generateLocation(r.Path, upstreamName, upstream, r.Action, vsc.cfgParams) locations = append(locations, loc) } @@ -278,7 +278,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE } else { upstreamName := upstreamNamer.GetNameForUpstream(r.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(r.Path, upstreamName, upstream, vsc.cfgParams) + loc := generateLocation(r.Path, upstreamName, upstream, r.Action, vsc.cfgParams) locations = append(locations, loc) } } @@ -511,7 +511,15 @@ func generatePath(path string) string { return path } -func generateLocation(path string, upstreamName string, upstream conf_v1.Upstream, cfgParams *ConfigParams) version2.Location { +func generateLocation(path string, upstreamName string, upstream conf_v1.Upstream, action *conf_v1.Action, cfgParams *ConfigParams) version2.Location { + if action.Redirect != nil { + return generateLocationForRedirect(path, cfgParams.LocationSnippets, action.Redirect) + } + + return generateLocationForProxying(path, upstreamName, upstream, cfgParams) +} + +func generateLocationForProxying(path string, upstreamName string, upstream conf_v1.Upstream, cfgParams *ConfigParams) version2.Location { return version2.Location{ Path: generatePath(path), Snippets: cfgParams.LocationSnippets, @@ -531,6 +539,14 @@ func generateLocation(path string, upstreamName string, upstream conf_v1.Upstrea } } +func generateLocationForRedirect(path string, locationSnippets []string, redirect *conf_v1.ActionRedirect) version2.Location { + return version2.Location{ + Path: path, + Snippets: locationSnippets, + Redirect: generateActionRedirectConfig(redirect), + } +} + type routingCfg struct { Maps []version2.Map SplitClients []version2.SplitClient @@ -561,7 +577,7 @@ func generateSplits(splits []conf_v1.Split, upstreamNamer *upstreamNamer, crUpst path := fmt.Sprintf("@splits_%d_split_%d", scIndex, i) upstreamName := upstreamNamer.GetNameForUpstream(s.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(path, upstreamName, upstream, cfgParams) + loc := generateLocation(path, upstreamName, upstream, s.Action, cfgParams) locations = append(locations, loc) } @@ -668,7 +684,7 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr path := fmt.Sprintf("@matches_%d_match_%d", index, i) upstreamName := upstreamNamer.GetNameForUpstream(m.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(path, upstreamName, upstream, cfgParams) + loc := generateLocation(path, upstreamName, upstream, m.Action, cfgParams) locations = append(locations, loc) } } @@ -682,7 +698,7 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr path := fmt.Sprintf("@matches_%d_default", index) upstreamName := upstreamNamer.GetNameForUpstream(route.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(path, upstreamName, upstream, cfgParams) + loc := generateLocation(path, upstreamName, upstream, route.Action, cfgParams) locations = append(locations, loc) } @@ -806,6 +822,23 @@ func generateTLSRedirectConfig(tls *conf_v1.TLS) *version2.TLSRedirect { return redirect } +func generateActionRedirectConfig(actionRedirect *conf_v1.ActionRedirect) *version2.ActionRedirect { + if actionRedirect == nil { + return nil + } + + redirect := &version2.ActionRedirect{ + URL: actionRedirect.URL, + Code: 301, + } + + if actionRedirect.Code != 0 { + redirect.Code = actionRedirect.Code + } + + return redirect +} + func generateTLSRedirectBasedOn(basedOn string) string { if basedOn == "x-forwarded-proto" { return "$http_x_forwarded_proto" diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 71083eacd8..b0030d9930 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -1100,7 +1100,7 @@ func TestGenerateBuffer(t *testing.T) { } } -func TestGenerateLocation(t *testing.T) { +func TestGenerateLocationForProxying(t *testing.T) { cfgParams := ConfigParams{ ProxyConnectTimeout: "30s", ProxyReadTimeout: "31s", @@ -1132,9 +1132,35 @@ func TestGenerateLocation(t *testing.T) { ProxyNextUpstreamTries: 0, } - result := generateLocation(path, upstreamName, conf_v1.Upstream{}, &cfgParams) + result := generateLocationForProxying(path, upstreamName, conf_v1.Upstream{}, &cfgParams) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateLocation() returned %v but expected %v", result, expected) + t.Errorf("generateLocationForProxying() returned %v but expected %v", result, expected) + } +} + +func TestGenerateLocationForRedirect(t *testing.T) { + cfgParams := ConfigParams{ + LocationSnippets: []string{"# location snippet"}, + } + path := "/" + redirect := &conf_v1.Action{ + Redirect: &conf_v1.ActionRedirect{ + URL: "http://www.nginx.com", + }, + } + + expected := version2.Location{ + Path: "/", + Snippets: []string{"# location snippet"}, + Redirect: &version2.ActionRedirect{ + URL: "http://www.nginx.com", + Code: 301, + }, + } + + result := generateLocationForRedirect(path, cfgParams.LocationSnippets, redirect.Redirect) + if !reflect.DeepEqual(result, expected) { + t.Errorf("generateLocationForRedirect() returned %v but expected %v", result, expected) } } @@ -1290,6 +1316,43 @@ func TestGenerateTLSRedirectBasedOn(t *testing.T) { } } +func TestGenerateActionRedirectConfig(t *testing.T) { + tests := []struct { + redirect *conf_v1.ActionRedirect + expected *version2.ActionRedirect + }{ + { + redirect: nil, + expected: nil, + }, + { + redirect: &conf_v1.ActionRedirect{ + URL: "http://www.nginx.com", + }, + expected: &version2.ActionRedirect{ + URL: "http://www.nginx.com", + Code: 301, + }, + }, + { + redirect: &conf_v1.ActionRedirect{ + URL: "$scheme://www.nginx.com", + Code: 302, + }, + expected: &version2.ActionRedirect{ + URL: "$scheme://www.nginx.com", + Code: 302, + }, + }, + } + for _, test := range tests { + result := generateActionRedirectConfig(test.redirect) + if !reflect.DeepEqual(result, test.expected) { + t.Errorf("generateActionRedirectConfig() returned %v, but expected %v", result, test.expected) + } + } +} + func TestCreateUpstreamsForPlus(t *testing.T) { virtualServerEx := VirtualServerEx{ VirtualServer: &conf_v1.VirtualServer{ diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index cd69b2bb83..8884188bec 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -107,7 +107,14 @@ type Route struct { // Action defines an action. type Action struct { - Pass string `json:"pass"` + Pass string `json:"pass"` + Redirect *ActionRedirect `json:"redirect"` +} + +// ActionRedirect defines a redirect in an Action. +type ActionRedirect struct { + URL string `json:"url"` + Code int `json:"code"` } // Split defines a split. diff --git a/pkg/apis/configuration/v1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1/zz_generated.deepcopy.go index 0f18de92f6..a763d440e9 100644 --- a/pkg/apis/configuration/v1/zz_generated.deepcopy.go +++ b/pkg/apis/configuration/v1/zz_generated.deepcopy.go @@ -11,6 +11,11 @@ import ( // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Action) DeepCopyInto(out *Action) { *out = *in + if in.Redirect != nil { + in, out := &in.Redirect, &out.Redirect + *out = new(ActionRedirect) + **out = **in + } return } @@ -24,6 +29,22 @@ func (in *Action) DeepCopy() *Action { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ActionRedirect) DeepCopyInto(out *ActionRedirect) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ActionRedirect. +func (in *ActionRedirect) DeepCopy() *ActionRedirect { + if in == nil { + return nil + } + out := new(ActionRedirect) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Condition) DeepCopyInto(out *Condition) { *out = *in @@ -93,7 +114,7 @@ func (in *Match) DeepCopyInto(out *Match) { if in.Action != nil { in, out := &in.Action, &out.Action *out = new(Action) - **out = **in + (*in).DeepCopyInto(*out) } if in.Splits != nil { in, out := &in.Splits, &out.Splits @@ -121,7 +142,7 @@ func (in *Route) DeepCopyInto(out *Route) { if in.Action != nil { in, out := &in.Action, &out.Action *out = new(Action) - **out = **in + (*in).DeepCopyInto(*out) } if in.Splits != nil { in, out := &in.Splits, &out.Splits @@ -172,7 +193,7 @@ func (in *Split) DeepCopyInto(out *Split) { if in.Action != nil { in, out := &in.Action, &out.Action *out = new(Action) - **out = **in + (*in).DeepCopyInto(*out) } return } diff --git a/pkg/apis/configuration/validation/validation.go b/pkg/apis/configuration/validation/validation.go index fe2bc8f166..cfc9dbf84b 100644 --- a/pkg/apis/configuration/validation/validation.go +++ b/pkg/apis/configuration/validation/validation.go @@ -72,7 +72,7 @@ func validateTLSRedirect(redirect *v1.TLSRedirect, fieldPath *field.Path) field. } if redirect.Code != nil { - allErrs = append(allErrs, validateTLSRedirectStatusCode(*redirect.Code, fieldPath.Child("code"))...) + allErrs = append(allErrs, validateRedirectStatusCode(*redirect.Code, fieldPath.Child("code"))...) } if redirect.BasedOn != "" && redirect.BasedOn != "scheme" && redirect.BasedOn != "x-forwarded-proto" { @@ -82,17 +82,17 @@ func validateTLSRedirect(redirect *v1.TLSRedirect, fieldPath *field.Path) field. return allErrs } -var validTLSRedirectStatusCodes = map[int]bool{ +var validRedirectStatusCodes = map[int]bool{ 301: true, 302: true, 307: true, 308: true, } -func validateTLSRedirectStatusCode(code int, fieldPath *field.Path) field.ErrorList { +func validateRedirectStatusCode(code int, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if _, ok := validTLSRedirectStatusCodes[code]; !ok { + if _, ok := validRedirectStatusCodes[code]; !ok { allErrs = append(allErrs, field.Invalid(fieldPath, code, "status code out of accepted range. accepted values are '301', '302', '307', '308'")) } @@ -596,15 +596,131 @@ func validateRoute(route v1.Route, fieldPath *field.Path, upstreamNames sets.Str func validateAction(action *v1.Action, fieldPath *field.Path, upstreamNames sets.String) field.ErrorList { allErrs := field.ErrorList{} - if action.Pass == "" { - return append(allErrs, field.Required(fieldPath.Child("pass"), "")) + if action.Pass == "" && action.Redirect == nil { + return append(allErrs, field.Required(fieldPath, "action must specify exactly one of `pass` or `redirect`")) } - allErrs = append(allErrs, validateReferencedUpstream(action.Pass, fieldPath.Child("pass"), upstreamNames)...) + if action.Pass != "" && action.Redirect != nil { + return append(allErrs, field.Required(fieldPath, "action must specify exactly one of `pass` or `redirect`")) + } + + if action.Pass != "" { + allErrs = append(allErrs, validateReferencedUpstream(action.Pass, fieldPath.Child("pass"), upstreamNames)...) + } + + if action.Redirect != nil { + allErrs = append(allErrs, validateActionRedirect(action.Redirect, fieldPath.Child("redirect"))...) + } + + return allErrs +} + +func validateActionRedirect(redirect *v1.ActionRedirect, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + allErrs = append(allErrs, validateRedirectURL(redirect.URL, fieldPath.Child("url"))...) + + if redirect.Code != 0 { + allErrs = append(allErrs, validateRedirectStatusCode(redirect.Code, fieldPath.Child("code"))...) + } + + return allErrs +} + +var nginxVariableRegexp = regexp.MustCompile(`\$\{([^}]*)\}`) + +// captureVariables returns a slice of vars enclosed in ${}. For example "${a} ${b}" would return ["a", "b"]. +func captureVariables(s string) []string { + var nVars []string + + res := nginxVariableRegexp.FindAllStringSubmatch(s, -1) + for _, n := range res { + nVars = append(nVars, n[1]) + } + + return nVars +} + +// validRedirectVariableNames includes NGINX variables allowed to be used in redirects. +var validRedirectVariableNames = map[string]bool{ + "scheme": true, + "http_x_forwarded_proto": true, + "request_uri": true, + "host": true, +} + +func validateRedirectURL(redirectURL string, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if redirectURL == "" { + return append(allErrs, field.Required(fieldPath, "must specify a url")) + } + + allErrs = append(allErrs, validateRedirectURLFmt(redirectURL, fieldPath)...) + + nginxVars := captureVariables(redirectURL) + for _, nVar := range nginxVars { + allErrs = append(allErrs, validateVariable(nVar, validRedirectVariableNames, fieldPath)...) + } return allErrs } +const redirectFmt string = `([^"\\]|\\.)*` +const redirectFmtErrMsg string = `must have all '"' (double quotes) escaped and must not end with an unescaped '\' (backslash)` + +var redirectFmtRegexp = regexp.MustCompile("^" + redirectFmt + "$") + +func validateRedirectURLFmt(str string, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if !redirectFmtRegexp.MatchString(str) { + msg := validation.RegexError(redirectFmtErrMsg, redirectFmt, "http://www.nginx.com", "$scheme://$host/green/", `\"http://www.nginx.com\"`) + return append(allErrs, field.Invalid(fieldPath, str, msg)) + } + + if strings.HasSuffix(str, "$") { + return append(allErrs, field.Invalid(fieldPath, str, "must not end with $")) + } + + for i, c := range str { + if c == '$' { + msg := "variables must be enclosed in curly braces, for example ${host}" + + if str[i+1] != '{' { + return append(allErrs, field.Invalid(fieldPath, str, msg)) + } + + if !strings.Contains(str[i+1:], "}") { + return append(allErrs, field.Invalid(fieldPath, str, msg)) + } + } + } + + return allErrs +} + +func validateVariable(nVar string, validVars map[string]bool, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if !validVars[nVar] { + msg := fmt.Sprintf("'%v' contains an invalid NGINX variable. Accepted variables are: %v", nVar, mapToPrettyString(validVars)) + allErrs = append(allErrs, field.Invalid(fieldPath, nVar, msg)) + } + + return allErrs +} + +func mapToPrettyString(m map[string]bool) string { + var out []string + + for k := range m { + out = append(out, k) + } + + return strings.Join(out, ", ") +} + func validateRouteField(value string, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/apis/configuration/validation/validation_test.go b/pkg/apis/configuration/validation/validation_test.go index b9b8b5d12f..65a2717b3e 100644 --- a/pkg/apis/configuration/validation/validation_test.go +++ b/pkg/apis/configuration/validation/validation_test.go @@ -1,6 +1,7 @@ package validation import ( + "reflect" "testing" v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" @@ -814,26 +815,227 @@ func TestValidateRouteFails(t *testing.T) { } func TestValidateAction(t *testing.T) { - action := &v1.Action{ - Pass: "test", - } upstreamNames := map[string]sets.Empty{ "test": {}, } + tests := []struct { + action *v1.Action + msg string + }{ + { + action: &v1.Action{ + Pass: "test", + }, + msg: "base pass action", + }, + { + action: &v1.Action{ + Redirect: &v1.ActionRedirect{ + URL: "http://www.nginx.com", + }, + }, + msg: "base redirect action", + }, + { + action: &v1.Action{ + Redirect: &v1.ActionRedirect{ + URL: "http://www.nginx.com", + Code: 302, + }, + }, + + msg: "redirect action with status code set", + }, + } - allErrs := validateAction(action, field.NewPath("action"), upstreamNames) - if len(allErrs) > 0 { - t.Errorf("validateAction() returned errors %v for valid input", allErrs) + for _, test := range tests { + allErrs := validateAction(test.action, field.NewPath("action"), upstreamNames) + if len(allErrs) > 0 { + t.Errorf("validateAction() returned errors %v for valid input for the case of %s", allErrs, test.msg) + } } } func TestValidateActionFails(t *testing.T) { - action := &v1.Action{} upstreamNames := map[string]sets.Empty{} - allErrs := validateAction(action, field.NewPath("action"), upstreamNames) - if len(allErrs) == 0 { - t.Error("validateAction() returned no errors for invalid input") + tests := []struct { + action *v1.Action + msg string + }{ + + { + action: &v1.Action{}, + msg: "empty action", + }, + { + action: &v1.Action{ + Redirect: &v1.ActionRedirect{}, + }, + msg: "missing required field url", + }, + { + action: &v1.Action{ + Pass: "test", + Redirect: &v1.ActionRedirect{ + URL: "http://www.nginx.com", + }, + }, + msg: "multiple actions defined", + }, + { + action: &v1.Action{ + Redirect: &v1.ActionRedirect{ + URL: "http://www.nginx.com", + Code: 305, + }, + }, + msg: "redirect action with invalid status code set", + }, + } + + for _, test := range tests { + allErrs := validateAction(test.action, field.NewPath("action"), upstreamNames) + if len(allErrs) == 0 { + t.Errorf("validateAction() returned no errors for invalid input for the case of %s", test.msg) + } + } +} + +func TestCaptureVariables(t *testing.T) { + tests := []struct { + s string + expected []string + }{ + { + "${scheme}://${host}", + []string{"scheme", "host"}, + }, + { + "http://www.nginx.org", + nil, + }, + { + "${}", + []string{""}, + }, + } + for _, test := range tests { + result := captureVariables(test.s) + if !reflect.DeepEqual(result, test.expected) { + t.Errorf("captureVariables(%s) returned %v but expected %v", test.s, result, test.expected) + } + } +} + +func TestValidateRedirectURL(t *testing.T) { + tests := []struct { + redirectURL string + msg string + }{ + { + redirectURL: "http://www.nginx.com", + msg: "base redirect url", + }, + { + redirectURL: "${scheme}://${host}/sorry", + msg: "multi variable redirect url", + }, + { + redirectURL: "${http_x_forwarded_proto}://${host}/sorry", + msg: "x-forwarded-proto redirect url use case", + }, + { + redirectURL: "${host}${request_uri}", + msg: "use multi variables, no scheme set", + }, + { + redirectURL: "${scheme}://www.${host}${request_uri}", + msg: "use multi variables", + }, + { + redirectURL: "http://example.com/redirect?source=abc", + msg: "arg variable use", + }, + { + redirectURL: `\"${scheme}://${host}\"`, + msg: "url with escaped quotes", + }, + { + redirectURL: "{abc}", + msg: "url with curly braces with no $ prefix", + }, + } + + for _, test := range tests { + allErrs := validateRedirectURL(test.redirectURL, field.NewPath("url")) + if len(allErrs) > 0 { + t.Errorf("validateRedirectURL(%s) returned errors %v for valid input for the case of %s", test.redirectURL, allErrs, test.msg) + } + } +} + +func TestValidateRedirectURLFails(t *testing.T) { + tests := []struct { + redirectURL string + msg string + }{ + + { + redirectURL: "", + msg: "url is blank", + }, + { + redirectURL: "$scheme://www.$host.com", + msg: "usage of nginx variable in url without ${}", + }, + { + redirectURL: "${scheme}://www.${invalid}.com", + msg: "invalid nginx variable in url", + }, + { + redirectURL: "${scheme}://www.${{host}.com", + msg: "leading curly brace", + }, + { + redirectURL: "${host.abc}.com", + msg: "multi var in curly brace", + }, + { + redirectURL: "${scheme}://www.${host{host}}.com", + msg: "nested nginx vars", + }, + { + redirectURL: `"${scheme}://${host}"`, + msg: "url in unescaped quotes", + }, + { + redirectURL: `"${scheme}://${host}`, + msg: "url with unescaped quote prefix", + }, + { + redirectURL: `\\"${scheme}://${host}\\"`, + msg: "url with escaped backslash", + }, + { + redirectURL: `${scheme}://${host}$`, + msg: "url with ending $", + }, + { + redirectURL: `http://${}`, + msg: "url containing blank var", + }, + { + redirectURL: `http://${abca`, + msg: "url containing a var without ending }", + }, + } + + for _, test := range tests { + allErrs := validateRedirectURL(test.redirectURL, field.NewPath("action")) + if len(allErrs) == 0 { + t.Errorf("validateRedirectURL(%s) returned no errors for invalid input for the case of %s", test.redirectURL, test.msg) + } } } @@ -2343,7 +2545,7 @@ func TestValidateSessionCookieFails(t *testing.T) { } } -func TestValidateTLSRedirectStatusCode(t *testing.T) { +func TestValidateRedirectStatusCode(t *testing.T) { tests := []struct { code int }{ @@ -2353,14 +2555,14 @@ func TestValidateTLSRedirectStatusCode(t *testing.T) { {code: 308}, } for _, test := range tests { - allErrs := validateTLSRedirectStatusCode(test.code, field.NewPath("code")) + allErrs := validateRedirectStatusCode(test.code, field.NewPath("code")) if len(allErrs) != 0 { - t.Errorf("validateTLSRedirectStatusCode(%v) returned errors %v for valid input", test.code, allErrs) + t.Errorf("validateRedirectStatusCode(%v) returned errors %v for valid input", test.code, allErrs) } } } -func TestValidateTLSRedirectStatusCodeFails(t *testing.T) { +func TestValidateRedirectStatusCodeFails(t *testing.T) { tests := []struct { code int }{ @@ -2369,9 +2571,56 @@ func TestValidateTLSRedirectStatusCodeFails(t *testing.T) { {code: 305}, } for _, test := range tests { - allErrs := validateTLSRedirectStatusCode(test.code, field.NewPath("code")) + allErrs := validateRedirectStatusCode(test.code, field.NewPath("code")) + if len(allErrs) == 0 { + t.Errorf("validateRedirectStatusCode(%v) returned no errors for invalid input", test.code) + } + } +} + +func TestValidateVariable(t *testing.T) { + var validVars = map[string]bool{ + "scheme": true, + "http_x_forwarded_proto": true, + "request_uri": true, + "host": true, + } + + tests := []struct { + nVar string + }{ + {"scheme"}, + {"http_x_forwarded_proto"}, + {"request_uri"}, + {"host"}, + } + for _, test := range tests { + allErrs := validateVariable(test.nVar, validVars, field.NewPath("url")) + if len(allErrs) != 0 { + t.Errorf("validateVariable(%v) returned errors %v for valid input", test.nVar, allErrs) + } + } +} + +func TestValidateVariableFails(t *testing.T) { + var validVars = map[string]bool{ + "host": true, + } + + tests := []struct { + nVar string + }{ + {""}, + {"hostinvalid.com"}, + {"$a"}, + {"host${host}"}, + {"host${host}}"}, + {"host$${host}"}, + } + for _, test := range tests { + allErrs := validateVariable(test.nVar, validVars, field.NewPath("url")) if len(allErrs) == 0 { - t.Errorf("validateTLSRedirectStatusCode(%v) returned no errors for invalid input", test.code) + t.Errorf("validateVariable(%v) returned no errors for invalid input", test.nVar) } } }