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

Use native per-route config for basic auth #3182

Merged
merged 6 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/Masterminds/semver/v3 v3.2.1
github.com/cncf/xds/go v0.0.0-20231128003011-0fa0005c9caa
github.com/davecgh/go-spew v1.1.1
github.com/envoyproxy/go-control-plane v0.12.1-0.20240322155512-db0b36a50fa8
github.com/envoyproxy/go-control-plane v0.12.1-0.20240410145647-bdba4bba15fc
github.com/envoyproxy/ratelimit v1.4.1-0.20230427142404-e2a87f41d3a7
github.com/evanphx/json-patch/v5 v5.9.0
github.com/fatih/color v1.16.0
Expand Down Expand Up @@ -99,7 +99,7 @@ require (
github.com/morikuni/aec v1.0.0 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/opencontainers/image-spec v1.1.0-rc5 // indirect
github.com/planetscale/vtprotobuf v0.5.1-0.20231212170721-e7d721933795 // indirect
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
github.com/rivo/uniseg v0.2.0 // indirect
github.com/rubenv/sql-migrate v1.5.2 // indirect
github.com/shopspring/decimal v1.3.1 // indirect
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT
github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g=
github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.12.1-0.20240322155512-db0b36a50fa8 h1:Zghtu+wdlGvrmutCyhU9Ew5ozU18PVpxP+zGSgyUpFs=
github.com/envoyproxy/go-control-plane v0.12.1-0.20240322155512-db0b36a50fa8/go.mod h1:YtsM9q/kVkKyvmemY+BF/ZK7I93OWsx4uk4Do2Mr/OA=
github.com/envoyproxy/go-control-plane v0.12.1-0.20240410145647-bdba4bba15fc h1:FJoupBhZkbUXmzGxgAic3rEHeZf8jgvREB7uMfBI23w=
github.com/envoyproxy/go-control-plane v0.12.1-0.20240410145647-bdba4bba15fc/go.mod h1:Dj0RQ153G7gNYzcQCihXUreYTQbuJNuL7IT7v9+jTr4=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/envoyproxy/protoc-gen-validate v1.0.4 h1:gVPz/FMfvh57HdSJQyvBtF00j8JU4zdyUgIUNhlgg0A=
github.com/envoyproxy/protoc-gen-validate v1.0.4/go.mod h1:qys6tmnRsYrQqIhm2bvKZH4Blx/1gTIZ2UKVY1M+Yew=
Expand Down Expand Up @@ -546,8 +546,8 @@ github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/planetscale/vtprotobuf v0.5.1-0.20231212170721-e7d721933795 h1:pH+U6pJP0BhxqQ4njBUjOg0++WMMvv3eByWzB+oATBY=
github.com/planetscale/vtprotobuf v0.5.1-0.20231212170721-e7d721933795/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8=
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo=
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/poy/onpar v1.1.2 h1:QaNrNiZx0+Nar5dLgTVp5mXkyoVFIbepjyEoGSnhbAY=
Expand Down
4 changes: 2 additions & 2 deletions internal/xds/extensions/extensions.gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,12 @@ import (
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/http/early_header_mutation/header_mutation/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/http/header_formatters/preserve_case/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/http/header_validators/envoy_default/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/http/injected_credentials/generic/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/http/injected_credentials/oauth2/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/http/original_ip_detection/custom_header/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/http/original_ip_detection/xff/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/http/stateful_session/cookie/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/http/stateful_session/header/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/injected_credentials/generic/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/injected_credentials/oauth2/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/internal_redirect/allow_listed_routes/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/internal_redirect/previous_routes/v3"
_ "github.com/envoyproxy/go-control-plane/envoy/extensions/internal_redirect/safe_cross_scheme/v3"
Expand Down
137 changes: 79 additions & 58 deletions internal/xds/translator/basicauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package translator

import (
"errors"
"fmt"

corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
Expand All @@ -31,97 +32,81 @@ type basicAuth struct {

var _ httpFilter = &basicAuth{}

// patchHCM builds and appends the basic_auth Filters to the HTTP Connection Manager
// patchHCM builds and appends the basic_auth Filter to the HTTP Connection Manager
// if applicable, and it does not already exist.
// Note: this method creates an basic_auth filter for each route that contains an BasicAuth config.
// The filter is disabled by default. It is enabled on the route level.
func (*basicAuth) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error {
var errs error

if mgr == nil {
return errors.New("hcm is nil")
}

if irListener == nil {
return errors.New("ir listener is nil")
}
if hcmContainsFilter(mgr, basicAuthFilter) {
return nil
}

for _, route := range irListener.Routes {
if !routeContainsBasicAuth(route) {
continue
}

// Only generates one BasicAuth Envoy filter for each unique name.
// For example, if there are two routes under the same gateway with the
// same BasicAuth config, only one BasicAuth filter will be generated.
if hcmContainsFilter(mgr, basicAuthFilterName(route.Security.BasicAuth)) {
continue
}
var (
irBasicAuth *ir.BasicAuth
filter *hcmv3.HttpFilter
err error
)

filter, err := buildHCMBasicAuthFilter(route.Security.BasicAuth)
if err != nil {
errs = errors.Join(errs, err)
continue
for _, route := range irListener.Routes {
if route.Security != nil && route.Security.BasicAuth != nil {
irBasicAuth = route.Security.BasicAuth
break
}

mgr.HttpFilters = append(mgr.HttpFilters, filter)
}
if irBasicAuth == nil {
return nil
}

return errs
// We use the first route that contains the basicAuth config to build the filter.
// The HCM-level filter config doesn't matter since it is overwritten at the route level.
if filter, err = buildHCMBasicAuthFilter(irBasicAuth); err != nil {
return err
}
mgr.HttpFilters = append(mgr.HttpFilters, filter)
return err
}

// buildHCMBasicAuthFilter returns a basic_auth HTTP filter from the provided IR HTTPRoute.
func buildHCMBasicAuthFilter(basicAuth *ir.BasicAuth) (*hcmv3.HttpFilter, error) {
basicAuthProto := basicAuthConfig(basicAuth)
var (
basicAuthProto *basicauthv3.BasicAuth
basicAuthAny *anypb.Any
err error
)

if err := basicAuthProto.ValidateAll(); err != nil {
basicAuthProto = &basicauthv3.BasicAuth{
Users: &corev3.DataSource{
Specifier: &corev3.DataSource_InlineBytes{
InlineBytes: basicAuth.Users,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we using the first route's security policy basic config to define the top-level HCM "global" config? It's overriden on each route, but makes XDS look a bit strange... Maybe just leave it blank?

Copy link
Member Author

@zhaohuabing zhaohuabing Apr 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'd like to leave it blank as well, but the Users field is mandatory.

},
},
}
if err = basicAuthProto.ValidateAll(); err != nil {
return nil, err
}

basicAuthAny, err := anypb.New(basicAuthProto)
if err != nil {
if basicAuthAny, err = anypb.New(basicAuthProto); err != nil {
return nil, err
}

return &hcmv3.HttpFilter{
Name: basicAuthFilterName(basicAuth),
Disabled: true,
Name: basicAuthFilter,
ConfigType: &hcmv3.HttpFilter_TypedConfig{
TypedConfig: basicAuthAny,
},
Disabled: true,
}, nil
}

func basicAuthFilterName(basicAuth *ir.BasicAuth) string {
return perRouteFilterName(basicAuthFilter, basicAuth.Name)
}

func basicAuthConfig(basicAuth *ir.BasicAuth) *basicauthv3.BasicAuth {
return &basicauthv3.BasicAuth{
Users: &corev3.DataSource{
Specifier: &corev3.DataSource_InlineBytes{
InlineBytes: basicAuth.Users,
},
},
}
}

// routeContainsBasicAuth returns true if BasicAuth exists for the provided route.
func routeContainsBasicAuth(irRoute *ir.HTTPRoute) bool {
if irRoute != nil &&
irRoute.Security != nil &&
irRoute.Security.BasicAuth != nil {
return true
}
return false
}

func (*basicAuth) patchResources(*types.ResourceVersionTable, []*ir.HTTPRoute) error {
return nil
}

// patchRoute patches the provided route with the basicAuth config if applicable.
// Note: this method enables the corresponding basicAuth filter for the provided route.
// Note: this method overwrites the HCM level filter config with the per route filter config.
func (*basicAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {
if route == nil {
return errors.New("xds route is nil")
Expand All @@ -132,9 +117,45 @@ func (*basicAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error
if irRoute.Security == nil || irRoute.Security.BasicAuth == nil {
return nil
}
filterName := basicAuthFilterName(irRoute.Security.BasicAuth)
if err := enableFilterOnRoute(route, filterName); err != nil {

var (
perFilterCfg map[string]*anypb.Any
basicAuthAny *anypb.Any
err error
)

perFilterCfg = route.GetTypedPerFilterConfig()
if _, ok := perFilterCfg[basicAuthFilter]; 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",
basicAuthFilter, route)
}

// Overwrite the HCM level filter config with the per route filter config.
basicAuthProto := basicAuthPerRouteConfig(irRoute.Security.BasicAuth)
if err = basicAuthProto.ValidateAll(); err != nil {
return err
}

if basicAuthAny, err = anypb.New(basicAuthProto); err != nil {
return err
}

if perFilterCfg == nil {
route.TypedPerFilterConfig = make(map[string]*anypb.Any)
}
route.TypedPerFilterConfig[basicAuthFilter] = basicAuthAny

return nil
}

func basicAuthPerRouteConfig(basicAuth *ir.BasicAuth) *basicauthv3.BasicAuthPerRoute {
return &basicauthv3.BasicAuthPerRoute{
Users: &corev3.DataSource{
Specifier: &corev3.DataSource_InlineBytes{
InlineBytes: basicAuth.Users,
},
},
}
}
4 changes: 2 additions & 2 deletions internal/xds/translator/httpfilters.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func registerHTTPFilter(filter httpFilter) {
// - PatchRouteWithPerRouteConfig: EG enables the corresponding filter for each
// route in the typedFilterConfig of that route.
//
// The filter types that haven't native per-route support: oauth2, basic authn, ext_authz.
// The filter types that haven't native per-route support: oauth2, ext_authz.
// Note: The filter types that have native per-route configuration support should
// always se their own native per-route configuration.
type httpFilter interface {
Expand Down Expand Up @@ -97,7 +97,7 @@ func newOrderedHTTPFilter(filter *hcmv3.HttpFilter) *OrderedHTTPFilter {
order = 2
case isFilterType(filter, extAuthFilter):
order = 3
case isFilterType(filter, basicAuthFilter):
case filter.Name == basicAuthFilter:
order = 4
case isFilterType(filter, oauth2Filter):
order = 5
Expand Down
4 changes: 2 additions & 2 deletions internal/xds/translator/httpfilters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func Test_sortHTTPFilters(t *testing.T) {
httpFilterForTest(wellknown.CORS),
httpFilterForTest(jwtAuthn),
httpFilterForTest(oauth2Filter + "-route1"),
httpFilterForTest(basicAuthFilter + "-route1"),
httpFilterForTest(basicAuthFilter),
httpFilterForTest(wellknown.HTTPRateLimit),
httpFilterForTest(wellknown.Fault),
httpFilterForTest(extAuthFilter + "-route1"),
Expand All @@ -35,7 +35,7 @@ func Test_sortHTTPFilters(t *testing.T) {
httpFilterForTest(wellknown.Fault),
httpFilterForTest(wellknown.CORS),
httpFilterForTest(extAuthFilter + "-route1"),
httpFilterForTest(basicAuthFilter + "-route1"),
httpFilterForTest(basicAuthFilter),
httpFilterForTest(oauth2Filter + "-route1"),
httpFilterForTest(jwtAuthn),
httpFilterForTest(wellknown.HTTPRateLimit),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,11 @@
maxConcurrentStreams: 100
httpFilters:
- disabled: true
name: envoy.filters.http.basic_auth/securitypolicy/default/policy-for-http-route-1
name: envoy.filters.http.basic_auth
typedConfig:
'@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuth
users:
inlineBytes: dXNlcjE6e1NIQX10RVNzQm1FL3lOWTNsYjZhMEw2dlZRRVpOcXc9CnVzZXIyOntTSEF9RUo5TFBGRFhzTjl5blNtYnh2anA3NUJtbHg4PQo=
- disabled: true
name: envoy.filters.http.basic_auth/securitypolicy/default/policy-for-gateway-1
typedConfig:
'@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuth
users:
inlineBytes: Zm9vOntTSEF9WXMyM0FnLzVJT1dxWkN3OVFHYVZEZEh3SDAwPQpmb28xOntTSEF9ZGpaMTFxSFkwS09pamV5bUs3YUt2WXV2aHZNPQo=
- name: envoy.filters.http.router
typedConfig:
'@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
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: {}
envoy.filters.http.basic_auth:
'@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuthPerRoute
users:
inlineBytes: dXNlcjE6e1NIQX10RVNzQm1FL3lOWTNsYjZhMEw2dlZRRVpOcXc9CnVzZXIyOntTSEF9RUo5TFBGRFhzTjl5blNtYnh2anA3NUJtbHg4PQo=
- match:
pathSeparatedPrefix: /foo2
name: httproute/default/httproute-1/rule/1/match/0/www_foo_com
Expand All @@ -24,9 +25,10 @@
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: {}
envoy.filters.http.basic_auth:
'@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuthPerRoute
users:
inlineBytes: dXNlcjE6e1NIQX10RVNzQm1FL3lOWTNsYjZhMEw2dlZRRVpOcXc9CnVzZXIyOntTSEF9RUo5TFBGRFhzTjl5blNtYnh2anA3NUJtbHg4PQo=
- domains:
- www.bar.com
name: default/gateway-1/http/www_bar_com
Expand All @@ -39,6 +41,7 @@
upgradeConfigs:
- upgradeType: websocket
typedPerFilterConfig:
envoy.filters.http.basic_auth/securitypolicy/default/policy-for-gateway-1:
'@type': type.googleapis.com/envoy.config.route.v3.FilterConfig
config: {}
envoy.filters.http.basic_auth:
'@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuthPerRoute
users:
inlineBytes: Zm9vOntTSEF9WXMyM0FnLzVJT1dxWkN3OVFHYVZEZEh3SDAwPQpmb28xOntTSEF9ZGpaMTFxSFkwS09pamV5bUs3YUt2WXV2aHZNPQo=
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
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
name: envoy.filters.http.basic_auth
typedConfig:
'@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuth
users:
Expand Down Expand Up @@ -94,7 +94,7 @@
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
name: envoy.filters.http.basic_auth
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 @@ -18,9 +18,10 @@
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: {}
envoy.filters.http.basic_auth:
'@type': type.googleapis.com/envoy.extensions.filters.http.basic_auth.v3.BasicAuthPerRoute
users:
inlineBytes: dXNlcjE6e1NIQX10RVNzQm1FL3lOWTNsYjZhMEw2dlZRRVpOcXc9CnVzZXIyOntTSEF9RUo5TFBGRFhzTjl5blNtYnh2anA3NUJtbHg4PQo=
- match:
pathSeparatedPrefix: /foo2
name: httproute/default/httproute-2/rule/0/match/0/www_foo_com
Expand Down