fix: add missing http filters to the http filter chain

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

Hide file tree

Showing 9 changed files with 374 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,
@@ -410,23 +407,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,81 @@
+# 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: "first-listener"
+  address: "0.0.0.0"
+  port: 10080
+  hostnames:
+  - "www.foo.com"
+  routes:
+  - name: "first-route"
+    hostname: "www.foo.com"
+    pathMatch:
+      prefix: "/foo1"
+    destination:
+      name: "first-route-dest"
+      settings:
+      - endpoints:
+        - host: "192.168.1.1"
+          port: 50000
+    basicAuth:
+      users: "dXNlcjE6e1NIQX10RVNzQm1FL3lOWTNsYjZhMEw2dlZRRVpOcXc9CnVzZXIyOntTSEF9RUo5TFBGRFhzTjl5blNtYnh2anA3NUJtbHg4PQo="
+  - name: "second-route"
+    hostname: "www.foo.com"
+    pathMatch:
+      prefix: "/foo2"
+    destination:
+      name: "second-route-dest"
+      settings:
+      - endpoints:
+        - host: "192.168.1.2"
+          port: 50000
+    cors:
+      allowOrigins:
+      - name: example.com
+        stringMatch:
+        safeRegex: "*.example.com"
+      - name: foo.bar.com
+        stringMatch:
+        exact: foo.bar.com
+      allowMethods:
+      - GET
+      - POST
+      allowHeaders:
+      - "x-header-1"
+      - "x-header-2"
+      exposeHeaders:
+      - "x-header-3"
+      - "x-header-4"
+      allowCredentials: true
+      maxAge: 1000s
+- name: "second-listener"
+  address: "0.0.0.0"
+  port: 10080
+  hostnames:
+  - "www.bar.com"
+  routes:
+  - name: "first-route"
+    hostname: "www.bar.com"
+    pathMatch:
+      prefix: "/bar"
+    destination:
+      name: "first-route-dest"
+      settings:
+      - endpoints:
+        - host: "192.168.1.3"
+          port: 50000
+    oidc:
+      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,70 @@
+- 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
+  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,24 @@
+- clusterName: first-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 192.168.1.1
+            portValue: 50000
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: first-route-dest/backend/0
+- clusterName: second-route-dest
+  endpoints:
+  - lbEndpoints:
+    - endpoint:
+        address:
+          socketAddress:
+            address: 192.168.1.2
+            portValue: 50000
+      loadBalancingWeight: 1
+    loadBalancingWeight: 1
+    locality:
+      region: second-route-dest/backend/0
diff --git a/...or/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml b/...or/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml
@@ -0,0 +1,82 @@
+- address:
+    socketAddress:
+      address: 0.0.0.0
+      portValue: 10080
+  defaultFilterChain:
+    filters:
+    - name: envoy.filters.network.http_connection_manager
+      typedConfig:
+        '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+        commonHttpProtocolOptions:
+          headersWithUnderscoresAction: REJECT_REQUEST
+        http2ProtocolOptions:
+          initialConnectionWindowSize: 1048576
+          initialStreamWindowSize: 65536
+          maxConcurrentStreams: 100
+        httpFilters:
+        - name: envoy.filters.http.cors
+          typedConfig:
+            '@type': type.googleapis.com/envoy.extensions.filters.http.cors.v3.Cors
+        - disabled: true
+          name: envoy.filters.http.basic_auth/first-route
+          typedConfig:
+            '@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuth
+            users:
+              inlineBytes: dXNlcjE6e1NIQX10RVNzQm1FL3lOWTNsYjZhMEw2dlZRRVpOcXc9CnVzZXIyOntTSEF9RUo5TFBGRFhzTjl5blNtYnh2anA3NUJtbHg4PQo=
+        - disabled: true
+          name: envoy.filters.http.oauth2/first-route
+          typedConfig:
+            '@type': type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2
+            config:
+              authScopes:
+              - openid
+              - email
+              - profile
+              authType: BASIC_AUTH
+              authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
+              credentials:
+                clientId: client.oauth.foo.com
+                cookieNames:
+                  bearerToken: BearerToken-5F93C2E4
+                  idToken: IdToken-5F93C2E4
+                  oauthExpires: OauthExpires-5F93C2E4
+                  oauthHmac: OauthHMAC-5F93C2E4
+                  refreshToken: RefreshToken-5F93C2E4
+                hmacSecret:
+                  name: first-route/oauth2/hmac_secret
+                  sdsConfig:
+                    ads: {}
+                    resourceApiVersion: V3
+                tokenSecret:
+                  name: first-route/oauth2/client_secret
+                  sdsConfig:
+                    ads: {}
+                    resourceApiVersion: V3
+              forwardBearerToken: true
+              redirectPathMatcher:
+                path:
+                  exact: /foo/oauth2/callback
+              redirectUri: https://www.example.com/foo/oauth2/callback
+              signoutPath:
+                path:
+                  exact: /foo/logout
+              tokenEndpoint:
+                cluster: oauth_foo_com_443
+                timeout: 10s
+                uri: https://oauth.foo.com/token
+        - name: envoy.filters.http.router
+          typedConfig:
+            '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
+            suppressEnvoyHeaders: true
+        normalizePath: true
+        rds:
+          configSource:
+            ads: {}
+            resourceApiVersion: V3
+          routeConfigName: first-listener
+        serverHeaderTransformation: PASS_THROUGH
+        statPrefix: http
+        useRemoteAddress: true
+  drainType: MODIFY_ONLY
+  name: first-listener
+  perConnectionBufferLimitBytes: 32768
diff --git a/...lator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.routes.yaml b/...lator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.routes.yaml
@@ -0,0 +1,52 @@
+- ignorePortInHostMatching: true
+  name: first-listener
+  virtualHosts:
+  - domains:
+    - www.foo.com
+    name: first-listener/www_foo_com
+    routes:
+    - match:
+        pathSeparatedPrefix: /foo1
+      name: first-route
+      route:
+        cluster: first-route-dest
+        upgradeConfigs:
+        - upgradeType: websocket
+      typedPerFilterConfig:
+        envoy.filters.http.basic_auth/first-route:
+          '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig
+          config: {}
+    - match:
+        pathSeparatedPrefix: /foo2
+      name: second-route
+      route:
+        cluster: second-route-dest
+        upgradeConfigs:
+        - upgradeType: websocket
+      typedPerFilterConfig:
+        envoy.filters.http.cors:
+          '@type': type.googleapis.com/envoy.extensions.filters.http.cors.v3.CorsPolicy
+          allowCredentials: true
+          allowHeaders: x-header-1, x-header-2
+          allowMethods: GET, POST
+          allowOriginStringMatch:
+          - safeRegex:
+              regex: '*.example.com'
+          - exact: foo.bar.com
+          exposeHeaders: x-header-3, x-header-4
+          maxAge: "1000"
+  - domains:
+    - www.bar.com
+    name: second-listener/www_bar_com
+    routes:
+    - match:
+        pathSeparatedPrefix: /bar
+      name: first-route
+      route:
+        cluster: first-route-dest
+        upgradeConfigs:
+        - upgradeType: websocket
+      typedPerFilterConfig:
+        envoy.filters.http.oauth2/first-route:
+          '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig
+          config: {}