address review's comments

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>
Kong · Oct 23, 2023 · a54d0dd · a54d0dd
1 parent 5f4ec24
commit a54d0dd
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 16 deletions.
diff --git a/CHANGELOG.md b/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/
 

diff --git a/internal/dataplane/parser/translators/httproute.go b/internal/dataplane/parser/translators/httproute.go
@@ -352,8 +352,15 @@ 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)
+// ConvertFiltersToPlugins converts HTTPRouteFilter into Kong plugins.
+// The plugins can be set in two different ways:
+// - Direct conversion from the respective HTTPRouteFilter.
+// - ExtensionRef to plugins annotation from the ExtensionRef filter.
+func ConvertFiltersToPlugins(route *kongstate.Route, filters []gatewayapi.HTTPRouteFilter, path string, tags []*string) error {
+	plugins, pluginAnnotation, err := generatePluginsFromHTTPRouteFilters(filters, path, tags)
+	if err != nil {
+		return err
+	}
 	if route.Plugins == nil {
 		route.Plugins = make([]kong.Plugin, 0)
 	}
@@ -370,14 +377,18 @@ 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 plugin, generated by the conversion of all the filters but ExtensionRef.
+// - 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
@@ -393,7 +404,10 @@ func generatePluginsFromHTTPRouteFilters(filters []gatewayapi.HTTPRouteFilter, p
 			kongPlugins = append(kongPlugins, generateResponseHeaderModifierKongPlugin(filter.ResponseHeaderModifier))
 
 		case gatewayapi.HTTPRouteFilterExtensionRef:
-			plugin := generateExtensionRefKongPlugin(filter.ExtensionRef)
+			plugin, err := generateExtensionRefKongPlugin(filter.ExtensionRef)
+			if err != nil {
+				return nil, "", err
+			}
 			if len(pluginsAnnotation) > 0 {
 				pluginsAnnotation = fmt.Sprintf("%s,%s", pluginsAnnotation, plugin)
 			} else {
@@ -411,7 +425,7 @@ func generatePluginsFromHTTPRouteFilters(filters []gatewayapi.HTTPRouteFilter, p
 		p.Tags = tags
 	}
 
-	return kongPlugins, pluginsAnnotation
+	return kongPlugins, pluginsAnnotation, nil
 }
 
 // generateRequestRedirectKongPlugin generates configurations of plugins to satisfy the specification
@@ -459,8 +473,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

diff --git a/internal/dataplane/parser/translators/httproute_test.go b/internal/dataplane/parser/translators/httproute_test.go
@@ -1,6 +1,7 @@
 package translators
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/kong/go-kong/kong"
@@ -17,14 +18,15 @@ func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) {
 		path                      string
 		expectedPlugins           []kong.Plugin
 		expectedPluginsAnnotation string
+		expectedErr               error
 	}{
 		{
 			name:            "no filters",
 			filters:         []gatewayapi.HTTPRouteFilter{},
 			expectedPlugins: []kong.Plugin{},
 		},
 		{
-			name: "request header modifier",
+			name: "request header modifier filter",
 			filters: []gatewayapi.HTTPRouteFilter{
 				{
 					Type: gatewayapi.HTTPRouteFilterRequestHeaderModifier,
@@ -74,7 +76,7 @@ func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) {
 			},
 		},
 		{
-			name: "request redirect modifier",
+			name: "request redirect modifier filter",
 			filters: []gatewayapi.HTTPRouteFilter{
 				{
 					Type: gatewayapi.HTTPRouteFilterRequestRedirect,
@@ -105,7 +107,7 @@ func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) {
 			},
 		},
 		{
-			name: "response header modifier",
+			name: "response header modifier filter",
 			filters: []gatewayapi.HTTPRouteFilter{
 				{
 					Type: gatewayapi.HTTPRouteFilterResponseHeaderModifier,
@@ -155,20 +157,20 @@ func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) {
 			},
 		},
 		{
-			name: "extension-refs",
+			name: "valid extensionrefs filters",
 			filters: []gatewayapi.HTTPRouteFilter{
 				{
 					Type: gatewayapi.HTTPRouteFilterExtensionRef,
 					ExtensionRef: &gatewayapi.LocalObjectReference{
-						Group: gatewayapi.Group("configuration.konghq.com/v1"),
+						Group: gatewayapi.Group("configuration.konghq.com"),
 						Kind:  gatewayapi.Kind("KongPlugin"),
 						Name:  "plugin1",
 					},
 				},
 				{
 					Type: gatewayapi.HTTPRouteFilterExtensionRef,
 					ExtensionRef: &gatewayapi.LocalObjectReference{
-						Group: gatewayapi.Group("configuration.konghq.com/v1"),
+						Group: gatewayapi.Group("configuration.konghq.com"),
 						Kind:  gatewayapi.Kind("KongPlugin"),
 						Name:  "plugin2",
 					},
@@ -177,12 +179,43 @@ func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) {
 			expectedPluginsAnnotation: "plugin1,plugin2",
 			expectedPlugins:           []kong.Plugin{},
 		},
+		{
+			name: "invalid extensionrefs filter group",
+			filters: []gatewayapi.HTTPRouteFilter{
+				{
+					Type: gatewayapi.HTTPRouteFilterExtensionRef,
+					ExtensionRef: &gatewayapi.LocalObjectReference{
+						Group: gatewayapi.Group("wrong.group"),
+						Kind:  gatewayapi.Kind("KongPlugin"),
+						Name:  "plugin1",
+					},
+				},
+			},
+			expectedPluginsAnnotation: "",
+			expectedErr:               errors.New("plugin wrong.group/KongPlugin unsupported"),
+		},
+		{
+			name: "invalid extensionrefs filter kind",
+			filters: []gatewayapi.HTTPRouteFilter{
+				{
+					Type: gatewayapi.HTTPRouteFilterExtensionRef,
+					ExtensionRef: &gatewayapi.LocalObjectReference{
+						Group: gatewayapi.Group("configuration.konghq.com"),
+						Kind:  gatewayapi.Kind("WrongKind"),
+						Name:  "plugin1",
+					},
+				},
+			},
+			expectedPluginsAnnotation: "",
+			expectedErr:               errors.New("plugin configuration.konghq.com/WrongKind unsupported"),
+		},
 	}
 
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
-			plugins, pluginsAnnotation := generatePluginsFromHTTPRouteFilters(tc.filters, tc.path, nil)
+			plugins, pluginsAnnotation, err := generatePluginsFromHTTPRouteFilters(tc.filters, tc.path, nil)
+			require.Equal(t, tc.expectedErr, err)
 			require.Equal(t, tc.expectedPlugins, plugins)
 			require.Equal(t, tc.expectedPluginsAnnotation, pluginsAnnotation)
 		})