Skip to content

Add partitioned cookie support for cookie-affinity #11242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/examples/affinity/cookie/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Session affinity can be configured using the following annotations:
|nginx.ingress.kubernetes.io/session-cookie-domain|Domain that will be set on the cookie|string|
|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"`
|nginx.ingress.kubernetes.io/session-cookie-partitioned|Will set `Partitioned` attribute on the cookie|`"true"` or `"false"` (defaults to false)|
|nginx.ingress.kubernetes.io/session-cookie-max-age|Time until the cookie expires, corresponds to the `Max-Age` cookie directive|number of seconds|
|nginx.ingress.kubernetes.io/session-cookie-expires|Legacy version of the previous annotation for compatibility with older browsers, generates an `Expires` cookie directive by adding the seconds to the current date|number of seconds|
|nginx.ingress.kubernetes.io/session-cookie-change-on-failure|When set to `false` nginx ingress will send request to upstream pointed by sticky cookie even if previous attempt failed. When set to `true` and previous attempt failed, sticky cookie will be changed to point to another upstream.|`true` or `false` (defaults to `false`)|
Expand Down
49 changes: 49 additions & 0 deletions docs/examples/affinity/cookie/ingress-partitioned.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: cookie-partitioned
annotations:
nginx.ingress.kubernetes.io/affinity: "cookie"
nginx.ingress.kubernetes.io/session-cookie-name: "PARTITIONEDCOOKIENAME"
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-partitioned: "true"
spec:
ingressClassName: nginx
rules:
- host: stickyingress-partitioned.example.com
http:
paths:
- path: /
pathType: Prefix
backend:
service:
name: http-svc
port:
number: 80
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: cookie-partitioned-false
annotations:
nginx.ingress.kubernetes.io/affinity: "cookie"
nginx.ingress.kubernetes.io/session-cookie-name: "PARTITIONEDFALSECOOKIENAME"
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-partitioned: "false"
spec:
ingressClassName: nginx
rules:
- host: stickyingress-partitioned-false.example.com
http:
paths:
- path: /
pathType: Prefix
backend:
service:
name: http-svc
port:
number: 80
1 change: 1 addition & 0 deletions docs/user-guide/nginx-configuration/annotations-risk.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
| SessionAffinity | session-cookie-expires | Medium | ingress |
| SessionAffinity | session-cookie-max-age | Medium | ingress |
| SessionAffinity | session-cookie-name | Medium | ingress |
| SessionAffinity | session-cookie-partitioned | Low | ingress |
| SessionAffinity | session-cookie-path | Medium | ingress |
| SessionAffinity | session-cookie-samesite | Low | ingress |
| SessionAffinity | session-cookie-secure | Low | ingress |
Expand Down
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 @@ -104,6 +104,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
|[nginx.ingress.kubernetes.io/session-cookie-path](#cookie-affinity)|string|
|[nginx.ingress.kubernetes.io/session-cookie-samesite](#cookie-affinity)|string|"None", "Lax" or "Strict"|
|[nginx.ingress.kubernetes.io/session-cookie-secure](#cookie-affinity)|string|
|[nginx.ingress.kubernetes.io/session-cookie-partitioned](#cookie-affinity)|"true" or "false"|
|[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"|
|[nginx.ingress.kubernetes.io/stream-snippet](#stream-snippet)|string|
Expand Down Expand Up @@ -192,6 +193,8 @@ Use `nginx.ingress.kubernetes.io/session-cookie-domain` to set the `Domain` attr

Use `nginx.ingress.kubernetes.io/session-cookie-samesite` to apply a `SameSite` attribute to the sticky cookie. Browser accepted values are `None`, `Lax`, and `Strict`. Some browsers reject cookies with `SameSite=None`, including those created before the `SameSite=None` specification (e.g. Chrome 5X). Other browsers mistakenly treat `SameSite=None` cookies as `SameSite=Strict` (e.g. Safari running on OSX 14). To omit `SameSite=None` from browsers with these incompatibilities, add the annotation `nginx.ingress.kubernetes.io/session-cookie-conditional-samesite-none: "true"`.

Use `nginx.ingress.kubernetes.io/session-cookie-partitioned` to apply a `Partitioned` attribute to the sticky cookie.

Use `nginx.ingress.kubernetes.io/session-cookie-expires` to control the cookie expires, its value is a number of seconds until the cookie expires.

Use `nginx.ingress.kubernetes.io/session-cookie-path` to control the cookie path when use-regex is set to true.
Expand Down
4 changes: 3 additions & 1 deletion go.work.sum
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ cloud.google.com/go/compute v1.24.0/go.mod h1:kw1/T+h/+tK2LJK0wiPPx1intgdAM3j/g3
cloud.google.com/go/compute v1.25.1/go.mod h1:oopOIR53ly6viBYxaDhBfJwzUAxf1zE//uf3IB011ls=
cloud.google.com/go/compute/metadata v0.3.0 h1:Tz+eQXMEqDIKRsmY3cHTL6FVaynIjX2QxYC4trgAKZc=
cloud.google.com/go/compute/metadata v0.3.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k=
cloud.google.com/go/compute/metadata v0.5.0/go.mod h1:aHnloV2TPI38yx4s9+wAZhHykWvVCfu7hQbF+9CWoiY=
cloud.google.com/go/contactcenterinsights v1.6.0 h1:jXIpfcH/VYSE1SYcPzO0n1VVb+sAamiLOgCw45JbOQk=
cloud.google.com/go/contactcenterinsights v1.9.1/go.mod h1:bsg/R7zGLYMVxFFzfh9ooLTruLRCG9fnzhH9KznHhbM=
cloud.google.com/go/contactcenterinsights v1.10.0/go.mod h1:bsg/R7zGLYMVxFFzfh9ooLTruLRCG9fnzhH9KznHhbM=
Expand Down Expand Up @@ -1112,6 +1113,7 @@ google.golang.org/grpc v1.61.0/go.mod h1:VUbo7IFqmF1QtCAstipjG0GIoq49KvMe9+h1jFL
google.golang.org/grpc v1.61.1/go.mod h1:VUbo7IFqmF1QtCAstipjG0GIoq49KvMe9+h1jFLBNJs=
google.golang.org/grpc v1.62.1/go.mod h1:IWTG0VlJLCh1SkC58F7np9ka9mx/WNkjl4PGJaiq+QE=
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0 h1:M1YKkFIboKNieVO5DLUEVzQfGwJD30Nv2jfUgzb5UcE=
google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc=
gopkg.in/errgo.v2 v2.1.0 h1:0vLT13EuvQ0hNvakwLuFZ/jYrLp5F3kcWHXdRggjCE8=
gopkg.in/evanphx/json-patch.v5 v5.6.0 h1:BMT6KIwBD9CaU91PJCZIe46bDmBWa9ynTQgJIOpfQBk=
Expand All @@ -1127,13 +1129,13 @@ k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAE
k8s.io/gengo/v2 v2.0.0-20240228010128-51d4e06bde70/go.mod h1:VH3AT8AaQOqiGjMF9p0/IM1Dj+82ZwjfxUP1IxaHE+8=
k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y=
k8s.io/klog/v2 v2.80.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0=
k8s.io/klog/v2 v2.120.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
k8s.io/kms v0.29.3/go.mod h1:TBGbJKpRUMk59neTMDMddjIDL+D4HuFUbpuiuzmOPg0=
k8s.io/kms v0.30.0 h1:ZlnD/ei5lpvUlPw6eLfVvH7d8i9qZ6HwUQgydNVks8g=
k8s.io/kms v0.30.0/go.mod h1:GrMurD0qk3G4yNgGcsCEmepqf9KyyIrTXYR2lyUOJC4=
k8s.io/kms v0.30.1/go.mod h1:GrMurD0qk3G4yNgGcsCEmepqf9KyyIrTXYR2lyUOJC4=
k8s.io/kms v0.31.0 h1:KchILPfB1ZE+ka7223mpU5zeFNkmb45jl7RHnlImUaI=
k8s.io/kms v0.31.0/go.mod h1:OZKwl1fan3n3N5FFxnW5C4V3ygrah/3YXeJWS3O6+94=
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98=
k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
lukechampine.com/uint128 v1.2.0 h1:mBi/5l91vocEN8otkC5bDLhi2KdCticRiwbdB0O+rjI=
Expand Down
8 changes: 4 additions & 4 deletions images/nginx/rootfs/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ export LUA_RESTY_CACHE=99e7578465b40f36f596d099b82eab404f2b42ed
# Check for recent changes: https://github.com/openresty/lua-resty-core/compare/v0.1.27...master
export LUA_RESTY_CORE=v0.1.28

# Check for recent changes: https://github.com/cloudflare/lua-resty-cookie/compare/f418d77082eaef48331302e84330488fdc810ef4...master
export LUA_RESTY_COOKIE_VERSION=f418d77082eaef48331302e84330488fdc810ef4
# Check for recent changes: https://github.com/utix/lua-resty-cookie/compare/v0.3.0...master
export LUA_RESTY_COOKIE_VERSION=v0.3.0

# Check for recent changes: https://github.com/openresty/lua-resty-dns/compare/8bb53516e2933e61c317db740a9b7c2048847c2f...master
export LUA_RESTY_DNS=8bb53516e2933e61c317db740a9b7c2048847c2f
Expand Down Expand Up @@ -249,8 +249,8 @@ get_src 39baab9e2b31cc48cecf896cea40ef6e80559054fd8a6e440cc804a858ea84d4 \
get_src a77b9de160d81712f2f442e1de8b78a5a7ef0d08f13430ff619f79235db974d4 \
"https://github.com/openresty/lua-cjson/archive/$LUA_CJSON_VERSION.tar.gz" "lua-cjson"

get_src 5ed48c36231e2622b001308622d46a0077525ac2f751e8cc0c9905914254baa4 \
"https://github.com/cloudflare/lua-resty-cookie/archive/$LUA_RESTY_COOKIE_VERSION.tar.gz" "lua-resty-cookie"
get_src 6a9261708e817f5a2941c1860b74bef10b2499bca02c82cf7451400f59d6c6b7 \
"https://github.com/utix/lua-resty-cookie/archive/$LUA_RESTY_COOKIE_VERSION.tar.gz" "lua-resty-cookie"

get_src 573184006b98ccee2594b0d134fa4d05e5d2afd5141cbad315051ccf7e9b6403 \
"https://github.com/openresty/lua-resty-lrucache/archive/$LUA_RESTY_CACHE.tar.gz" "lua-resty-lrucache"
Expand Down
16 changes: 16 additions & 0 deletions internal/ingress/annotations/sessionaffinity/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ const (
// This is used to control whether SameSite=None should be conditionally applied based on the User-Agent
annotationAffinityCookieConditionalSameSiteNone = "session-cookie-conditional-samesite-none"

// This is used to set the Partitioned flag on the cookie
annotationAffinityCookiePartitioned = "session-cookie-partitioned"

// This is used to control the cookie change after request failure
annotationAffinityCookieChangeOnFailure = "session-cookie-change-on-failure"

Expand Down Expand Up @@ -141,6 +144,12 @@ var sessionAffinityAnnotations = parser.Annotation{
Risk: parser.AnnotationRiskLow,
Documentation: `This annotation is used to omit SameSite=None from browsers with SameSite attribute incompatibilities`,
},
annotationAffinityCookiePartitioned: {
Validator: parser.ValidateBool,
Scope: parser.AnnotationScopeIngress,
Risk: parser.AnnotationRiskLow,
Documentation: `This annotation sets the cookie as Partitioned`,
},
annotationAffinityCookieChangeOnFailure: {
Validator: parser.ValidateBool,
Scope: parser.AnnotationScopeIngress,
Expand Down Expand Up @@ -184,6 +193,8 @@ type Cookie struct {
SameSite string `json:"samesite"`
// Flag that conditionally applies SameSite=None attribute on cookie if user agent accepts it.
ConditionalSameSiteNone bool `json:"conditional-samesite-none"`
// Partitioned flag to be set
Partitioned bool `json:"partitioned"`
}

type affinity struct {
Expand Down Expand Up @@ -241,6 +252,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", annotationAffinityCookieConditionalSameSiteNone)
}

cookie.Partitioned, err = parser.GetBoolAnnotation(annotationAffinityCookiePartitioned, ing, a.annotationConfig.Annotations)
if err != nil {
klog.V(3).InfoS("Invalid or no annotation value found. Ignoring", "ingress", klog.KObj(ing), "annotation", annotationAffinityCookiePartitioned)
}

cookie.ChangeOnFailure, err = parser.GetBoolAnnotation(annotationAffinityCookieChangeOnFailure, ing, a.annotationConfig.Annotations)
if err != nil {
klog.V(3).InfoS("Invalid or no annotation value found. Ignoring", "ingress", klog.KObj(ing), "annotation", annotationAffinityCookieChangeOnFailure)
Expand Down
11 changes: 9 additions & 2 deletions internal/ingress/annotations/sessionaffinity/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package sessionaffinity

import (
"strconv"
"testing"

api "k8s.io/api/core/v1"
Expand Down Expand Up @@ -70,6 +71,7 @@ func buildIngress() *networking.Ingress {
}

func TestIngressAffinityCookieConfig(t *testing.T) {
trueString := strconv.FormatBool(true)
ing := buildIngress()
data := map[string]string{}
data[parser.GetAnnotationWithPrefix(annotationAffinityType)] = "cookie"
Expand All @@ -80,8 +82,9 @@ func TestIngressAffinityCookieConfig(t *testing.T) {
data[parser.GetAnnotationWithPrefix(annotationAffinityCookiePath)] = "/foo"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieDomain)] = "foo.bar"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieSameSite)] = "Strict"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieChangeOnFailure)] = "true"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieSecure)] = "true"
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieChangeOnFailure)] = trueString
data[parser.GetAnnotationWithPrefix(annotationAffinityCookieSecure)] = trueString
data[parser.GetAnnotationWithPrefix(annotationAffinityCookiePartitioned)] = trueString
ing.SetAnnotations(data)

affin, err := NewParser(&resolver.Mock{}).Parse(ing)
Expand Down Expand Up @@ -133,4 +136,8 @@ func TestIngressAffinityCookieConfig(t *testing.T) {
if !nginxAffinity.Cookie.Secure {
t.Errorf("expected secure parameter set to true but returned %v", nginxAffinity.Cookie.Secure)
}

if !nginxAffinity.Cookie.Partitioned {
t.Errorf("expected partitioned parameter set to true but returned %v", nginxAffinity.Cookie.Partitioned)
}
}
1 change: 1 addition & 0 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,7 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in
ups.SessionAffinity.CookieSessionAffinity.Domain = anns.SessionAffinity.Cookie.Domain
ups.SessionAffinity.CookieSessionAffinity.SameSite = anns.SessionAffinity.Cookie.SameSite
ups.SessionAffinity.CookieSessionAffinity.ConditionalSameSiteNone = anns.SessionAffinity.Cookie.ConditionalSameSiteNone
ups.SessionAffinity.CookieSessionAffinity.Partitioned = anns.SessionAffinity.Cookie.Partitioned
ups.SessionAffinity.CookieSessionAffinity.ChangeOnFailure = anns.SessionAffinity.Cookie.ChangeOnFailure

locs := ups.SessionAffinity.CookieSessionAffinity.Locations
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/ingress/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ type CookieSessionAffinity struct {
Domain string `json:"domain,omitempty"`
SameSite string `json:"samesite,omitempty"`
ConditionalSameSiteNone bool `json:"conditional_samesite_none,omitempty"`
Partitioned bool `json:"partitioned,omitempty"`
ChangeOnFailure bool `json:"change_on_failure,omitempty"`
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/ingress/types_equals.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ func (csa1 *CookieSessionAffinity) Equal(csa2 *CookieSessionAffinity) bool {
if csa1.ConditionalSameSiteNone != csa2.ConditionalSameSiteNone {
return false
}
if csa1.Partitioned != csa2.Partitioned {
return false
}

return true
}
Expand Down
1 change: 1 addition & 0 deletions rootfs/etc/nginx/lua/balancer/sticky.lua
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ function _M.set_cookie(self, value)
httponly = true,
samesite = cookie_samesite,
secure = cookie_secure,
partitioned = self.cookie_session_affinity.partitioned,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tao12345666333 need someone with lua knowledge here :)

}

if self.cookie_session_affinity.expires and self.cookie_session_affinity.expires ~= "" then
Expand Down
50 changes: 50 additions & 0 deletions rootfs/etc/nginx/lua/test/balancer/sticky_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,56 @@ describe("Sticky", function()
end)
end)

describe("Partitioned settings", function()
local mocked_cookie_new = cookie.new

before_each(function()
reset_sticky_balancer()
end)

after_each(function()
cookie.new = mocked_cookie_new
end)

local function test_set_cookie_with(sticky_balancer_type, expected_path, partitioned, expected_partitioned)
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, expected_path)
assert.equal(payload.domain, nil)
assert.equal(payload.httponly, true)
assert.equal(payload.secure, true)
assert.equal(payload.partitioned, expected_partitioned)
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["test.com"] = {"/"}
b.sessionAffinityConfig.cookieSessionAffinity.secure = true
b.sessionAffinityConfig.cookieSessionAffinity.partitioned = partitioned
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 Partitioned when user specifies partitioned=true", function()
test_set_cookie_with(sticky_balanced, "/", true, true)
end)
it("returns a cookie without Partitioned when user specifies partitioned=false", function()
test_set_cookie_with(sticky_balanced, "/", false, false)
end)
it("returns a cookie without Partitioned when user does not specify partitioned", function()
test_set_cookie_with(sticky_balanced, "/", nil, nil)
end)
end)

describe("get_cookie()", function()

describe("legacy cookie value", function()
Expand Down