refactor: simplify filter patching with an unified interface

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

internal/xds/translator/cors.go

@@ -20,11 +20,17 @@ 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 {
+type CORS struct {
+}
+
+// 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")
 	}
@@ -88,9 +94,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")
 	}
@@ -152,3 +157,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,35 @@ 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{&CORS{}, &JWT{}, &OIDC{}}
+
+// httpFilter is the interface for all the HTTP filters.
+type httpFilter interface {
+	// patchHCM patches the HttpConnectionManager with the filter.
+	patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error
+
+	// patchRouteConfig patches the RouteConfiguration with the filter.
+	// Right now, it's only used for disable filters without native per-route
+	// configuration support.
+	patchRouteConfig(rc *routev3.RouteConfiguration, irListener *ir.HTTPListener) error
+
+	// patchRoute patches the Route with the filter's per-rout 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 secret for the oauth2 client secret.
+	patchResources(tCtx *types.ResourceVersionTable, routes []*ir.HTTPRoute) error
+}
+
 type OrderedHTTPFilter struct {
 	filter *hcmv3.HttpFilter
 	order  int
@@ -97,29 +122,21 @@ 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
-	}
-
-	// Add the jwt authn filter, if needed.
-	if err := patchHCMWithJWTAuthnFilter(mgr, irListener); err != nil {
-		return err
+	for _, filter := range httpFilters {
+		if err := filter.patchHCM(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)
@@ -144,33 +161,30 @@ 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.
 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
-	}
 
-	// Add the jwt per route config to the route, if needed.
-	if err := patchRouteWithJWT(route, irRoute); err != nil {
-		return err
+	for _, filter := range httpFilters {
+		if err := filter.patchRoute(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 +196,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,12 @@ 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 {
+type JWT struct {
+}
+
+// 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 +196,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 +230,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 +314,7 @@ func routeContainsJWTAuthn(irRoute *ir.HTTPRoute) bool {
 
 	return false
 }
+
+func (*JWT) patchRouteConfig(*routev3.RouteConfiguration, *ir.HTTPListener) error {
+	return nil
+}

internal/xds/translator/oidc.go

@@ -34,10 +34,13 @@ const (
 	defaultSignoutPath          = "/signout"
 )
 
-// patchHCMWithOAuth2Filters builds and appends the oauth2 Filters to the HTTP
-// Connection Manager if applicable, and it does not already exist.
+type OIDC struct {
+}
+
+// patchHCM builds and appends the oauth2 Filters to the HTTP Connection Manager
+// if applicable, and it does not already exist.
 // Note: this method creates an oauth2 filter for each route that contains an OIDC config.
-func patchHCMWithOAuth2Filters(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error {
+func (*OIDC) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error {
 	var errs error
 
 	if mgr == nil {
@@ -177,6 +180,17 @@ func routeContainsOIDC(irRoute *ir.HTTPRoute) bool {
 	return false
 }
 
+func (*OIDC) patchResources(tCtx *types.ResourceVersionTable,
+	routes []*ir.HTTPRoute) error {
+	if err := createOAuth2TokenEndpointClusters(tCtx, routes); err != nil {
+		return err
+	}
+	if err := createOAuth2Secrets(tCtx, routes); err != nil {
+		return err
+	}
+	return nil
+}
+
 // createOAuth2TokenEndpointClusters creates token endpoint clusters from the
 // provided routes, if needed.
 func createOAuth2TokenEndpointClusters(tCtx *types.ResourceVersionTable,
@@ -340,11 +354,11 @@ func generateHMACSecretKey() ([]byte, error) {
 	return key, nil
 }
 
-// patchRouteCfgWithOAuth2Filter patches the provided route configuration with
-// the oauth2 filter if applicable.
+// patchRouteCfg patches the provided route configuration with the oauth2 filter
+// if applicable.
 // Note: this method disables all the oauth2 filters by default. The filter will
 // be enabled per-route in the typePerFilterConfig of the route.
-func patchRouteCfgWithOAuth2Filter(routeCfg *routev3.RouteConfiguration, irListener *ir.HTTPListener) error {
+func (*OIDC) patchRouteConfig(routeCfg *routev3.RouteConfiguration, irListener *ir.HTTPListener) error {
 	if routeCfg == nil {
 		return errors.New("route configuration is nil")
 	}
@@ -386,10 +400,9 @@ func patchRouteCfgWithOAuth2Filter(routeCfg *routev3.RouteConfiguration, irListe
 	return errs
 }
 
-// patchRouteWithOAuth2 patches the provided route with the oauth2 config if
-// applicable.
+// patchRoute patches the provided route with the oauth2 config if applicable.
 // Note: this method enables the corresponding oauth2 filter for the provided route.
-func patchRouteWithOAuth2(route *routev3.Route, irRoute *ir.HTTPRoute) error {
+func (*OIDC) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {
 	if route == nil {
 		return errors.New("xds route is nil")
 	}

internal/xds/translator/testdata/out/xds-ir/jwt-ratelimit.clusters.yaml

@@ -40,6 +40,28 @@
   outlierDetection: {}
   perConnectionBufferLimitBytes: 32768
   type: EDS
+- commonLbConfig:
+    localityWeightedLbConfig: {}
+  connectTimeout: 10s
+  dnsLookupFamily: V4_ONLY
+  edsClusterConfig:
+    edsConfig:
+      ads: {}
+      resourceApiVersion: V3
+    serviceName: "192_168_1_250_443"
+  lbPolicy: LEAST_REQUEST
+  name: "192_168_1_250_443"
+  outlierDetection: {}
+  perConnectionBufferLimitBytes: 32768
+  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
+  type: EDS
 - commonLbConfig:
     localityWeightedLbConfig: {}
   connectTimeout: 10s
@@ -82,25 +104,3 @@
       '@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
       explicitHttpConfig:
         http2ProtocolOptions: {}
-- commonLbConfig:
-    localityWeightedLbConfig: {}
-  connectTimeout: 10s
-  dnsLookupFamily: V4_ONLY
-  edsClusterConfig:
-    edsConfig:
-      ads: {}
-      resourceApiVersion: V3
-    serviceName: "192_168_1_250_443"
-  lbPolicy: LEAST_REQUEST
-  name: "192_168_1_250_443"
-  outlierDetection: {}
-  perConnectionBufferLimitBytes: 32768
-  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
-  type: EDS

internal/xds/translator/translator.go

@@ -248,25 +248,16 @@ func (t *Translator) processHTTPListenerXdsTranslation(tCtx *types.ResourceVersi
 			return err
 		}
 
-		// TODO: Make this into a generic interface for API Gateway features.
-		//       https://github.com/envoyproxy/gateway/issues/882
-		// Check if a ratelimit cluster exists, if not, add it, if its needed.
-		if err := t.createRateLimitServiceCluster(tCtx, httpListener); err != nil {
-			return err
-		}
-
-		// Create authn jwks clusters, if needed.
-		if err := createJWKSClusters(tCtx, httpListener.Routes); err != nil {
+		// Add all the other needed resources referenced by this filter to the
+		// resource version table.
+		if err := patchResources(tCtx, httpListener.Routes); err != nil {
 			return err
 		}
 
-		// Create oauth2 token endpoint clusters, if needed.
-		if err := createOAuth2TokenEndpointClusters(tCtx, httpListener.Routes); err != nil {
-			return err
-		}
-
-		// Create oauth2 client and HMAC secrets, if needed.
-		if err := createOAuth2Secrets(tCtx, httpListener.Routes); err != nil {
+		// RateLimit filter is handled separately because it relies on the global
+		// rate limit server configuration.
+		// Check if a ratelimit cluster exists, if not, add it, if it's needed.
+		if err := t.createRateLimitServiceCluster(tCtx, httpListener); err != nil {
 			return err
 		}