DevFW-CICD/ingress-nginx-helm
12
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat: add session-cookie-secure annotation (#7399)

Browse source
This commit is contained in:
Vincent LE GOFF 2021-09-02 00:23:40 +02:00 committed by GitHub
parent 8a1a5e93c7
commit f2e743f561
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 50 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
docs/examples/affinity/cookie/README.md
View file

@ -12,6 +12,7 @@ Session affinity can be configured using the following annotations:
|nginx.ingress.kubernetes.io/affinity-mode|The affinity mode defines how sticky a session is. Use `balanced` to redistribute some sessions when scaling pods or `persistent` for maximum stickiness.|`balanced` (default) or `persistent`|
|nginx.ingress.kubernetes.io/affinity-canary-behavior|Defines session affinity behavior of canaries. By default the behavior is `sticky`, and canaries respect session affinity configuration. Set this to `legacy` to restore original canary behavior, when session affinity parameters were not respected.|`sticky` (default) or `legacy`|
|nginx.ingress.kubernetes.io/session-cookie-name|Name of the cookie that will be created|string (defaults to `INGRESSCOOKIE`)|
|nginx.ingress.kubernetes.io/session-cookie-secure|Set the cookie as secure regardless the protocol of the incoming request|`"true"` or `"false"`|
|nginx.ingress.kubernetes.io/session-cookie-path|Path that will be set on the cookie (required if your [Ingress paths][ingress-paths] use regular expressions)|string (defaults to the currently [matched path][ingress-paths])|
|nginx.ingress.kubernetes.io/session-cookie-samesite|SameSite attribute to apply to the cookie|Browser accepted values are `None`, `Lax`, and `Strict`|
|nginx.ingress.kubernetes.io/session-cookie-conditional-samesite-none|Will omit `SameSite=None` attribute for older browsers which reject the more-recently defined `SameSite=None` value|`"true"` or `"false"`

1
docs/examples/affinity/cookie/ingress-samesite.yaml
View file

@ -5,6 +5,7 @@ metadata:
annotations:
nginx.ingress.kubernetes.io/affinity: "cookie"
nginx.ingress.kubernetes.io/session-cookie-name: "SSNONE"
nginx.ingress.kubernetes.io/session-cookie-secure: "true"
nginx.ingress.kubernetes.io/session-cookie-expires: "172800"
nginx.ingress.kubernetes.io/session-cookie-max-age: "172800"
nginx.ingress.kubernetes.io/session-cookie-samesite: "None"

11
internal/ingress/annotations/sessionaffinity/main.go
View file

@ -37,6 +37,10 @@ const (
defaultAffinityCookieName = "INGRESSCOOKIE"
// This is used to force the Secure flag on the cookie even if the
// incoming request is not secured. (https://github.com/kubernetes/ingress-nginx/issues/6812)
annotationAffinityCookieSecure = "session-cookie-secure"
// This is used to control the cookie expires, its value is a number of seconds until the
// cookie expires
annotationAffinityCookieExpires = "session-cookie-expires"
@ -85,6 +89,8 @@ type Cookie struct {
Path string `json:"path"`
// Flag that allows cookie regeneration on request failure
ChangeOnFailure bool `json:"changeonfailure"`
// Secure flag to be set
Secure bool `json:"secure"`
// SameSite attribute value
SameSite string `json:"samesite"`
// Flag that conditionally applies SameSite=None attribute on cookie if user agent accepts it.
@ -126,6 +132,11 @@ func (a affinity) cookieAffinityParse(ing *networking.Ingress) *Cookie {
klog.V(3).InfoS("Invalid or no annotation value found. Ignoring", "ingress", klog.KObj(ing), "annotation", annotationAffinityCookieSameSite)
}
cookie.Secure, err = parser.GetBoolAnnotation(annotationAffinityCookieSecure, ing)
if err != nil {
klog.V(3).InfoS("Invalid or no annotation value found. Ignoring", "ingress", klog.KObj(ing), "annotation", annotationAffinityCookieSecure)
}
cookie.ConditionalSameSiteNone, err = parser.GetBoolAnnotation(annotationAffinityCookieConditionalSameSiteNone, ing)
if err != nil {
klog.V(3).InfoS("Invalid or no annotation value found. Ignoring", "ingress", klog.KObj(ing), "annotation", annotationAffinityCookieConditionalSameSiteNone)

5
internal/ingress/annotations/sessionaffinity/main_test.go
View file

@ -79,6 +79,7 @@ func TestIngressAffinityCookieConfig(t *testing.T) {
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieMaxAge)] = "3000"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookiePath)] = "/foo"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieChangeOnFailure)] = "true"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieSecure)] = "true"
ing.SetAnnotations(data)
affin, _ := NewParser(&resolver.Mock{}).Parse(ing)
@ -114,4 +115,8 @@ func TestIngressAffinityCookieConfig(t *testing.T) {
if !nginxAffinity.Cookie.ChangeOnFailure {
t.Errorf("expected change of failure parameter set to true but returned %v", nginxAffinity.Cookie.ChangeOnFailure)
}
if !nginxAffinity.Cookie.Secure {
t.Errorf("expected secure parameter set to true but returned %v", nginxAffinity.Cookie.Secure)
}
}

1
internal/ingress/controller/controller.go
View file

@ -667,6 +667,7 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in
ups.SessionAffinity.CookieSessionAffinity.Name = anns.SessionAffinity.Cookie.Name
ups.SessionAffinity.CookieSessionAffinity.Expires = anns.SessionAffinity.Cookie.Expires
ups.SessionAffinity.CookieSessionAffinity.MaxAge = anns.SessionAffinity.Cookie.MaxAge
ups.SessionAffinity.CookieSessionAffinity.Secure = anns.SessionAffinity.Cookie.Secure
ups.SessionAffinity.CookieSessionAffinity.Path = cookiePath
ups.SessionAffinity.CookieSessionAffinity.SameSite = anns.SessionAffinity.Cookie.SameSite
ups.SessionAffinity.CookieSessionAffinity.ConditionalSameSiteNone = anns.SessionAffinity.Cookie.ConditionalSameSiteNone

1
internal/ingress/types.go
View file

@ -155,6 +155,7 @@ type CookieSessionAffinity struct {
Expires string `json:"expires,omitempty"`
MaxAge string `json:"maxage,omitempty"`
Locations map[string][]string `json:"locations,omitempty"`
Secure bool `json:"secure,omitempty"`
Path string `json:"path,omitempty"`
SameSite string `json:"samesite,omitempty"`
ConditionalSameSiteNone bool `json:"conditional_samesite_none,omitempty"`

3
internal/ingress/types_equals.go
View file

@ -182,6 +182,9 @@ func (csa1 *CookieSessionAffinity) Equal(csa2 *CookieSessionAffinity) bool {
if csa1.SameSite != csa2.SameSite {
return false
}
if csa1.Secure != csa2.Secure {
return false
}
if csa1.ConditionalSameSiteNone != csa2.ConditionalSameSiteNone {
return false
}

7
rootfs/etc/nginx/lua/balancer/sticky.lua
View file

@ -87,13 +87,18 @@ function _M.set_cookie(self, value)
end
end
local cookie_secure = self.cookie_session_affinity.secure
if cookie_secure == nil then
cookie_secure = ngx.var.https == "on"
end
local cookie_data = {
key = self:cookie_name(),
value = value .. COOKIE_VALUE_DELIMITER .. self.backend_key,
path = cookie_path,
httponly = true,
samesite = cookie_samesite,
secure = ngx.var.https == "on",
secure = cookie_secure,
}
if self.cookie_session_affinity.expires and self.cookie_session_affinity.expires ~= "" then

38
rootfs/etc/nginx/lua/test/balancer/sticky_test.lua
View file

@ -422,7 +422,7 @@ describe("Sticky", function()
cookie.new = mocked_cookie_new
end)
local function test_set_cookie_with(sticky_balancer_type, samesite, conditional_samesite_none, expected_path, expected_samesite)
local function test_set_cookie_with(sticky_balancer_type, samesite, conditional_samesite_none, expected_path, expected_samesite, secure, expected_secure)
local s = {}
cookie.new = function(self)
local cookie_instance = {
@ -432,7 +432,7 @@ describe("Sticky", function()
assert.equal(payload.samesite, expected_samesite)
assert.equal(payload.domain, nil)
assert.equal(payload.httponly, true)
assert.equal(payload.secure, false)
assert.equal(payload.secure, expected_secure)
return true, nil
end,
get = function(k) return false end,
@ -445,57 +445,61 @@ describe("Sticky", function()
b.sessionAffinityConfig.cookieSessionAffinity.locations["test.com"] = {"/"}
b.sessionAffinityConfig.cookieSessionAffinity.samesite = samesite
b.sessionAffinityConfig.cookieSessionAffinity.conditional_samesite_none = conditional_samesite_none
b.sessionAffinityConfig.cookieSessionAffinity.secure = secure
local sticky_balancer_instance = sticky_balancer_type:new(b)
assert.has_no.errors(function() sticky_balancer_instance:balance() end)
assert.spy(s).was_called()
end
it("returns a cookie with SameSite=Strict when user specifies samesite strict", function()
test_set_cookie_with(sticky_balanced, "Strict", false, "/", "Strict")
it("returns a secure cookie with SameSite=Strict when user specifies samesite strict and secure=true", function()
test_set_cookie_with(sticky_balanced, "Lax", false, "/", "Lax", true, true)
end)
it("returns a cookie with SameSite=Strict when user specifies samesite strict and conditional samesite none", function()
test_set_cookie_with(sticky_balanced, "Strict", true, "/", "Strict")
test_set_cookie_with(sticky_balanced, "Strict", true, "/", "Strict", nil, false)
end)
it("returns a cookie with SameSite=Lax when user specifies samesite lax", function()
test_set_cookie_with(sticky_balanced, "Lax", false, "/", "Lax")
test_set_cookie_with(sticky_balanced, "Lax", false, "/", "Lax", nil, false)
end)
it("returns a cookie with SameSite=Lax when user specifies samesite lax and conditional samesite none", function()
test_set_cookie_with(sticky_balanced, "Lax", true, "/", "Lax")
test_set_cookie_with(sticky_balanced, "Lax", true, "/", "Lax", nil, false)
end)
it("returns a cookie with SameSite=None when user specifies samesite None", function()
test_set_cookie_with(sticky_balanced, "None", false, "/", "None")
test_set_cookie_with(sticky_balanced, "None", false, "/", "None", nil, false)
end)
it("returns a cookie with SameSite=None when user specifies samesite None and conditional samesite none with supported user agent", function()
mock_ngx({ var = { location_path = "/", host = "test.com" , http_user_agent = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.2704.103 Safari/537.36"} })
test_set_cookie_with(sticky_balanced, "None", true, "/", "None")
test_set_cookie_with(sticky_balanced, "None", true, "/", "None", nil, false)
end)
it("returns a cookie without SameSite=None when user specifies samesite None and conditional samesite none with unsupported user agent", function()
mock_ngx({ var = { location_path = "/", host = "test.com" , http_user_agent = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36"} })
test_set_cookie_with(sticky_balanced, "None", true, "/", nil)
test_set_cookie_with(sticky_balanced, "None", true, "/", nil, nil, false)
end)
it("returns a secure cookie with SameSite=Strict when user specifies samesite strict and secure=true", function()
test_set_cookie_with(sticky_persistent, "Lax", false, "/", "Lax", true, true)
end)
it("returns a cookie with SameSite=Strict when user specifies samesite strict", function()
test_set_cookie_with(sticky_persistent, "Strict", false, "/", "Strict")
test_set_cookie_with(sticky_persistent, "Strict", false, "/", "Strict", nil, false)
end)
it("returns a cookie with SameSite=Strict when user specifies samesite strict and conditional samesite none", function()
test_set_cookie_with(sticky_persistent, "Strict", true, "/", "Strict")
test_set_cookie_with(sticky_persistent, "Strict", true, "/", "Strict", nil, false)
end)
it("returns a cookie with SameSite=Lax when user specifies samesite lax", function()
test_set_cookie_with(sticky_persistent, "Lax", false, "/", "Lax")
test_set_cookie_with(sticky_persistent, "Lax", false, "/", "Lax", nil, false)
end)
it("returns a cookie with SameSite=Lax when user specifies samesite lax and conditional samesite none", function()
test_set_cookie_with(sticky_persistent, "Lax", true, "/", "Lax")
test_set_cookie_with(sticky_persistent, "Lax", true, "/", "Lax", nil, false)
end)
it("returns a cookie with SameSite=None when user specifies samesite None", function()
test_set_cookie_with(sticky_persistent, "None", false, "/", "None")
test_set_cookie_with(sticky_persistent, "None", false, "/", "None", nil, false)
end)
it("returns a cookie with SameSite=None when user specifies samesite None and conditional samesite none with supported user agent", function()
mock_ngx({ var = { location_path = "/", host = "test.com" , http_user_agent = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.2704.103 Safari/537.36"} })
test_set_cookie_with(sticky_persistent, "None", true, "/", "None")
test_set_cookie_with(sticky_persistent, "None", true, "/", "None", nil, false)
end)
it("returns a cookie without SameSite=None when user specifies samesite None and conditional samesite none with unsupported user agent", function()
mock_ngx({ var = { location_path = "/", host = "test.com" , http_user_agent = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36"} })
test_set_cookie_with(sticky_persistent, "None", true, "/", nil)
test_set_cookie_with(sticky_persistent, "None", true, "/", nil, nil, false)
end)
end)