feat: HeadersWithUnderscoreActions on ClientTrafficPolicy (envoyproxy…

…#3052) * feat: HeadersWithUnderscoreActions on ClientTrafficPolicy Signed-off-by: David Alger <[email protected]> * Add XDS translator test Signed-off-by: David Alger <[email protected]> * Rename using preferred func prefix Signed-off-by: David Alger <[email protected]> --------- Signed-off-by: David Alger <[email protected]>
ShyunnY · Apr 1, 2024 · 69ddc66 · 69ddc66
1 parent ac683da
commit 69ddc66
Show file tree

Hide file tree

Showing 15 changed files with 448 additions and 2 deletions.
diff --git a/api/v1alpha1/clienttrafficpolicy_types.go b/api/v1alpha1/clienttrafficpolicy_types.go
@@ -99,8 +99,30 @@ type HeaderSettings struct {
 	// and responses.
 	// +optional
 	EnableEnvoyHeaders *bool `json:"enableEnvoyHeaders,omitempty"`
+
+	// WithUnderscoresAction configures the action to take when an HTTP header with underscores
+	// is encountered. The default action is to reject the request.
+	// +optional
+	WithUnderscoresAction *WithUnderscoresAction `json:"withUnderscoresAction,omitempty"`
 }
 
+// WithUnderscoresAction configures the action to take when an HTTP header with underscores
+// is encountered.
+// +kubebuilder:validation:Enum=Allow;RejectRequest;DropHeader
+type WithUnderscoresAction string
+
+const (
+	// WithUnderscoresActionAllow allows headers with underscores to be passed through.
+	WithUnderscoresActionAllow WithUnderscoresAction = "Allow"
+	// WithUnderscoresActionRejectRequest rejects the client request. HTTP/1 requests are rejected with
+	// the 400 status. HTTP/2 requests end with the stream reset.
+	WithUnderscoresActionRejectRequest WithUnderscoresAction = "RejectRequest"
+	// WithUnderscoresActionDropHeader drops the client header with name containing underscores. The header
+	// is dropped before the filter chain is invoked and as such filters will not see
+	// dropped headers.
+	WithUnderscoresActionDropHeader WithUnderscoresAction = "DropHeader"
+)
+
 // ClientIPDetectionSettings provides configuration for determining the original client IP address for requests.
 //
 // +kubebuilder:validation:XValidation:rule="!(has(self.xForwardedFor) && has(self.customHeader))",message="customHeader cannot be used in conjunction with xForwardedFor"

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml
@@ -139,6 +139,15 @@ spec:
                     description: EnableEnvoyHeaders configures Envoy Proxy to add
                       the "X-Envoy-" headers to requests and responses.
                     type: boolean
+                  withUnderscoresAction:
+                    description: WithUnderscoresAction configures the action to take
+                      when an HTTP header with underscores is encountered. The default
+                      action is to reject the request.
+                    enum:
+                    - Allow
+                    - RejectRequest
+                    - DropHeader
+                    type: string
                 type: object
               http1:
                 description: HTTP1 provides HTTP/1 configuration on the listener.

diff --git a/internal/gatewayapi/clienttrafficpolicy.go b/internal/gatewayapi/clienttrafficpolicy.go
@@ -531,7 +531,8 @@ func translateListenerHeaderSettings(headerSettings *egv1a1.HeaderSettings, http
 		return
 	}
 	httpIR.Headers = &ir.HeaderSettings{
-		EnableEnvoyHeaders: ptr.Deref(headerSettings.EnableEnvoyHeaders, false),
+		EnableEnvoyHeaders:    ptr.Deref(headerSettings.EnableEnvoyHeaders, false),
+		WithUnderscoresAction: ir.WithUnderscoresAction(ptr.Deref(headerSettings.WithUnderscoresAction, egv1a1.WithUnderscoresActionRejectRequest)),
 	}
 }
 

diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-headers.in.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-headers.in.yaml
@@ -7,6 +7,7 @@ clientTrafficPolicies:
   spec:
     headers:
       enableEnvoyHeaders: true
+      withUnderscoresAction: Allow
     targetRef:
       group: gateway.networking.k8s.io
       kind: Gateway

diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-headers.out.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-headers.out.yaml
@@ -8,6 +8,7 @@ clientTrafficPolicies:
   spec:
     headers:
       enableEnvoyHeaders: true
+      withUnderscoresAction: Allow
     targetRef:
       group: gateway.networking.k8s.io
       kind: Gateway
@@ -129,6 +130,7 @@ xdsIR:
     - address: 0.0.0.0
       headers:
         enableEnvoyHeaders: true
+        withUnderscoresAction: Allow
       hostnames:
       - '*'
       isHTTP2: false
@@ -140,6 +142,7 @@ xdsIR:
     - address: 0.0.0.0
       headers:
         enableEnvoyHeaders: true
+        withUnderscoresAction: Allow
       hostnames:
       - '*'
       isHTTP2: false

diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -361,6 +361,14 @@ type PathSettings struct {
 	EscapedSlashesAction PathEscapedSlashAction `json:"escapedSlashesAction" yaml:"escapedSlashesAction"`
 }
 
+type WithUnderscoresAction egv1a1.WithUnderscoresAction
+
+const (
+	WithUnderscoresActionAllow         = WithUnderscoresAction(egv1a1.WithUnderscoresActionAllow)
+	WithUnderscoresActionRejectRequest = WithUnderscoresAction(egv1a1.WithUnderscoresActionRejectRequest)
+	WithUnderscoresActionDropHeader    = WithUnderscoresAction(egv1a1.WithUnderscoresActionDropHeader)
+)
+
 // ClientIPDetectionSettings provides configuration for determining the original client IP address for requests.
 // +k8s:deepcopy-gen=true
 type ClientIPDetectionSettings egv1a1.ClientIPDetectionSettings
@@ -394,6 +402,11 @@ type HeaderSettings struct {
 	// The default is to suppress these headers.
 	// Refer to https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/router/v3/router.proto#extensions-filters-http-router-v3-router
 	EnableEnvoyHeaders bool `json:"enableEnvoyHeaders,omitempty" yaml:"enableEnvoyHeaders,omitempty"`
+
+	// WithUnderscoresAction configures the action to take when an HTTP header with underscores
+	// is encountered. The default action is to reject the request.
+	// Refer to https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/protocol.proto#envoy-v3-api-enum-config-core-v3-httpprotocoloptions-headerswithunderscoresaction
+	WithUnderscoresAction WithUnderscoresAction `json:"withUnderscoresAction,omitempty" yaml:"withUnderscoresAction,omitempty"`
 }
 
 // ClientTimeout sets the timeout configuration for downstream connections

diff --git a/internal/xds/translator/listener.go b/internal/xds/translator/listener.go
@@ -236,7 +236,7 @@ func (t *Translator) addXdsHTTPFilterChain(xdsListener *listenerv3.Listener, irL
 		MergeSlashes:                 irListener.Path.MergeSlashes,
 		PathWithEscapedSlashesAction: translateEscapePath(irListener.Path.EscapedSlashesAction),
 		CommonHttpProtocolOptions: &corev3.HttpProtocolOptions{
-			HeadersWithUnderscoresAction: corev3.HttpProtocolOptions_REJECT_REQUEST,
+			HeadersWithUnderscoresAction: buildHeadersWithUnderscoresAction(irListener.Headers),
 		},
 		Tracing: hcmTracing,
 	}
@@ -737,3 +737,17 @@ func buildTCPProxyHashPolicy(lb *ir.LoadBalancer) []*typev3.HashPolicy {
 
 	return nil
 }
+
+func buildHeadersWithUnderscoresAction(in *ir.HeaderSettings) corev3.HttpProtocolOptions_HeadersWithUnderscoresAction {
+	if in != nil {
+		switch in.WithUnderscoresAction {
+		case ir.WithUnderscoresActionAllow:
+			return corev3.HttpProtocolOptions_ALLOW
+		case ir.WithUnderscoresActionRejectRequest:
+			return corev3.HttpProtocolOptions_REJECT_REQUEST
+		case ir.WithUnderscoresActionDropHeader:
+			return corev3.HttpProtocolOptions_DROP_HEADER
+		}
+	}
+	return corev3.HttpProtocolOptions_REJECT_REQUEST
+}
diff --git a/internal/xds/translator/testdata/in/xds-ir/headers-with-underscores-action.yaml b/internal/xds/translator/testdata/in/xds-ir/headers-with-underscores-action.yaml
@@ -0,0 +1,64 @@
+http:
+- name: "first-listener"
+  address: "0.0.0.0"
+  port: 8081
+  hostnames:
+  - "*"
+  routes:
+  - name: "first-route"
+    hostname: "*"
+    destination:
+      name: "first-route-dest"
+      settings:
+      - endpoints:
+        - host: "1.1.1.1"
+          port: 8081
+- name: "second-listener"
+  address: "0.0.0.0"
+  port: 8082
+  hostnames:
+  - "*"
+  routes:
+  - name: "second-route"
+    hostname: "*"
+    destination:
+      name: "second-route-dest"
+      settings:
+      - endpoints:
+        - host: "2.2.2.2"
+          port: 8082
+  headers:
+    withUnderscoresAction: Allow
+- name: "third-listener"
+  address: "0.0.0.0"
+  port: 8083
+  hostnames:
+  - "*"
+  routes:
+  - name: "third-route"
+    hostname: "*"
+    destination:
+      name: "third-route-dest"
+      settings:
+      - endpoints:
+        - host: "3.3.3.3"
+          port: 8083
+  headers:
+    withUnderscoresAction: RejectRequest
+- name: "fourth-listener"
+  address: "0.0.0.0"
+  port: 8084
+  hostnames:
+  - "*"
+  routes:
+  - name: "fourth-route"
+    hostname: "*"
+    destination:
+      name: "fourth-route-dest"
+      settings:
+      - endpoints:
+        - host: "4.4.4.4"
+          port: 8084
+  headers:
+    withUnderscoresAction: DropHeader
+
diff --git a/internal/xds/translator/testdata/out/xds-ir/headers-with-underscores-action.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/headers-with-underscores-action.clusters.yaml
@@ -0,0 +1,68 @@
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: first-route-dest
+  lbPolicy: LEAST_REQUEST
+  name: first-route-dest
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: second-route-dest
+  lbPolicy: LEAST_REQUEST
+  name: second-route-dest
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: third-route-dest
+  lbPolicy: LEAST_REQUEST
+  name: third-route-dest
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: fourth-route-dest
+  lbPolicy: LEAST_REQUEST
+  name: fourth-route-dest
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
diff --git a/internal/xds/translator/testdata/out/xds-ir/headers-with-underscores-action.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/headers-with-underscores-action.endpoints.yaml
@@ -0,0 +1,48 @@
+- clusterName: first-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 1.1.1.1
+            portValue: 8081
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: first-route-dest/backend/0
+- clusterName: second-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 2.2.2.2
+            portValue: 8082
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: second-route-dest/backend/0
+- clusterName: third-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 3.3.3.3
+            portValue: 8083
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: third-route-dest/backend/0
+- clusterName: fourth-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 4.4.4.4
+            portValue: 8084
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: fourth-route-dest/backend/0