Skip to content

Commit

Permalink
Only set cookies on paths that enable session affinity
Browse files Browse the repository at this point in the history
  • Loading branch information
Zenara Daley committed Nov 19, 2018
1 parent 82721e5 commit 2b109b3
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 5 deletions.
3 changes: 3 additions & 0 deletions docs/user-guide/nginx-configuration/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ If the Application Root is exposed in a different path and needs to be redirecte
The annotation `nginx.ingress.kubernetes.io/affinity` enables and sets the affinity type in all Upstreams of an Ingress. This way, a request will always be directed to the same upstream server.
The only affinity type available for NGINX is `cookie`.

!!! attention
If more than one Ingress is defined for a host and at least one Ingress uses `nginx.ingress.kubernetes.io/affinity: cookie`, then only paths on the Ingress using `nginx.ingress.kubernetes.io/affinity` will use session cookie affinity. All paths defined on other Ingresses for the host will be load balanced through the random selection of a backend server.

!!! example
Please check the [affinity](../../examples/affinity/cookie/README.md) example.

Expand Down
14 changes: 13 additions & 1 deletion rootfs/etc/nginx/lua/balancer/sticky.lua
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ function _M.new(self, backend)
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,
Expand Down Expand Up @@ -81,7 +82,18 @@ function _M.balance(self)
if not key then
local random_str = string.format("%s.%s", ngx.now(), ngx.worker.pid())
key = encrypted_endpoint_string(self, random_str)
set_cookie(self, key)

if self.cookie_locations then
local locs = self.cookie_locations[ngx.var.host]
if locs ~= nil then
for _, path in pairs(locs) do
if ngx.var.location_path == path then
set_cookie(self, key)
break
end
end
end
end
end

return self.instance:find(key)
Expand Down
44 changes: 40 additions & 4 deletions rootfs/etc/nginx/lua/test/balancer/sticky_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ end

describe("Sticky", function()
before_each(function()
mock_ngx({ var = { location_path = "/" } })
mock_ngx({ var = { location_path = "/", host = "test.com" } })
end)

after_each(function()
Expand Down Expand Up @@ -102,7 +102,8 @@ describe("Sticky", function()
cookie.new = mocked_cookie_new
end)

context("when client doesn't have a cookie set", function()

context("when client doesn't have a cookie set and location is in cookie_locations", function()
it("picks an endpoint for the client", function()
local sticky_balancer_instance = sticky:new(test_backend)
local peer = sticky_balancer_instance:balance()
Expand All @@ -119,7 +120,7 @@ describe("Sticky", function()
local expected_len = #util[test_backend_hash_fn .. "_digest"]("anything")
assert.equal(#payload.value, expected_len)
assert.equal(payload.path, ngx.var.location_path)
assert.equal(payload.domain, nil)
assert.equal(payload.domain, ngx.var.host)
assert.equal(payload.httponly, true)
return true, nil
end,
Expand All @@ -128,12 +129,47 @@ describe("Sticky", function()
s = spy.on(cookie_instance, "set")
return cookie_instance, false
end
local sticky_balancer_instance = sticky:new(get_test_backend())
local b = get_test_backend()
b.sessionAffinityConfig.cookieSessionAffinity.locations = {}
b.sessionAffinityConfig.cookieSessionAffinity.locations["test.com"] = {"/"}
local sticky_balancer_instance = sticky:new(b)
assert.has_no.errors(function() sticky_balancer_instance:balance() end)
assert.spy(s).was_called()
end)
end)

context("when client doesn't have a cookie set and location not in cookie_locations", function()
it("picks an endpoint for the client", function()
local sticky_balancer_instance = sticky:new(test_backend)
local peer = sticky_balancer_instance:balance()
assert.equal(peer, test_backend_endpoint)
end)

it("does not set a cookie on the client", function()
local s = {}
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)
local expected_len = #util[test_backend_hash_fn .. "_digest"]("anything")
assert.equal(#payload.value, expected_len)
assert.equal(payload.path, ngx.var.location_path)
assert.equal(payload.domain, ngx.var.host)
assert.equal(payload.httponly, true)
return true, nil
end,
get = function(k) return false end,
}
s = spy.on(cookie_instance, "set")
return cookie_instance, false
end
local sticky_balancer_instance = sticky:new(get_test_backend())
assert.has_no.errors(function() sticky_balancer_instance:balance() end)
assert.spy(s).was_not_called()
end)
end)

context("when client has a cookie set", function()
it("does not set a cookie", function()
local s = {}
Expand Down
36 changes: 36 additions & 0 deletions test/e2e/annotations/affinity.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,4 +284,40 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions",
Expect(err).ToNot(HaveOccurred())
Expect(logs).To(ContainSubstring(`session-cookie-path should be set when use-regex is true`))
})

It("should not set affinity across all server locations when using separate ingresses", func() {
host := "cookie.foo.com"

annotations := map[string]string{
"nginx.ingress.kubernetes.io/affinity": "cookie",
}
ing1 := framework.NewSingleIngress("ingress1", "/foo/bar", host, f.IngressController.Namespace, "http-svc", 80, &annotations)
f.EnsureIngress(ing1)

ing2 := framework.NewSingleIngress("ingress2", "/foo", host, f.IngressController.Namespace, "http-svc", 80, &map[string]string{})
f.EnsureIngress(ing2)

f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, `location /foo/bar`) && strings.Contains(server, `location /foo`)
})

resp, _, errs := gorequest.New().
Get(f.IngressController.HTTPURL+"/foo").
Set("Host", host).
End()

Expect(errs).Should(BeEmpty())
Expect(resp.StatusCode).Should(Equal(http.StatusOK))
Expect(resp.Header.Get("Set-Cookie")).Should(Equal(""))

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))
Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("Path=/foo/bar"))
})
})

0 comments on commit 2b109b3

Please sign in to comment.