From 419cd078a7f7df717642a7d43c86cfbc7aaaab25 Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Tue, 24 Oct 2023 19:40:11 +0200 Subject: [PATCH] feat: HTTPRoute extensionRef filter converted onto plugin (#4838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: HTTPRoute extensionRef filter converted onto plugin Signed-off-by: Mattia Lavacca * address review's comments Signed-off-by: Mattia Lavacca * Apply suggestions from code review Co-authored-by: Jakub Warczarek * Apply suggestions from code review Co-authored-by: Patryk Małek --------- Signed-off-by: Mattia Lavacca Co-authored-by: Jakub Warczarek Co-authored-by: Patryk Małek --- CHANGELOG.md | 4 +- .../dataplane/parser/translate_httproute.go | 26 +++++-- .../parser/translate_httproute_test.go | 11 --- .../dataplane/parser/translators/httproute.go | 65 ++++++++++++++-- .../parser/translators/httproute_atc.go | 22 +++--- .../parser/translators/httproute_atc_test.go | 3 - .../parser/translators/httproute_test.go | 74 +++++++++++++++++-- internal/gatewayapi/aliases.go | 1 + 8 files changed, 159 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 517775f31f..dcf149e18c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -163,7 +163,6 @@ Adding a new version? You'll need three changes: to manage its own mTLS negotiation. [#4942](https://github.com/Kong/kubernetes-ingress-controller/pull/4942) - ### Added - Added support for expression-based Kong routes for `TLSRoute`. This requires @@ -178,6 +177,9 @@ 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/translate_httproute.go b/internal/dataplane/parser/translate_httproute.go index 23190ee5c2..2b964beb80 100644 --- a/internal/dataplane/parser/translate_httproute.go +++ b/internal/dataplane/parser/translate_httproute.go @@ -266,12 +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 { - plugins := translators.GeneratePluginsFromHTTPRouteFilters(filters, "", tags) - r.Plugins = append(r.Plugins, plugins...) + if err := translators.SetRoutePlugins(&r, filters, "", tags); err != nil { + return nil, err + } routes = []kongstate.Route{r} } @@ -285,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) @@ -320,8 +324,9 @@ func getRoutesFromMatches( } // generate kong plugins from rule.filters - plugins := translators.GeneratePluginsFromHTTPRouteFilters(filters, path, tags) - matchRoute.Plugins = append(matchRoute.Plugins, plugins...) + if err := translators.SetRoutePlugins(matchRoute, filters, path, tags); err != nil { + return nil, err + } routes = append(routes, *route) } else { @@ -344,7 +349,7 @@ func getRoutesFromMatches( } } } - return routes + return routes, nil } func generateKongRoutePathFromHTTPRouteMatch(match gatewayapi.HTTPRouteMatch) []string { @@ -428,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 diff --git a/internal/dataplane/parser/translate_httproute_test.go b/internal/dataplane/parser/translate_httproute_test.go index 00a46665e3..ee5b6adc4d 100644 --- a/internal/dataplane/parser/translate_httproute_test.go +++ b/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, }, }, diff --git a/internal/dataplane/parser/translators/httproute.go b/internal/dataplane/parser/translators/httproute.go index cd7a78ff9c..95b12e921b 100644 --- a/internal/dataplane/parser/translators/httproute.go +++ b/internal/dataplane/parser/translators/httproute.go @@ -9,6 +9,8 @@ import ( "github.com/kong/go-kong/kong" + "github.com/kong/kubernetes-ingress-controller/v2/internal/annotations" + "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate" "github.com/kong/kubernetes-ingress-controller/v2/internal/gatewayapi" ) @@ -350,14 +352,44 @@ func mustMarshalJSON[T any](val T) string { return string(key) } -// GeneratePluginsFromHTTPRouteFilters converts HTTPRouteFilter into Kong plugins. +// 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 + } + route.Plugins = append(route.Plugins, plugins...) + if len(pluginAnnotation) > 0 { + if route.Ingress.Annotations == nil { + route.Ingress.Annotations = make(map[string]string) + } + const pluginAnnotationKey = annotations.AnnotationPrefix + annotations.PluginsKey + if _, ok := route.Ingress.Annotations[pluginAnnotationKey]; !ok { + route.Ingress.Annotations[pluginAnnotationKey] = pluginAnnotation + } else { + route.Ingress.Annotations[pluginAnnotationKey] = fmt.Sprintf("%s,%s", + route.Ingress.Annotations[pluginAnnotationKey], + 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 { +// 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 strings.Builder for _, filter := range filters { switch filter.Type { case gatewayapi.HTTPRouteFilterRequestHeaderModifier: @@ -369,10 +401,24 @@ func GeneratePluginsFromHTTPRouteFilters(filters []gatewayapi.HTTPRouteFilter, p case gatewayapi.HTTPRouteFilterResponseHeaderModifier: kongPlugins = append(kongPlugins, generateResponseHeaderModifierKongPlugin(filter.ResponseHeaderModifier)) - case gatewayapi.HTTPRouteFilterExtensionRef, - gatewayapi.HTTPRouteFilterRequestMirror, + case gatewayapi.HTTPRouteFilterExtensionRef: + plugin, err := generateExtensionRefKongPlugin(filter.ExtensionRef) + if err != nil { + return nil, "", err + } + if len(pluginsAnnotation.String()) > 0 { + _, err := pluginsAnnotation.WriteString("," + plugin) + if err != nil { + return nil, "", err + } + } else { + pluginsAnnotation.WriteString(plugin) + } + + case gatewayapi.HTTPRouteFilterRequestMirror, gatewayapi.HTTPRouteFilterURLRewrite: // not supported + return nil, "", fmt.Errorf("httpFilter %s unsupported", filter.Type) } } for _, p := range kongPlugins { @@ -381,7 +427,7 @@ func GeneratePluginsFromHTTPRouteFilters(filters []gatewayapi.HTTPRouteFilter, p p.Tags = tags } - return kongPlugins + return kongPlugins, pluginsAnnotation.String(), nil } // generateRequestRedirectKongPlugin generates configurations of plugins to satisfy the specification @@ -429,6 +475,13 @@ func generateRequestRedirectKongPlugin(modifier *gatewayapi.HTTPRequestRedirectF return plugins } +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 // kong.Plugin of type request-transformer. func generateRequestHeaderModifierKongPlugin(modifier *gatewayapi.HTTPHeaderFilter) kong.Plugin { diff --git a/internal/dataplane/parser/translators/httproute_atc.go b/internal/dataplane/parser/translators/httproute_atc.go index aba0fcbb7a..ef1a109e24 100644 --- a/internal/dataplane/parser/translators/httproute_atc.go +++ b/internal/dataplane/parser/translators/httproute_atc.go @@ -66,8 +66,9 @@ func GenerateKongExpressionRoutesFromHTTPRouteMatches( atc.ApplyExpression(&r.Route, routeMatcher, 1) // generate plugins. - plugins := GeneratePluginsFromHTTPRouteFilters(translation.Filters, "", tags) - r.Plugins = plugins + if err := SetRoutePlugins(&r, translation.Filters, "", tags); err != nil { + return nil, err + } return []kongstate.Route{r}, nil } @@ -104,9 +105,9 @@ func generateKongExpressionRoutesWithRequestRedirectFilter( if match.Path != nil && match.Path.Value != nil { path = *match.Path.Value } - plugins := GeneratePluginsFromHTTPRouteFilters(translation.Filters, path, tags) - matchRoute.Plugins = plugins - + if err := SetRoutePlugins(&matchRoute, translation.Filters, path, tags); err != nil { + return nil, err + } routes = append(routes, matchRoute) } return routes, nil @@ -540,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) @@ -559,7 +560,7 @@ func KongExpressionRouteFromHTTPRouteMatchWithPriority( match.MatchIndex, ) - r := kongstate.Route{ + r := &kongstate.Route{ Route: kong.Route{ Name: kong.String(routeName), PreserveHost: kong.Bool(true), @@ -593,11 +594,12 @@ func KongExpressionRouteFromHTTPRouteMatchWithPriority( path = *match.Match.Path.Value } - plugins := GeneratePluginsFromHTTPRouteFilters(rule.Filters, path, tags) - r.Plugins = plugins + 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. diff --git a/internal/dataplane/parser/translators/httproute_atc_test.go b/internal/dataplane/parser/translators/httproute_atc_test.go index b097c8aee7..14c927ca2d 100644 --- a/internal/dataplane/parser/translators/httproute_atc_test.go +++ b/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, }, }, diff --git a/internal/dataplane/parser/translators/httproute_test.go b/internal/dataplane/parser/translators/httproute_test.go index 0451222818..6a6ad935b1 100644 --- 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" @@ -12,10 +13,12 @@ import ( func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) { testCases := []struct { - name string - filters []gatewayapi.HTTPRouteFilter - path string - expectedPlugins []kong.Plugin + name string + filters []gatewayapi.HTTPRouteFilter + path string + expectedPlugins []kong.Plugin + expectedPluginsAnnotation string + expectedErr error }{ { name: "no filters", @@ -23,7 +26,7 @@ func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) { expectedPlugins: []kong.Plugin{}, }, { - name: "request header modifier", + name: "request header modifier filter", filters: []gatewayapi.HTTPRouteFilter{ { Type: gatewayapi.HTTPRouteFilterRequestHeaderModifier, @@ -73,7 +76,7 @@ func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) { }, }, { - name: "request redirect modifier", + name: "request redirect modifier filter", filters: []gatewayapi.HTTPRouteFilter{ { Type: gatewayapi.HTTPRouteFilterRequestRedirect, @@ -104,7 +107,7 @@ func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) { }, }, { - name: "response header modifier", + name: "response header modifier filter", filters: []gatewayapi.HTTPRouteFilter{ { Type: gatewayapi.HTTPRouteFilterResponseHeaderModifier, @@ -153,13 +156,68 @@ func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) { }, }, }, + { + name: "valid extensionrefs filters", + filters: []gatewayapi.HTTPRouteFilter{ + { + Type: gatewayapi.HTTPRouteFilterExtensionRef, + ExtensionRef: &gatewayapi.LocalObjectReference{ + Group: gatewayapi.Group("configuration.konghq.com"), + Kind: gatewayapi.Kind("KongPlugin"), + Name: "plugin1", + }, + }, + { + Type: gatewayapi.HTTPRouteFilterExtensionRef, + ExtensionRef: &gatewayapi.LocalObjectReference{ + Group: gatewayapi.Group("configuration.konghq.com"), + Kind: gatewayapi.Kind("KongPlugin"), + Name: "plugin2", + }, + }, + }, + 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 - plugins := GeneratePluginsFromHTTPRouteFilters(tc.filters, tc.path, nil) t.Run(tc.name, func(t *testing.T) { + 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) }) } } diff --git a/internal/gatewayapi/aliases.go b/internal/gatewayapi/aliases.go index 5213293947..3dfb39a5ae 100644 --- a/internal/gatewayapi/aliases.go +++ b/internal/gatewayapi/aliases.go @@ -40,6 +40,7 @@ type ( HTTPRouteList = gatewayv1.HTTPRouteList HTTPRouteMatch = gatewayv1.HTTPRouteMatch HTTPRouteRule = gatewayv1.HTTPRouteRule + LocalObjectReference = gatewayv1.LocalObjectReference HTTPRouteSpec = gatewayv1.HTTPRouteSpec HTTPRouteStatus = gatewayv1.HTTPRouteStatus Hostname = gatewayv1.Hostname