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.

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

Add redirect support in VS/VSR #778

Merged
merged 4 commits into from
Nov 28, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
22 changes: 21 additions & 1 deletion docs/virtualserver-and-virtualserverroute.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -400,7 +401,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
Expand Down
7 changes: 7 additions & 0 deletions internal/configs/version2/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ type Location struct {
ProxyNextUpstreamTimeout string
ProxyNextUpstreamTries int
HasKeepalive bool
Redirect *ActionRedirect
}

// SplitClient defines a split_clients.
Expand Down Expand Up @@ -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
Expand Down
18 changes: 12 additions & 6 deletions internal/configs/version2/nginx-plus.virtualserver.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -146,6 +151,7 @@ server {
proxy_next_upstream {{ $l.ProxyNextUpstream }};
proxy_next_upstream_timeout {{ $l.ProxyNextUpstreamTimeout }};
proxy_next_upstream_tries {{ $l.ProxyNextUpstreamTries }};
{{ end }}
}
{{ end }}
}
18 changes: 12 additions & 6 deletions internal/configs/version2/nginx.virtualserver.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -115,6 +120,7 @@ server {
proxy_next_upstream {{ $l.ProxyNextUpstream }};
proxy_next_upstream_timeout {{ $l.ProxyNextUpstreamTimeout }};
proxy_next_upstream_tries {{ $l.ProxyNextUpstreamTries }};
{{ end }}
}
{{ end }}
}
45 changes: 39 additions & 6 deletions internal/configs/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}

Expand Down Expand Up @@ -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"
Expand Down
69 changes: 66 additions & 3 deletions internal/configs/virtualserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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() = %v, want %v", result, test.expected)
Dean-Coakley marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

func TestCreateUpstreamsForPlus(t *testing.T) {
virtualServerEx := VirtualServerEx{
VirtualServer: &conf_v1.VirtualServer{
Expand Down
9 changes: 8 additions & 1 deletion pkg/apis/configuration/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading