From 425d2655493cb339f39b2a0d5357638719252dad Mon Sep 17 00:00:00 2001
From: Max Verigin <max.verigin@gmail.com>
Date: Wed, 16 Sep 2020 12:22:08 -0700
Subject: [PATCH 1/9] store all empty endpoint fields, not just name

To access fields other than the name in an empty endpoint, temporarily
store the object itself, rather than just the name string.
---
 pkg/haproxy/dynupdate.go | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/pkg/haproxy/dynupdate.go b/pkg/haproxy/dynupdate.go
index 621cc8137..716cd04ad 100644
--- a/pkg/haproxy/dynupdate.go
+++ b/pkg/haproxy/dynupdate.go
@@ -165,13 +165,13 @@ func (d *dynUpdater) checkBackendPair(pair *backendPair) bool {
 	// map endpoints of old and new config together
 	endpoints := make(map[string]*epPair, len(oldBack.Endpoints))
 	targets := make([]string, 0, len(oldBack.Endpoints))
-	var empty []string
+	var empty []*hatypes.Endpoint
 	for _, endpoint := range oldBack.Endpoints {
 		if endpoint.Enabled {
 			endpoints[endpoint.Target] = &epPair{old: endpoint}
 			targets = append(targets, endpoint.Target)
 		} else {
-			empty = append(empty, endpoint.Name)
+			empty = append(empty, endpoint)
 		}
 	}
 
@@ -202,14 +202,14 @@ func (d *dynUpdater) checkBackendPair(pair *backendPair) bool {
 			if !d.execDisableEndpoint(curBack.ID, pair.old) || pair.old.Label != "" {
 				updated = false
 			}
-			empty = append(empty, pair.old.Name)
-		} else if !d.checkEndpointPair(curBack.ID, pair) {
+			empty = append(empty, pair.old)
+		} else if !d.checkEndpointPair(curBack, pair) {
 			updated = false
 		}
 	}
 	for i := range added {
 		// reusing empty slots from oldBack
-		added[i].Name = empty[i]
+		added[i].Name = empty[i].Name
 		if !d.execEnableEndpoint(curBack.ID, nil, added[i]) || added[i].Label != "" {
 			updated = false
 		}
@@ -217,18 +217,18 @@ func (d *dynUpdater) checkBackendPair(pair *backendPair) bool {
 
 	// copy remaining empty slots from oldBack to curBack, so it can be used in a future update
 	for i := len(added); i < len(empty); i++ {
-		curBack.AddEmptyEndpoint().Name = empty[i]
+		curBack.AddEmptyEndpoint().Name = empty[i].Name
 	}
 	curBack.SortEndpoints()
 
 	return updated
 }
 
-func (d *dynUpdater) checkEndpointPair(backname string, pair *epPair) bool {
+func (d *dynUpdater) checkEndpointPair(backend *hatypes.Backend, pair *epPair) bool {
 	if reflect.DeepEqual(pair.old, pair.cur) {
 		return true
 	}
-	updated := d.execEnableEndpoint(backname, pair.old, pair.cur)
+	updated := d.execEnableEndpoint(backend.ID, pair.old, pair.cur)
 	if !updated || pair.old.Label != "" || pair.cur.Label != "" {
 		return false
 	}

From 4ecd687f94d0e3e1497a1e60c6cf4b7cc51f0823 Mon Sep 17 00:00:00 2001
From: Max Verigin <max.verigin@gmail.com>
Date: Wed, 16 Sep 2020 12:46:54 -0700
Subject: [PATCH 2/9] extract cookie affinity calculation into function

---
 pkg/haproxy/types/backend.go              | 5 +++++
 rootfs/etc/templates/haproxy/haproxy.tmpl | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/pkg/haproxy/types/backend.go b/pkg/haproxy/types/backend.go
index f175108fc..cc9327aae 100644
--- a/pkg/haproxy/types/backend.go
+++ b/pkg/haproxy/types/backend.go
@@ -100,6 +100,11 @@ func (b *Backend) SortEndpoints() {
 	})
 }
 
+// CookieAffinity ...
+func (b *Backend) CookieAffinity() bool {
+	return !b.ModeTCP && b.Cookie.Name != "" && !b.Cookie.Dynamic
+}
+
 // FindBackendPath ...
 func (b *Backend) FindBackendPath(link PathLink) *BackendPath {
 	// IMPLEMENT change to a map
diff --git a/rootfs/etc/templates/haproxy/haproxy.tmpl b/rootfs/etc/templates/haproxy/haproxy.tmpl
index 686fa4ad2..8def14168 100644
--- a/rootfs/etc/templates/haproxy/haproxy.tmpl
+++ b/rootfs/etc/templates/haproxy/haproxy.tmpl
@@ -607,7 +607,7 @@ backend {{ $backend.ID }}
     server {{ $ep.Name }} {{ $ep.IP }}:{{ $ep.Port }}
         {{- if not $ep.Enabled }} disabled{{ end }}
         {{- "" }} weight {{ $ep.Weight }}
-        {{- if and (not $backend.ModeTCP) ($backend.Cookie.Name) (not $backend.Cookie.Dynamic) }} cookie {{ $ep.Name }}{{ end }}
+        {{- if $backend.CookieAffinity }} cookie {{ $ep.Name }}{{ end }}
         {{- template "backend" map $backend }}
 {{- end }}
 {{- end }}

From 7ffbf637b7929d9a41d9eb7ed762ce388ab52322 Mon Sep 17 00:00:00 2001
From: Max Verigin <max.verigin@gmail.com>
Date: Wed, 16 Sep 2020 14:00:33 -0700
Subject: [PATCH 3/9] add CookieValue field to separate logic from Name

Currently, the CookieValue will just be set to Name when cookie affinity
is enabled, which is how it already works. Separating CookieValue out to
a different field allows us to add future logic which will compute the
CookieValue differently from the name.
---
 pkg/converters/ingress/ingress.go         | 23 +++++++++++++++++++++++
 pkg/haproxy/instance_test.go              |  3 +++
 pkg/haproxy/types/backend.go              |  4 ++++
 pkg/haproxy/types/types.go                | 17 +++++++++--------
 rootfs/etc/templates/haproxy/haproxy.tmpl |  2 +-
 5 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/pkg/converters/ingress/ingress.go b/pkg/converters/ingress/ingress.go
index 301fa20cf..6b59cd774 100644
--- a/pkg/converters/ingress/ingress.go
+++ b/pkg/converters/ingress/ingress.go
@@ -169,6 +169,7 @@ func (c *converter) syncFull() {
 		c.syncIngress(ing)
 	}
 	c.fullSyncAnnotations()
+	c.syncEndpointCookies()
 }
 
 func (c *converter) syncPartial() {
@@ -281,6 +282,7 @@ func (c *converter) syncPartial() {
 		c.syncIngress(ing)
 	}
 	c.partialSyncAnnotations()
+	c.syncChangedEndpointCookies()
 }
 
 // trackAddedIngress add tracking hostnames and backends to new ingress objects
@@ -436,6 +438,18 @@ func (c *converter) syncIngress(ing *networking.Ingress) {
 	}
 }
 
+func (c *converter) syncEndpointCookies() {
+	for _, backend := range c.haproxy.Backends().Items() {
+		c.syncBackendEndpointCookies(backend)
+	}
+}
+
+func (c *converter) syncChangedEndpointCookies() {
+	for _, backend := range c.haproxy.Backends().ItemsAdd() {
+		c.syncBackendEndpointCookies(backend)
+	}
+}
+
 func (c *converter) fullSyncAnnotations() {
 	c.updater.UpdateGlobalConfig(c.haproxy, c.globalConfig)
 	for _, host := range c.haproxy.Hosts().Items() {
@@ -597,6 +611,15 @@ func (c *converter) addBackend(source *annotations.Source, hostname, uri, fullSv
 	return backend, nil
 }
 
+func (c *converter) syncBackendEndpointCookies(backend *hatypes.Backend) {
+	cookieAffinity := backend.CookieAffinity()
+	for _, ep := range backend.Endpoints {
+		if cookieAffinity {
+			ep.CookieValue = ep.Name
+		}
+	}
+}
+
 func (c *converter) addTLS(source *annotations.Source, hostname, secretName string) convtypes.CrtFile {
 	if secretName != "" {
 		tlsFile, err := c.cache.GetTLSSecretPath(
diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go
index 6716400dd..6c6e713d8 100644
--- a/pkg/haproxy/instance_test.go
+++ b/pkg/haproxy/instance_test.go
@@ -54,6 +54,9 @@ func TestBackends(t *testing.T) {
 				b.Cookie.Name = "ingress-controller"
 				b.Cookie.Strategy = "insert"
 				b.Cookie.Keywords = "indirect nocache httponly"
+				e1 := *endpointS1
+				b.Endpoints = []*hatypes.Endpoint{&e1}
+				b.Endpoints[0].CookieValue = "s1"
 			},
 			srvsuffix: "cookie s1",
 			expected: `
diff --git a/pkg/haproxy/types/backend.go b/pkg/haproxy/types/backend.go
index cc9327aae..b90886d69 100644
--- a/pkg/haproxy/types/backend.go
+++ b/pkg/haproxy/types/backend.go
@@ -63,6 +63,10 @@ func (b *Backend) AcquireEndpoint(ip string, port int, targetRef string) *Endpoi
 func (b *Backend) AddEmptyEndpoint() *Endpoint {
 	endpoint := b.addEndpoint("127.0.0.1", 1023, "")
 	endpoint.Enabled = false
+	// we need to set the cookie value to something here so that when dynamic
+	// update enables these endpoints without a reload, they will use cookie
+	// affinity (if it's enabled).
+	endpoint.CookieValue = endpoint.Name
 	return endpoint
 }
 
diff --git a/pkg/haproxy/types/types.go b/pkg/haproxy/types/types.go
index 1632b613d..ea586dd66 100644
--- a/pkg/haproxy/types/types.go
+++ b/pkg/haproxy/types/types.go
@@ -496,14 +496,15 @@ type Backend struct {
 
 // Endpoint ...
 type Endpoint struct {
-	Enabled   bool
-	Label     string
-	IP        string
-	Name      string
-	Port      int
-	Target    string
-	TargetRef string
-	Weight    int
+	Enabled     bool
+	Label       string
+	IP          string
+	Name        string
+	Port        int
+	Target      string
+	TargetRef   string
+	Weight      int
+	CookieValue string
 }
 
 // BlueGreenConfig ...
diff --git a/rootfs/etc/templates/haproxy/haproxy.tmpl b/rootfs/etc/templates/haproxy/haproxy.tmpl
index 8def14168..1dab6434c 100644
--- a/rootfs/etc/templates/haproxy/haproxy.tmpl
+++ b/rootfs/etc/templates/haproxy/haproxy.tmpl
@@ -607,7 +607,7 @@ backend {{ $backend.ID }}
     server {{ $ep.Name }} {{ $ep.IP }}:{{ $ep.Port }}
         {{- if not $ep.Enabled }} disabled{{ end }}
         {{- "" }} weight {{ $ep.Weight }}
-        {{- if $backend.CookieAffinity }} cookie {{ $ep.Name }}{{ end }}
+        {{- if and ($backend.CookieAffinity) ($ep.CookieValue) }} cookie {{ $ep.CookieValue }}{{ end }}
         {{- template "backend" map $backend }}
 {{- end }}
 {{- end }}

From 57a470508ea93edfe9c04d5087670337f03253f3 Mon Sep 17 00:00:00 2001
From: Max Verigin <max.verigin@gmail.com>
Date: Wed, 16 Sep 2020 14:19:14 -0700
Subject: [PATCH 4/9] Add 'session-cookie-preserve' backend annotation

The 'session-cookie-preserve` annotation is able to be used when cookie
affinity is enabled. The purpose is to be able to intelligently control
dynamic updating in such a way where it avoids causing an update that
would make the cookie value for a server no longer match an Enabled
server (mismatch on disabling a server is fine).

The reason cookie values matching their assigned backend is important,
is because the "preserve" mode of HAProxy allows servers to emit a
"Set-Cookie" header to effectively assign their own server. Because of
this, the specific cookie value assigned to each backend server must be
consistent and known by both HAProxy as well as the backend server
itself.

This change makes usage of "preserve" mode in "session-cookie-keywords"
no longer (or at least, less) desirable, but it should still be allowed
for backward compatibility and maximum customizability, therefore a
warning will simply be logged when it is used in that way.
---
 docs/content/en/docs/configuration/keys.md    |   7 +-
 pkg/converters/ingress/annotations/backend.go |   6 +
 .../ingress/annotations/backend_test.go       |  25 +++
 pkg/converters/ingress/defaults.go            |   1 +
 pkg/converters/ingress/types/annotations.go   |   1 +
 pkg/haproxy/dynupdate.go                      |  11 +-
 pkg/haproxy/dynupdate_test.go                 | 144 ++++++++++++++++++
 pkg/haproxy/instance_test.go                  |  15 ++
 pkg/haproxy/types/backend.go                  |   3 +-
 pkg/haproxy/types/types.go                    |   1 +
 rootfs/etc/templates/haproxy/haproxy.tmpl     |   1 +
 11 files changed, 211 insertions(+), 4 deletions(-)

diff --git a/docs/content/en/docs/configuration/keys.md b/docs/content/en/docs/configuration/keys.md
index f34325fd5..fdc04f1b3 100644
--- a/docs/content/en/docs/configuration/keys.md
+++ b/docs/content/en/docs/configuration/keys.md
@@ -199,6 +199,7 @@ The table below describes all supported configuration keys.
 | [`session-cookie-dynamic`](#affinity)                | [true\|false]                           | Backend |                    |
 | [`session-cookie-keywords`](#affinity)               | cookie options                          | Backend | `indirect nocache httponly`     |
 | [`session-cookie-name`](#affinity)                   | cookie name                             | Backend |                    |
+| [`session-cookie-preserve`](#affinity)               | [true\|false]                           | Backend | `false`            |
 | [`session-cookie-shared`](#affinity)                 | [true\|false]                           | Backend | `false`            |
 | [`session-cookie-strategy`](#affinity)               | [insert\|prefix\|rewrite]               | Backend |                    |
 | [`slots-min-free`](#dynamic-scaling)                 | minimum number of free slots            | Backend | `0`                |
@@ -347,6 +348,7 @@ See also:
 | `session-cookie-dynamic`    | `Backend` | `true`                      |       |
 | `session-cookie-keywords`   | `Backend` | `indirect nocache httponly` | v0.11 |
 | `session-cookie-name`       | `Backend` | `INGRESSCOOKIE`             |       |
+| `session-cookie-preserve`   | `Backend` | `false`                     | v0.12 |
 | `session-cookie-shared`     | `Backend` | `false`                     | v0.8  |
 | `session-cookie-strategy`   | `Backend` | `insert`                    |       |
 
@@ -355,8 +357,9 @@ Configure if HAProxy should maintain client requests to the same backend server.
 * `affinity`: the only supported option is `cookie`. If declared, clients will receive a cookie with a hash of the server it should be fidelized to.
 * `cookie-key`: defines a secret key used with the IP address and port number of a backend server to dynamically create a cookie to that server. Defaults to `Ingress` if not provided.
 * `session-cookie-dynamic`: indicates whether or not dynamic cookie value will be used. With the default of `true`, a cookie value will be generated by HAProxy using a hash of the server IP address, TCP port, and dynamic cookie secret key. When `false`, the server name will be used as the cookie name. Note that setting this to `false` will have no impact if [use-resolver](#dns-resolvers) is set.
-* `session-cookie-keywords`: additional options to the `cookie` option like `nocache`, `preserve`, `httponly`. For the sake of backwards compatibility the default is `indirect nocache httponly` if not declared and `strategy` is `insert`.
+* `session-cookie-keywords`: additional options to the `cookie` option like `nocache`, `httponly`. For the sake of backwards compatibility the default is `indirect nocache httponly` if not declared and `strategy` is `insert`.
 * `session-cookie-name`: the name of the cookie. `INGRESSCOOKIE` is the default value if not declared.
+* `session-cookie-preserve`: indicates whether the session cookie will be set to `preserve` mode. If this mode is enabled, haproxy will allow backend servers to use a `Set-Cookie` HTTP header to emit their own persistence cookie value, meaning the backend servers have knowledge of which cookie value should route to which server. Since the cookie value is tightly coupled with a particular backend server in this scenario, this mode will cause dynamic updating to understand that it must keep the same cookie value associated with the same backend server. If this is disabled, dynamic updating is free to assign servers in a way that can make their cookie value no longer matching.
 * `session-cookie-shared`: defines if the persistence cookie should be shared between all domains that uses this backend. Defaults to `false`. If `true` the `Set-Cookie` response will declare all the domains that shares this backend, indicating to the HTTP agent that all of them should use the same backend server.
 * `session-cookie-strategy`: the cookie strategy to use (insert, rewrite, prefix). `insert` is the default value if not declared.
 
@@ -498,7 +501,7 @@ Configures how to name backend servers.
 * `ip`: Uses target's `<ip>:<port>` as the server name.
 
 {{% alert title="Note" %}}
-HAProxy Ingress won't refuse to change the default naming if dynamic update is `true`, this would however lead to undesired behaviour: empty slots would still be named as sequences, old-named backend servers will dynamically receive new workloads with new pod names or IP numbers which does not relates with the name anymore, making the naming useless, if not wrong.
+HAProxy Ingress won't refuse to change the default naming if dynamic update is `true`, this would however lead to undesired behaviour: empty slots would still be named as sequences, old-named backend servers will dynamically receive new workloads with new pod names or IP numbers which do not relate with the name anymore, making the naming useless, if not wrong. If you have [cookie affinity](#affinity) enabled, dynamic updating can cause the cookie values to get out of sync with the servers. This can be avoided by using `session-cookie-preserve` with a value of `true`.
 {{% /alert %}}
 
 ---
diff --git a/pkg/converters/ingress/annotations/backend.go b/pkg/converters/ingress/annotations/backend.go
index ebae495ee..be2acae1b 100644
--- a/pkg/converters/ingress/annotations/backend.go
+++ b/pkg/converters/ingress/annotations/backend.go
@@ -61,7 +61,13 @@ func (c *updater) buildBackendAffinity(d *backData) {
 	}
 	d.backend.Cookie.Keywords = keywords
 	d.backend.Cookie.Dynamic = d.mapper.Get(ingtypes.BackSessionCookieDynamic).Bool()
+	d.backend.Cookie.Preserve = d.mapper.Get(ingtypes.BackSessionCookiePreserve).Bool()
 	d.backend.Cookie.Shared = d.mapper.Get(ingtypes.BackSessionCookieShared).Bool()
+
+	if strings.Contains(d.backend.Cookie.Keywords, "preserve") {
+		// just warn, no error, for keeping backwards compatibility where "preserve" may have been used in the "keywords" section
+		c.logger.Warn("session-cookie-keywords contains 'preserve'; consider using 'session-cookie-preserve' instead for better dynamic update cookie persistence")
+	}
 }
 
 func (c *updater) buildBackendAuthHTTP(d *backData) {
diff --git a/pkg/converters/ingress/annotations/backend_test.go b/pkg/converters/ingress/annotations/backend_test.go
index 14f227574..f0e46a92f 100644
--- a/pkg/converters/ingress/annotations/backend_test.go
+++ b/pkg/converters/ingress/annotations/backend_test.go
@@ -121,6 +121,31 @@ func TestAffinity(t *testing.T) {
 			expCookie:  hatypes.Cookie{Name: "INGRESSCOOKIE", Strategy: "prefix", Keywords: "nocache"},
 			expLogging: "",
 		},
+		// 10 - test that warning is logged when using "preserve" in the keywords annotation instead of in "session-cookie-preserve" annotation
+		{
+			ann: map[string]string{
+				ingtypes.BackAffinity:              "cookie",
+				ingtypes.BackSessionCookieName:     "serverId",
+				ingtypes.BackSessionCookieStrategy: "insert",
+				ingtypes.BackSessionCookieDynamic:  "false",
+				ingtypes.BackSessionCookieKeywords: "preserve nocache",
+			},
+			expCookie:  hatypes.Cookie{Name: "serverId", Strategy: "insert", Dynamic: false, Preserve: false, Keywords: "preserve nocache"},
+			expLogging: "WARN session-cookie-keywords contains 'preserve'; consider using 'session-cookie-preserve' instead for better dynamic update cookie persistence",
+		},
+		// 11 - test "session-cookie-preserve" cookie annotation is applied
+		{
+			ann: map[string]string{
+				ingtypes.BackAffinity:              "cookie",
+				ingtypes.BackSessionCookieName:     "serverId",
+				ingtypes.BackSessionCookieStrategy: "insert",
+				ingtypes.BackSessionCookieDynamic:  "false",
+				ingtypes.BackSessionCookiePreserve: "true",
+				ingtypes.BackSessionCookieKeywords: "nocache",
+			},
+			expCookie:  hatypes.Cookie{Name: "serverId", Strategy: "insert", Dynamic: false, Preserve: true, Keywords: "nocache"},
+			expLogging: "",
+		},
 	}
 
 	source := &Source{
diff --git a/pkg/converters/ingress/defaults.go b/pkg/converters/ingress/defaults.go
index 2efc3a66d..7996533c1 100644
--- a/pkg/converters/ingress/defaults.go
+++ b/pkg/converters/ingress/defaults.go
@@ -52,6 +52,7 @@ func createDefaults() map[string]string {
 		types.BackHSTSPreload:            "false",
 		types.BackInitialWeight:          "1",
 		types.BackSessionCookieDynamic:   "true",
+		types.BackSessionCookiePreserve:  "false",
 		types.BackSSLRedirect:            "true",
 		types.BackSSLCipherSuitesBackend: defaultSSLCipherSuites,
 		types.BackSSLCiphersBackend:      defaultSSLCiphers,
diff --git a/pkg/converters/ingress/types/annotations.go b/pkg/converters/ingress/types/annotations.go
index dbda75f53..7c2c79229 100644
--- a/pkg/converters/ingress/types/annotations.go
+++ b/pkg/converters/ingress/types/annotations.go
@@ -119,6 +119,7 @@ const (
 	BackSessionCookieDynamic   = "session-cookie-dynamic"
 	BackSessionCookieKeywords  = "session-cookie-keywords"
 	BackSessionCookieName      = "session-cookie-name"
+	BackSessionCookiePreserve  = "session-cookie-preserve"
 	BackSessionCookieShared    = "session-cookie-shared"
 	BackSessionCookieStrategy  = "session-cookie-strategy"
 	BackSSLCipherSuitesBackend = "ssl-cipher-suites-backend"
diff --git a/pkg/haproxy/dynupdate.go b/pkg/haproxy/dynupdate.go
index 716cd04ad..cb229ffc7 100644
--- a/pkg/haproxy/dynupdate.go
+++ b/pkg/haproxy/dynupdate.go
@@ -210,7 +210,11 @@ func (d *dynUpdater) checkBackendPair(pair *backendPair) bool {
 	for i := range added {
 		// reusing empty slots from oldBack
 		added[i].Name = empty[i].Name
-		if !d.execEnableEndpoint(curBack.ID, nil, added[i]) || added[i].Label != "" {
+		if curBack.Cookie.Preserve && added[i].CookieValue != empty[i].CookieValue {
+			// if cookie doesn't match here and preserving the value is
+			// important, don't even enable the endpoint before reloading
+			updated = false
+		} else if !d.execEnableEndpoint(curBack.ID, nil, added[i]) || added[i].Label != "" {
 			updated = false
 		}
 	}
@@ -228,6 +232,11 @@ func (d *dynUpdater) checkEndpointPair(backend *hatypes.Backend, pair *epPair) b
 	if reflect.DeepEqual(pair.old, pair.cur) {
 		return true
 	}
+	if backend.Cookie.Preserve && pair.old.CookieValue != pair.cur.CookieValue {
+		// if cookie doesn't match here and preserving the value is
+		// important, don't even enable the endpoint before reloading
+		return false
+	}
 	updated := d.execEnableEndpoint(backend.ID, pair.old, pair.cur)
 	if !updated || pair.old.Label != "" || pair.cur.Label != "" {
 		return false
diff --git a/pkg/haproxy/dynupdate_test.go b/pkg/haproxy/dynupdate_test.go
index 8bc56dbe8..75dd3bdf9 100644
--- a/pkg/haproxy/dynupdate_test.go
+++ b/pkg/haproxy/dynupdate_test.go
@@ -582,6 +582,150 @@ set server default_app_8080/srv002 weight 1`,
 			},
 			dynamic: true,
 		},
+		// 23 - test that we're able to update when a cookie value of acquired
+		// existing endpoint exactly matches and cookie affinity is enabled
+		// even with "preserve" cookie mode
+		{
+			doconfig1: func(c *testConfig) {
+				b1 := c.config.Backends().AcquireBackend("default", "default_backend", "8080")
+				c.config.Backends().SetDefaultBackend(b1)
+				b2 := c.config.Backends().AcquireBackend("default", "app", "8080")
+				// some of these are unnecessary but the attempt is to have as
+				// realistic config as possible for a more reliable test
+				b2.Cookie.Name = "serverId"
+				b2.Cookie.Strategy = "insert"
+				b2.Cookie.Keywords = "nocache"
+				b2.Cookie.Dynamic = false
+				b2.Cookie.Preserve = true
+				b2.ModeTCP = false
+				b2.AcquireEndpoint("172.17.0.2", 8080, "").CookieValue = "5017b276-6886-4ae0-b75e-bd1fcb9b1e3b"
+				b2.AcquireEndpoint("172.17.0.3", 8080, "").CookieValue = "3e4c9c86-0fc4-4aa9-9d96-bf0c49797006"
+			},
+			doconfig2: func(c *testConfig) {
+				b1 := c.config.Backends().AcquireBackend("default", "default_backend", "8080")
+				b1.Dynamic.DynUpdate = true
+				c.config.Backends().SetDefaultBackend(b1)
+				b2 := c.config.Backends().AcquireBackend("default", "app", "8080")
+				b2.Dynamic.DynUpdate = true
+				// some of these are unnecessary but the attempt is to have as
+				// realistic config as possible for a more reliable test
+				b2.Cookie.Name = "serverId"
+				b2.Cookie.Strategy = "insert"
+				b2.Cookie.Keywords = "nocache"
+				b2.Cookie.Dynamic = false
+				b2.Cookie.Preserve = true
+				b2.ModeTCP = false
+				b2.AcquireEndpoint("172.17.0.2", 8080, "").CookieValue = "5017b276-6886-4ae0-b75e-bd1fcb9b1e3b"
+				// acquiring a different ip on srv002 should succeed dynamically
+				// because the cookie didn't change
+				b2.AcquireEndpoint("172.17.0.4", 8080, "").CookieValue = "3e4c9c86-0fc4-4aa9-9d96-bf0c49797006"
+			},
+			expected: []string{
+				"srv001:172.17.0.2:8080:1",
+				"srv002:172.17.0.4:8080:1",
+			},
+			dynamic: true,
+			cmd: `
+set server default_app_8080/srv002 addr 172.17.0.4 port 8080
+set server default_app_8080/srv002 state ready
+set server default_app_8080/srv002 weight 1
+`,
+			logging: `INFO-V(2) updated endpoint '172.17.0.4:8080' weight '1' state 'ready' on backend/server 'default_app_8080/srv002'`,
+		},
+		// 24 - test that we're unable to update when a cookie value of acquired
+		// existing endpoint doesn't match and "preserve" cookie mode is enabled
+		{
+			doconfig1: func(c *testConfig) {
+				b1 := c.config.Backends().AcquireBackend("default", "default_backend", "8080")
+				c.config.Backends().SetDefaultBackend(b1)
+				b2 := c.config.Backends().AcquireBackend("default", "app", "8080")
+				// some of these are unnecessary but the attempt is to have as
+				// realistic config as possible for a more reliable test
+				b2.Cookie.Name = "serverId"
+				b2.Cookie.Strategy = "insert"
+				b2.Cookie.Keywords = "nocache"
+				b2.Cookie.Dynamic = false
+				b2.Cookie.Preserve = true
+				b2.ModeTCP = false
+				b2.AcquireEndpoint("172.17.0.2", 8080, "").CookieValue = "5017b276-6886-4ae0-b75e-bd1fcb9b1e3b"
+				b2.AcquireEndpoint("172.17.0.3", 8080, "").CookieValue = "3e4c9c86-0fc4-4aa9-9d96-bf0c49797006"
+			},
+			doconfig2: func(c *testConfig) {
+				b1 := c.config.Backends().AcquireBackend("default", "default_backend", "8080")
+				b1.Dynamic.DynUpdate = true
+				c.config.Backends().SetDefaultBackend(b1)
+				b2 := c.config.Backends().AcquireBackend("default", "app", "8080")
+				b2.Dynamic.DynUpdate = true
+				// some of these are unnecessary but the attempt is to have as
+				// realistic config as possible for a more reliable test
+				b2.Cookie.Name = "serverId"
+				b2.Cookie.Strategy = "insert"
+				b2.Cookie.Keywords = "nocache"
+				b2.Cookie.Dynamic = false
+				b2.Cookie.Preserve = true
+				b2.ModeTCP = false
+				b2.AcquireEndpoint("172.17.0.2", 8080, "").CookieValue = "5017b276-6886-4ae0-b75e-bd1fcb9b1e3b"
+				// changing this cookie value should cause it to not be able to
+				// dynupdate
+				b2.AcquireEndpoint("172.17.0.4", 8080, "").CookieValue = "a7b4db22-8689-4b56-8f49-1c1638c30acd"
+			},
+			expected: []string{
+				"srv001:172.17.0.2:8080:1",
+				"srv002:172.17.0.4:8080:1",
+			},
+			dynamic: false,
+			cmd:     ``,
+			logging: ``,
+		},
+		// 25 - test that we're able to update when a cookie value of acquired
+		// existing endpoint doesn't match and "preserve" cookie mode is not enabled
+		// (eg. it doesn't care to preserve the cookie value, so still updates)
+		{
+			doconfig1: func(c *testConfig) {
+				b1 := c.config.Backends().AcquireBackend("default", "default_backend", "8080")
+				c.config.Backends().SetDefaultBackend(b1)
+				b2 := c.config.Backends().AcquireBackend("default", "app", "8080")
+				// some of these are unnecessary but the attempt is to have as
+				// realistic config as possible for a more reliable test
+				b2.Cookie.Name = "serverId"
+				b2.Cookie.Strategy = "insert"
+				b2.Cookie.Keywords = "nocache"
+				b2.Cookie.Dynamic = false
+				b2.Cookie.Preserve = false
+				b2.ModeTCP = false
+				b2.AcquireEndpoint("172.17.0.2", 8080, "").CookieValue = "5017b276-6886-4ae0-b75e-bd1fcb9b1e3b"
+				b2.AcquireEndpoint("172.17.0.3", 8080, "").CookieValue = "3e4c9c86-0fc4-4aa9-9d96-bf0c49797006"
+			},
+			doconfig2: func(c *testConfig) {
+				b1 := c.config.Backends().AcquireBackend("default", "default_backend", "8080")
+				b1.Dynamic.DynUpdate = true
+				c.config.Backends().SetDefaultBackend(b1)
+				b2 := c.config.Backends().AcquireBackend("default", "app", "8080")
+				b2.Dynamic.DynUpdate = true
+				// some of these are unnecessary but the attempt is to have as
+				// realistic config as possible for a more reliable test
+				b2.Cookie.Name = "serverId"
+				b2.Cookie.Strategy = "insert"
+				b2.Cookie.Keywords = "nocache"
+				b2.Cookie.Dynamic = false
+				b2.Cookie.Preserve = false
+				b2.ModeTCP = false
+				b2.AcquireEndpoint("172.17.0.2", 8080, "").CookieValue = "5017b276-6886-4ae0-b75e-bd1fcb9b1e3b"
+				// we can still update even though the cookie changes below because
+				// "preserve" cookie strategy is disabled
+				b2.AcquireEndpoint("172.17.0.4", 8080, "").CookieValue = "a7b4db22-8689-4b56-8f49-1c1638c30acd"
+			},
+			expected: []string{
+				"srv001:172.17.0.2:8080:1",
+				"srv002:172.17.0.4:8080:1",
+			},
+			dynamic: true,
+			cmd: `
+set server default_app_8080/srv002 addr 172.17.0.4 port 8080
+set server default_app_8080/srv002 state ready
+set server default_app_8080/srv002 weight 1`,
+			logging: `INFO-V(2) updated endpoint '172.17.0.4:8080' weight '1' state 'ready' on backend/server 'default_app_8080/srv002'`,
+		},
 	}
 	for i, test := range testCases {
 		c := setup(t)
diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go
index 6c6e713d8..c1192163e 100644
--- a/pkg/haproxy/instance_test.go
+++ b/pkg/haproxy/instance_test.go
@@ -632,6 +632,21 @@ d1.local/ path01`,
     server s32 172.17.0.132:8080 weight 100
     server s33 172.17.0.133:8080 weight 100`,
 		},
+		// simulates a config where the cookie "preserve" option is used
+		{
+			doconfig: func(g *hatypes.Global, h *hatypes.Host, b *hatypes.Backend) {
+				b.Cookie.Name = "serverId"
+				b.Cookie.Strategy = "insert"
+				b.Cookie.Preserve = true
+				b.Cookie.Keywords = "nocache"
+				ep1 := *endpointS1
+				b.Endpoints = []*hatypes.Endpoint{&ep1}
+				b.Endpoints[0].CookieValue = "web-abcde"
+			},
+			srvsuffix: "cookie web-abcde",
+			expected: `
+    cookie serverId insert preserve nocache`,
+		},
 	}
 	for _, test := range testCases {
 		c := setup(t)
diff --git a/pkg/haproxy/types/backend.go b/pkg/haproxy/types/backend.go
index b90886d69..0eb273314 100644
--- a/pkg/haproxy/types/backend.go
+++ b/pkg/haproxy/types/backend.go
@@ -65,7 +65,8 @@ func (b *Backend) AddEmptyEndpoint() *Endpoint {
 	endpoint.Enabled = false
 	// we need to set the cookie value to something here so that when dynamic
 	// update enables these endpoints without a reload, they will use cookie
-	// affinity (if it's enabled).
+	// affinity (if it's enabled). This happens when session-cookie-preserve
+	// is false.
 	endpoint.CookieValue = endpoint.Name
 	return endpoint
 }
diff --git a/pkg/haproxy/types/types.go b/pkg/haproxy/types/types.go
index ea586dd66..91d0813a8 100644
--- a/pkg/haproxy/types/types.go
+++ b/pkg/haproxy/types/types.go
@@ -666,6 +666,7 @@ type UserlistConfig struct {
 type Cookie struct {
 	Name     string
 	Dynamic  bool
+	Preserve bool
 	Shared   bool
 	Strategy string
 	Keywords string
diff --git a/rootfs/etc/templates/haproxy/haproxy.tmpl b/rootfs/etc/templates/haproxy/haproxy.tmpl
index 1dab6434c..ec61c3111 100644
--- a/rootfs/etc/templates/haproxy/haproxy.tmpl
+++ b/rootfs/etc/templates/haproxy/haproxy.tmpl
@@ -503,6 +503,7 @@ backend {{ $backend.ID }}
 {{- if $backend.Cookie.Name }}
 {{- $cookie := $backend.Cookie }}
     cookie {{ $cookie.Name }} {{ $cookie.Strategy }}
+        {{- if $cookie.Preserve }} preserve{{ end }}
         {{- if $cookie.Keywords }} {{ $cookie.Keywords }}{{ end }}
         {{- if $cookie.Shared }}
             {{- range $hostname := $backend.Hostnames }} domain {{ $hostname }}{{ end }}

From 379a859954c4a5cea99880aeab3bd656fe8e503e Mon Sep 17 00:00:00 2001
From: Max Verigin <max.verigin@gmail.com>
Date: Wed, 16 Sep 2020 14:29:13 -0700
Subject: [PATCH 5/9] Add 'session-cookie-value-strategy' annotation

This new backend annotation allows the way that a cookie value for a
backend server is calculated to be controlled more precisely.

Prior to this, the value of a cookie for a backend server was always set
to the Name of an endpoint, which was in turn controlled by
'backend-server-naming'. While that gives a few options for changing the
cookie value, the downside was that it required changing the name of the
server. Additionally, there are possible cookie values which would make
poor backend server names (such as a guid/uuid) but may make good cookie
values.

The new annotation defaults to setting the naming strategy to the Name
of the server, which is the existing behaviour. In addition, it adds the
ability to set the value to a Pod UID.
---
 docs/content/en/docs/configuration/keys.md    | 23 ++++++++-------
 pkg/converters/ingress/annotations/backend.go | 13 +++++++++
 pkg/converters/ingress/defaults.go            |  1 +
 pkg/converters/ingress/ingress.go             | 14 +++++++++-
 pkg/converters/ingress/types/annotations.go   |  1 +
 pkg/haproxy/instance_test.go                  | 15 ++++++++++
 pkg/haproxy/types/types.go                    | 28 +++++++++++++------
 7 files changed, 75 insertions(+), 20 deletions(-)

diff --git a/docs/content/en/docs/configuration/keys.md b/docs/content/en/docs/configuration/keys.md
index fdc04f1b3..1d1cf053e 100644
--- a/docs/content/en/docs/configuration/keys.md
+++ b/docs/content/en/docs/configuration/keys.md
@@ -202,6 +202,7 @@ The table below describes all supported configuration keys.
 | [`session-cookie-preserve`](#affinity)               | [true\|false]                           | Backend | `false`            |
 | [`session-cookie-shared`](#affinity)                 | [true\|false]                           | Backend | `false`            |
 | [`session-cookie-strategy`](#affinity)               | [insert\|prefix\|rewrite]               | Backend |                    |
+| [`session-cookie-value-strategy`](#affinity)         | [server-name\|pod-uid]                  | Backend | `server-name`      |
 | [`slots-min-free`](#dynamic-scaling)                 | minimum number of free slots            | Backend | `0`                |
 | [`ssl-cipher-suites`](#ssl-ciphers)                  | colon-separated list                    | Host    | [see description](#ssl-ciphers) |
 | [`ssl-cipher-suites-backend`](#ssl-ciphers)          | colon-separated list                    | Backend | [see description](#ssl-ciphers) |
@@ -341,16 +342,17 @@ See also:
 
 ## Affinity
 
-| Configuration key           | Scope     | Default                     | Since |
-|-----------------------------|-----------|-----------------------------|-------|
-| `affinity`                  | `Backend` | `false`                     |       |
-| `cookie-key`                | `Global`  | `Ingress`                   |       |
-| `session-cookie-dynamic`    | `Backend` | `true`                      |       |
-| `session-cookie-keywords`   | `Backend` | `indirect nocache httponly` | v0.11 |
-| `session-cookie-name`       | `Backend` | `INGRESSCOOKIE`             |       |
-| `session-cookie-preserve`   | `Backend` | `false`                     | v0.12 |
-| `session-cookie-shared`     | `Backend` | `false`                     | v0.8  |
-| `session-cookie-strategy`   | `Backend` | `insert`                    |       |
+| Configuration key               | Scope     | Default                     | Since |
+|---------------------------------|-----------|-----------------------------|-------|
+| `affinity`                      | `Backend` | `false`                     |       |
+| `cookie-key`                    | `Global`  | `Ingress`                   |       |
+| `session-cookie-dynamic`        | `Backend` | `true`                      |       |
+| `session-cookie-keywords`       | `Backend` | `indirect nocache httponly` | v0.11 |
+| `session-cookie-name`           | `Backend` | `INGRESSCOOKIE`             |       |
+| `session-cookie-preserve`       | `Backend` | `false`                     | v0.12 |
+| `session-cookie-shared`         | `Backend` | `false`                     | v0.8  |
+| `session-cookie-strategy`       | `Backend` | `insert`                    |       |
+| `session-cookie-value-strategy` | `Backend` | `server-name`               | v0.12 |
 
 Configure if HAProxy should maintain client requests to the same backend server.
 
@@ -362,6 +364,7 @@ Configure if HAProxy should maintain client requests to the same backend server.
 * `session-cookie-preserve`: indicates whether the session cookie will be set to `preserve` mode. If this mode is enabled, haproxy will allow backend servers to use a `Set-Cookie` HTTP header to emit their own persistence cookie value, meaning the backend servers have knowledge of which cookie value should route to which server. Since the cookie value is tightly coupled with a particular backend server in this scenario, this mode will cause dynamic updating to understand that it must keep the same cookie value associated with the same backend server. If this is disabled, dynamic updating is free to assign servers in a way that can make their cookie value no longer matching.
 * `session-cookie-shared`: defines if the persistence cookie should be shared between all domains that uses this backend. Defaults to `false`. If `true` the `Set-Cookie` response will declare all the domains that shares this backend, indicating to the HTTP agent that all of them should use the same backend server.
 * `session-cookie-strategy`: the cookie strategy to use (insert, rewrite, prefix). `insert` is the default value if not declared.
+* `session-cookie-value-strategy`: the strategy to use to calculate the cookie value of a server (`server-name`, `pod-uid`). `server-name` is the default if not declared, and indicates that the cookie will be set based on the name defined in `backend-server-naming`. `pod-uid` indicates that the cookie will be set to the `UID` of the pod running the target server.
 
 Note for `dynamic-scaling` users only, v0.5 or older: the hash of the server is built based on it's name.
 When the slots are scaled down, the remaining servers might change it's server name on
diff --git a/pkg/converters/ingress/annotations/backend.go b/pkg/converters/ingress/annotations/backend.go
index be2acae1b..4f5e94b0e 100644
--- a/pkg/converters/ingress/annotations/backend.go
+++ b/pkg/converters/ingress/annotations/backend.go
@@ -68,6 +68,19 @@ func (c *updater) buildBackendAffinity(d *backData) {
 		// just warn, no error, for keeping backwards compatibility where "preserve" may have been used in the "keywords" section
 		c.logger.Warn("session-cookie-keywords contains 'preserve'; consider using 'session-cookie-preserve' instead for better dynamic update cookie persistence")
 	}
+
+	cookieStrategy := d.mapper.Get(ingtypes.BackSessionCookieValue).Value
+	switch cookieStrategy {
+	case "pod-uid":
+		d.backend.EpCookieStrategy = hatypes.EpCookiePodUid
+	case "server-name":
+		d.backend.EpCookieStrategy = hatypes.EpCookieName
+	default:
+		c.logger.Warn("invalid session-cookie-value-strategy '%s', using 'server-name' instead", cookieStrategy)
+		fallthrough
+	case "":
+		d.backend.EpCookieStrategy = hatypes.EpCookieName
+	}
 }
 
 func (c *updater) buildBackendAuthHTTP(d *backData) {
diff --git a/pkg/converters/ingress/defaults.go b/pkg/converters/ingress/defaults.go
index 7996533c1..66bf27d14 100644
--- a/pkg/converters/ingress/defaults.go
+++ b/pkg/converters/ingress/defaults.go
@@ -53,6 +53,7 @@ func createDefaults() map[string]string {
 		types.BackInitialWeight:          "1",
 		types.BackSessionCookieDynamic:   "true",
 		types.BackSessionCookiePreserve:  "false",
+		types.BackSessionCookieValue:     "server-name",
 		types.BackSSLRedirect:            "true",
 		types.BackSSLCipherSuitesBackend: defaultSSLCipherSuites,
 		types.BackSSLCiphersBackend:      defaultSSLCiphers,
diff --git a/pkg/converters/ingress/ingress.go b/pkg/converters/ingress/ingress.go
index 6b59cd774..d8d136e45 100644
--- a/pkg/converters/ingress/ingress.go
+++ b/pkg/converters/ingress/ingress.go
@@ -615,7 +615,19 @@ func (c *converter) syncBackendEndpointCookies(backend *hatypes.Backend) {
 	cookieAffinity := backend.CookieAffinity()
 	for _, ep := range backend.Endpoints {
 		if cookieAffinity {
-			ep.CookieValue = ep.Name
+			switch backend.EpCookieStrategy {
+			default:
+				ep.CookieValue = ep.Name
+			case hatypes.EpCookiePodUid:
+				if ep.TargetRef != "" {
+					pod, err := c.cache.GetPod(ep.TargetRef)
+					if err == nil {
+						ep.CookieValue = fmt.Sprintf("%v", pod.UID)
+					} else {
+						c.logger.Error("error calculating cookie value for pod %s: %v", ep.TargetRef, err)
+					}
+				}
+			}
 		}
 	}
 }
diff --git a/pkg/converters/ingress/types/annotations.go b/pkg/converters/ingress/types/annotations.go
index 7c2c79229..b2cf2a834 100644
--- a/pkg/converters/ingress/types/annotations.go
+++ b/pkg/converters/ingress/types/annotations.go
@@ -122,6 +122,7 @@ const (
 	BackSessionCookiePreserve  = "session-cookie-preserve"
 	BackSessionCookieShared    = "session-cookie-shared"
 	BackSessionCookieStrategy  = "session-cookie-strategy"
+	BackSessionCookieValue     = "session-cookie-value-strategy"
 	BackSSLCipherSuitesBackend = "ssl-cipher-suites-backend"
 	BackSSLCiphersBackend      = "ssl-ciphers-backend"
 	BackSSLFingerprintLower    = "ssl-fingerprint-lower"
diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go
index c1192163e..2cfd4c7e9 100644
--- a/pkg/haproxy/instance_test.go
+++ b/pkg/haproxy/instance_test.go
@@ -631,6 +631,21 @@ d1.local/ path01`,
     server s31 172.17.0.131:8080 weight 100
     server s32 172.17.0.132:8080 weight 100
     server s33 172.17.0.133:8080 weight 100`,
+		},
+		// simulates a config where the cookie value is a pod id
+		{
+			doconfig: func(g *hatypes.Global, h *hatypes.Host, b *hatypes.Backend) {
+				b.Cookie.Name = "serverId"
+				b.Cookie.Strategy = "insert"
+				b.Cookie.Keywords = "nocache"
+				b.EpCookieStrategy = hatypes.EpCookiePodUid
+				ep1 := *endpointS1
+				b.Endpoints = []*hatypes.Endpoint{&ep1}
+				b.Endpoints[0].CookieValue = "9d344d6c-6069-4aee-85e6-9348e70c71e6"
+			},
+			srvsuffix: "cookie 9d344d6c-6069-4aee-85e6-9348e70c71e6",
+			expected: `
+    cookie serverId insert nocache`,
 		},
 		// simulates a config where the cookie "preserve" option is used
 		{
diff --git a/pkg/haproxy/types/types.go b/pkg/haproxy/types/types.go
index 91d0813a8..1b9fe6379 100644
--- a/pkg/haproxy/types/types.go
+++ b/pkg/haproxy/types/types.go
@@ -410,6 +410,15 @@ const (
 	EpTargetRef
 )
 
+// EndpointCookieStrategy ...
+type EndpointCookieStrategy int
+
+// ...
+const (
+	EpCookieName EndpointCookieStrategy = iota
+	EpCookiePodUid
+)
+
 // Backends ...
 type Backends struct {
 	items, itemsAdd, itemsDel map[string]*Backend
@@ -434,15 +443,16 @@ type Backend struct {
 	//
 	// IMPLEMENT
 	// use BackendID
-	shard     int
-	ID        string
-	Namespace string
-	Name      string
-	Port      string
-	Endpoints []*Endpoint
-	EpNaming  EndpointNaming
-	Paths     []*BackendPath
-	PathsMap  *HostsMap
+	shard            int
+	ID               string
+	Namespace        string
+	Name             string
+	Port             string
+	Endpoints        []*Endpoint
+	EpNaming         EndpointNaming
+	EpCookieStrategy EndpointCookieStrategy
+	Paths            []*BackendPath
+	PathsMap         *HostsMap
 	//
 	// per backend config
 	//

From 474d3c5f9bb044346cef58112f6e557844178696 Mon Sep 17 00:00:00 2001
From: Max Verigin <max.verigin@gmail.com>
Date: Sat, 26 Sep 2020 10:42:31 -0700
Subject: [PATCH 6/9] Add source to warning message for cookie keywords

---
 pkg/converters/ingress/annotations/backend.go  | 18 +++++++++---------
 .../ingress/annotations/backend_test.go        |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/pkg/converters/ingress/annotations/backend.go b/pkg/converters/ingress/annotations/backend.go
index 4f5e94b0e..fe083a079 100644
--- a/pkg/converters/ingress/annotations/backend.go
+++ b/pkg/converters/ingress/annotations/backend.go
@@ -55,20 +55,20 @@ func (c *updater) buildBackendAffinity(d *backData) {
 	}
 	d.backend.Cookie.Name = name
 	d.backend.Cookie.Strategy = strategyName
-	keywords := d.mapper.Get(ingtypes.BackSessionCookieKeywords).Value
-	if strategyName == "insert" && keywords == "" {
-		keywords = "indirect nocache httponly"
+	keywords := d.mapper.Get(ingtypes.BackSessionCookieKeywords)
+	keywordsValue := keywords.Value
+	if strategyName == "insert" && keywordsValue == "" {
+		keywordsValue = "indirect nocache httponly"
 	}
-	d.backend.Cookie.Keywords = keywords
+	if strings.Contains(keywordsValue, "preserve") {
+		// just warn, no error, for keeping backwards compatibility where "preserve" may have been used in the "keywords" section
+		c.logger.Warn("session-cookie-keywords on %s contains 'preserve'; consider using 'session-cookie-preserve' instead for better dynamic update cookie persistence", keywords.Source)
+	}
+	d.backend.Cookie.Keywords = keywordsValue
 	d.backend.Cookie.Dynamic = d.mapper.Get(ingtypes.BackSessionCookieDynamic).Bool()
 	d.backend.Cookie.Preserve = d.mapper.Get(ingtypes.BackSessionCookiePreserve).Bool()
 	d.backend.Cookie.Shared = d.mapper.Get(ingtypes.BackSessionCookieShared).Bool()
 
-	if strings.Contains(d.backend.Cookie.Keywords, "preserve") {
-		// just warn, no error, for keeping backwards compatibility where "preserve" may have been used in the "keywords" section
-		c.logger.Warn("session-cookie-keywords contains 'preserve'; consider using 'session-cookie-preserve' instead for better dynamic update cookie persistence")
-	}
-
 	cookieStrategy := d.mapper.Get(ingtypes.BackSessionCookieValue).Value
 	switch cookieStrategy {
 	case "pod-uid":
diff --git a/pkg/converters/ingress/annotations/backend_test.go b/pkg/converters/ingress/annotations/backend_test.go
index f0e46a92f..76680d8a2 100644
--- a/pkg/converters/ingress/annotations/backend_test.go
+++ b/pkg/converters/ingress/annotations/backend_test.go
@@ -131,7 +131,7 @@ func TestAffinity(t *testing.T) {
 				ingtypes.BackSessionCookieKeywords: "preserve nocache",
 			},
 			expCookie:  hatypes.Cookie{Name: "serverId", Strategy: "insert", Dynamic: false, Preserve: false, Keywords: "preserve nocache"},
-			expLogging: "WARN session-cookie-keywords contains 'preserve'; consider using 'session-cookie-preserve' instead for better dynamic update cookie persistence",
+			expLogging: "WARN session-cookie-keywords on ingress 'default/ing1' contains 'preserve'; consider using 'session-cookie-preserve' instead for better dynamic update cookie persistence",
 		},
 		// 11 - test "session-cookie-preserve" cookie annotation is applied
 		{

From e7cf7f5ca3b779da2a376c299622235a0558c21d Mon Sep 17 00:00:00 2001
From: Max Verigin <max.verigin@gmail.com>
Date: Sat, 26 Sep 2020 10:58:10 -0700
Subject: [PATCH 7/9] Add source and test for cookie value strategy warn

---
 pkg/converters/ingress/annotations/backend.go      |  6 +++---
 pkg/converters/ingress/annotations/backend_test.go | 10 ++++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/pkg/converters/ingress/annotations/backend.go b/pkg/converters/ingress/annotations/backend.go
index fe083a079..122803486 100644
--- a/pkg/converters/ingress/annotations/backend.go
+++ b/pkg/converters/ingress/annotations/backend.go
@@ -69,14 +69,14 @@ func (c *updater) buildBackendAffinity(d *backData) {
 	d.backend.Cookie.Preserve = d.mapper.Get(ingtypes.BackSessionCookiePreserve).Bool()
 	d.backend.Cookie.Shared = d.mapper.Get(ingtypes.BackSessionCookieShared).Bool()
 
-	cookieStrategy := d.mapper.Get(ingtypes.BackSessionCookieValue).Value
-	switch cookieStrategy {
+	cookieStrategy := d.mapper.Get(ingtypes.BackSessionCookieValue)
+	switch cookieStrategy.Value {
 	case "pod-uid":
 		d.backend.EpCookieStrategy = hatypes.EpCookiePodUid
 	case "server-name":
 		d.backend.EpCookieStrategy = hatypes.EpCookieName
 	default:
-		c.logger.Warn("invalid session-cookie-value-strategy '%s', using 'server-name' instead", cookieStrategy)
+		c.logger.Warn("invalid session-cookie-value-strategy '%s' on %s, using 'server-name' instead", cookieStrategy.Value, cookieStrategy.Source)
 		fallthrough
 	case "":
 		d.backend.EpCookieStrategy = hatypes.EpCookieName
diff --git a/pkg/converters/ingress/annotations/backend_test.go b/pkg/converters/ingress/annotations/backend_test.go
index 76680d8a2..c38205991 100644
--- a/pkg/converters/ingress/annotations/backend_test.go
+++ b/pkg/converters/ingress/annotations/backend_test.go
@@ -146,6 +146,16 @@ func TestAffinity(t *testing.T) {
 			expCookie:  hatypes.Cookie{Name: "serverId", Strategy: "insert", Dynamic: false, Preserve: true, Keywords: "nocache"},
 			expLogging: "",
 		},
+		// 12 - test that there is a fallback to using the "name" cookie value strategy
+		{
+			ann: map[string]string{
+				ingtypes.BackAffinity:             "cookie",
+				ingtypes.BackSessionCookieDynamic: "false",
+				ingtypes.BackSessionCookieValue:   "err",
+			},
+			expCookie:  hatypes.Cookie{Name: "INGRESSCOOKIE", Strategy: "insert", Dynamic: false, Keywords: "indirect nocache httponly"},
+			expLogging: "WARN invalid session-cookie-value-strategy 'err' on ingress 'default/ing1', using 'server-name' instead",
+		},
 	}
 
 	source := &Source{

From 0d58edb40287f5f1e19770498566e8776fb5cd50 Mon Sep 17 00:00:00 2001
From: Max Verigin <max.verigin@gmail.com>
Date: Sat, 26 Sep 2020 11:02:17 -0700
Subject: [PATCH 8/9] Fallback to name cookie value strategy on error

---
 pkg/converters/ingress/ingress.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pkg/converters/ingress/ingress.go b/pkg/converters/ingress/ingress.go
index d8d136e45..a50eab170 100644
--- a/pkg/converters/ingress/ingress.go
+++ b/pkg/converters/ingress/ingress.go
@@ -624,7 +624,8 @@ func (c *converter) syncBackendEndpointCookies(backend *hatypes.Backend) {
 					if err == nil {
 						ep.CookieValue = fmt.Sprintf("%v", pod.UID)
 					} else {
-						c.logger.Error("error calculating cookie value for pod %s: %v", ep.TargetRef, err)
+						ep.CookieValue = ep.Name
+						c.logger.Error("error calculating cookie value for pod %s; falling back to 'server-name' strategy: %v", ep.TargetRef, err)
 					}
 				}
 			}

From 099b1f64b6c5a5fc3c48514f9d2e671b826acd71 Mon Sep 17 00:00:00 2001
From: Max Verigin <max.verigin@gmail.com>
Date: Sat, 26 Sep 2020 11:28:42 -0700
Subject: [PATCH 9/9] Docs for cookie naming with disable-pod-list

---
 docs/content/en/docs/configuration/command-line.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/content/en/docs/configuration/command-line.md b/docs/content/en/docs/configuration/command-line.md
index 98118300e..223e8bdfd 100644
--- a/docs/content/en/docs/configuration/command-line.md
+++ b/docs/content/en/docs/configuration/command-line.md
@@ -124,7 +124,7 @@ This is a mandatory argument used in the [deployment](https://github.com/jcmorai
 
 Since v0.11
 
-Disables in memory pod list and also pod watch for changes. Pod list and watch is used by `drain-support` option, which will not work if pod list is disabled. Blue/green also uses pod list if enabled, otherwise k8s api is called if needed. The default value is `false`, which means pods will be watched and listed in memory.
+Disables in memory pod list and also pod watch for changes. Pod list and watch is used by `drain-support` option, which will not work if pod list is disabled. Blue/green and `session-cookie-value-strategy` set to `pod-uid` also use pod list if enabled, otherwise k8s api is called if needed. The default value is `false`, which means pods will be watched and listed in memory.
 
 ---