diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 2951d6ff7ec..4a85f726a7f 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -378,7 +378,10 @@ func (t *Translator) translateSecurityPolicyForRoute( } if policy.Spec.ExtAuth != nil { - if extAuth, err = t.buildExtAuth(policy, resources); err != nil { + if extAuth, err = t.buildExtAuth( + utils.NamespacedName(route).String(), + policy, + resources); err != nil { errs = errors.Join(errs, err) } } @@ -458,7 +461,10 @@ func (t *Translator) translateSecurityPolicyForGateway( } if policy.Spec.ExtAuth != nil { - if extAuth, err = t.buildExtAuth(policy, resources); err != nil { + if extAuth, err = t.buildExtAuth( + utils.NamespacedName(gateway).String(), + policy, + resources); err != nil { errs = errors.Join(errs, err) } } @@ -793,6 +799,7 @@ func (t *Translator) buildBasicAuth( } func (t *Translator) buildExtAuth( + name string, policy *egv1a1.SecurityPolicy, resources *Resources) (*ir.ExtAuth, error) { var ( @@ -845,6 +852,7 @@ func (t *Translator) buildExtAuth( } extAuth := &ir.ExtAuth{ + Name: name, HeadersToExtAuth: policy.Spec.ExtAuth.HeadersToExtAuth, } diff --git a/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.out.yaml index 5b413d59432..b3dc1299f34 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.out.yaml @@ -324,6 +324,7 @@ xdsIR: headersToExtAuth: - header1 - header2 + name: default/httproute-1 hostname: www.foo.com isHTTP2: false name: httproute/default/httproute-1/rule/0/match/0/www_foo_com @@ -364,6 +365,7 @@ xdsIR: - header1 - header2 path: /auth + name: default/gateway-1 hostname: www.bar.com isHTTP2: false name: httproute/default/httproute-2/rule/0/match/0/www_bar_com diff --git a/internal/gatewayapi/testdata/securitypolicy-with-extauth.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-extauth.in.yaml index 12142460fa3..a451b576774 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-extauth.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-extauth.in.yaml @@ -29,10 +29,16 @@ httpRoutes: rules: - matches: - path: - value: /foo + value: /foo1 backendRefs: - name: service-1 port: 8080 + - matches: + - path: + value: /foo2 + backendRefs: + - name: service-2 + port: 8080 - apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute metadata: @@ -50,7 +56,7 @@ httpRoutes: - path: value: /bar backendRefs: - - name: service-1 + - name: service-3 port: 8080 services: - apiVersion: v1 diff --git a/internal/gatewayapi/testdata/securitypolicy-with-extauth.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-extauth.out.yaml index 93666da42bb..25a291a8ca3 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-extauth.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-extauth.out.yaml @@ -59,7 +59,13 @@ httpRoutes: port: 8080 matches: - path: - value: /foo + value: /foo1 + - backendRefs: + - name: service-2 + port: 8080 + matches: + - path: + value: /foo2 status: parents: - conditions: @@ -93,7 +99,7 @@ httpRoutes: sectionName: http rules: - backendRefs: - - name: service-1 + - name: service-3 port: 8080 matches: - path: @@ -253,13 +259,49 @@ xdsIR: headersToExtAuth: - header1 - header2 + name: default/httproute-1 hostname: www.foo.com isHTTP2: false name: httproute/default/httproute-1/rule/0/match/0/www_foo_com pathMatch: distinct: false name: "" - prefix: /foo + prefix: /foo1 + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-1/rule/1 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + extAuth: + grpc: + authority: grpc-backend.default:9000 + destination: + name: securitypolicy/default/policy-for-http-route/grpc-backend + settings: + - addressType: IP + endpoints: + - host: 8.8.8.8 + port: 9000 + protocol: GRPC + weight: 1 + headersToExtAuth: + - header1 + - header2 + name: default/httproute-1 + hostname: www.foo.com + isHTTP2: false + name: httproute/default/httproute-1/rule/1/match/0/www_foo_com + pathMatch: + distinct: false + name: "" + prefix: /foo2 - backendWeights: invalid: 0 valid: 0 @@ -288,6 +330,7 @@ xdsIR: - header1 - header2 path: /auth + name: default/gateway-1 hostname: www.bar.com isHTTP2: false name: httproute/default/httproute-2/rule/0/match/0/www_bar_com diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 0d7c5df03b2..d2194fb95e8 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -575,6 +575,10 @@ type BasicAuth struct { // // +k8s:deepcopy-gen=true type ExtAuth struct { + // Name is a unique name for an ExtAuth configuration. + // The xds translator only generates one external authorization filter for each unique name. + Name string `json:"name" yaml:"name"` + // GRPC defines the gRPC External Authorization service. // Only one of GRPCService or HTTPService may be specified. GRPC *GRPCExtAuthService `json:"grpc,omitempty"` diff --git a/internal/xds/translator/basicauth.go b/internal/xds/translator/basicauth.go index 6ae7b9163e5..85cd77fa3c3 100644 --- a/internal/xds/translator/basicauth.go +++ b/internal/xds/translator/basicauth.go @@ -129,7 +129,8 @@ func (*basicAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error if irRoute.BasicAuth == nil { return nil } - if err := enableFilterOnRoute(basicAuthFilter, route, irRoute); err != nil { + filterName := basicAuthFilterName(irRoute) + if err := enableFilterOnRoute(route, filterName); err != nil { return err } return nil diff --git a/internal/xds/translator/extauth.go b/internal/xds/translator/extauth.go index 069aac77eb7..28e509253aa 100644 --- a/internal/xds/translator/extauth.go +++ b/internal/xds/translator/extauth.go @@ -55,7 +55,14 @@ func (*extAuth) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPLi continue } - filter, err := buildHCMExtAuthFilter(route) + // Only generates one OAuth2 Envoy filter for each unique name. + // For example, if there are two routes under the same gateway with the + // same OIDC config, only one OAuth2 filter will be generated. + if hcmContainsFilter(mgr, extAuthFilterName(route.ExtAuth)) { + continue + } + + filter, err := buildHCMExtAuthFilter(route.ExtAuth) if err != nil { errs = errors.Join(errs, err) continue @@ -68,8 +75,8 @@ func (*extAuth) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPLi } // buildHCMExtAuthFilter returns an ext_authz HTTP filter from the provided IR HTTPRoute. -func buildHCMExtAuthFilter(route *ir.HTTPRoute) (*hcmv3.HttpFilter, error) { - extAuthProto := extAuthConfig(route.ExtAuth) +func buildHCMExtAuthFilter(extAuth *ir.ExtAuth) (*hcmv3.HttpFilter, error) { + extAuthProto := extAuthConfig(extAuth) if err := extAuthProto.ValidateAll(); err != nil { return nil, err } @@ -80,7 +87,7 @@ func buildHCMExtAuthFilter(route *ir.HTTPRoute) (*hcmv3.HttpFilter, error) { } return &hcmv3.HttpFilter{ - Name: extAuthFilterName(route), + Name: extAuthFilterName(extAuth), Disabled: true, ConfigType: &hcmv3.HttpFilter_TypedConfig{ TypedConfig: extAuthAny, @@ -88,8 +95,8 @@ func buildHCMExtAuthFilter(route *ir.HTTPRoute) (*hcmv3.HttpFilter, error) { }, nil } -func extAuthFilterName(route *ir.HTTPRoute) string { - return perRouteFilterName(extAuthFilter, route.Name) +func extAuthFilterName(extAuth *ir.ExtAuth) string { + return perRouteFilterName(extAuthFilter, extAuth.Name) } func extAuthConfig(extAuth *ir.ExtAuth) *extauthv3.ExtAuthz { @@ -276,7 +283,8 @@ func (*extAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { if irRoute.ExtAuth == nil { return nil } - if err := enableFilterOnRoute(extAuthFilter, route, irRoute); err != nil { + filterName := extAuthFilterName(irRoute.ExtAuth) + if err := enableFilterOnRoute(route, filterName); err != nil { return err } return nil diff --git a/internal/xds/translator/oidc.go b/internal/xds/translator/oidc.go index 58b417f2441..2a10254ec64 100644 --- a/internal/xds/translator/oidc.go +++ b/internal/xds/translator/oidc.go @@ -94,7 +94,7 @@ func buildHCMOAuth2Filter(route *ir.HTTPRoute) (*hcmv3.HttpFilter, error) { } func oauth2FilterName(route *ir.HTTPRoute) string { - return fmt.Sprintf("%s_%s", oauth2Filter, route.Name) + return perRouteFilterName(oauth2Filter, route.Name) } func oauth2Config(route *ir.HTTPRoute) (*oauth2v3.OAuth2, error) { @@ -340,8 +340,8 @@ func (*oidc) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { if irRoute.OIDC == nil { return nil } - - if err := enableFilterOnRoute(oauth2Filter, route, irRoute); err != nil { + filterName := oauth2FilterName(irRoute) + if err := enableFilterOnRoute(route, filterName); err != nil { return err } return nil diff --git a/internal/xds/translator/testdata/in/xds-ir/ext-auth.yaml b/internal/xds/translator/testdata/in/xds-ir/ext-auth.yaml index d1451e968e3..e8dd3181425 100644 --- a/internal/xds/translator/testdata/in/xds-ir/ext-auth.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/ext-auth.yaml @@ -19,6 +19,34 @@ http: - host: "10.0.0.1" port: 50000 extAuth: + name: default/httproute-1 + http: + authority: http-backend.envoy-gateway:80 + headersToBackend: + - header1 + - header2 + path: /auth + destination: + name: securitypolicy/default/policy-for-first-route/http-backend + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 80 + protocol: HTTP + weight: 1 + - name: httproute/default/httproute-1/rule/1/match/0/www_example_com + hostname: "*" + pathMatch: + exact: "foo" + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - endpoints: + - host: "10.0.0.1" + port: 50000 + extAuth: + name: default/httproute-1 http: authority: http-backend.envoy-gateway:80 headersToBackend: @@ -45,6 +73,7 @@ http: - host: "10.0.0.2" port: 60000 extAuth: + name: default/gateway-1 grpc: authority: grpc-backend.default:9000 destination: diff --git a/internal/xds/translator/testdata/out/xds-ir/basic-auth.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/basic-auth.listeners.yaml index 3b72788ce31..36ab0dbb7ba 100644 --- a/internal/xds/translator/testdata/out/xds-ir/basic-auth.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/basic-auth.listeners.yaml @@ -15,7 +15,7 @@ maxConcurrentStreams: 100 httpFilters: - disabled: true - name: envoy.filters.http.basic_auth_first-route + name: envoy.filters.http.basic_auth/first-route typedConfig: '@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuth users: diff --git a/internal/xds/translator/testdata/out/xds-ir/basic-auth.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/basic-auth.routes.yaml index f87be11474a..22938d503e2 100644 --- a/internal/xds/translator/testdata/out/xds-ir/basic-auth.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/basic-auth.routes.yaml @@ -13,6 +13,6 @@ upgradeConfigs: - upgradeType: websocket typedPerFilterConfig: - envoy.filters.http.basic_auth_first-route: + envoy.filters.http.basic_auth/first-route: '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig config: {} diff --git a/internal/xds/translator/testdata/out/xds-ir/ext-auth.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/ext-auth.listeners.yaml index 52735036294..1fa830a1f83 100644 --- a/internal/xds/translator/testdata/out/xds-ir/ext-auth.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/ext-auth.listeners.yaml @@ -15,7 +15,7 @@ maxConcurrentStreams: 100 httpFilters: - disabled: true - name: envoy.filters.http.ext_authz_httproute/default/httproute-1/rule/0/match/0/www_example_com + name: envoy.filters.http.ext_authz/default/httproute-1 typedConfig: '@type': type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz httpService: @@ -30,7 +30,7 @@ uri: http://http-backend.envoy-gateway:80/auth transportApiVersion: V3 - disabled: true - name: envoy.filters.http.ext_authz_httproute/default/httproute-2/rule/0/match/0/www_example_com + name: envoy.filters.http.ext_authz/default/gateway-1 typedConfig: '@type': type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz allowedHeaders: diff --git a/internal/xds/translator/testdata/out/xds-ir/ext-auth.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/ext-auth.routes.yaml index 0b039af3da3..7ca5275220c 100644 --- a/internal/xds/translator/testdata/out/xds-ir/ext-auth.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/ext-auth.routes.yaml @@ -13,7 +13,18 @@ upgradeConfigs: - upgradeType: websocket typedPerFilterConfig: - envoy.filters.http.ext_authz_httproute/default/httproute-1/rule/0/match/0/www_example_com: + envoy.filters.http.ext_authz/default/httproute-1: + '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig + config: {} + - match: + path: foo + name: httproute/default/httproute-1/rule/1/match/0/www_example_com + route: + cluster: httproute/default/httproute-1/rule/0 + upgradeConfigs: + - upgradeType: websocket + typedPerFilterConfig: + envoy.filters.http.ext_authz/default/httproute-1: '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig config: {} - match: @@ -24,6 +35,6 @@ upgradeConfigs: - upgradeType: websocket typedPerFilterConfig: - envoy.filters.http.ext_authz_httproute/default/httproute-2/rule/0/match/0/www_example_com: + envoy.filters.http.ext_authz/default/gateway-1: '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig config: {} diff --git a/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml index 73388e331ed..132b1d06a48 100644 --- a/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml @@ -15,7 +15,7 @@ maxConcurrentStreams: 100 httpFilters: - disabled: true - name: envoy.filters.http.oauth2_first-route + name: envoy.filters.http.oauth2/first-route typedConfig: '@type': type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2 config: @@ -56,7 +56,7 @@ timeout: 10s uri: https://oauth.foo.com/token - disabled: true - name: envoy.filters.http.oauth2_second-route + name: envoy.filters.http.oauth2/second-route typedConfig: '@type': type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2 config: diff --git a/internal/xds/translator/testdata/out/xds-ir/oidc.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/oidc.routes.yaml index a093d6967ac..d597d98514e 100644 --- a/internal/xds/translator/testdata/out/xds-ir/oidc.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/oidc.routes.yaml @@ -13,7 +13,7 @@ upgradeConfigs: - upgradeType: websocket typedPerFilterConfig: - envoy.filters.http.oauth2_first-route: + envoy.filters.http.oauth2/first-route: '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig config: {} - match: @@ -24,6 +24,6 @@ upgradeConfigs: - upgradeType: websocket typedPerFilterConfig: - envoy.filters.http.oauth2_second-route: + envoy.filters.http.oauth2/second-route: '@type': type.googleapis.com/envoy.config.route.v3.FilterConfig config: {} diff --git a/internal/xds/translator/utils.go b/internal/xds/translator/utils.go index 8ebcc74c677..64f946373b7 100644 --- a/internal/xds/translator/utils.go +++ b/internal/xds/translator/utils.go @@ -14,9 +14,8 @@ import ( "strings" 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" "google.golang.org/protobuf/types/known/anypb" - - "github.com/envoyproxy/gateway/internal/ir" ) const ( @@ -80,21 +79,17 @@ func clusterName(host string, port uint32) string { } // enableFilterOnRoute enables a filterType on the provided route. -func enableFilterOnRoute(filterType string, route *routev3.Route, irRoute *ir.HTTPRoute) error { +func enableFilterOnRoute(route *routev3.Route, filterName string) error { if route == nil { return errors.New("xds route is nil") } - if irRoute == nil { - return errors.New("ir route is nil") - } - filterName := perRouteFilterName(filterType, irRoute.Name) filterCfg := route.GetTypedPerFilterConfig() if _, ok := filterCfg[filterName]; ok { // This should not happen since this is the only place where the filter // config is added in a route. return fmt.Errorf("route already contains filter config: %s, %+v", - filterType, route) + filterName, route) } // Enable the corresponding filter for this route. @@ -114,6 +109,16 @@ func enableFilterOnRoute(filterType string, route *routev3.Route, irRoute *ir.HT return nil } -func perRouteFilterName(filterType, routeName string) string { - return fmt.Sprintf("%s_%s", filterType, routeName) +// perRouteFilterName generates a unique filter name for the provided filterType and configName. +func perRouteFilterName(filterType, configName string) string { + return fmt.Sprintf("%s/%s", filterType, configName) +} + +func hcmContainsFilter(mgr *hcmv3.HttpConnectionManager, filterName string) bool { + for _, existingFilter := range mgr.HttpFilters { + if existingFilter.Name == filterName { + return true + } + } + return false }