fix: add missing http filters to the http filter chain

Signed-off-by: huabing zhao <[email protected]>
envoyproxy · Mar 19, 2024 · a3e863d · a3e863d
1 parent 6a57cd1
commit a3e863d
Show file tree

Hide file tree

Showing 9 changed files with 475 additions and 24 deletions.
diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go
@@ -134,10 +134,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security
 				continue
 			}
 
-			err := validatePortOverlapForSecurityPolicyRoute(xdsIR, targetedRoute)
-			if err == nil {
-				err = t.translateSecurityPolicyForRoute(policy, targetedRoute, resources, xdsIR)
-			}
+			err := t.translateSecurityPolicyForRoute(policy, targetedRoute, resources, xdsIR)
 
 			if err != nil {
 				status.SetTranslationErrorForPolicyAncestors(&policy.Status,
@@ -414,23 +411,6 @@ func (t *Translator) translateSecurityPolicyForRoute(
 	return errs
 }
 
-func validatePortOverlapForSecurityPolicyRoute(xds XdsIRMap, route RouteContext) error {
-	var errs error
-	prefix := irRoutePrefix(route)
-	for _, ir := range xds {
-		for _, http := range ir.HTTP {
-			for _, r := range http.Routes {
-				if strings.HasPrefix(r.Name, prefix) {
-					if sameListeners := listenersWithSameHTTPPort(ir, http); len(sameListeners) != 0 {
-						errs = errors.Join(errs, fmt.Errorf("affects multiple listeners: %s", strings.Join(sameListeners, ", ")))
-					}
-				}
-			}
-		}
-	}
-	return errs
-}
-
 func (t *Translator) translateSecurityPolicyForGateway(
 	policy *egv1a1.SecurityPolicy, gateway *GatewayContext,
 	resources *Resources, xdsIR XdsIRMap) error {

diff --git a/internal/xds/translator/httpfilters.go b/internal/xds/translator/httpfilters.go
@@ -165,9 +165,18 @@ func (t *Translator) patchHCMWithFilters(
 	// rate limit server configuration.
 	t.patchHCMWithRateLimit(mgr, irListener)
 
-	// Add the router filter
-	headerSettings := ptr.Deref(irListener.Headers, ir.HeaderSettings{})
-	mgr.HttpFilters = append(mgr.HttpFilters, filters.GenerateRouterFilter(headerSettings.EnableEnvoyHeaders))
+	// Add the router filter if it doesn't exist.
+	hasRouter := false
+	for _, filter := range mgr.HttpFilters {
+		if filter.Name == wellknown.Router {
+			hasRouter = true
+			break
+		}
+	}
+	if !hasRouter {
+		headerSettings := ptr.Deref(irListener.Headers, ir.HeaderSettings{})
+		mgr.HttpFilters = append(mgr.HttpFilters, filters.GenerateRouterFilter(headerSettings.EnableEnvoyHeaders))
+	}
 
 	// Sort the filters in the correct order.
 	mgr.HttpFilters = sortHTTPFilters(mgr.HttpFilters)

diff --git a/...ds/translator/testdata/in/xds-ir/multiple-listeners-same-port-with-different-filters.yaml b/...ds/translator/testdata/in/xds-ir/multiple-listeners-same-port-with-different-filters.yaml
@@ -0,0 +1,117 @@
+# This is a test file for multiple Gateway HTTP listeners on the same port with different filters.
+# These HTTP listeners should be merged into a single HTTP connection manager,
+# and the filters should be merged into the DefaultFilterChain of the HTTP connection manager.
+http:
+  - name: default/gateway-1/http
+    address: 0.0.0.0
+    hostnames:
+      - 'www.foo.com'
+    isHTTP2: false
+    path:
+      escapedSlashesAction: UnescapeAndRedirect
+      mergeSlashes: true
+    port: 10080
+    routes:
+      - name: httproute/default/httproute-1/rule/0/match/0/www_foo_com
+        hostname: www.foo.com
+        isHTTP2: false
+        pathMatch:
+          distinct: false
+          name: ""
+          prefix: /foo1
+        backendWeights:
+          invalid: 0
+          valid: 0
+        destination:
+          name: httproute/default/httproute-1/rule/0
+          settings:
+            - addressType: IP
+              endpoints:
+                - host: 192.168.1.1
+                  port: 8080
+              protocol: HTTP
+              weight: 1
+        basicAuth:
+          name: securitypolicy/default/policy-for-http-route-1
+          users: "dXNlcjE6e1NIQX10RVNzQm1FL3lOWTNsYjZhMEw2dlZRRVpOcXc9CnVzZXIyOntTSEF9RUo5TFBGRFhzTjl5blNtYnh2anA3NUJtbHg4PQo="
+      - name: httproute/default/httproute-2/rule/0/match/0/www_foo_com
+        hostname: www.foo.com
+        isHTTP2: false
+        pathMatch:
+          distinct: false
+          name: ""
+          prefix: /foo2
+        backendWeights:
+          invalid: 0
+          valid: 0
+        destination:
+          name: httproute/default/httproute-2/rule/0
+          settings:
+            - addressType: IP
+              endpoints:
+                - host: 192.168.1.2
+                  port: 8080
+              protocol: HTTP
+              weight: 1
+        extAuth:
+          name: securitypolicy/default/policy-for-http-route-2
+          failOpen: true
+          http:
+            authority: http-backend.envoy-gateway:80
+            destination:
+              name: securitypolicy/default/policy-for-http-route-2/envoy-gateway/http-backend
+              settings:
+                - addressType: IP
+                  endpoints:
+                    - host: 7.7.7.7
+                      port: 80
+                  protocol: HTTP
+                  weight: 1
+            headersToBackend:
+              - header1
+              - header2
+            path: /auth
+  - name: default/gateway-2/http
+    address: 0.0.0.0
+    hostnames:
+      - 'www.bar.com'
+    isHTTP2: false
+    path:
+      escapedSlashesAction: UnescapeAndRedirect
+      mergeSlashes: true
+    port: 10080
+    routes:
+      - name: httproute/default/httproute-3/rule/0/match/0/www_bar_com
+        hostname: www.bar.com
+        isHTTP2: false
+        pathMatch:
+          distinct: false
+          name: ""
+          prefix: /bar
+        backendWeights:
+          invalid: 0
+          valid: 0
+        destination:
+          name: httproute/default/httproute-3/rule/0
+          settings:
+            - addressType: IP
+              endpoints:
+                - host: 192.168.1.3
+                  port: 8080
+              protocol: HTTP
+              weight: 1
+        oidc:
+          name: securitypolicy/default/policy-for-gateway-2
+          clientID: client.oauth.foo.com
+          clientSecret: Y2xpZW50MTpzZWNyZXQK
+          provider:
+            authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
+            tokenEndpoint: https://oauth.foo.com/token
+          scopes:
+            - openid
+            - email
+            - profile
+          redirectURL: "https://www.example.com/foo/oauth2/callback"
+          redirectPath: "/foo/oauth2/callback"
+          logoutPath: "/foo/logout"
+          cookieSuffix: 5F93C2E4
diff --git a/...tor/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.clusters.yaml b/...tor/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.clusters.yaml
@@ -0,0 +1,104 @@
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: httproute/default/httproute-1/rule/0
+  lbPolicy: LEAST_REQUEST
+  name: httproute/default/httproute-1/rule/0
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: httproute/default/httproute-2/rule/0
+  lbPolicy: LEAST_REQUEST
+  name: httproute/default/httproute-2/rule/0
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: securitypolicy/default/policy-for-http-route-2/envoy-gateway/http-backend
+  lbPolicy: LEAST_REQUEST
+  name: securitypolicy/default/policy-for-http-route-2/envoy-gateway/http-backend
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: httproute/default/httproute-3/rule/0
+  lbPolicy: LEAST_REQUEST
+  name: httproute/default/httproute-3/rule/0
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  type: EDS
+- circuitBreakers:
+    thresholds:
+    - maxRetries: 1024
+  commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  dnsRefreshRate: 30s
+  lbPolicy: LEAST_REQUEST
+  loadAssignment:
+    clusterName: oauth_foo_com_443
+    endpoints:
+    - lbEndpoints:
+      - endpoint:
+          address:
+            socketAddress:
+              address: oauth.foo.com
+              portValue: 443
+        loadBalancingWeight: 1
+      loadBalancingWeight: 1
+      locality:
+        region: oauth_foo_com_443/backend/0
+  name: oauth_foo_com_443
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  respectDnsTtl: true
+  transportSocket:
+    name: envoy.transport_sockets.tls
+    typedConfig:
+      '@type': type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext
+      commonTlsContext:
+        validationContext:
+          trustedCa:
+            filename: /etc/ssl/certs/ca-certificates.crt
+      sni: oauth.foo.com
+  type: STRICT_DNS
diff --git a/...or/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.endpoints.yaml b/...or/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.endpoints.yaml
@@ -0,0 +1,48 @@
+- clusterName: httproute/default/httproute-1/rule/0
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 192.168.1.1
+            portValue: 8080
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: httproute/default/httproute-1/rule/0/backend/0
+- clusterName: httproute/default/httproute-2/rule/0
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 192.168.1.2
+            portValue: 8080
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: httproute/default/httproute-2/rule/0/backend/0
+- clusterName: securitypolicy/default/policy-for-http-route-2/envoy-gateway/http-backend
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 7.7.7.7
+            portValue: 80
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: securitypolicy/default/policy-for-http-route-2/envoy-gateway/http-backend/backend/0
+- clusterName: httproute/default/httproute-3/rule/0
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 192.168.1.3
+            portValue: 8080
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: httproute/default/httproute-3/rule/0/backend/0