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 session persistence in VS/VSR #728

Merged
merged 6 commits into from
Oct 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
39 changes: 36 additions & 3 deletions docs/virtualserver-and-virtualserverroute.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ This document is the reference documentation for the resources. To see additiona
- [Upstream.TLS](#upstreamtls)
- [Upstream.Queue](#upstreamqueue)
- [Upstream.Healthcheck](#upstreamhealthcheck)
- [Upstream.SessionCookie](#upstreamsessioncookie)
- [Header](#header)
- [Split](#split)
- [Rules](#rules)
Expand Down Expand Up @@ -233,7 +234,7 @@ The buffers field configures the buffers used for reading a response from the up

```yaml
number: 4
size: 8K
size: 8K
```
See the [proxy_buffers](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffers) directive for additional information.

Expand Down Expand Up @@ -296,7 +297,7 @@ healthCheck:
| Field | Description | Type | Required |
| ----- | ----------- | ---- | -------- |
| `enable` | Enables a health check for an upstream server. The default is `false`. | `boolean` | No |
| `path` | The path used for health check requests. The default is `/`. | `string` | No |
| `path` | The path used for health check requests. The default is `/`. | `string` | No |
| `interval` | The interval between two consecutive health checks. The default is `5s`. | `string` | No |
| `jitter` | The time within which each health check will be randomly delayed. By default, there is no delay. | `string` | No |
| `fails` | The number of consecutive failed health checks of a particular upstream server after which this server will be considered unhealthy. The default is `1`. | `integer` | No |
Expand All @@ -307,7 +308,39 @@ healthCheck:
| `read-timeout` | The timeout for reading a response from an upstream server. By default, the `read-timeout` of the upstream is used. | `string` | No |
| `send-timeout` | The timeout for transmitting a request to an upstream server. By default, the `send-timeout` of the upstream is used. | `string` | No |
| `headers` | The request headers used for health check requests. NGINX Plus always sets the `Host`, `User-Agent` and `Connection` headers for health check requests. | [`[]header`](#Header) | No |
| `statusMatch` | The expected response status codes of a health check. By default, the response should have status code 2xx or 3xx. Examples: `“200”`, `“! 500”`, `"301-303 307"`. See the documentation of the [match](https://nginx.org/en/docs/http/ngx_http_upstream_hc_module.html?#match) directive. | `string` | No |
| `statusMatch` | The expected response status codes of a health check. By default, the response should have status code 2xx or 3xx. Examples: `“200”`, `“! 500”`, `"301-303 307"`. See the documentation of the [match](https://nginx.org/en/docs/http/ngx_http_upstream_hc_module.html?#match) directive. | `string` | No |

### Upstream.SessionCookie

The SessionCookie field configures session persistence which allows requests from the same client to be passed to the same upstream server. The information about the designated upstream server is passed in a session cookie generated by NGINX Plus.

In the example below, we configure session persistence with a session cookie for an upstream and configure all the available parameters:

```yaml
Dean-Coakley marked this conversation as resolved.
Show resolved Hide resolved
name: tea
service: tea-svc
port: 80
sessionCookie:
enable: true
name: srv_id
path: /
expires: 1h
domain: .example.com
httpOnly: false
secure: true
```
See the [`sticky`](https://nginx.org/en/docs/http/ngx_http_upstream_module.html?#sticky) directive for additional information. The session cookie corresponds to the `sticky cookie` method.


| Field | Description | Type | Required |
| ----- | ----------- | ---- | -------- |
| `enable` | Enables session persistence with a session cookie for an upstream server. The default is `false`. | `boolean` | No |
| `name` | The name of the cookie. | `string` | Yes |
| `path` | The path for which the cookie is set. | `string` | No |
| `expires` | The time for which a browser should keep the cookie. Can be set to the special value `max`, which will cause the cookie to expire on `31 Dec 2037 23:55:55 GMT`. | `string` | No |
| `domain` | The domain for which the cookie is set. | `string` | No |
| `httpOnly` | Adds the `HttpOnly` attribute to the cookie. | `boolean` | No |
| `secure` | Adds the `Secure` attribute to the cookie. | `boolean` | No |

### Header

Expand Down
12 changes: 12 additions & 0 deletions internal/configs/version2/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type Upstream struct {
FailTimeout string
UpstreamZoneSize string
Queue *Queue
SessionCookie *SessionCookie
}

// UpstreamServer defines an upstream server.
Expand Down Expand Up @@ -98,6 +99,17 @@ type HealthCheck struct {
Match string
}

// SessionCookie defines a session cookie for an upstream.
type SessionCookie struct {
Enable bool
Name string
Path string
Expires string
Domain string
HTTPOnly bool
Secure bool
}

// Distribution maps weight to a value in a SplitClient.
type Distribution struct {
Weight string
Expand Down
6 changes: 6 additions & 0 deletions internal/configs/version2/nginx-plus.virtualserver.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ upstream {{ $u.Name }} {
{{ if $u.Queue }}
queue {{ $u.Queue.Size }} timeout={{ $u.Queue.Timeout }};
{{ end }}

{{ with $u.SessionCookie }}
{{ if .Enable }}
sticky cookie {{ .Name }}{{ if .Expires }} expires={{ .Expires }}{{ end }}{{ if .Domain }} domain={{ .Domain }}{{ end }}{{ if .HTTPOnly }} httponly{{ end }}{{ if .Secure }} secure{{ end }}{{ if .Path }} path={{ .Path }}{{ end }};
{{ end }}
{{ end }}
}
{{ end }}

Expand Down
1 change: 1 addition & 0 deletions internal/configs/version2/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var virtualServerCfg = VirtualServerConfig{
SlowStart: "10s",
UpstreamZoneSize: "256k",
Queue: &Queue{Size: 10, Timeout: "60s"},
SessionCookie: &SessionCookie{Enable: true, Name: "test", Path: "/tea", Expires: "25s"},
},
{
Name: "coffee-v1",
Expand Down
17 changes: 17 additions & 0 deletions internal/configs/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ func (vsc *virtualServerConfigurator) generateUpstream(owner runtime.Object, ups
if vsc.isPlus {
ups.SlowStart = vsc.generateSlowStartForPlus(owner, upstream, lbMethod)
ups.Queue = generateQueueForPlus(upstream.Queue, "60s")
ups.SessionCookie = generateSessionCookie(upstream.SessionCookie)
}

return ups
Expand Down Expand Up @@ -412,6 +413,22 @@ func generateHealthCheck(upstream conf_v1alpha1.Upstream, upstreamName string, c
return hc
}

func generateSessionCookie(sc *conf_v1alpha1.SessionCookie) *version2.SessionCookie {
if sc == nil || !sc.Enable {
return nil
}

return &version2.SessionCookie{
Enable: true,
Name: sc.Name,
Path: sc.Path,
Expires: sc.Expires,
Domain: sc.Domain,
HTTPOnly: sc.HTTPOnly,
Secure: sc.Secure,
}
}

func generateStatusMatchName(upstreamName string) string {
return fmt.Sprintf("%s_match", upstreamName)
}
Expand Down
30 changes: 30 additions & 0 deletions internal/configs/virtualserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2412,3 +2412,33 @@ func TestGenerateQueueForPlus(t *testing.T) {
}

}

func TestGenerateSessionCookie(t *testing.T) {
tests := []struct {
sc *conf_v1alpha1.SessionCookie
expected *version2.SessionCookie
msg string
}{
{
sc: &conf_v1alpha1.SessionCookie{Enable: true, Name: "test"},
expected: &version2.SessionCookie{Enable: true, Name: "test"},
msg: "session cookie with name",
},
{
sc: nil,
expected: nil,
msg: "session cookie with nil",
},
{
sc: &conf_v1alpha1.SessionCookie{Name: "test"},
expected: nil,
msg: "session cookie not enabled",
},
}
for _, test := range tests {
result := generateSessionCookie(test.sc)
if !reflect.DeepEqual(result, test.expected) {
t.Errorf("generateSessionCookie() returned %v, but expected %v for the case of: %v", result, test.expected, test.msg)
}
}
}
12 changes: 12 additions & 0 deletions pkg/apis/configuration/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type Upstream struct {
HealthCheck *HealthCheck `json:"healthCheck"`
SlowStart string `json:"slow-start"`
Queue *UpstreamQueue `json:"queue"`
SessionCookie *SessionCookie `json:"sessionCookie"`
}

// UpstreamBuffers defines Buffer Configuration for an Upstream
Expand Down Expand Up @@ -84,6 +85,17 @@ type Header struct {
Value string `json:"value"`
}

// SessionCookie defines the parameters for session persistence.
type SessionCookie struct {
Enable bool `json:"enable"`
Name string `json:"name"`
Path string `json:"path"`
Expires string `json:"expires"`
Domain string `json:"domain"`
HTTPOnly bool `json:"httpOnly"`
Secure bool `json:"secure"`
}

// Route defines a route.
type Route struct {
Path string `json:"path"`
Expand Down
21 changes: 21 additions & 0 deletions pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 39 additions & 0 deletions pkg/apis/configuration/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,40 @@ func validateUpstreamHealthCheck(hc *v1alpha1.HealthCheck, fieldPath *field.Path
return allErrs
}

func validateSessionCookie(sc *v1alpha1.SessionCookie, fieldPath *field.Path) field.ErrorList {
Dean-Coakley marked this conversation as resolved.
Show resolved Hide resolved
allErrs := field.ErrorList{}

if sc == nil {
return allErrs
}

if sc.Name == "" {
allErrs = append(allErrs, field.Required(fieldPath.Child("name"), ""))
} else {
for _, msg := range isCookieName(sc.Name) {
allErrs = append(allErrs, field.Invalid(fieldPath.Child("name"), sc.Name, msg))
}
}

if sc.Path != "" {
allErrs = append(allErrs, validatePath(sc.Path, fieldPath.Child("path"))...)
}

if sc.Expires != "max" {
Dean-Coakley marked this conversation as resolved.
Show resolved Hide resolved
allErrs = append(allErrs, validateTime(sc.Expires, fieldPath.Child("expires"))...)
}

if sc.Domain != "" {
// A Domain prefix of "." is allowed.
Dean-Coakley marked this conversation as resolved.
Show resolved Hide resolved
domain := strings.TrimPrefix(sc.Domain, ".")
for _, msg := range validation.IsDNS1123Subdomain(domain) {
allErrs = append(allErrs, field.Invalid(fieldPath, sc.Domain, msg))
}
}

return allErrs
}

func validateStatusMatch(s string, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

Expand Down Expand Up @@ -375,6 +409,7 @@ func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path, isP
allErrs = append(allErrs, validateSize(u.ProxyBufferSize, idxPath.Child("buffer-size"))...)
allErrs = append(allErrs, rejectPlusResourcesInOSS(u, idxPath, isPlus)...)
allErrs = append(allErrs, validateQueue(u.Queue, idxPath.Child("queue"), isPlus)...)
allErrs = append(allErrs, validateSessionCookie(u.SessionCookie, idxPath.Child("sessionCookie"))...)

for _, msg := range validation.IsValidPortNum(int(u.Port)) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("port"), u.Port, msg))
Expand Down Expand Up @@ -814,6 +849,10 @@ func rejectPlusResourcesInOSS(upstream v1alpha1.Upstream, idxPath *field.Path, i
allErrs = append(allErrs, field.Forbidden(idxPath.Child("slow-start"), "slow start is only supported in NGINX Plus"))
}

if upstream.SessionCookie != nil {
allErrs = append(allErrs, field.Forbidden(idxPath.Child("sessionCookie"), "sticky cookies are only supported in NGINX Plus"))
}

return allErrs
}

Expand Down
77 changes: 75 additions & 2 deletions pkg/apis/configuration/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2024,8 +2024,9 @@ func TestValidateIntFromStringFails(t *testing.T) {

func TestRejectPlusResourcesInOSS(t *testing.T) {
upstream := v1alpha1.Upstream{
SlowStart: "10s",
HealthCheck: &v1alpha1.HealthCheck{},
SlowStart: "10s",
HealthCheck: &v1alpha1.HealthCheck{},
SessionCookie: &v1alpha1.SessionCookie{},
}

allErrsPlus := rejectPlusResourcesInOSS(upstream, field.NewPath("upstreams").Index(0), true)
Expand Down Expand Up @@ -2110,3 +2111,75 @@ func TestValidateQueueFails(t *testing.T) {
}
}
}

func TestValidateSessionCookie(t *testing.T) {
tests := []struct {
sc *v1alpha1.SessionCookie
fieldPath *field.Path
msg string
}{
{
sc: &v1alpha1.SessionCookie{Enable: true, Name: "min"},
fieldPath: field.NewPath("sessionCookie"),
msg: "min valid config",
},
{
sc: &v1alpha1.SessionCookie{Enable: true, Name: "test", Expires: "max"},
fieldPath: field.NewPath("sessionCookie"),
msg: "valid config with expires max",
},
{
sc: &v1alpha1.SessionCookie{
Enable: true, Name: "test", Path: "/tea", Expires: "1", Domain: ".example.com", HTTPOnly: false, Secure: true,
},
fieldPath: field.NewPath("sessionCookie"),
msg: "max valid config",
},
}
for _, test := range tests {
allErrs := validateSessionCookie(test.sc, test.fieldPath)
if len(allErrs) != 0 {
t.Errorf("validateSessionCookie() returned errors %v for valid input for the case of: %s", allErrs, test.msg)
}
}
}

func TestValidateSessionCookieFails(t *testing.T) {
tests := []struct {
sc *v1alpha1.SessionCookie
fieldPath *field.Path
msg string
}{
{
sc: &v1alpha1.SessionCookie{Enable: true},
fieldPath: field.NewPath("sessionCookie"),
msg: "missing required field: Name",
},
{
sc: &v1alpha1.SessionCookie{Enable: false},
fieldPath: field.NewPath("sessionCookie"),
msg: "session cookie not enabled",
},
{
sc: &v1alpha1.SessionCookie{Enable: true, Name: "$ecret-Name"},
fieldPath: field.NewPath("sessionCookie"),
msg: "invalid name format",
},
{
sc: &v1alpha1.SessionCookie{Enable: true, Name: "test", Expires: "EGGS"},
fieldPath: field.NewPath("sessionCookie"),
msg: "invalid time format",
},
{
sc: &v1alpha1.SessionCookie{Enable: true, Name: "test", Path: "/ coffee"},
fieldPath: field.NewPath("sessionCookie"),
msg: "invalid path format",
},
}
for _, test := range tests {
allErrs := validateSessionCookie(test.sc, test.fieldPath)
if len(allErrs) == 0 {
t.Errorf("validateSessionCookie() returned no errors for invalid input for the case of: %v", test.msg)
}
}
}