kubernetes-ingress icon indicating copy to clipboard operation
kubernetes-ingress copied to clipboard

BUG/MINOR: don't add cookie to server if dynamic cookie enabled

Open cspargo opened this issue 3 years ago • 2 comments

the annotation haproxy.org/cookie-persistence is supposed to enable dynamic cookies for the backend, but i've found that it is still returning static cookies.

I tested with 1.7.9 and 1.8.0 and see the same behavior. If I define an ingress with the annotation

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: test
  namespace: test
  annotations:
    haproxy.org/cookie-persistence: "COOKIE"

The generated backend configuration looks like the following. Note the dynamic cookie option and key is set, but the server entries each specify a cookie (SRV_1 and SRV_2). If cookies are present on the servers, it disables the dynamic mode.

backend test_app_http
  mode http
  balance roundrobin
  option forwardfor
  no option abortonclose
  default-server check
  cookie COOKIE dynamic indirect nocache insert
  dynamic-cookie-key ohph7OoGhong
  server SRV_1 100.65.210.25:80 enabled cookie SRV_1
  server SRV_2 100.64.16.185:80 enabled cookie SRV_2

the cookie that is returned is the static cookie, like this:

set-cookie: COOKIE=SRV_1; path=/

or this

set-cookie: COOKIE=SRV_2; path=/

When running multiple haproxy pods, this has resulted in clients being sent to the incorrect server, if the order of the servers is different in each pod.

This change changes the logic so that it will not add a cookie to the server entries if the dynamic option is set.

Using the same ingress configuration with the patched version results in this backend configuration, without a cookie set on the servers

backend test_app_http
  mode http
  balance roundrobin
  option forwardfor
  no option abortonclose
  default-server check
  cookie COOKIE dynamic indirect nocache insert
  dynamic-cookie-key ohph7OoGhong
  server SRV_1 100.64.16.185:80 enabled
  server SRV_2 100.65.210.25:80 enabled

The cookies that are returned are now the expected dynamic values generated from a hash of the server addresses:

set-cookie: COOKIE=d257e60a895117a1; path=/

and

set-cookie: COOKIE=a2960a4362d3f17f; path=/

cspargo avatar Jun 15 '22 07:06 cspargo

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 15 '22 08:07 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 01 '22 23:09 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Oct 02 '22 07:10 stale[bot]

Dear bot, please don't close this issue

jgranieczny avatar Oct 05 '22 07:10 jgranieczny

Quite an important fix for our use case, keep this up :+1:

lukasged avatar Oct 11 '22 13:10 lukasged

The commit does not contain id of this issue. I got into 1.8 here https://github.com/haproxytech/kubernetes-ingress/commit/3ed52f338799415e75b1a2e3491946f35a33c690 on 18.1.2023 And to the 1.9 here https://github.com/haproxytech/kubernetes-ingress/commit/084368ac73ff1f8db2a85d4321c41c9c08415f0c on the same date

Hence the fix is available from v1.8.9 resp. v1.9.1

dosmanak avatar Feb 06 '23 11:02 dosmanak