Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove duplicated http filters for ExtAuth #2893

Merged
merged 4 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions internal/gatewayapi/securitypolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -793,6 +799,7 @@ func (t *Translator) buildBasicAuth(
}

func (t *Translator) buildExtAuth(
name string,
policy *egv1a1.SecurityPolicy,
resources *Resources) (*ir.ExtAuth, error) {
var (
Expand Down Expand Up @@ -845,6 +852,7 @@ func (t *Translator) buildExtAuth(
}

extAuth := &ir.ExtAuth{
Name: name,
HeadersToExtAuth: policy.Spec.ExtAuth.HeadersToExtAuth,
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -50,7 +56,7 @@ httpRoutes:
- path:
value: /bar
backendRefs:
- name: service-1
- name: service-3
port: 8080
services:
- apiVersion: v1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -93,7 +99,7 @@ httpRoutes:
sectionName: http
rules:
- backendRefs:
- name: service-1
- name: service-3
port: 8080
matches:
- path:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions internal/ir/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
3 changes: 2 additions & 1 deletion internal/xds/translator/basicauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 15 additions & 7 deletions internal/xds/translator/extauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -80,16 +87,16 @@ 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,
},
}, 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 {
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions internal/xds/translator/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions internal/xds/translator/testdata/in/xds-ir/ext-auth.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -45,6 +73,7 @@ http:
- host: "10.0.0.2"
port: 60000
extAuth:
name: default/gateway-1
grpc:
authority: grpc-backend.default:9000
destination:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {}
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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: {}
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading