feat(translator): Implement header hash policy for consistent hash lo…

…ad balancers (#3357) * Map consistent hash configuration Signed-off-by: Harry Turton <harryt@canva.com> * Implement mapping and add tests Signed-off-by: Harry Turton <harryt@canva.com> * Small refactors, add yaml keys Signed-off-by: Harry Turton <harryt@canva.com> * Add documentation and lints Signed-off-by: Harry Turton <harryt@canva.com> * Add additional tests Signed-off-by: Harry Turton <harryt@canva.com> --------- Signed-off-by: Harry Turton <harryt@canva.com>
envoyproxy · May 9, 2024 · 7863f81 · 7863f81
1 parent bb348dc
commit 7863f81
Show file tree

Hide file tree

Showing 14 changed files with 199 additions and 16 deletions.
diff --git a/api/v1alpha1/loadbalancer_types.go b/api/v1alpha1/loadbalancer_types.go
@@ -57,15 +57,14 @@ const (
 //
 // +kubebuilder:validation:XValidation:rule="self.type == 'Header' ? has(self.header) : !has(self.header)",message="If consistent hash type is header, the header field must be set."
 type ConsistentHash struct {
-	// Valid Type values are  "SourceIP".
+	// ConsistentHashType defines the type of input to hash on. Valid Type values are "SourceIP" or "Header".
 	//
 	// +unionDiscriminator
 	Type ConsistentHashType `json:"type"`
 
 	// Header configures the header hash policy when the consistent hash type is set to Header.
 	//
 	// +optional
-	// +notImplementedHide
 	Header *Header `json:"header,omitempty"`
 }
 

diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml
@@ -423,7 +423,8 @@ spec:
                         - name
                         type: object
                       type:
-                        description: Valid Type values are  "SourceIP".
+                        description: ConsistentHashType defines the type of input
+                          to hash on. Valid Type values are "SourceIP" or "Header".
                         enum:
                         - SourceIP
                         - Header

diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go
@@ -764,11 +764,7 @@ func (t *Translator) buildLoadBalancer(policy *egv1a1.BackendTrafficPolicy) *ir.
 	switch policy.Spec.LoadBalancer.Type {
 	case egv1a1.ConsistentHashLoadBalancerType:
 		lb = &ir.LoadBalancer{
-			ConsistentHash: &ir.ConsistentHash{},
-		}
-		if policy.Spec.LoadBalancer.ConsistentHash != nil &&
-			policy.Spec.LoadBalancer.ConsistentHash.Type == egv1a1.SourceIPConsistentHashType {
-			lb.ConsistentHash.SourceIP = ptr.To(true)
+			ConsistentHash: t.buildConsistentHashLoadBalancer(policy),
 		}
 	case egv1a1.LeastRequestLoadBalancerType:
 		lb = &ir.LoadBalancer{}
@@ -805,6 +801,23 @@ func (t *Translator) buildLoadBalancer(policy *egv1a1.BackendTrafficPolicy) *ir.
 	return lb
 }
 
+func (t *Translator) buildConsistentHashLoadBalancer(policy *egv1a1.BackendTrafficPolicy) *ir.ConsistentHash {
+	switch policy.Spec.LoadBalancer.ConsistentHash.Type {
+	case egv1a1.SourceIPConsistentHashType:
+		return &ir.ConsistentHash{
+			SourceIP: ptr.To(true),
+		}
+	case egv1a1.HeaderConsistentHashType:
+		return &ir.ConsistentHash{
+			Header: &ir.Header{
+				Name: policy.Spec.LoadBalancer.ConsistentHash.Header.Name,
+			},
+		}
+	default:
+		return &ir.ConsistentHash{}
+	}
+}
+
 func (t *Translator) buildProxyProtocol(policy *egv1a1.BackendTrafficPolicy) *ir.ProxyProtocol {
 	var pp *ir.ProxyProtocol
 	switch policy.Spec.ProxyProtocol.Version {

diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -1575,7 +1575,13 @@ 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"`
+	SourceIP *bool   `json:"sourceIP,omitempty" yaml:"sourceIP,omitempty"`
+	Header   *Header `json:"header,omitempty" yaml:"header,omitempty"`
+}
+
+// Header consistent hash type settings
+type Header struct {
+	Name string `json:"name" yaml:"name"`
 }
 
 type ProxyProtocolVersion string

diff --git a/internal/ir/xds_test.go b/internal/ir/xds_test.go
@@ -1243,15 +1243,25 @@ func TestValidateLoadBalancer(t *testing.T) {
 			want: nil,
 		},
 		{
-			name: "consistent hash",
+			name: "consistent hash with source IP hash policy",
 			input: LoadBalancer{
 				ConsistentHash: &ConsistentHash{
 					SourceIP: ptr.To(true),
 				},
 			},
 			want: nil,
 		},
-
+		{
+			name: "consistent hash with header hash policy",
+			input: LoadBalancer{
+				ConsistentHash: &ConsistentHash{
+					Header: &Header{
+						Name: "name",
+					},
+				},
+			},
+			want: nil,
+		},
 		{
 			name: "least request and random set",
 			input: LoadBalancer{

diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go
diff --git a/internal/xds/translator/listener_test.go b/internal/xds/translator/listener_test.go
@@ -64,7 +64,7 @@ func Test_buildTCPProxyHashPolicy(t *testing.T) {
 			want: nil,
 		},
 		{
-			name: "ConsistentHash without SourceIP",
+			name: "ConsistentHash without hash policy",
 			lb:   &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{}},
 			want: nil,
 		},

diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go
@@ -440,7 +440,20 @@ func buildHashPolicy(httpRoute *ir.HTTPRoute) []*routev3.RouteAction_HashPolicy
 		return nil
 	}
 
-	if httpRoute.LoadBalancer.ConsistentHash.SourceIP != nil && *httpRoute.LoadBalancer.ConsistentHash.SourceIP {
+	switch {
+	case httpRoute.LoadBalancer.ConsistentHash.Header != nil:
+		hashPolicy := &routev3.RouteAction_HashPolicy{
+			PolicySpecifier: &routev3.RouteAction_HashPolicy_Header_{
+				Header: &routev3.RouteAction_HashPolicy_Header{
+					HeaderName: httpRoute.LoadBalancer.ConsistentHash.Header.Name,
+				},
+			},
+		}
+		return []*routev3.RouteAction_HashPolicy{hashPolicy}
+	case httpRoute.LoadBalancer.ConsistentHash.SourceIP != nil:
+		if !*httpRoute.LoadBalancer.ConsistentHash.SourceIP {
+			return nil
+		}
 		hashPolicy := &routev3.RouteAction_HashPolicy{
 			PolicySpecifier: &routev3.RouteAction_HashPolicy_ConnectionProperties_{
 				ConnectionProperties: &routev3.RouteAction_HashPolicy_ConnectionProperties{
@@ -449,9 +462,9 @@ func buildHashPolicy(httpRoute *ir.HTTPRoute) []*routev3.RouteAction_HashPolicy
 			},
 		}
 		return []*routev3.RouteAction_HashPolicy{hashPolicy}
+	default:
+		return nil
 	}
-
-	return nil
 }
 
 func buildRetryPolicy(route *ir.HTTPRoute) (*routev3.RetryPolicy, error) {

diff --git a/internal/xds/translator/route_test.go b/internal/xds/translator/route_test.go
@@ -0,0 +1,84 @@
+// Copyright Envoy Gateway Authors
+// SPDX-License-Identifier: Apache-2.0
+// The full text of the Apache license is available in the LICENSE file at
+// the root of the repo.
+
+package translator
+
+import (
+	"reflect"
+	"testing"
+
+	routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
+	"k8s.io/utils/ptr"
+
+	"github.com/envoyproxy/gateway/internal/ir"
+)
+
+func Test_buildHashPolicy(t *testing.T) {
+	tests := []struct {
+		name      string
+		httpRoute *ir.HTTPRoute
+		want      []*routev3.RouteAction_HashPolicy
+	}{
+		{
+			name:      "Nil HttpRoute",
+			httpRoute: nil,
+			want:      nil,
+		},
+		{
+			name:      "Nil LoadBalancer in HttpRoute",
+			httpRoute: &ir.HTTPRoute{},
+			want:      nil,
+		},
+		{
+			name:      "Nil ConsistentHash in LoadBalancer",
+			httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{}},
+			want:      nil,
+		},
+		{
+			name:      "ConsistentHash with nil SourceIP and Header",
+			httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{}}},
+			want:      nil,
+		},
+		{
+			name:      "ConsistentHash with SourceIP set to false",
+			httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{SourceIP: ptr.To(false)}}},
+			want:      nil,
+		},
+		{
+			name:      "ConsistentHash with SourceIP set to true",
+			httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{SourceIP: ptr.To(true)}}},
+			want: []*routev3.RouteAction_HashPolicy{
+				{
+					PolicySpecifier: &routev3.RouteAction_HashPolicy_ConnectionProperties_{
+						ConnectionProperties: &routev3.RouteAction_HashPolicy_ConnectionProperties{
+							SourceIp: true,
+						},
+					},
+				},
+			},
+		},
+		{
+			name:      "ConsistentHash with Header",
+			httpRoute: &ir.HTTPRoute{LoadBalancer: &ir.LoadBalancer{ConsistentHash: &ir.ConsistentHash{Header: &ir.Header{Name: "name"}}}},
+			want: []*routev3.RouteAction_HashPolicy{
+				{
+					PolicySpecifier: &routev3.RouteAction_HashPolicy_Header_{
+						Header: &routev3.RouteAction_HashPolicy_Header{
+							HeaderName: "name",
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := buildHashPolicy(tt.httpRoute)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("buildHashPolicy() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml b/internal/xds/translator/testdata/in/xds-ir/load-balancer.yaml
@@ -73,3 +73,15 @@ http:
       - endpoints:
         - host: "1.2.3.4"
           port: 50000
+  - name: "seventh-route"
+    hostname: "*"
+    loadBalancer:
+      consistentHash:
+        header:
+          name: name
+    destination:
+      name: "seventh-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
@@ -104,3 +104,20 @@
     slowStartConfig:
       slowStartWindow: 300s
   type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: seventh-route-dest
+  lbPolicy: MAGLEV
+  name: seventh-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
@@ -70,3 +70,15 @@
     loadBalancingWeight: 1
     locality:
       region: sixth-route-dest/backend/0
+- clusterName: seventh-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 1.2.3.4
+            portValue: 50000
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: seventh-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
@@ -50,3 +50,13 @@
         cluster: sixth-route-dest
         upgradeConfigs:
         - upgradeType: websocket
+    - match:
+        prefix: /
+      name: seventh-route
+      route:
+        cluster: seventh-route-dest
+        hashPolicy:
+        - header:
+            headerName: name
+        upgradeConfigs:
+        - upgradeType: websocket
diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md
@@ -562,7 +562,8 @@ _Appears in:_
 
 | Field | Type | Required | Description |
 | ---   | ---  | ---      | ---         |
-| `type` | _[ConsistentHashType](#consistenthashtype)_ |  true  | Valid Type values are  "SourceIP". |
+| `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. |
 
 
 #### ConsistentHashType