Remove session-cookie-hash annotation

This commit is contained in:
Alex Kursell 2019-03-04 10:34:48 -05:00
parent 79c52cf094
commit d3ac73be79
18 changed files with 22 additions and 189 deletions

View file

@ -10,7 +10,6 @@ Session stickiness is achieved through 3 annotations on the Ingress, as shown in
| --- | --- | --- |
|nginx.ingress.kubernetes.io/affinity|Sets the affinity type|string (in NGINX only ``cookie`` is possible|
|nginx.ingress.kubernetes.io/session-cookie-name|Name of the cookie that will be used|string (default to INGRESSCOOKIE)|
|nginx.ingress.kubernetes.io/session-cookie-hash|Type of hash that will be used in cookie value|sha1/md5/index|
|nginx.ingress.kubernetes.io/session-cookie-expires|The value is a date as UNIX timestamp that the cookie will expire on, it corresponds to cookie Expires directive|number of seconds|
|nginx.ingress.kubernetes.io/session-cookie-max-age|Number of seconds until the cookie expires that will correspond to cookie `Max-Age` directive|number of seconds|
@ -37,7 +36,6 @@ Rules:
/ nginx-service:80 (<none>)
Annotations:
affinity: cookie
session-cookie-hash: sha1
session-cookie-name: INGRESSCOOKIE
session-cookie-expires: 172800
session-cookie-max-age: 172800

View file

@ -5,7 +5,6 @@ metadata:
annotations:
nginx.ingress.kubernetes.io/affinity: "cookie"
nginx.ingress.kubernetes.io/session-cookie-name: "route"
nginx.ingress.kubernetes.io/session-cookie-hash: "sha1"
nginx.ingress.kubernetes.io/session-cookie-expires: "172800"
nginx.ingress.kubernetes.io/session-cookie-max-age: "172800"

View file

@ -70,7 +70,6 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
|[nginx.ingress.kubernetes.io/server-snippet](#server-snippet)|string|
|[nginx.ingress.kubernetes.io/service-upstream](#service-upstream)|"true" or "false"|
|[nginx.ingress.kubernetes.io/session-cookie-name](#cookie-affinity)|string|
|[nginx.ingress.kubernetes.io/session-cookie-hash](#cookie-affinity)|string|
|[nginx.ingress.kubernetes.io/session-cookie-path](#cookie-affinity)|string|
|[nginx.ingress.kubernetes.io/ssl-redirect](#server-side-https-enforcement-through-redirect)|"true" or "false"|
|[nginx.ingress.kubernetes.io/ssl-passthrough](#ssl-passthrough)|"true" or "false"|
@ -149,18 +148,8 @@ The only affinity type available for NGINX is `cookie`.
If you use the ``cookie`` affinity type you can also specify the name of the cookie that will be used to route the requests with the annotation `nginx.ingress.kubernetes.io/session-cookie-name`. The default is to create a cookie named 'INGRESSCOOKIE'.
In case of NGINX the annotation `nginx.ingress.kubernetes.io/session-cookie-hash` defines which algorithm will be used to hash the used upstream. Default value is `md5` and possible values are `md5`, `sha1` and `index`.
The NGINX annotation `nginx.ingress.kubernetes.io/session-cookie-path` defines the path that will be set on the cookie. This is optional unless the annotation `nginx.ingress.kubernetes.io/use-regex` is set to true; Session cookie paths do not support regex.
!!! attention
The `index` option is not an actual hash; an in-memory index is used instead, which has less overhead.
However, with `index`, matching against a changing upstream server list is inconsistent.
So, at reload, if upstream servers have changed, index values are not guaranteed to correspond to the same server as before!
**Use `index` with caution** and only if you need to!
In NGINX this feature is implemented by the third party module [nginx-sticky-module-ng](https://bitbucket.org/nginx-goodies/nginx-sticky-module-ng). The workflow used to define which upstream server will be used is explained [here](https://bitbucket.org/nginx-goodies/nginx-sticky-module-ng/raw/08a395c66e425540982c00482f55034e1fee67b6/docs/sticky.pdf)
### Authentication

View file

@ -41,7 +41,6 @@ var (
defaultCorsMethods = "GET, PUT, POST, DELETE, PATCH, OPTIONS"
defaultCorsHeaders = "DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Authorization"
annotationAffinityCookieName = parser.GetAnnotationWithPrefix("session-cookie-name")
annotationAffinityCookieHash = parser.GetAnnotationWithPrefix("session-cookie-hash")
annotationUpstreamHashBy = parser.GetAnnotationWithPrefix("upstream-hash-by")
annotationCustomHTTPErrors = parser.GetAnnotationWithPrefix("custom-http-errors")
)
@ -200,24 +199,19 @@ func TestAffinitySession(t *testing.T) {
fooAnns := []struct {
annotations map[string]string
affinitytype string
hash string
name string
}{
{map[string]string{annotationAffinityType: "cookie", annotationAffinityCookieHash: "md5", annotationAffinityCookieName: "route"}, "cookie", "md5", "route"},
{map[string]string{annotationAffinityType: "cookie", annotationAffinityCookieHash: "xpto", annotationAffinityCookieName: "route1"}, "cookie", "md5", "route1"},
{map[string]string{annotationAffinityType: "cookie", annotationAffinityCookieHash: "", annotationAffinityCookieName: ""}, "cookie", "md5", "INGRESSCOOKIE"},
{map[string]string{}, "", "", ""},
{nil, "", "", ""},
{map[string]string{annotationAffinityType: "cookie", annotationAffinityCookieName: "route"}, "cookie", "route"},
{map[string]string{annotationAffinityType: "cookie", annotationAffinityCookieName: "route1"}, "cookie", "route1"},
{map[string]string{annotationAffinityType: "cookie", annotationAffinityCookieName: ""}, "cookie", "INGRESSCOOKIE"},
{map[string]string{}, "", ""},
{nil, "", ""},
}
for _, foo := range fooAnns {
ing.SetAnnotations(foo.annotations)
r := ec.Extract(ing).SessionAffinity
t.Logf("Testing pass %v %v %v", foo.affinitytype, foo.hash, foo.name)
if r.Cookie.Hash != foo.hash {
t.Errorf("Returned %v but expected %v for Hash", r.Cookie.Hash, foo.hash)
}
t.Logf("Testing pass %v %v", foo.affinitytype, foo.name)
if r.Cookie.Name != foo.name {
t.Errorf("Returned %v but expected %v for Name", r.Cookie.Name, foo.name)

View file

@ -34,11 +34,6 @@ const (
defaultAffinityCookieName = "INGRESSCOOKIE"
// This is the algorithm used by nginx to generate a value for the session cookie, if
// 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 number of seconds until the
// cookie expires
annotationAffinityCookieExpires = "session-cookie-expires"
@ -52,7 +47,6 @@ const (
)
var (
affinityCookieHashRegex = regexp.MustCompile(`^(index|md5|sha1)$`)
affinityCookieExpiresRegex = regexp.MustCompile(`(^0|-?[1-9]\d*$)`)
)
@ -67,8 +61,6 @@ type Config struct {
type Cookie struct {
// The name of the cookie that will be used in case of cookie affinity type.
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 duration to control cookie expires
Expires string `json:"expires"`
// The number of seconds until the cookie expires
@ -90,12 +82,6 @@ func (a affinity) cookieAffinityParse(ing *extensions.Ingress) *Cookie {
cookie.Name = defaultAffinityCookieName
}
cookie.Hash, err = parser.GetStringAnnotation(annotationAffinityCookieHash, ing)
if err != nil || !affinityCookieHashRegex.MatchString(cookie.Hash) {
klog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Setting it to default %v", ing.Name, annotationAffinityCookieHash, defaultAffinityCookieHash)
cookie.Hash = defaultAffinityCookieHash
}
cookie.Expires, err = parser.GetStringAnnotation(annotationAffinityCookieExpires, ing)
if err != nil || !affinityCookieExpiresRegex.MatchString(cookie.Expires) {
klog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieExpires)

View file

@ -67,7 +67,6 @@ func TestIngressAffinityCookieConfig(t *testing.T) {
data := map[string]string{}
data[parser.GetAnnotationWithPrefix(annotationAffinityType)] = "cookie"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieHash)] = "sha123"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieName)] = "INGRESSCOOKIE"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieExpires)] = "4500"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieMaxAge)] = "3000"
@ -84,10 +83,6 @@ func TestIngressAffinityCookieConfig(t *testing.T) {
t.Errorf("expected cookie as affinity but returned %v", nginxAffinity.Type)
}
if nginxAffinity.Cookie.Hash != "md5" {
t.Errorf("expected md5 as session-cookie-hash but returned %v", nginxAffinity.Cookie.Hash)
}
if nginxAffinity.Cookie.Name != "INGRESSCOOKIE" {
t.Errorf("expected route as session-cookie-name but returned %v", nginxAffinity.Cookie.Name)
}

View file

@ -548,7 +548,6 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in
}
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
ups.SessionAffinity.CookieSessionAffinity.Path = cookiePath

View file

@ -145,7 +145,6 @@ type SessionAffinityConfig struct {
// +k8s:deepcopy-gen=true
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"`

View file

@ -224,9 +224,6 @@ func (csa1 *CookieSessionAffinity) Equal(csa2 *CookieSessionAffinity) bool {
if csa1.Name != csa2.Name {
return false
}
if csa1.Hash != csa2.Hash {
return false
}
if csa1.Path != csa2.Path {
return false
}

View file

@ -2,29 +2,20 @@ local balancer_resty = require("balancer.resty")
local resty_chash = require("resty.chash")
local util = require("util")
local ck = require("resty.cookie")
local math = require("math")
local _M = balancer_resty:new({ factory = resty_chash, name = "sticky" })
local DEFAULT_COOKIE_NAME = "route"
local function get_digest_func(hash)
local digest_func = util.md5_digest
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),
digest_func = digest_func,
traffic_shaping_policy = backend.trafficShapingPolicy,
alternative_backends = backend.alternativeBackends,
cookie_session_affinity = backend["sessionAffinityConfig"]["cookieSessionAffinity"]
@ -34,15 +25,6 @@ function _M.new(self, backend)
return o
end
local function encrypted_endpoint_string(self, endpoint_string)
local encrypted, err = self.digest_func(endpoint_string)
if err ~= nil then
ngx.log(ngx.ERR, err)
end
return encrypted
end
local function set_cookie(self, value)
local cookie, err = ck:new()
if not cookie then
@ -86,8 +68,7 @@ function _M.balance(self)
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)
key = string.format("%s.%s.%s", ngx.now(), ngx.worker.pid(), math.random(999999))
if self.cookie_session_affinity.locations then
local locs = self.cookie_session_affinity.locations[ngx.var.host]
@ -118,7 +99,6 @@ function _M.sync(self, backend)
end
self.cookie_session_affinity = backend.sessionAffinityConfig.cookieSessionAffinity
self.digest_func = get_digest_func(backend.sessionAffinityConfig.cookieSessionAffinity.hash)
end
return _M

View file

@ -68,26 +68,6 @@ describe("Sticky", function()
assert.equal(sticky_balancer_instance:cookie_name(), default_cookie_name)
end)
end)
context("when backend specifies hash function", function()
it("returns an instance with the corresponding hash implementation", function()
local sticky_balancer_instance = sticky:new(test_backend)
local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash
local test_backend_hash_implementation = util[test_backend_hash_fn .. "_digest"]
assert.equal(sticky_balancer_instance.digest_func, test_backend_hash_implementation)
end)
end)
context("when backend does not specify hash function", function()
it("returns an instance with the default implementation (md5)", function()
local temp_backend = util.deepcopy(test_backend)
temp_backend.sessionAffinityConfig.cookieSessionAffinity.hash = nil
local sticky_balancer_instance = sticky:new(temp_backend)
local default_hash_fn = "md5"
local default_hash_implementation = util[default_hash_fn .. "_digest"]
assert.equal(sticky_balancer_instance.digest_func, default_hash_implementation)
end)
end)
end)
describe("balance()", function()
@ -112,12 +92,9 @@ describe("Sticky", function()
it("sets 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, nil)
assert.equal(payload.httponly, true)
@ -141,12 +118,9 @@ describe("Sticky", function()
ngx.var.https = "on"
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, nil)
assert.equal(payload.httponly, true)
@ -177,12 +151,9 @@ describe("Sticky", function()
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)

View file

@ -29,7 +29,7 @@ local function reset_backends()
{ address = "10.184.97.100", port = "8080", maxFails = 0, failTimeout = 0 },
{ address = "10.184.98.239", port = "8080", maxFails = 0, failTimeout = 0 },
},
sessionAffinityConfig = { name = "", cookieSessionAffinity = { name = "", hash = "" } },
sessionAffinityConfig = { name = "", cookieSessionAffinity = { name = "" } },
},
{ name = "my-dummy-app-1", ["load-balance"] = "round_robin", },
{
@ -38,12 +38,12 @@ local function reset_backends()
},
{
name = "my-dummy-app-3", ["load-balance"] = "ewma",
sessionAffinityConfig = { name = "cookie", cookieSessionAffinity = { name = "route", hash = "sha1" } }
sessionAffinityConfig = { name = "cookie", cookieSessionAffinity = { name = "route" } }
},
{ name = "my-dummy-app-4", ["load-balance"] = "ewma", },
{
name = "my-dummy-app-5", ["load-balance"] = "ewma", ["upstream-hash-by"] = "$request_uri",
sessionAffinityConfig = { name = "cookie", cookieSessionAffinity = { name = "route", hash = "sha1" } }
sessionAffinityConfig = { name = "cookie", cookieSessionAffinity = { name = "route" } }
},
}
end

View file

@ -10,7 +10,7 @@ function get_backends()
{
name = "my-dummy-backend-1", ["load-balance"] = "sticky",
endpoints = { { address = "10.183.7.40", port = "8080", maxFails = 0, failTimeout = 0 } },
sessionAffinityConfig = { name = "cookie", cookieSessionAffinity = { name = "route", hash = "sha1" } },
sessionAffinityConfig = { name = "cookie", cookieSessionAffinity = { name = "route" } },
},
{
name = "my-dummy-backend-2", ["load-balance"] = "ewma",

View file

@ -1,8 +1,5 @@
local string_len = string.len
local string_sub = string.sub
local resty_str = require("resty.string")
local resty_sha1 = require("resty.sha1")
local resty_md5 = require("resty.md5")
local _M = {}
@ -18,30 +15,6 @@ function _M.get_nodes(endpoints)
return nodes
end
local function hash_digest(hash_factory, message)
local hash = hash_factory:new()
if not hash then
return nil, "failed to create object"
end
local ok = hash:update(message)
if not ok then
return nil, "failed to add data"
end
local binary_digest = hash:final()
if binary_digest == nil then
return nil, "failed to create digest"
end
return resty_str.to_hex(binary_digest), nil
end
function _M.sha1_digest(message)
return hash_digest(resty_sha1, message)
end
function _M.md5_digest(message)
return hash_digest(resty_md5, message)
end
-- given an Nginx variable i.e $request_uri
-- it returns value of ngx.var[request_uri]
function _M.lua_ngx_var(ngx_var)

View file

@ -19,7 +19,6 @@ package annotations
import (
"fmt"
"net/http"
"regexp"
"strings"
"time"
@ -65,13 +64,9 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions",
Set("Host", host).
End()
md5Regex := regexp.MustCompile("SERVERID=[0-9a-f]{32}")
match := md5Regex.FindStringSubmatch(resp.Header.Get("Set-Cookie"))
Expect(len(match)).Should(BeNumerically("==", 1))
Expect(errs).Should(BeEmpty())
Expect(resp.StatusCode).Should(Equal(http.StatusOK))
Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring(match[0]))
Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("SERVERID="))
})
It("should change cookie name on ingress definition change", func() {
@ -114,36 +109,6 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions",
Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("OTHERCOOKIENAME"))
})
It("should set sticky cookie with sha1 hash", func() {
host := "sha1.foo.com"
annotations := map[string]string{
"nginx.ingress.kubernetes.io/affinity": "cookie",
"nginx.ingress.kubernetes.io/session-cookie-hash": "sha1",
}
ing := framework.NewSingleIngress(host, "/", host, f.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.GetURL(framework.HTTP)).
Set("Host", host).
End()
sha1Regex := regexp.MustCompile("INGRESSCOOKIE=[0-9a-f]{40}")
match := sha1Regex.FindStringSubmatch(resp.Header.Get("Set-Cookie"))
Expect(len(match)).Should(BeNumerically("==", 1))
Expect(errs).Should(BeEmpty())
Expect(resp.StatusCode).Should(Equal(http.StatusOK))
Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring(match[0]))
})
It("should set the path to /something on the generated cookie", func() {
host := "path.foo.com"
annotations := map[string]string{
@ -296,13 +261,9 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions",
Set("Host", host).
End()
md5Regex := regexp.MustCompile("SERVERID=[0-9a-f]{32}")
match := md5Regex.FindStringSubmatch(resp.Header.Get("Set-Cookie"))
Expect(len(match)).Should(BeNumerically("==", 1))
Expect(errs).Should(BeEmpty())
Expect(resp.StatusCode).Should(Equal(http.StatusOK))
Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring(match[0]))
Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("SERVERID="))
Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("Path=/foo/bar"))
})

View file

@ -55,8 +55,7 @@
"sessionAffinityConfig": {
"name": "",
"cookieSessionAffinity": {
"name": "",
"hash": ""
"name": ""
}
}
}, {
@ -128,8 +127,7 @@
"sessionAffinityConfig": {
"name": "",
"cookieSessionAffinity": {
"name": "",
"hash": ""
"name": ""
}
}
}, {
@ -194,8 +192,7 @@
"sessionAffinityConfig": {
"name": "",
"cookieSessionAffinity": {
"name": "",
"hash": ""
"name": ""
}
}
}],

View file

@ -55,8 +55,7 @@
"sessionAffinityConfig": {
"name": "",
"cookieSessionAffinity": {
"name": "",
"hash": ""
"name": ""
}
}
}, {
@ -128,8 +127,7 @@
"sessionAffinityConfig": {
"name": "",
"cookieSessionAffinity": {
"name": "",
"hash": ""
"name": ""
}
}
}, {
@ -194,8 +192,7 @@
"sessionAffinityConfig": {
"name": "",
"cookieSessionAffinity": {
"name": "",
"hash": ""
"name": ""
}
}
}],

View file

@ -26,8 +26,7 @@
"SessionAffinity": {
"name": "",
"CookieSessionAffinity": {
"name": "",
"hash": ""
"name": ""
}
}
}, {
@ -80,8 +79,7 @@
"SessionAffinity": {
"name": "",
"CookieSessionAffinity": {
"name": "",
"hash": ""
"name": ""
}
}
}],