From c180a0998b7a49105a74bbcf0a0b5b25a99edbf6 Mon Sep 17 00:00:00 2001 From: Alex Kursell Date: Tue, 19 Feb 2019 17:27:08 -0500 Subject: [PATCH] Fix session-cookie-* annotation reloading --- internal/ingress/types_equals.go | 6 ++ rootfs/etc/nginx/lua/balancer/sticky.lua | 56 ++++++++++++----- .../nginx/lua/test/balancer/sticky_test.lua | 4 +- test/e2e/annotations/affinity.go | 62 ++++++++++++++++--- 4 files changed, 102 insertions(+), 26 deletions(-) diff --git a/internal/ingress/types_equals.go b/internal/ingress/types_equals.go index 968c365298..ddda04c7d8 100644 --- a/internal/ingress/types_equals.go +++ b/internal/ingress/types_equals.go @@ -230,6 +230,12 @@ func (csa1 *CookieSessionAffinity) Equal(csa2 *CookieSessionAffinity) bool { if csa1.Path != csa2.Path { return false } + if csa1.Expires != csa2.Expires { + return false + } + if csa1.MaxAge != csa2.MaxAge { + return false + } return true } diff --git a/rootfs/etc/nginx/lua/balancer/sticky.lua b/rootfs/etc/nginx/lua/balancer/sticky.lua index f60d1810e0..6b062b4d2a 100644 --- a/rootfs/etc/nginx/lua/balancer/sticky.lua +++ b/rootfs/etc/nginx/lua/balancer/sticky.lua @@ -4,24 +4,30 @@ local util = require("util") local ck = require("resty.cookie") local _M = balancer_resty:new({ factory = resty_chash, name = "sticky" }) +local DEFAULT_COOKIE_NAME = "route" -function _M.new(self, backend) - local nodes = util.get_nodes(backend.endpoints) +local function get_digest_func(hash) local digest_func = util.md5_digest - if backend["sessionAffinityConfig"]["cookieSessionAffinity"]["hash"] == "sha1" then + if hash == "sha1" then digest_func = util.sha1_digest end + return digest_func +end + +function _M.cookie_name(self) + return self.cookie_session_affinity.name or DEFAULT_COOKIE_NAME +end + +function _M.new(self, backend) + local nodes = util.get_nodes(backend.endpoints) + local digest_func = get_digest_func(backend["sessionAffinityConfig"]["cookieSessionAffinity"]["hash"]) 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"], - cookie_path = backend["sessionAffinityConfig"]["cookieSessionAffinity"]["path"], - cookie_locations = backend["sessionAffinityConfig"]["cookieSessionAffinity"]["locations"], digest_func = digest_func, traffic_shaping_policy = backend.trafficShapingPolicy, alternative_backends = backend.alternativeBackends, + cookie_session_affinity = backend["sessionAffinityConfig"]["cookieSessionAffinity"] } setmetatable(o, self) self.__index = self @@ -43,25 +49,25 @@ local function set_cookie(self, value) ngx.log(ngx.ERR, err) end - local cookie_path = self.cookie_path + local cookie_path = self.cookie_session_affinity.path if not cookie_path then cookie_path = ngx.var.location_path end local cookie_data = { - key = self.cookie_name, + key = self:cookie_name(), value = value, path = cookie_path, httponly = true, secure = ngx.var.https == "on", } - if self.cookie_expires and self.cookie_expires ~= "" then - cookie_data.expires = ngx.cookie_time(ngx.time() + tonumber(self.cookie_expires)) + if self.cookie_session_affinity.expires and self.cookie_session_affinity.expires ~= "" then + cookie_data.expires = ngx.cookie_time(ngx.time() + tonumber(self.cookie_session_affinity.expires)) end - if self.cookie_max_age and self.cookie_max_age ~= "" then - cookie_data.max_age = tonumber(self.cookie_max_age) + if self.cookie_session_affinity.maxage and self.cookie_session_affinity.maxage ~= "" then + cookie_data.max_age = tonumber(self.cookie_session_affinity.maxage) end local ok @@ -78,13 +84,13 @@ function _M.balance(self) return end - local key = cookie:get(self.cookie_name) + local key = cookie:get(self:cookie_name()) if not key then local random_str = string.format("%s.%s", ngx.now(), ngx.worker.pid()) key = encrypted_endpoint_string(self, random_str) - if self.cookie_locations then - local locs = self.cookie_locations[ngx.var.host] + if self.cookie_session_affinity.locations then + local locs = self.cookie_session_affinity.locations[ngx.var.host] if locs ~= nil then for _, path in pairs(locs) do if ngx.var.location_path == path then @@ -99,4 +105,20 @@ function _M.balance(self) return self.instance:find(key) end +function _M.sync(self, backend) + balancer_resty.sync(self, backend) + + -- Reload the balancer if any of the annotations have changed. + local changed = not util.deep_compare( + self.cookie_session_affinity, + backend.sessionAffinityConfig.cookieSessionAffinity + ) + if not changed then + return + end + + self.cookie_session_affinity = backend.sessionAffinityConfig.cookieSessionAffinity + self.digest_func = get_digest_func(backend.sessionAffinityConfig.cookieSessionAffinity.hash) +end + return _M diff --git a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua index 8b9741d3f6..06a6235573 100644 --- a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua +++ b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua @@ -55,7 +55,7 @@ describe("Sticky", function() it("returns an instance containing the corresponding cookie name", function() local sticky_balancer_instance = sticky:new(test_backend) local test_backend_cookie_name = test_backend.sessionAffinityConfig.cookieSessionAffinity.name - assert.equal(sticky_balancer_instance.cookie_name, test_backend_cookie_name) + assert.equal(sticky_balancer_instance:cookie_name(), test_backend_cookie_name) end) end) @@ -65,7 +65,7 @@ describe("Sticky", function() temp_backend.sessionAffinityConfig.cookieSessionAffinity.name = nil local sticky_balancer_instance = sticky:new(temp_backend) local default_cookie_name = "route" - assert.equal(sticky_balancer_instance.cookie_name, default_cookie_name) + assert.equal(sticky_balancer_instance:cookie_name(), default_cookie_name) end) end) diff --git a/test/e2e/annotations/affinity.go b/test/e2e/annotations/affinity.go index 37ee031462..bc65f90ebe 100644 --- a/test/e2e/annotations/affinity.go +++ b/test/e2e/annotations/affinity.go @@ -58,6 +58,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", func(server string) bool { return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) }) + time.Sleep(waitForLuaSync) resp, _, errs := gorequest.New(). Get(f.IngressController.HTTPURL). @@ -73,8 +74,48 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring(match[0])) }) + It("should change cookie name on ingress definition change", func() { + host := "change.foo.com" + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/affinity": "cookie", + "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", + } + + ing := framework.NewSingleIngress(host, "/", 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)) + }) + time.Sleep(waitForLuaSync) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("SERVERID")) + + ing.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/session-cookie-name"] = "OTHERCOOKIENAME" + f.EnsureIngress(ing) + + time.Sleep(waitForLuaSync) + + resp, _, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("OTHERCOOKIENAME")) + }) + It("should set sticky cookie with sha1 hash", func() { - host := "sticky.foo.com" + host := "sha1.foo.com" annotations := map[string]string{ "nginx.ingress.kubernetes.io/affinity": "cookie", "nginx.ingress.kubernetes.io/session-cookie-hash": "sha1", @@ -87,6 +128,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", func(server string) bool { return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) }) + time.Sleep(waitForLuaSync) resp, _, errs := gorequest.New(). Get(f.IngressController.HTTPURL). @@ -103,7 +145,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", }) It("should set the path to /something on the generated cookie", func() { - host := "example.com" + host := "path.foo.com" annotations := map[string]string{ "nginx.ingress.kubernetes.io/affinity": "cookie", "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", @@ -116,6 +158,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", func(server string) bool { return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) }) + time.Sleep(waitForLuaSync) resp, _, errs := gorequest.New(). Get(f.IngressController.HTTPURL+"/something"). @@ -128,7 +171,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", }) It("does not set the path to / on the generated cookie if there's more than one rule referring to the same backend", func() { - host := "example.com" + host := "morethanonerule.foo.com" annotations := map[string]string{ "nginx.ingress.kubernetes.io/affinity": "cookie", "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", @@ -173,6 +216,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", func(server string) bool { return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) }) + time.Sleep(waitForLuaSync) resp, _, errs := gorequest.New(). Get(f.IngressController.HTTPURL+"/something"). @@ -194,7 +238,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", }) It("should set cookie with expires", func() { - host := "cookie.foo.com" + host := "cookieexpires.foo.com" annotations := map[string]string{ "nginx.ingress.kubernetes.io/affinity": "cookie", "nginx.ingress.kubernetes.io/session-cookie-name": "ExpiresCookie", @@ -209,6 +253,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", func(server string) bool { return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) }) + time.Sleep(waitForLuaSync) resp, _, errs := gorequest.New(). Get(f.IngressController.HTTPURL). @@ -225,7 +270,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", }) It("should work with use-regex annotation and session-cookie-path", func() { - host := "cookie.foo.com" + host := "useregex.foo.com" annotations := map[string]string{ "nginx.ingress.kubernetes.io/affinity": "cookie", "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", @@ -240,6 +285,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", func(server string) bool { return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) }) + time.Sleep(waitForLuaSync) resp, _, errs := gorequest.New(). Get(f.IngressController.HTTPURL+"/foo/bar"). @@ -257,7 +303,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", }) It("should warn user when use-regex is true and session-cookie-path is not set", func() { - host := "cookie.foo.com" + host := "useregexwarn.foo.com" annotations := map[string]string{ "nginx.ingress.kubernetes.io/affinity": "cookie", "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", @@ -271,6 +317,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", func(server string) bool { return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) }) + time.Sleep(waitForLuaSync) resp, _, errs := gorequest.New(). Get(f.IngressController.HTTPURL+"/foo/bar"). @@ -286,7 +333,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", }) It("should not set affinity across all server locations when using separate ingresses", func() { - host := "cookie.foo.com" + host := "separate.foo.com" annotations := map[string]string{ "nginx.ingress.kubernetes.io/affinity": "cookie", @@ -301,6 +348,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", func(server string) bool { return strings.Contains(server, `location /foo/bar`) && strings.Contains(server, `location /foo`) }) + time.Sleep(waitForLuaSync) resp, _, errs := gorequest.New(). Get(f.IngressController.HTTPURL+"/foo").