Skip to content

Commit

Permalink
Merge pull request #4867 from aledbf/sticky-fix
Browse files Browse the repository at this point in the history
Fix sticky session for ingress without host
  • Loading branch information
k8s-ci-robot committed Jan 2, 2020
2 parents 8db541e + 41a3e04 commit f42f559
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 6 deletions.
2 changes: 1 addition & 1 deletion internal/ingress/zz_generated.deepcopy.go

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

11 changes: 8 additions & 3 deletions rootfs/etc/nginx/lua/balancer/sticky.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 38 additions & 1 deletion rootfs/etc/nginx/lua/test/balancer/sticky_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
24 changes: 24 additions & 0 deletions test/e2e/annotations/affinity.go
Original file line number Diff line number Diff line change
Expand Up @@ -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="))
})
})
6 changes: 5 additions & 1 deletion test/e2e/framework/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
{
Expand Down

0 comments on commit f42f559

Please sign in to comment.