Repository: envoyproxy/gateway
Pull request #3683 (2024-06-27 00:38:20)

feat: cookie based consistent hashing
Fixes: https://github.com/envoyproxy/gateway/issues/2624


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/envoyproxy/gateway/pull/3683?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy) Report
Attention: Patch coverage is `0%` with `20 lines` in your changes missing coverage. Please review.
> Project coverage is 68.28%. Comparing base [(`d784c32`)](https://app.codecov.io/gh/envoyproxy/gateway/commit/d784c3237765aa258c3526b32765dd9c3abf6415?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy) to head [(`d44050a`)](https://app.codecov.io/gh/envoyproxy/gateway/commit/d44050a53575a968db825d5b9222e13aaea267c1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy).

| [Files](https://app.codecov.io/gh/envoyproxy/gateway/pull/3683?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy) | Patch % | Lines |
|---|---|---|
| [internal/xds/translator/route.go](https://app.codecov.io/gh/envoyproxy/gateway/pull/3683?src=pr&el=tree&filepath=internal%2Fxds%2Ftranslator%2Froute.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy#diff-aW50ZXJuYWwveGRzL3RyYW5zbGF0b3Ivcm91dGUuZ28=) | 0.00% | [18 Missing :warning: ](https://app.codecov.io/gh/envoyproxy/gateway/pull/3683?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy) |
| [internal/gatewayapi/backendtrafficpolicy.go](https://app.codecov.io/gh/envoyproxy/gateway/pull/3683?src=pr&el=tree&filepath=internal%2Fgatewayapi%2Fbackendtrafficpolicy.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy#diff-aW50ZXJuYWwvZ2F0ZXdheWFwaS9iYWNrZW5kdHJhZmZpY3BvbGljeS5nbw==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/envoyproxy/gateway/pull/3683?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #3683      +/-   ##
==========================================
- Coverage   68.31%   68.28%   -0.03%     
==========================================
  Files         170      170              
  Lines       20780    20800      +20     
==========================================
+ Hits        14196    14204       +8     
- Misses       5565     5580      +15     
+ Partials     1019     1016       -3     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/envoyproxy/gateway/pull/3683?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=envoyproxy).

arkodg: > LGTM, need docs and tests.

yeah adding issues to track that, will pick those up and complete them by v1.1
zirain: /retest

Patch:
diff --git a/api/v1alpha1/loadbalancer_types.go b/api/v1alpha1/loadbalancer_types.go
index 9fdaf26d5..3fd144bdf 100644
--- a/api/v1alpha1/loadbalancer_types.go
+++ b/api/v1alpha1/loadbalancer_types.go
@@ -71,7 +71,6 @@ type ConsistentHash struct {
 	// Cookie configures the cookie hash policy when the consistent hash type is set to Cookie.
 	//
 	// +optional
-	// +notImplementedHide
 	Cookie *Cookie `json:"cookie,omitempty"`
 
 	// The table size for consistent hashing, must be prime number limited to 5000011.
@@ -103,12 +102,10 @@ type Cookie struct {
 	// Max-Age attribute value.
 	//
 	// +optional
-	// +notImplementedHide
 	TTL *metav1.Duration `json:"ttl,omitempty"`
 	// Additional Attributes to set for the generated cookie.
 	//
 	// +optional
-	// +notImplementedHide
 	Attributes map[string]string `json:"attributes,omitempty"`
 }
 
diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go
index 0750f0085..6babdfa81 100644
--- a/internal/gatewayapi/backendtrafficpolicy.go
+++ b/internal/gatewayapi/backendtrafficpolicy.go
@@ -864,6 +864,8 @@ func (t *Translator) buildConsistentHashLoadBalancer(policy *egv1a1.BackendTraff
 		consistentHash.Header = &ir.Header{
 			Name: policy.Spec.LoadBalancer.ConsistentHash.Header.Name,
 		}
+	case egv1a1.CookieConsistentHashType:
+		consistentHash.Cookie = policy.Spec.LoadBalancer.ConsistentHash.Cookie
 	}
 
 	return consistentHash, nil
diff --git a/internal/ir/xds.go b/internal/ir/xds.go
index 42704cffe..8dc95e783 100644
--- a/internal/ir/xds.go
+++ b/internal/ir/xds.go
@@ -1795,9 +1795,10 @@ type Random struct{}
 // +k8s:deepcopy-gen=true
 type ConsistentHash struct {
 	// Hash based on the Source IP Address
-	SourceIP  *bool   `json:"sourceIP,omitempty" yaml:"sourceIP,omitempty"`
-	Header    *Header `json:"header,omitempty" yaml:"header,omitempty"`
-	TableSize *uint64 `json:"tableSize,omitempty" yaml:"tableSize,omitempty"`
+	SourceIP  *bool          `json:"sourceIP,omitempty" yaml:"sourceIP,omitempty"`
+	Header    *Header        `json:"header,omitempty" yaml:"header,omitempty"`
+	Cookie    *egv1a1.Cookie `json:"cookie,omitempty" yaml:"cookie,omitempty"`
+	TableSize *uint64        `json:"tableSize,omitempty" yaml:"tableSize,omitempty"`
 }
 
 // Header consistent hash type settings
diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go
index 171c2c28d..d76ea363c 100644
--- a/internal/ir/zz_generated.deepcopy.go
+++ b/internal/ir/zz_generated.deepcopy.go
@@ -509,6 +509,11 @@ func (in *ConsistentHash) DeepCopyInto(out *ConsistentHash) {
 		*out = new(Header)
 		**out = **in
 	}
+	if in.Cookie != nil {
+		in, out := &in.Cookie, &out.Cookie
+		*out = new(v1alpha1.Cookie)
+		(*in).DeepCopyInto(*out)
+	}
 	if in.TableSize != nil {
 		in, out := &in.TableSize, &out.TableSize
 		*out = new(uint64)
diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go
index ba1baa6c2..139b359a1 100644
--- a/internal/xds/translator/route.go
+++ b/internal/xds/translator/route.go
@@ -493,6 +493,28 @@ func buildHashPolicy(httpRoute *ir.HTTPRoute) []*routev3.RouteAction_HashPolicy
 			},
 		}
 		return []*routev3.RouteAction_HashPolicy{hashPolicy}
+	case ch.Cookie != nil:
+		hashPolicy := &routev3.RouteAction_HashPolicy{
+			PolicySpecifier: &routev3.RouteAction_HashPolicy_Cookie_{
+				Cookie: &routev3.RouteAction_HashPolicy_Cookie{
+					Name: ch.Cookie.Name,
+				},
+			},
+		}
+		if ch.Cookie.TTL != nil {
+			hashPolicy.GetCookie().Ttl = durationpb.New(ch.Cookie.TTL.Duration)
+		}
+		if ch.Cookie.Attributes != nil {
+			attributes := make([]*routev3.RouteAction_HashPolicy_CookieAttribute, len(ch.Cookie.Attributes))
+			i := 0
+			for name, value := range ch.Cookie.Attributes {
+				attributes[i].Name = name
+				attributes[i].Value = value
+				i++
+			}
+			hashPolicy.GetCookie().Attributes = attributes
+		}
+		return []*routev3.RouteAction_HashPolicy{hashPolicy}
 	case ch.SourceIP != nil:
 		if !*ch.SourceIP {
 			return nil


Test patch:
diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.in.yaml
index f8bb9de05..9da4a34e6 100644
--- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.in.yaml
+++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.in.yaml
@@ -81,6 +81,25 @@ httpRoutes:
       backendRefs:
       - name: service-2
         port: 8080
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: HTTPRoute
+  metadata:
+    namespace: default
+    name: httproute-3
+  spec:
+    hostnames:
+    - gateway.envoyproxy.io
+    parentRefs:
+    - namespace: envoy-gateway
+      name: gateway-2
+      sectionName: http
+    rules:
+    - matches:
+      - path:
+          value: "/test3"
+      backendRefs:
+      - name: service-2
+        port: 8080
 backendTrafficPolicies:
 - apiVersion: gateway.envoyproxy.io/v1alpha1
   kind: BackendTrafficPolicy
@@ -137,3 +156,19 @@ backendTrafficPolicies:
       type: LeastRequest
       slowStart:
         window: 300s
+- apiVersion: gateway.envoyproxy.io/v1alpha1
+  kind: BackendTrafficPolicy
+  metadata:
+    namespace: default
+    name: policy-for-route3
+  spec:
+    targetRef:
+      group: gateway.networking.k8s.io
+      kind: HTTPRoute
+      name: httproute-3
+    loadBalancer:
+      type: ConsistentHash
+      consistentHash:
+        type: Cookie
+        cookie:
+          name: "test"
diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.out.yaml
index d1891d883..b2378edf7 100644
--- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.out.yaml
+++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.out.yaml
@@ -60,6 +60,38 @@ backendTrafficPolicies:
         status: "True"
         type: Accepted
       controllerName: gateway.envoyproxy.io/gatewayclass-controller
+- apiVersion: gateway.envoyproxy.io/v1alpha1
+  kind: BackendTrafficPolicy
+  metadata:
+    creationTimestamp: null
+    name: policy-for-route3
+    namespace: default
+  spec:
+    loadBalancer:
+      consistentHash:
+        cookie:
+          name: test
+        type: Cookie
+      type: ConsistentHash
+    targetRef:
+      group: gateway.networking.k8s.io
+      kind: HTTPRoute
+      name: httproute-3
+  status:
+    ancestors:
+    - ancestorRef:
+        group: gateway.networking.k8s.io
+        kind: Gateway
+        name: gateway-2
+        namespace: envoy-gateway
+        sectionName: http
+      conditions:
+      - lastTransitionTime: null
+        message: Policy has been accepted.
+        reason: Accepted
+        status: "True"
+        type: Accepted
+      controllerName: gateway.envoyproxy.io/gatewayclass-controller
 - apiVersion: gateway.envoyproxy.io/v1alpha1
   kind: BackendTrafficPolicy
   metadata:
@@ -117,7 +149,7 @@ backendTrafficPolicies:
         type: Accepted
       - lastTransitionTime: null
         message: 'This policy is being overridden by other backendTrafficPolicies
-          for these routes: [default/httproute-1 default/httproute-2]'
+          for these routes: [default/httproute-1 default/httproute-2 default/httproute-3]'
         reason: Overridden
         status: "True"
         type: Overridden
@@ -180,7 +212,7 @@ gateways:
       protocol: HTTP
   status:
     listeners:
-    - attachedRoutes: 2
+    - attachedRoutes: 3
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane
@@ -314,6 +346,44 @@ httpRoutes:
         name: gateway-2
         namespace: envoy-gateway
         sectionName: http
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: HTTPRoute
+  metadata:
+    creationTimestamp: null
+    name: httproute-3
+    namespace: default
+  spec:
+    hostnames:
+    - gateway.envoyproxy.io
+    parentRefs:
+    - name: gateway-2
+      namespace: envoy-gateway
+      sectionName: http
+    rules:
+    - backendRefs:
+      - name: service-2
+        port: 8080
+      matches:
+      - path:
+          value: /test3
+  status:
+    parents:
+    - conditions:
+      - lastTransitionTime: null
+        message: Route is accepted
+        reason: Accepted
+        status: "True"
+        type: Accepted
+      - lastTransitionTime: null
+        message: Resolved all the Object references for the Route
+        reason: ResolvedRefs
+        status: "True"
+        type: ResolvedRefs
+      controllerName: gateway.envoyproxy.io/gatewayclass-controller
+      parentRef:
+        name: gateway-2
+        namespace: envoy-gateway
+        sectionName: http
 infraIR:
   envoy-gateway/gateway-1:
     proxy:
@@ -430,6 +500,31 @@ xdsIR:
             leastRequest:
               slowStart:
                 window: 5m0s
+      - destination:
+          name: httproute/default/httproute-3/rule/0
+          settings:
+          - addressType: IP
+            endpoints:
+            - host: 7.7.7.7
+              port: 8080
+            protocol: HTTP
+            weight: 1
+        hostname: gateway.envoyproxy.io
+        isHTTP2: false
+        metadata:
+          kind: HTTPRoute
+          name: httproute-3
+          namespace: default
+        name: httproute/default/httproute-3/rule/0/match/0/gateway_envoyproxy_io
+        pathMatch:
+          distinct: false
+          name: ""
+          prefix: /test3
+        traffic:
+          loadBalancer:
+            consistentHash:
+              cookie:
+                name: test
       - destination:
           name: httproute/default/httproute-1/rule/0
           settings:
diff --git a/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml b/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml
index 3886f0cfd..998318fdb 100644
--- a/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml
+++ b/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml
@@ -104,3 +104,16 @@ http:
       - endpoints:
         - host: "1.2.3.4"
           port: 50000
+  - name: "ninth-route"
+    hostname: "*"
+    traffic:
+      loadBalancer:
+        consistentHash:
+          cookie:
+            name: "test"
+    destination:
+      name: "ninth-route-dest"
+      settings:
+      - endpoints:
+        - host: "1.2.3.4"
+          port: 50000
diff --git a/internal/xds/translator/testdata/out/xds-ir/load-balancer.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/load-balancer.clusters.yaml
index 204667fb0..cc88feed9 100644
--- a/internal/xds/translator/testdata/out/xds-ir/load-balancer.clusters.yaml
+++ b/internal/xds/translator/testdata/out/xds-ir/load-balancer.clusters.yaml
@@ -140,3 +140,20 @@
   outlierDetection: {}
   perConnectionBufferLimitBytes: 32768
   type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: ninth-route-dest
+  lbPolicy: MAGLEV
+  name: ninth-route-dest
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
diff --git a/internal/xds/translator/testdata/out/xds-ir/load-balancer.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/load-balancer.endpoints.yaml
index e262710c4..31d553105 100644
--- a/internal/xds/translator/testdata/out/xds-ir/load-balancer.endpoints.yaml
+++ b/internal/xds/translator/testdata/out/xds-ir/load-balancer.endpoints.yaml
@@ -94,3 +94,15 @@
     loadBalancingWeight: 1
     locality:
       region: eighth-route-dest/backend/0
+- clusterName: ninth-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 1.2.3.4
+            portValue: 50000
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: ninth-route-dest/backend/0
diff --git a/internal/xds/translator/testdata/out/xds-ir/load-balancer.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/load-balancer.routes.yaml
index bb34436d3..8fcdb470f 100644
--- a/internal/xds/translator/testdata/out/xds-ir/load-balancer.routes.yaml
+++ b/internal/xds/translator/testdata/out/xds-ir/load-balancer.routes.yaml
@@ -67,3 +67,13 @@
         cluster: eighth-route-dest
         upgradeConfigs:
         - upgradeType: websocket
+    - match:
+        prefix: /
+      name: ninth-route
+      route:
+        cluster: ninth-route-dest
+        hashPolicy:
+        - cookie:
+            name: test
+        upgradeConfigs:
+        - upgradeType: websocket
diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md
index e72e28dc5..fc824c824 100644
--- a/site/content/en/latest/api/extension_types.md
+++ b/site/content/en/latest/api/extension_types.md
@@ -757,6 +757,7 @@ _Appears in:_
 | ---   | ---  | ---      | ---         |
 | `type` | _[ConsistentHashType](#consistenthashtype)_ |  true  | ConsistentHashType defines the type of input to hash on. Valid Type values are "SourceIP" or "Header". |
 | `header` | _[Header](#header)_ |  false  | Header configures the header hash policy when the consistent hash type is set to Header. |
+| `cookie` | _[Cookie](#cookie)_ |  false  | Cookie configures the cookie hash policy when the consistent hash type is set to Cookie. |
 | `tableSize` | _integer_ |  false  | The table size for consistent hashing, must be prime number limited to 5000011. |
 
 
@@ -789,6 +790,8 @@ _Appears in:_
 | Field | Type | Required | Description |
 | ---   | ---  | ---      | ---         |
 | `name` | _string_ |  true  | Name of the cookie to hash.<br />If this cookie does not exist in the request, Envoy will generate a cookie and set<br />the TTL on the response back to the client based on Layer 4<br />attributes of the backend endpoint, to ensure that these future requests<br />go to the same backend endpoint. Make sure to set the TTL field for this case. |
+| `ttl` | _[Duration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#duration-v1-meta)_ |  false  | TTL of the generated cookie if the cookie is not present. This value sets the<br />Max-Age attribute value. |
+| `attributes` | _object (keys:string, values:string)_ |  false  | Additional Attributes to set for the generated cookie. |
 
 
 #### CustomHeaderExtensionSettings
diff --git a/site/content/zh/latest/api/extension_types.md b/site/content/zh/latest/api/extension_types.md
index e72e28dc5..fc824c824 100644
--- a/site/content/zh/latest/api/extension_types.md
+++ b/site/content/zh/latest/api/extension_types.md
@@ -757,6 +757,7 @@ _Appears in:_
 | ---   | ---  | ---      | ---         |
 | `type` | _[ConsistentHashType](#consistenthashtype)_ |  true  | ConsistentHashType defines the type of input to hash on. Valid Type values are "SourceIP" or "Header". |
 | `header` | _[Header](#header)_ |  false  | Header configures the header hash policy when the consistent hash type is set to Header. |
+| `cookie` | _[Cookie](#cookie)_ |  false  | Cookie configures the cookie hash policy when the consistent hash type is set to Cookie. |
 | `tableSize` | _integer_ |  false  | The table size for consistent hashing, must be prime number limited to 5000011. |
 
 
@@ -789,6 +790,8 @@ _Appears in:_
 | Field | Type | Required | Description |
 | ---   | ---  | ---      | ---         |
 | `name` | _string_ |  true  | Name of the cookie to hash.<br />If this cookie does not exist in the request, Envoy will generate a cookie and set<br />the TTL on the response back to the client based on Layer 4<br />attributes of the backend endpoint, to ensure that these future requests<br />go to the same backend endpoint. Make sure to set the TTL field for this case. |
+| `ttl` | _[Duration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#duration-v1-meta)_ |  false  | TTL of the generated cookie if the cookie is not present. This value sets the<br />Max-Age attribute value. |
+| `attributes` | _object (keys:string, values:string)_ |  false  | Additional Attributes to set for the generated cookie. |
 
 
 #### CustomHeaderExtensionSettings