address review's comments

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

CHANGELOG.md

@@ -159,7 +159,6 @@ Adding a new version? You'll need three changes:
   version `gateway.networking.k8s.io/v1`.
 [#4935](https://github.com/Kong/kubernetes-ingress-controller/pull/4935)
 
-
 ### Added
 
 - Added support for expression-based Kong routes for `TLSRoute`. This requires
@@ -174,6 +173,10 @@ Adding a new version? You'll need three changes:
   [#4762](https://github.com/Kong/kubernetes-ingress-controller/pull/4762)
 - Support Query Parameter matching of `HTTPRoute` when expression router enabled.
   [#4780](https://github.com/Kong/kubernetes-ingress-controller/pull/4780)
+- Support `ExtensionRef` HTTPRoute filter. It is now possibile to set a KongPlugin
+  reference in the `HTTPRoute`s' `ExtensionRef` filter field.
+  [#4838](https://github.com/Kong/kubernetes-ingress-controller/pull/4838)
+
 
 [KIC Annotations reference]: https://docs.konghq.com/kubernetes-ingress-controller/latest/references/annotations/
 

internal/dataplane/parser/translate_httproute.go

@@ -266,11 +266,16 @@ func generateKongRoutesFromHTTPRouteMatches(
 		return filter.Type == gatewayapi.HTTPRouteFilterRequestRedirect
 	})
 
-	routes := getRoutesFromMatches(matches, &r, filters, tags, hasRedirectFilter)
+	routes, err := getRoutesFromMatches(matches, &r, filters, tags, hasRedirectFilter)
+	if err != nil {
+		return nil, err
+	}
 
 	// if the redirect filter has not been set, we still need to set the route plugins
 	if !hasRedirectFilter {
-		translators.ConvertFiltersToPlugins(&r, filters, "", tags)
+		if err := translators.SetRoutePlugins(&r, filters, "", tags); err != nil {
+			return nil, err
+		}
 		routes = []kongstate.Route{r}
 	}
 
@@ -284,7 +289,7 @@ func getRoutesFromMatches(
 	filters []gatewayapi.HTTPRouteFilter,
 	tags []*string,
 	hasRedirectFilter bool,
-) []kongstate.Route {
+) ([]kongstate.Route, error) {
 	seenMethods := make(map[string]struct{})
 	routes := make([]kongstate.Route, 0)
 
@@ -319,7 +324,9 @@ func getRoutesFromMatches(
 			}
 
 			// generate kong plugins from rule.filters
-			translators.ConvertFiltersToPlugins(matchRoute, filters, path, tags)
+			if err := translators.SetRoutePlugins(matchRoute, filters, path, tags); err != nil {
+				return nil, err
+			}
 
 			routes = append(routes, *route)
 		} else {
@@ -342,7 +349,7 @@ func getRoutesFromMatches(
 			}
 		}
 	}
-	return routes
+	return routes, nil
 }
 
 func generateKongRoutePathFromHTTPRouteMatch(match gatewayapi.HTTPRouteMatch) []string {
@@ -426,9 +433,14 @@ func (p *Parser) ingressRulesFromSplitHTTPRouteMatchWithPriority(
 		return err
 	}
 
+	additionalRoutes, err := translators.KongExpressionRouteFromHTTPRouteMatchWithPriority(httpRouteMatchWithPriority)
+	if err != nil {
+		return err
+	}
+
 	kongService.Routes = append(
 		kongService.Routes,
-		translators.KongExpressionRouteFromHTTPRouteMatchWithPriority(httpRouteMatchWithPriority),
+		*additionalRoutes,
 	)
 	// cache the service to avoid duplicates in further loop iterations
 	rules.ServiceNameToServices[serviceName] = kongService

internal/dataplane/parser/translate_httproute_test.go

@@ -1599,7 +1599,6 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) {
 							Expression:   kong.String(`http.path == "/v1/foo"`),
 							PreserveHost: kong.Bool(true),
 						},
-						Plugins:          []kong.Plugin{},
 						ExpressionRoutes: true,
 					},
 					{
@@ -1608,7 +1607,6 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) {
 							Expression:   kong.String(`http.path == "/v1/barr"`),
 							PreserveHost: kong.Bool(true),
 						},
-						Plugins:          []kong.Plugin{},
 						ExpressionRoutes: true,
 					},
 				},
@@ -1703,7 +1701,6 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) {
 							Expression:   kong.String(`(http.host == "foo.com") && (http.path == "/v1/foo")`),
 							PreserveHost: kong.Bool(true),
 						},
-						Plugins:          []kong.Plugin{},
 						ExpressionRoutes: true,
 					},
 				},
@@ -1714,7 +1711,6 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) {
 							Expression:   kong.String(`(http.host =^ ".bar.com") && (http.path == "/v1/foo")`),
 							PreserveHost: kong.Bool(true),
 						},
-						Plugins:          []kong.Plugin{},
 						ExpressionRoutes: true,
 					},
 				},
@@ -1725,7 +1721,6 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) {
 							Expression:   kong.String(`(http.host == "foo.com") && (http.path == "/v1/barr")`),
 							PreserveHost: kong.Bool(true),
 						},
-						Plugins:          []kong.Plugin{},
 						ExpressionRoutes: true,
 					},
 				},
@@ -1736,7 +1731,6 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) {
 							Expression:   kong.String(`(http.host =^ ".bar.com") && (http.path == "/v1/barr")`),
 							PreserveHost: kong.Bool(true),
 						},
-						Plugins:          []kong.Plugin{},
 						ExpressionRoutes: true,
 					},
 				},
@@ -1793,7 +1787,6 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) {
 							Expression:   kong.String(`(http.host == "foo.com") && (tls.sni == "foo.com") && (http.path == "/v1/foo")`),
 							PreserveHost: kong.Bool(true),
 						},
-						Plugins:          []kong.Plugin{},
 						ExpressionRoutes: true,
 					},
 				},
@@ -1896,7 +1889,6 @@ func TestIngressRulesFromSplitHTTPRouteMatchWithPriority(t *testing.T) {
 					StripPath:    kong.Bool(false),
 					Priority:     kong.Int(1024),
 				},
-				Plugins:          []kong.Plugin{},
 				ExpressionRoutes: true,
 			},
 		},
@@ -2037,7 +2029,6 @@ func TestIngressRulesFromSplitHTTPRouteMatchWithPriority(t *testing.T) {
 					StripPath:    kong.Bool(false),
 					Priority:     kong.Int(1024),
 				},
-				Plugins:          []kong.Plugin{},
 				ExpressionRoutes: true,
 			},
 		},
@@ -2090,7 +2081,6 @@ func TestIngressRulesFromSplitHTTPRouteMatchWithPriority(t *testing.T) {
 					StripPath:    kong.Bool(false),
 					Priority:     kong.Int(1024),
 				},
-				Plugins:          []kong.Plugin{},
 				ExpressionRoutes: true,
 			},
 		},
@@ -2140,7 +2130,6 @@ func TestIngressRulesFromSplitHTTPRouteMatchWithPriority(t *testing.T) {
 					StripPath:    kong.Bool(false),
 					Priority:     kong.Int(1024),
 				},
-				Plugins:          []kong.Plugin{},
 				ExpressionRoutes: true,
 			},
 		},

internal/dataplane/parser/translators/httproute.go

@@ -352,12 +352,21 @@ func mustMarshalJSON[T any](val T) string {
 	return string(key)
 }
 
-func ConvertFiltersToPlugins(route *kongstate.Route, filters []gatewayapi.HTTPRouteFilter, path string, tags []*string) {
-	plugins, pluginAnnotation := generatePluginsFromHTTPRouteFilters(filters, path, tags)
-	if route.Plugins == nil {
-		route.Plugins = make([]kong.Plugin, 0)
+// SetRoutePlugins converts HTTPRouteFilter into Kong plugins. The plugins are set into the given kongstate.Route.
+// The plugins can be set in two different ways:
+// - Direct conversion from the respective HTTPRouteFilter.
+// - ExtensionRef to plugins annotation from the ExtensionRef filter.
+func SetRoutePlugins(route *kongstate.Route, filters []gatewayapi.HTTPRouteFilter, path string, tags []*string) error {
+	plugins, pluginAnnotation, err := generatePluginsFromHTTPRouteFilters(filters, path, tags)
+	if err != nil {
+		return err
+	}
+	if len(plugins) > 0 {
+		if route.Plugins == nil {
+			route.Plugins = make([]kong.Plugin, 0)
+		}
+		route.Plugins = append(route.Plugins, plugins...)
 	}
-	route.Plugins = append(route.Plugins, plugins...)
 	if len(pluginAnnotation) > 0 {
 		if route.Ingress.Annotations == nil {
 			route.Ingress.Annotations = make(map[string]string)
@@ -370,17 +379,21 @@ func ConvertFiltersToPlugins(route *kongstate.Route, filters []gatewayapi.HTTPRo
 				pluginAnnotation)
 		}
 	}
+	return nil
 }
 
 // generatePluginsFromHTTPRouteFilters converts HTTPRouteFilter into Kong plugins.
 // path is the parameter to be used by the redirect plugin, to perform redirection.
-func generatePluginsFromHTTPRouteFilters(filters []gatewayapi.HTTPRouteFilter, path string, tags []*string) ([]kong.Plugin, string) {
+// It returns two values:
+// - A set of plugins generated by the conversion of all the provided filters, excluding  ExtensionRefs.
+// - A plugins annotation value, generated by the ExtensionRef filter.
+func generatePluginsFromHTTPRouteFilters(filters []gatewayapi.HTTPRouteFilter, path string, tags []*string) ([]kong.Plugin, string, error) {
 	kongPlugins := make([]kong.Plugin, 0)
 	if len(filters) == 0 {
-		return kongPlugins, ""
+		return kongPlugins, "", nil
 	}
 
-	var pluginsAnnotation string
+	var pluginsAnnotation strings.Builder
 	for _, filter := range filters {
 		switch filter.Type {
 		case gatewayapi.HTTPRouteFilterRequestHeaderModifier:
@@ -393,16 +406,20 @@ func generatePluginsFromHTTPRouteFilters(filters []gatewayapi.HTTPRouteFilter, p
 			kongPlugins = append(kongPlugins, generateResponseHeaderModifierKongPlugin(filter.ResponseHeaderModifier))
 
 		case gatewayapi.HTTPRouteFilterExtensionRef:
-			plugin := generateExtensionRefKongPlugin(filter.ExtensionRef)
-			if len(pluginsAnnotation) > 0 {
-				pluginsAnnotation = fmt.Sprintf("%s,%s", pluginsAnnotation, plugin)
+			plugin, err := generateExtensionRefKongPlugin(filter.ExtensionRef)
+			if err != nil {
+				return nil, "", err
+			}
+			if len(pluginsAnnotation.String()) > 0 {
+				fmt.Fprintf(&pluginsAnnotation, ",%s", plugin)
 			} else {
-				pluginsAnnotation = plugin
+				pluginsAnnotation.WriteString(plugin)
 			}
 
 		case gatewayapi.HTTPRouteFilterRequestMirror,
 			gatewayapi.HTTPRouteFilterURLRewrite:
 			// not supported
+			return nil, "", fmt.Errorf("httpFilter %s unsupported", filter.Type)
 		}
 	}
 	for _, p := range kongPlugins {
@@ -411,7 +428,7 @@ func generatePluginsFromHTTPRouteFilters(filters []gatewayapi.HTTPRouteFilter, p
 		p.Tags = tags
 	}
 
-	return kongPlugins, pluginsAnnotation
+	return kongPlugins, pluginsAnnotation.String(), nil
 }
 
 // generateRequestRedirectKongPlugin generates configurations of plugins to satisfy the specification
@@ -459,8 +476,11 @@ func generateRequestRedirectKongPlugin(modifier *gatewayapi.HTTPRequestRedirectF
 	return plugins
 }
 
-func generateExtensionRefKongPlugin(modifier *gatewayapi.LocalObjectReference) string {
-	return string(modifier.Name)
+func generateExtensionRefKongPlugin(modifier *gatewayapi.LocalObjectReference) (string, error) {
+	if modifier.Group != "configuration.konghq.com" || modifier.Kind != "KongPlugin" {
+		return "", fmt.Errorf("plugin %s/%s unsupported", modifier.Group, modifier.Kind)
+	}
+	return string(modifier.Name), nil
 }
 
 // generateRequestHeaderModifierKongPlugin converts a gatewayapi.HTTPRequestHeaderFilter into a

internal/dataplane/parser/translators/httproute_atc.go

@@ -66,7 +66,9 @@ func GenerateKongExpressionRoutesFromHTTPRouteMatches(
 
 	atc.ApplyExpression(&r.Route, routeMatcher, 1)
 	// generate plugins.
-	ConvertFiltersToPlugins(&r, translation.Filters, "", tags)
+	if err := SetRoutePlugins(&r, translation.Filters, "", tags); err != nil {
+		return nil, err
+	}
 	return []kongstate.Route{r}, nil
 }
 
@@ -103,7 +105,9 @@ func generateKongExpressionRoutesWithRequestRedirectFilter(
 		if match.Path != nil && match.Path.Value != nil {
 			path = *match.Path.Value
 		}
-		ConvertFiltersToPlugins(&matchRoute, translation.Filters, path, tags)
+		if err := SetRoutePlugins(&matchRoute, translation.Filters, path, tags); err != nil {
+			return nil, err
+		}
 		routes = append(routes, matchRoute)
 	}
 	return routes, nil
@@ -537,7 +541,7 @@ func compareSplitHTTPRouteMatchesRelativePriority(match1, match2 SplitHTTPRouteM
 // based kong route with assigned priority.
 func KongExpressionRouteFromHTTPRouteMatchWithPriority(
 	httpRouteMatchWithPriority SplitHTTPRouteMatchToKongRoutePriority,
-) kongstate.Route {
+) (*kongstate.Route, error) {
 	match := httpRouteMatchWithPriority.Match
 	httproute := httpRouteMatchWithPriority.Match.Source
 	tags := util.GenerateTagsForObject(httproute)
@@ -556,7 +560,7 @@ func KongExpressionRouteFromHTTPRouteMatchWithPriority(
 		match.MatchIndex,
 	)
 
-	r := kongstate.Route{
+	r := &kongstate.Route{
 		Route: kong.Route{
 			Name:         kong.String(routeName),
 			PreserveHost: kong.Bool(true),
@@ -590,10 +594,12 @@ func KongExpressionRouteFromHTTPRouteMatchWithPriority(
 			path = *match.Match.Path.Value
 		}
 
-		ConvertFiltersToPlugins(&r, rule.Filters, path, tags)
+		if err := SetRoutePlugins(r, rule.Filters, path, tags); err != nil {
+			return nil, err
+		}
 	}
 
-	return r
+	return r, nil
 }
 
 // KongServiceNameFromSplitHTTPRouteMatch generates service name from split HTTPRoute match.

internal/dataplane/parser/translators/httproute_atc_test.go

@@ -79,7 +79,6 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) {
 						Expression:   kong.String(`(http.path == "/prefix") || (http.path ^= "/prefix/")`),
 						Priority:     kong.Int(1),
 					},
-					Plugins:          []kong.Plugin{},
 					ExpressionRoutes: true,
 				},
 			},
@@ -101,7 +100,6 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) {
 						Expression:   kong.String(`((http.path == "/prefix") || (http.path ^= "/prefix/")) || ((http.path == "/exact") && (http.method == "GET"))`),
 						Priority:     kong.Int(1),
 					},
-					Plugins:          []kong.Plugin{},
 					ExpressionRoutes: true,
 				},
 			},
@@ -244,7 +242,6 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) {
 						Expression:   kong.String(`((http.path == "/prefix/0") || (http.path ^= "/prefix/0/")) && (http.host == "a.foo.com") && (tls.sni == "a.foo.com")`),
 						Priority:     kong.Int(1),
 					},
-					Plugins:          []kong.Plugin{},
 					ExpressionRoutes: true,
 				},
 			},