refactor: simplify filter patching with an unified interface (#2210)

* refactor: simplify filter patching with an unified interface

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

fix: check if secret exist before creat it

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

* address comments

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

internal/xds/translator/cluster.go

@@ -27,6 +27,21 @@ const (
 	tcpClusterPerConnectionBufferLimitBytes = 32768
 )
 
+type xdsClusterArgs struct {
+	name         string
+	settings     []*ir.DestinationSetting
+	tSocket      *corev3.TransportSocket
+	endpointType EndpointType
+	loadBalancer *ir.LoadBalancer
+}
+
+type EndpointType int
+
+const (
+	EndpointTypeDNS EndpointType = iota
+	EndpointTypeStatic
+)
+
 func buildXdsCluster(args *xdsClusterArgs) *clusterv3.Cluster {
 	cluster := &clusterv3.Cluster{
 		Name:            args.name,

internal/xds/translator/cors.go

@@ -20,11 +20,23 @@ import (
 	"google.golang.org/protobuf/types/known/anypb"
 
 	"github.com/envoyproxy/gateway/internal/ir"
+	"github.com/envoyproxy/gateway/internal/xds/types"
 )
 
-// patchHCMWithCORSFilter builds and appends the CORS Filter to the HTTP
-// Connection Manager if applicable.
-func patchHCMWithCORSFilter(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error {
+func init() {
+	registerHTTPFilter(&cors{})
+}
+
+type cors struct {
+}
+
+var _ httpFilter = &cors{}
+
+// patchHCM builds and appends the CORS Filter to the HTTP Connection Manager if
+// applicable.
+func (*cors) patchHCM(
+	mgr *hcmv3.HttpConnectionManager,
+	irListener *ir.HTTPListener) error {
 	if mgr == nil {
 		return errors.New("hcm is nil")
 	}
@@ -49,7 +61,7 @@ func patchHCMWithCORSFilter(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTT
 		return err
 	}
 
-	// Ensure the cors filter is the first one in the filter chain.
+	// Ensure the CORS filter is the first one in the filter chain.
 	mgr.HttpFilters = append([]*hcmv3.HttpFilter{corsFilter}, mgr.HttpFilters...)
 
 	return nil
@@ -88,9 +100,8 @@ func listenerContainsCORS(irListener *ir.HTTPListener) bool {
 	return false
 }
 
-// patchRouteWithCORS patches the provided route with the CORS config if
-// applicable.
-func patchRouteWithCORS(route *routev3.Route, irRoute *ir.HTTPRoute) error {
+// patchRoute patches the provided route with the CORS config if applicable.
+func (*cors) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {
 	if route == nil {
 		return errors.New("xds route is nil")
 	}
@@ -103,7 +114,7 @@ func patchRouteWithCORS(route *routev3.Route, irRoute *ir.HTTPRoute) error {
 
 	filterCfg := route.GetTypedPerFilterConfig()
 	if _, ok := filterCfg[wellknown.CORS]; ok {
-		// This should not happen since this is the only place where the cors
+		// This should not happen since this is the only place where the CORS
 		// filter is added in a route.
 		return fmt.Errorf("route already contains cors config: %+v", route)
 	}
@@ -152,3 +163,11 @@ func patchRouteWithCORS(route *routev3.Route, irRoute *ir.HTTPRoute) error {
 
 	return nil
 }
+
+func (*cors) patchRouteConfig(*routev3.RouteConfiguration, *ir.HTTPListener) error {
+	return nil
+}
+
+func (c *cors) patchResources(*types.ResourceVersionTable, []*ir.HTTPRoute) error {
+	return nil
+}

internal/xds/translator/httpfilters.go

@@ -13,10 +13,60 @@ import (
 	hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
 	"github.com/envoyproxy/go-control-plane/pkg/wellknown"
 
+	"github.com/envoyproxy/gateway/internal/xds/filters"
+	"github.com/envoyproxy/gateway/internal/xds/types"
+
 	"github.com/envoyproxy/gateway/internal/ir"
-	xdsfilters "github.com/envoyproxy/gateway/internal/xds/filters"
 )
 
+var httpFilters []httpFilter
+
+// registerHTTPFilter registers the provided HTTP filter.
+func registerHTTPFilter(filter httpFilter) {
+	httpFilters = append(httpFilters, filter)
+}
+
+// httpFilter is the interface for all the HTTP filters.
+//
+// There are two ways to support per-route configuration for an HTTP filter:
+// - For the filters with native per-route configuration support:
+//   - patchHCM: EG adds the filter to the HCM filter chain only once.
+//   - patchRoute: EG adds the filter's native per-route configuration to each
+//     route's typedFilterConfig.
+//
+// - For the filters without native per-route configuration support:
+//   - patchHCM: EG adds a filter for each route in the HCM filter chain, the
+//     filter name is prefixed with the filter's type name, for example,
+//     "envoy.filters.http.oauth2", and suffixed with the route name. Each filter
+//     is configured with the route's per-route configuration.
+//   - patchRouteConfig: EG disables all the filters of this type in the
+//     typedFilterConfig of the route config.
+//   - 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
+// Note: The filter types that have native per-route configuration support should
+// always se their own native per-route configuration.
+type httpFilter interface {
+	// patchHCM patches the HttpConnectionManager with the filter.
+	patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error
+
+	// patchRouteConfig patches the provided RouteConfiguration with a filter's
+	// RouteConfiguration level configuration.
+	patchRouteConfig(rc *routev3.RouteConfiguration, irListener *ir.HTTPListener) error
+
+	// patchRoute patches the provide Route with a filter's Route level configuration.
+	patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error
+
+	// patchResources adds all the other needed resources referenced by this
+	// filter to the resource version table.
+	// for example:
+	// - a jwt filter needs to add the cluster for the jwks.
+	// - an oidc filter needs to add the cluster for token endpoint and the secret
+	//   for the oauth2 client secret and the hmac secret.
+	patchResources(tCtx *types.ResourceVersionTable, routes []*ir.HTTPRoute) error
+}
+
 type OrderedHTTPFilter struct {
 	filter *hcmv3.HttpFilter
 	order  int
@@ -58,6 +108,7 @@ func newOrderedHTTPFilter(filter *hcmv3.HttpFilter) *OrderedHTTPFilter {
 }
 
 // sort.Interface implementation.
+
 func (o OrderedHTTPFilters) Len() int {
 	return len(o)
 }
@@ -97,80 +148,61 @@ func (t *Translator) patchHCMWithFilters(
 	irListener *ir.HTTPListener) error {
 	// The order of filter patching is not relevant here.
 	// All the filters will be sorted in correct order after the patching is done.
+	//
 	// Important: don't forget to set the order for new filters in the
 	// newOrderedHTTPFilter method.
-	// TODO: Make this a generic interface for all API Gateway features.
-	//       https://github.com/envoyproxy/gateway/issues/882
-	t.patchHCMWithRateLimit(mgr, irListener)
-
-	// Add the cors filter, if needed
-	if err := patchHCMWithCORSFilter(mgr, irListener); err != nil {
-		return err
+	for _, filter := range httpFilters {
+		if err := filter.patchHCM(mgr, irListener); err != nil {
+			return err
+		}
 	}
 
-	// Add the jwt authn filter, if needed.
-	if err := patchHCMWithJWTAuthnFilter(mgr, irListener); err != nil {
-		return err
-	}
-
-	// Add oauth2 filters, if needed.
-	if err := patchHCMWithOAuth2Filters(mgr, irListener); err != nil {
-		return err
-	}
+	// RateLimit filter is handled separately because it relies on the global
+	// rate limit server configuration.
+	t.patchHCMWithRateLimit(mgr, irListener)
 
 	// Add the router filter
-	mgr.HttpFilters = append(mgr.HttpFilters, xdsfilters.HTTPRouter)
+	mgr.HttpFilters = append(mgr.HttpFilters, filters.HTTPRouter)
 
 	// Sort the filters in the correct order.
 	mgr.HttpFilters = sortHTTPFilters(mgr.HttpFilters)
 	return nil
 }
 
 // patchRouteCfgWithPerRouteConfig appends per-route filter configurations to the
-// route config.
-// This is a generic way to add per-route filter configurations for all filters
-// that has none-native per-route configuration support.
-//   - For the filter type that without native per-route configuration support, EG
-//     adds a filter for each route in the HCM filter chain.
-//   - patchRouteCfgWithPerRouteConfig disables all the filters in the
-//     typedFilterConfig of the route config.
-//   - PatchRouteWithPerRouteConfig enables the corresponding oauth2 filter for each
-//     route in the typedFilterConfig of the route.
-//
-// The filter types that have non-native per-route support: oauth2, basic authn
-// Note: The filter types that have native per-route configuration support should
-// use their own native per-route configuration.
+// provided listener's RouteConfiguration.
+// This method is used to disable the filters without native per-route support.
+// The disabled filters will be enabled by route in the patchRouteWithPerRouteConfig
+// method.
 func patchRouteCfgWithPerRouteConfig(
 	routeCfg *routev3.RouteConfiguration,
 	irListener *ir.HTTPListener) error {
 	// Only supports the oauth2 filter for now, other filters will be added later.
-	return patchRouteCfgWithOAuth2Filter(routeCfg, irListener)
+	for _, filter := range httpFilters {
+		if err := filter.patchRouteConfig(routeCfg, irListener); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
-// patchRouteWithPerRouteConfig appends per-route filter configurations to the route.
+// patchRouteWithPerRouteConfig appends per-route filter configuration to the
+// provided route.
 func patchRouteWithPerRouteConfig(
 	route *routev3.Route,
 	irRoute *ir.HTTPRoute) error {
-	// TODO: Convert this into a generic interface for API Gateway features.
-	//       https://github.com/envoyproxy/gateway/issues/882
-	if err :=
-		patchRouteWithRateLimit(route.GetRoute(), irRoute); err != nil {
-		return nil
-	}
 
-	// Add the cors per route config to the route, if needed.
-	if err := patchRouteWithCORS(route, irRoute); err != nil {
-		return err
+	for _, filter := range httpFilters {
+		if err := filter.patchRoute(route, irRoute); err != nil {
+			return err
+		}
 	}
 
-	// Add the jwt per route config to the route, if needed.
-	if err := patchRouteWithJWT(route, irRoute); err != nil {
-		return err
-	}
-
-	// Add the oauth2 per route config to the route, if needed.
-	if err := patchRouteWithOAuth2(route, irRoute); err != nil {
-		return err
+	// RateLimit filter is handled separately because it relies on the global
+	// rate limit server configuration.
+	if err :=
+		patchRouteWithRateLimit(route.GetRoute(), irRoute); err != nil {
+		return nil
 	}
 
 	return nil
@@ -182,3 +214,17 @@ func isOAuth2Filter(filter *hcmv3.HttpFilter) bool {
 	// route. The oauth2 filter name is prefixed with "envoy.filters.http.oauth2".
 	return strings.HasPrefix(filter.Name, oauth2Filter)
 }
+
+// patchResources adds all the other needed resources referenced by this
+// filter to the resource version table.
+// for example:
+// - a jwt filter needs to add the cluster for the jwks.
+// - an oidc filter needs to add the secret for the oauth2 client secret.
+func patchResources(tCtx *types.ResourceVersionTable, routes []*ir.HTTPRoute) error {
+	for _, filter := range httpFilters {
+		if err := filter.patchResources(tCtx, routes); err != nil {
+			return err
+		}
+	}
+	return nil
+}

internal/xds/translator/jwt_authn.go → internal/xds/translator/jwt.go

@@ -29,9 +29,18 @@ const (
 	envoyTrustBundle = "/etc/ssl/certs/ca-certificates.crt"
 )
 
-// patchHCMWithJWTAuthnFilter builds and appends the Jwt Filter to the HTTP
-// Connection Manager if applicable, and it does not already exist.
-func patchHCMWithJWTAuthnFilter(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error {
+func init() {
+	registerHTTPFilter(&jwt{})
+}
+
+type jwt struct {
+}
+
+var _ httpFilter = &jwt{}
+
+// patchHCM builds and appends the JWT Filter to the HTTP Connection Manager if
+// applicable, and it does not already exist.
+func (*jwt) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error {
 	if mgr == nil {
 		return errors.New("hcm is nil")
 	}
@@ -193,9 +202,9 @@ func buildXdsUpstreamTLSSocket() (*corev3.TransportSocket, error) {
 	}, nil
 }
 
-// patchRouteWithJWT patches the provided route with a JWT PerRouteConfig, if the
-// route doesn't contain it.
-func patchRouteWithJWT(route *routev3.Route, irRoute *ir.HTTPRoute) error {
+// patchRoute patches the provided route with a JWT PerRouteConfig, if the route
+// doesn't contain it.
+func (*jwt) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {
 	if route == nil {
 		return errors.New("xds route is nil")
 	}
@@ -227,8 +236,8 @@ func patchRouteWithJWT(route *routev3.Route, irRoute *ir.HTTPRoute) error {
 	return nil
 }
 
-// createJWKSClusters creates JWKS clusters from the provided routes, if needed.
-func createJWKSClusters(tCtx *types.ResourceVersionTable, routes []*ir.HTTPRoute) error {
+// patchResources creates JWKS clusters from the provided routes, if needed.
+func (*jwt) patchResources(tCtx *types.ResourceVersionTable, routes []*ir.HTTPRoute) error {
 	if tCtx == nil || tCtx.XdsResources == nil {
 		return errors.New("xds resource table is nil")
 	}
@@ -311,3 +320,7 @@ func routeContainsJWTAuthn(irRoute *ir.HTTPRoute) bool {
 
 	return false
 }
+
+func (*jwt) patchRouteConfig(*routev3.RouteConfiguration, *ir.HTTPListener) error {
+	return nil
+}