From 6c92c80073f3bd9509ba4cb7ae8c277355a3d427 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Sat, 28 Dec 2019 19:56:13 -0300 Subject: [PATCH] Fix sticky session for ingress without host --- rootfs/etc/nginx/lua/balancer/sticky.lua | 11 ++++-- .../nginx/lua/test/balancer/sticky_test.lua | 39 ++++++++++++++++++- test/e2e/annotations/affinity.go | 24 ++++++++++++ test/e2e/framework/framework.go | 6 ++- 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/rootfs/etc/nginx/lua/balancer/sticky.lua b/rootfs/etc/nginx/lua/balancer/sticky.lua index a5570911cf..91183caf7f 100644 --- a/rootfs/etc/nginx/lua/balancer/sticky.lua +++ b/rootfs/etc/nginx/lua/balancer/sticky.lua @@ -82,11 +82,16 @@ local function get_failed_upstreams() end local function should_set_cookie(self) - if self.cookie_session_affinity.locations and ngx.var.host then - local locs = self.cookie_session_affinity.locations[ngx.var.host] + local host = ngx.var.host + if ngx.var.server_name == '_' then + host = ngx.var.server_name + end + + if self.cookie_session_affinity.locations then + local locs = self.cookie_session_affinity.locations[host] if locs == nil then -- Based off of wildcard hostname in ../certificate.lua - local wildcard_host, _, err = ngx.re.sub(ngx.var.host, "^[^\\.]+\\.", "*.", "jo") + local wildcard_host, _, err = ngx.re.sub(host, "^[^\\.]+\\.", "*.", "jo") if err then ngx.log(ngx.ERR, "error: ", err); elseif wildcard_host then diff --git a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua index 53f1c1ea7e..2a258ec0d9 100644 --- a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua +++ b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua @@ -202,7 +202,7 @@ describe("Sticky", function() local sticky_balancer_instance = sticky:new(b) assert.has_no.errors(function() sticky_balancer_instance:balance() end) assert.spy(s).was_called() - end + end it("sets a cookie on the client", function() test(sticky_balanced) end) it("sets a cookie on the client", function() test(sticky_persistent) end) @@ -353,4 +353,41 @@ describe("Sticky", function() end) end) end) + + context("when client doesn't have a cookie set and no host header, matching default server '_'", + function() + before_each(function () + ngx.var.host = "not-default-server" + ngx.var.server_name = "_" + end) + + local function test(sticky) + local s = {} + cookie.new = function(self) + local cookie_instance = { + set = function(self, payload) + assert.equal(payload.key, test_backend.sessionAffinityConfig.cookieSessionAffinity.name) + assert.equal(payload.path, ngx.var.location_path) + assert.equal(payload.domain, nil) + assert.equal(payload.httponly, true) + assert.equal(payload.secure, false) + return true, nil + end, + get = function(k) return false end, + } + s = spy.on(cookie_instance, "set") + return cookie_instance, false + end + + local b = get_test_backend() + b.sessionAffinityConfig.cookieSessionAffinity.locations = {} + b.sessionAffinityConfig.cookieSessionAffinity.locations["_"] = {"/"} + local sticky_balancer_instance = sticky:new(b) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + end + + it("sets a cookie on the client", function() test(sticky_balanced) end) + it("sets a cookie on the client", function() test(sticky_persistent) end) + end) end) diff --git a/test/e2e/annotations/affinity.go b/test/e2e/annotations/affinity.go index c786b2af77..108844b72e 100644 --- a/test/e2e/annotations/affinity.go +++ b/test/e2e/annotations/affinity.go @@ -333,4 +333,28 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", Expect(resp.StatusCode).Should(Equal(http.StatusOK)) Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("Path=/foo/bar")) }) + + It("should set sticky cookie without host", func() { + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/affinity": "cookie", + "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", + } + + ing := framework.NewSingleIngress("default-no-host", "/", "", f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer("_", + func(server string) bool { + return strings.Contains(server, "server_name _") + }) + time.Sleep(waitForLuaSync) + + resp, _, errs := gorequest.New(). + Get(f.GetURL(framework.HTTP)). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("SERVERID=")) + }) }) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 302f73e679..7c43adaa48 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -447,7 +447,6 @@ func newSingleIngressWithRules(name, path, host, ns, service string, port int, a spec := networking.IngressSpec{ Rules: []networking.IngressRule{ { - Host: host, IngressRuleValue: networking.IngressRuleValue{ HTTP: &networking.HTTPIngressRuleValue{ Paths: []networking.HTTPIngressPath{ @@ -465,6 +464,11 @@ func newSingleIngressWithRules(name, path, host, ns, service string, port int, a }, } + // allow ingresses without host field + if host != "" { + spec.Rules[0].Host = host + } + if len(tlsHosts) > 0 { spec.TLS = []networking.IngressTLS{ {