From 50b29feb4af4e78fdcabd17dacf635de37e9893f Mon Sep 17 00:00:00 2001 From: Zenara Daley Date: Mon, 19 Nov 2018 09:15:24 -0500 Subject: [PATCH] Add annotation for session affinity path --- .../nginx-configuration/annotations.md | 6 ++ .../annotations/sessionaffinity/main.go | 11 ++++ .../annotations/sessionaffinity/main_test.go | 15 +++-- internal/ingress/controller/controller.go | 7 ++- internal/ingress/types.go | 1 + internal/ingress/types_equals.go | 3 + rootfs/etc/nginx/lua/balancer/sticky.lua | 8 ++- test/e2e/annotations/affinity.go | 61 +++++++++++++++++++ 8 files changed, 105 insertions(+), 7 deletions(-) diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 366ac9b2b..e155d1b5f 100644 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -65,6 +65,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz |[nginx.ingress.kubernetes.io/service-upstream](#service-upstream)|"true" or "false"| |[nginx.ingress.kubernetes.io/session-cookie-name](#cookie-affinity)|string| |[nginx.ingress.kubernetes.io/session-cookie-hash](#cookie-affinity)|string| +|[nginx.ingress.kubernetes.io/session-cookie-path](#cookie-affinity)|string| |[nginx.ingress.kubernetes.io/ssl-redirect](#server-side-https-enforcement-through-redirect)|"true" or "false"| |[nginx.ingress.kubernetes.io/ssl-passthrough](#ssl-passthrough)|"true" or "false"| |[nginx.ingress.kubernetes.io/upstream-hash-by](#custom-nginx-upstream-hashing)|string| @@ -119,6 +120,8 @@ If you use the ``cookie`` affinity type you can also specify the name of the coo In case of NGINX the annotation `nginx.ingress.kubernetes.io/session-cookie-hash` defines which algorithm will be used to hash the used upstream. Default value is `md5` and possible values are `md5`, `sha1` and `index`. +The NGINX annotation `nginx.ingress.kubernetes.io/session-cookie-path` defines the path that will be set on the cookie. This is optional unless the annotation `nginx.ingress.kubernetes.io/use-regex` is set to true; Session cookie paths do not support regex. + !!! attention The `index` option is not an actual hash; an in-memory index is used instead, which has less overhead. However, with `index`, matching against a changing upstream server list is inconsistent. @@ -639,6 +642,9 @@ nginx.ingress.kubernetes.io/backend-protocol: "HTTPS" ### Use Regex +!!! attention +When using this annotation with the NGINX annotation `nginx.ingress.kubernetes.io/affinity` of type `cookie`, `nginx.ingress.kubernetes.io/session-cookie-path` must be also set; Session cookie paths do not support regex. + Using the `nginx.ingress.kubernetes.io/use-regex` annotation will indicate whether or not the paths defined on an Ingress use regular expressions. The default value is `false`. The following will indicate that regular expression paths are being used: diff --git a/internal/ingress/annotations/sessionaffinity/main.go b/internal/ingress/annotations/sessionaffinity/main.go index 09599b6b5..2f9b7e41c 100644 --- a/internal/ingress/annotations/sessionaffinity/main.go +++ b/internal/ingress/annotations/sessionaffinity/main.go @@ -47,6 +47,9 @@ const ( // This is used to control the cookie expires, its value is a number of seconds until the // cookie expires annotationAffinityCookieMaxAge = "session-cookie-max-age" + + // This is used to control the cookie path when use-regex is set to true + annotationAffinityCookiePath = "session-cookie-path" ) var ( @@ -71,6 +74,8 @@ type Cookie struct { Expires string `json:"expires"` // The number of seconds until the cookie expires MaxAge string `json:"maxage"` + // The path that a cookie will be set on + Path string `json:"path"` } // cookieAffinityParse gets the annotation values related to Cookie Affinity @@ -106,6 +111,12 @@ func (a affinity) cookieAffinityParse(ing *extensions.Ingress) *Cookie { sm = "" } cookie.MaxAge = sm + + sp, err := parser.GetStringAnnotation(annotationAffinityCookiePath, ing) + if err != nil || sp == "" { + glog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieMaxAge) + } + cookie.Path = sp return cookie } diff --git a/internal/ingress/annotations/sessionaffinity/main_test.go b/internal/ingress/annotations/sessionaffinity/main_test.go index a3e89d662..e22fe0f4d 100644 --- a/internal/ingress/annotations/sessionaffinity/main_test.go +++ b/internal/ingress/annotations/sessionaffinity/main_test.go @@ -71,6 +71,7 @@ func TestIngressAffinityCookieConfig(t *testing.T) { data[parser.GetAnnotationWithPrefix(annotationAffinityCookieName)] = "INGRESSCOOKIE" data[parser.GetAnnotationWithPrefix(annotationAffinityCookieExpires)] = "4500" data[parser.GetAnnotationWithPrefix(annotationAffinityCookieMaxAge)] = "3000" + data[parser.GetAnnotationWithPrefix(annotationAffinityCookiePath)] = "/foo" ing.SetAnnotations(data) affin, _ := NewParser(&resolver.Mock{}).Parse(ing) @@ -80,22 +81,26 @@ func TestIngressAffinityCookieConfig(t *testing.T) { } if nginxAffinity.Type != "cookie" { - t.Errorf("expected cookie as sticky-type but returned %v", nginxAffinity.Type) + t.Errorf("expected cookie as affinity but returned %v", nginxAffinity.Type) } if nginxAffinity.Cookie.Hash != "md5" { - t.Errorf("expected md5 as sticky-hash but returned %v", nginxAffinity.Cookie.Hash) + t.Errorf("expected md5 as session-cookie-hash but returned %v", nginxAffinity.Cookie.Hash) } if nginxAffinity.Cookie.Name != "INGRESSCOOKIE" { - t.Errorf("expected route as sticky-name but returned %v", nginxAffinity.Cookie.Name) + t.Errorf("expected route as session-cookie-name but returned %v", nginxAffinity.Cookie.Name) } if nginxAffinity.Cookie.Expires != "4500" { - t.Errorf("expected 1h as sticky-expires but returned %v", nginxAffinity.Cookie.Expires) + t.Errorf("expected 1h as session-cookie-expires but returned %v", nginxAffinity.Cookie.Expires) } if nginxAffinity.Cookie.MaxAge != "3000" { - t.Errorf("expected 3000 as sticky-max-age but returned %v", nginxAffinity.Cookie.MaxAge) + t.Errorf("expected 3000 as session-cookie-max-age but returned %v", nginxAffinity.Cookie.MaxAge) + } + + if nginxAffinity.Cookie.Path != "/foo" { + t.Errorf("expected /foo as session-cookie-path but returned %v", nginxAffinity.Cookie.Path) } } diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 5739f805b..5eecbebe7 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -404,16 +404,21 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] } if anns.SessionAffinity.Type == "cookie" { + cookiePath := anns.SessionAffinity.Cookie.Path + if anns.Rewrite.UseRegex && cookiePath == "" { + glog.Warningf("session-cookie-path should be set when use-regex is true") + } + ups.SessionAffinity.CookieSessionAffinity.Name = anns.SessionAffinity.Cookie.Name ups.SessionAffinity.CookieSessionAffinity.Hash = anns.SessionAffinity.Cookie.Hash ups.SessionAffinity.CookieSessionAffinity.Expires = anns.SessionAffinity.Cookie.Expires ups.SessionAffinity.CookieSessionAffinity.MaxAge = anns.SessionAffinity.Cookie.MaxAge + ups.SessionAffinity.CookieSessionAffinity.Path = cookiePath locs := ups.SessionAffinity.CookieSessionAffinity.Locations if _, ok := locs[host]; !ok { locs[host] = []string{} } - locs[host] = append(locs[host], path.Path) } } diff --git a/internal/ingress/types.go b/internal/ingress/types.go index 6f10f3946..690984607 100644 --- a/internal/ingress/types.go +++ b/internal/ingress/types.go @@ -112,6 +112,7 @@ type CookieSessionAffinity struct { Expires string `json:"expires,omitempty"` MaxAge string `json:"maxage,omitempty"` Locations map[string][]string `json:"locations,omitempty"` + Path string `json:"path,omitempty"` } // Endpoint describes a kubernetes endpoint in a backend diff --git a/internal/ingress/types_equals.go b/internal/ingress/types_equals.go index 31e88d1cd..45c7042a6 100644 --- a/internal/ingress/types_equals.go +++ b/internal/ingress/types_equals.go @@ -168,6 +168,9 @@ func (csa1 *CookieSessionAffinity) Equal(csa2 *CookieSessionAffinity) bool { if csa1.Hash != csa2.Hash { return false } + if csa1.Path != csa2.Path { + return false + } return true } diff --git a/rootfs/etc/nginx/lua/balancer/sticky.lua b/rootfs/etc/nginx/lua/balancer/sticky.lua index c61cef446..14e435e4d 100644 --- a/rootfs/etc/nginx/lua/balancer/sticky.lua +++ b/rootfs/etc/nginx/lua/balancer/sticky.lua @@ -17,6 +17,7 @@ function _M.new(self, backend) cookie_name = backend["sessionAffinityConfig"]["cookieSessionAffinity"]["name"] or "route", cookie_expires = backend["sessionAffinityConfig"]["cookieSessionAffinity"]["expires"], cookie_max_age = backend["sessionAffinityConfig"]["cookieSessionAffinity"]["maxage"], + cookie_path = backend["sessionAffinityConfig"]["cookieSessionAffinity"]["path"], digest_func = digest_func, } setmetatable(o, self) @@ -39,10 +40,15 @@ local function set_cookie(self, value) ngx.log(ngx.ERR, err) end + local cookie_path = self.cookie_path + if not cookie_path then + cookie_path = ngx.var.location_path + end + local cookie_data = { key = self.cookie_name, value = value, - path = ngx.var.location_path, + path = cookie_path, domain = ngx.var.host, httponly = true, } diff --git a/test/e2e/annotations/affinity.go b/test/e2e/annotations/affinity.go index 6e175b6c3..53390b2cd 100644 --- a/test/e2e/annotations/affinity.go +++ b/test/e2e/annotations/affinity.go @@ -223,4 +223,65 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring(fmt.Sprintf("Expires=%s", expected))) Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("Max-Age=259200")) }) + + It("should work with use-regex annotation and session-cookie-path", func() { + host := "cookie.foo.com" + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/affinity": "cookie", + "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", + "nginx.ingress.kubernetes.io/use-regex": "true", + "nginx.ingress.kubernetes.io/session-cookie-path": "/foo/bar", + } + + ing := framework.NewSingleIngress(host, "/foo/.*", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) + }) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL+"/foo/bar"). + Set("Host", host). + End() + + md5Regex := regexp.MustCompile("SERVERID=[0-9a-f]{32}") + match := md5Regex.FindStringSubmatch(resp.Header.Get("Set-Cookie")) + Expect(len(match)).Should(BeNumerically("==", 1)) + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring(match[0])) + Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("Path=/foo/bar")) + }) + + It("should warn user when use-regex is true and session-cookie-path is not set", func() { + host := "cookie.foo.com" + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/affinity": "cookie", + "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", + "nginx.ingress.kubernetes.io/use-regex": "true", + } + + ing := framework.NewSingleIngress(host, "/foo/.*", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) + }) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL+"/foo/bar"). + Set("Host", host). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + + logs, err := f.NginxLogs() + Expect(err).ToNot(HaveOccurred()) + Expect(logs).To(ContainSubstring(`session-cookie-path should be set when use-regex is true`)) + }) })