From a3e863d1edba0f674dd634a9145b97322049a649 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Mon, 18 Mar 2024 17:22:29 +0800 Subject: [PATCH] fix: add missing http filters to the http filter chain Signed-off-by: huabing zhao --- internal/gatewayapi/securitypolicy.go | 22 +--- internal/xds/translator/httpfilters.go | 15 ++- ...ners-same-port-with-different-filters.yaml | 117 ++++++++++++++++++ ...-port-with-different-filters.clusters.yaml | 104 ++++++++++++++++ ...port-with-different-filters.endpoints.yaml | 48 +++++++ ...port-with-different-filters.listeners.yaml | 97 +++++++++++++++ ...me-port-with-different-filters.routes.yaml | 44 +++++++ internal/xds/translator/translator.go | 49 ++++++++ internal/xds/translator/translator_test.go | 3 + 9 files changed, 475 insertions(+), 24 deletions(-) create mode 100644 internal/xds/translator/testdata/in/xds-ir/multiple-listeners-same-port-with-different-filters.yaml create mode 100755 internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.clusters.yaml create mode 100755 internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.endpoints.yaml create mode 100755 internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml create mode 100755 internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.routes.yaml diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 434d9ce839a..0e1b9a4807f 100644 --- 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 index 738e5ee9480..ce107741cf1 100644 --- 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/internal/xds/translator/testdata/in/xds-ir/multiple-listeners-same-port-with-different-filters.yaml b/internal/xds/translator/testdata/in/xds-ir/multiple-listeners-same-port-with-different-filters.yaml new file mode 100644 index 00000000000..d0a777bb641 --- /dev/null +++ b/internal/xds/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/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.clusters.yaml new file mode 100755 index 00000000000..2b9b567cf39 --- /dev/null +++ b/internal/xds/translator/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/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.endpoints.yaml new file mode 100755 index 00000000000..1f4fcb70daf --- /dev/null +++ b/internal/xds/translator/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 diff --git a/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml new file mode 100755 index 00000000000..3b95e6ba272 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml @@ -0,0 +1,97 @@ +- 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: + - disabled: true + name: envoy.filters.http.ext_authz/securitypolicy/default/policy-for-http-route-2 + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz + failureModeAllow: true + httpService: + authorizationResponse: + allowedUpstreamHeaders: + patterns: + - exact: header1 + - exact: header2 + serverUri: + cluster: securitypolicy/default/policy-for-http-route-2/envoy-gateway/http-backend + timeout: 10s + uri: http://http-backend.envoy-gateway:80/auth + transportApiVersion: V3 + - disabled: true + name: envoy.filters.http.basic_auth/securitypolicy/default/policy-for-http-route-1 + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuth + users: + inlineBytes: dXNlcjE6e1NIQX10RVNzQm1FL3lOWTNsYjZhMEw2dlZRRVpOcXc9CnVzZXIyOntTSEF9RUo5TFBGRFhzTjl5blNtYnh2anA3NUJtbHg4PQo= + - disabled: true + name: envoy.filters.http.oauth2/securitypolicy/default/policy-for-gateway-2 + 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: oauth2/hmac_secret/securitypolicy/default/policy-for-gateway-2 + sdsConfig: + ads: {} + resourceApiVersion: V3 + tokenSecret: + name: oauth2/client_secret/securitypolicy/default/policy-for-gateway-2 + 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 + mergeSlashes: true + normalizePath: true + pathWithEscapedSlashesAction: UNESCAPE_AND_REDIRECT + rds: + configSource: + ads: {} + resourceApiVersion: V3 + routeConfigName: default/gateway-1/http + serverHeaderTransformation: PASS_THROUGH + statPrefix: http + useRemoteAddress: true + drainType: MODIFY_ONLY + name: default/gateway-1/http + perConnectionBufferLimitBytes: 32768 diff --git a/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.routes.yaml new file mode 100755 index 00000000000..2e60d54d862 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.routes.yaml @@ -0,0 +1,44 @@ +- ignorePortInHostMatching: true + name: default/gateway-1/http + virtualHosts: + - domains: + - www.foo.com + name: default/gateway-1/http/www_foo_com + routes: + - match: + pathSeparatedPrefix: /foo1 + name: httproute/default/httproute-1/rule/0/match/0/www_foo_com + route: + cluster: httproute/default/httproute-1/rule/0 + upgradeConfigs: + - upgradeType: websocket + typedPerFilterConfig: + envoy.filters.http.basic_auth/securitypolicy/default/policy-for-http-route-1: + '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig + config: {} + - match: + pathSeparatedPrefix: /foo2 + name: httproute/default/httproute-2/rule/0/match/0/www_foo_com + route: + cluster: httproute/default/httproute-2/rule/0 + upgradeConfigs: + - upgradeType: websocket + typedPerFilterConfig: + envoy.filters.http.ext_authz/securitypolicy/default/policy-for-http-route-2: + '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig + config: {} + - domains: + - www.bar.com + name: default/gateway-2/http/www_bar_com + routes: + - match: + pathSeparatedPrefix: /bar + name: httproute/default/httproute-3/rule/0/match/0/www_bar_com + route: + cluster: httproute/default/httproute-3/rule/0 + upgradeConfigs: + - upgradeType: websocket + typedPerFilterConfig: + envoy.filters.http.oauth2/securitypolicy/default/policy-for-gateway-2: + '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig + config: {} diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go index bd35d41891a..be72efb44ec 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -16,15 +16,18 @@ import ( endpointv3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3" listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" tlsv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" matcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" resourcev3 "github.com/envoyproxy/go-control-plane/pkg/resource/v3" "github.com/envoyproxy/go-control-plane/pkg/wellknown" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/wrapperspb" extensionTypes "github.com/envoyproxy/gateway/internal/extension/types" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/protocov" "github.com/envoyproxy/gateway/internal/xds/types" ) @@ -171,6 +174,20 @@ func (t *Translator) processHTTPListenerXdsTranslation( return err } } + } else { + // When the DefaultFilterChain is shared by multiple Gateway HTTP + // Listeners, we need to add the HTTP filters associated with the + // HTTPListener to the HCM if they have not yet been added. + hcm, err := findHCMinFilterChain(xdsListener.DefaultFilterChain) + if err != nil { + return err // should not happen + } + if err = t.patchHCMWithFilters(hcm, httpListener); err != nil { + return err + } + if err = updateHCMinFilterChain(xdsListener.DefaultFilterChain, hcm); err != nil { + return err + } } // Create a route config if we have not found one yet @@ -320,6 +337,38 @@ func (t *Translator) processHTTPListenerXdsTranslation( return errs } +func findHCMinFilterChain(filterChain *listenerv3.FilterChain) (*hcmv3.HttpConnectionManager, error) { + for _, filter := range filterChain.Filters { + if filter.Name == wellknown.HTTPConnectionManager { + hcm := &hcmv3.HttpConnectionManager{} + if err := anypb.UnmarshalTo(filter.GetTypedConfig(), hcm, proto.UnmarshalOptions{}); err != nil { + return nil, err + } + return hcm, nil + } + } + return nil, errors.New("http connection manager not found") +} + +func updateHCMinFilterChain(filterChain *listenerv3.FilterChain, hcm *hcmv3.HttpConnectionManager) error { + for i, filter := range filterChain.Filters { + if filter.Name == wellknown.HTTPConnectionManager { + mgrAny, err := protocov.ToAnyWithError(hcm) + if err != nil { + return err + } + + filterChain.Filters[i] = &listenerv3.Filter{ + Name: wellknown.HTTPConnectionManager, + ConfigType: &listenerv3.Filter_TypedConfig{ + TypedConfig: mgrAny, + }, + } + } + } + return nil +} + func buildHTTP3AltSvcHeader(port int) *corev3.HeaderValueOption { return &corev3.HeaderValueOption{ Append: &wrapperspb.BoolValue{Value: true}, diff --git a/internal/xds/translator/translator_test.go b/internal/xds/translator/translator_test.go index e0737a0df6f..f9ef79121c2 100644 --- a/internal/xds/translator/translator_test.go +++ b/internal/xds/translator/translator_test.go @@ -293,6 +293,9 @@ func TestTranslateXds(t *testing.T) { { name: "retry-partial-invalid", }, + { + name: "multiple-listeners-same-port-with-different-filters", + }, } for _, tc := range testCases {