From ad57c76b73bee1a5f2a7f820a18dc81dfab1c529 Mon Sep 17 00:00:00 2001 From: liuwei Date: Mon, 29 Oct 2018 15:21:10 +0800 Subject: [PATCH] Support cookie expires --- .../annotations/sessionaffinity/main.go | 36 +++++- .../annotations/sessionaffinity/main_test.go | 10 ++ internal/ingress/controller/controller.go | 2 + internal/ingress/types.go | 2 + rootfs/etc/nginx/lua/balancer/sticky.lua | 49 +++++++- .../nginx/lua/test/balancer/sticky_test.lua | 117 ++++++++++++++++++ 6 files changed, 209 insertions(+), 7 deletions(-) diff --git a/internal/ingress/annotations/sessionaffinity/main.go b/internal/ingress/annotations/sessionaffinity/main.go index 49de25d33..35ae09335 100644 --- a/internal/ingress/annotations/sessionaffinity/main.go +++ b/internal/ingress/annotations/sessionaffinity/main.go @@ -39,10 +39,21 @@ const ( // one isn't supplied and affinity is set to "cookie". annotationAffinityCookieHash = "session-cookie-hash" defaultAffinityCookieHash = "md5" + + // This is used to control the cookie expires, its value is a time period, the cookie will + // not expire during the time period. e.g. 1h (1 hour) the validated time unit includes: + // y, M, w, d, h, m and s + annotationAffinityCookieExpires = "session-cookie-expires" + + // This is used to control the cookie expires, its value is a number of seconds until the + // cookie expires + annotationAffinityCookieMaxAge = "session-cookie-max-age" ) var ( - affinityCookieHashRegex = regexp.MustCompile(`^(index|md5|sha1)$`) + affinityCookieHashRegex = regexp.MustCompile(`^(index|md5|sha1)$`) + affinityCookieExpiresRegex = regexp.MustCompile(`^(\d+(y|M|w|d|h|m|s))$`) + affinityCookieMaxAgeRegex = regexp.MustCompile(`(^0|-?[1-9]\d*$)`) ) // Config describes the per ingress session affinity config @@ -58,17 +69,23 @@ type Cookie struct { Name string `json:"name"` // The hash that will be used to encode the cookie in case of cookie affinity type Hash string `json:"hash"` + // The time period to control cookie expires + Expires string `json:"expires"` + // The number of seconds until the cookie expires + MaxAge string `json:"maxage"` } // cookieAffinityParse gets the annotation values related to Cookie Affinity // It also sets default values when no value or incorrect value is found func (a affinity) cookieAffinityParse(ing *extensions.Ingress) *Cookie { + cookie := &Cookie{} sn, err := parser.GetStringAnnotation(annotationAffinityCookieName, ing) if err != nil || sn == "" { glog.V(3).Infof("Ingress %v: No value found in annotation %v. Using the default %v", ing.Name, annotationAffinityCookieName, defaultAffinityCookieName) sn = defaultAffinityCookieName } + cookie.Name = sn sh, err := parser.GetStringAnnotation(annotationAffinityCookieHash, ing) @@ -76,11 +93,22 @@ func (a affinity) cookieAffinityParse(ing *extensions.Ingress) *Cookie { glog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Setting it to default %v", ing.Name, annotationAffinityCookieHash, defaultAffinityCookieHash) sh = defaultAffinityCookieHash } + cookie.Hash = sh - return &Cookie{ - Name: sn, - Hash: sh, + se, err := parser.GetStringAnnotation(annotationAffinityCookieExpires, ing) + if err != nil && !affinityCookieExpiresRegex.MatchString(se) { + glog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieExpires) + se = "" } + cookie.Expires = se + + sm, err := parser.GetStringAnnotation(annotationAffinityCookieMaxAge, ing) + if err != nil && !affinityCookieMaxAgeRegex.MatchString(sm) { + glog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieMaxAge) + sm = "" + } + cookie.MaxAge = sm + return cookie } // NewParser creates a new Affinity annotation parser diff --git a/internal/ingress/annotations/sessionaffinity/main_test.go b/internal/ingress/annotations/sessionaffinity/main_test.go index 3db53a2f7..d00e41c20 100644 --- a/internal/ingress/annotations/sessionaffinity/main_test.go +++ b/internal/ingress/annotations/sessionaffinity/main_test.go @@ -69,6 +69,8 @@ func TestIngressAffinityCookieConfig(t *testing.T) { data[parser.GetAnnotationWithPrefix(annotationAffinityType)] = "cookie" data[parser.GetAnnotationWithPrefix(annotationAffinityCookieHash)] = "sha123" data[parser.GetAnnotationWithPrefix(annotationAffinityCookieName)] = "INGRESSCOOKIE" + data[parser.GetAnnotationWithPrefix(annotationAffinityCookieExpires)] = "1h" + data[parser.GetAnnotationWithPrefix(annotationAffinityCookieMaxAge)] = "3000" ing.SetAnnotations(data) affin, _ := NewParser(&resolver.Mock{}).Parse(ing) @@ -88,4 +90,12 @@ func TestIngressAffinityCookieConfig(t *testing.T) { if nginxAffinity.Cookie.Name != "INGRESSCOOKIE" { t.Errorf("expected route as sticky-name but returned %v", nginxAffinity.Cookie.Name) } + + if nginxAffinity.Cookie.Expires != "1h" { + t.Errorf("expected 1h as sticky-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) + } } diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index d070c5e12..e211db233 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -405,6 +405,8 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] if anns.SessionAffinity.Type == "cookie" { 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 locs := ups.SessionAffinity.CookieSessionAffinity.Locations if _, ok := locs[host]; !ok { diff --git a/internal/ingress/types.go b/internal/ingress/types.go index fcf496108..fea5b0c28 100644 --- a/internal/ingress/types.go +++ b/internal/ingress/types.go @@ -109,6 +109,8 @@ type SessionAffinityConfig struct { type CookieSessionAffinity struct { Name string `json:"name"` Hash string `json:"hash"` + Expires string `json:"expires,omitempty"` + MaxAge string `json:"maxage,omitempty"` Locations map[string][]string `json:"locations,omitempty"` } diff --git a/rootfs/etc/nginx/lua/balancer/sticky.lua b/rootfs/etc/nginx/lua/balancer/sticky.lua index 4d4684146..fc9387dd5 100644 --- a/rootfs/etc/nginx/lua/balancer/sticky.lua +++ b/rootfs/etc/nginx/lua/balancer/sticky.lua @@ -15,6 +15,8 @@ function _M.new(self, backend) local o = { instance = self.factory:new(nodes), cookie_name = backend["sessionAffinityConfig"]["cookieSessionAffinity"]["name"] or "route", + cookie_expires = backend["sessionAffinityConfig"]["cookieSessionAffinity"]["expires"], + cookie_max_age = backend["sessionAffinityConfig"]["cookieSessionAffinity"]["maxage"], digest_func = digest_func, } setmetatable(o, self) @@ -31,20 +33,61 @@ local function encrypted_endpoint_string(self, endpoint_string) return encrypted end +local function parse_cookie_expires(expires) + local time = tonumber(string.sub(expires, 0, string.len(expires) - 1)) + if time == nil then + return nil, string.format("the time of expires (%s) is wrong", expires) + end + + local unit = string.sub(expires, -1) + if unit == "y" then + return time * 60 * 60 * 24 * 365 + elseif unit == "M" then + return time * 60 * 60 * 24 * 30 + elseif unit == "w" then + return time * 60 * 60 * 24 * 7 + elseif unit == "d" then + return time * 60 * 60 * 24 + elseif unit == "h" then + return time * 60 * 60 + elseif unit == "m" then + return time * 60 + elseif unit == "s" then + return time + else + return nil, string.format("the unit of expires (%s) is wrong, validated unit includes: y, M, w, d, h, m and s", expires) + end +end + local function set_cookie(self, value) local cookie, err = ck:new() if not cookie then ngx.log(ngx.ERR, err) end - local ok - ok, err = cookie:set({ + local cookie_data = { key = self.cookie_name, value = value, path = ngx.var.location_path, domain = ngx.var.host, httponly = true, - }) + } + + if self.cookie_expires then + local expires, err = parse_cookie_expires(self.cookie_expires) + if err then + ngx.log(ngx.WARN, string.format("error when parsing cookie expires: %s, ignoring it", tostring(err))) + else + cookie_data.expires = ngx.cookie_time(expires) + end + end + + if self.cookie_max_age then + cookie_data.max_age = tonumber(self.cookie_max_age) + end + + local ok + ok, err = cookie:set(cookie_data) if not ok then ngx.log(ngx.ERR, err) end diff --git a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua index d5c402236..264acf39c 100644 --- a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua +++ b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua @@ -88,6 +88,38 @@ describe("Sticky", function() assert.equal(sticky_balancer_instance.digest_func, default_hash_implementation) end) end) + + context("when backend specifies cookie expires", function() + it("returns an instance containing the corresponding cookie expires", function() + local temp_backend = util.deepcopy(test_backend) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "1y" + local sticky_balancer_instance = sticky:new(temp_backend) + assert.equal(sticky_balancer_instance.cookie_expires, "1y") + end) + end) + + context("when backend does not specify cookie expires", function() + it("returns an instance without cookie expires", function() + local sticky_balancer_instance = sticky:new(test_backend) + assert.equal(sticky_balancer_instance.cookie_expires, nil) + end) + end) + + context("when backend specifies cookie max-age", function() + it("returns an instance containing the corresponding cookie max-age", function() + local temp_backend = util.deepcopy(test_backend) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.maxage = 1000 + local sticky_balancer_instance = sticky:new(temp_backend) + assert.equal(sticky_balancer_instance.cookie_max_age, 1000) + end) + end) + + context("when backend does not specify cookie max-age", function() + it("returns an instance without cookie max-age", function() + local sticky_balancer_instance = sticky:new(test_backend) + assert.equal(sticky_balancer_instance.cookie_max_age, nil) + end) + end) end) describe("balance()", function() @@ -131,6 +163,91 @@ describe("Sticky", function() assert.has_no.errors(function() sticky_balancer_instance:balance() end) assert.spy(s).was_called() end) + + it("sets an expires cookie (with expires) on the client", function() + local s = {} + local expected_expires + cookie.new = function(self) + local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash + local cookie_instance = { + set = function(self, payload) + assert.equal(payload.key, test_backend.sessionAffinityConfig.cookieSessionAffinity.name) + assert.equal(payload.value, util[test_backend_hash_fn .. "_digest"](test_backend_endpoint)) + assert.equal(payload.path, ngx.var.location_path) + assert.equal(payload.domain, nil) + assert.equal(payload.httponly, true) + expected_expires = payload.expires + return true, nil + end, + get = function(k) return false end, + } + s = spy.on(cookie_instance, "set") + return cookie_instance, false + end + local temp_backend = util.deepcopy(get_test_backend()) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "1y" + local sticky_balancer_instance = sticky:new(temp_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + assert.equal(expected_expires, ngx.cookie_time(1 * 60 * 60 * 24 * 365)) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "10M" + sticky_balancer_instance = sticky:new(temp_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + assert.equal(expected_expires, ngx.cookie_time(10 * 60 * 60 * 24 * 30)) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "8w" + sticky_balancer_instance = sticky:new(temp_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + assert.equal(expected_expires, ngx.cookie_time(8 * 60 * 60 * 24 * 7)) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "200d" + sticky_balancer_instance = sticky:new(temp_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + assert.equal(expected_expires, ngx.cookie_time(200 * 60 * 60 * 24)) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "24h" + sticky_balancer_instance = sticky:new(temp_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + assert.equal(expected_expires, ngx.cookie_time(24 * 60 * 60)) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "30m" + sticky_balancer_instance = sticky:new(temp_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + assert.equal(expected_expires, ngx.cookie_time(30 * 60)) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "300s" + sticky_balancer_instance = sticky:new(temp_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + assert.equal(expected_expires, ngx.cookie_time(300)) + end) + + it("sets an expires cookie (with max-age) on the client", function() + local s = {} + local cookie_payload = {} + cookie.new = function(self) + local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash + local cookie_instance = { + set = function(self, payload) + assert.equal(payload.key, test_backend.sessionAffinityConfig.cookieSessionAffinity.name) + assert.equal(payload.value, util[test_backend_hash_fn .. "_digest"](test_backend_endpoint)) + assert.equal(payload.path, ngx.var.location_path) + assert.equal(payload.domain, nil) + assert.equal(payload.httponly, true) + assert.equal(payload.max_age, 1000) + return true, nil + end, + get = function(k) return false end, + } + s = spy.on(cookie_instance, "set") + return cookie_instance, false + end + local temp_backend = util.deepcopy(get_test_backend()) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.maxage = "1000" + local sticky_balancer_instance = sticky:new(temp_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + end) end) context("when client has a cookie set", function()