diff --git a/CHANGELOG.md b/CHANGELOG.md index 26c577d1c5..d3a5ebe97c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -152,6 +152,9 @@ Adding a new version? You'll need three changes: of the controller. The old `all-in-one-dbless.yaml` manifest has been deprecated and renamed to `all-in-one-dbless-legacy.yaml`. It will be removed in a future release. [#3629](https://github.com/Kong/kubernetes-ingress-controller/pull/3629) +- The RequestRedirect Gateway API filter is now supported and translated + to the proper set of Kong plugins. + [#3702](https://github.com/Kong/kubernetes-ingress-controller/pull/3702) ### Fixed diff --git a/internal/dataplane/parser/translate_errors.go b/internal/dataplane/parser/translate_errors.go index 2a0df2ede4..00411a4322 100644 --- a/internal/dataplane/parser/translate_errors.go +++ b/internal/dataplane/parser/translate_errors.go @@ -4,7 +4,6 @@ import "errors" var ( errRouteValidationNoRules = errors.New("no rules provided") - errRouteValidationMissingBackendRefs = errors.New("missing backendRef in rule") errRouteValidationQueryParamMatchesUnsupported = errors.New("query param matches are not yet supported") errRouteValidationNoMatchRulesOrHostnamesSpecified = errors.New("no match rules or hostnames specified") ) diff --git a/internal/dataplane/parser/translate_httproute.go b/internal/dataplane/parser/translate_httproute.go index 3e33761b60..b7205cc420 100644 --- a/internal/dataplane/parser/translate_httproute.go +++ b/internal/dataplane/parser/translate_httproute.go @@ -2,9 +2,11 @@ package parser import ( "fmt" + pathlib "path" "strings" "github.com/kong/go-kong/kong" + "github.com/samber/lo" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate" @@ -63,11 +65,6 @@ func validateHTTPRoute(httproute *gatewayv1beta1.HTTPRoute) error { return errRouteValidationNoRules } - for _, rule := range spec.Rules { - if len(rule.BackendRefs) == 0 { - return errRouteValidationMissingBackendRefs - } - } return nil } @@ -88,11 +85,11 @@ func (p *Parser) ingressRulesFromHTTPRouteWithCombinedServiceRoutes(httproute *g // generate the routes for the service and attach them to the service for _, kongRouteTranslation := range kongServiceTranslation.KongRoutes { - route, err := generateKongRouteFromTranslation(httproute, kongRouteTranslation, p.flagEnabledRegexPathPrefix) + routes, err := generateKongRouteFromTranslation(httproute, kongRouteTranslation, p.flagEnabledRegexPathPrefix) if err != nil { return err } - service.Routes = append(service.Routes, route) + service.Routes = append(service.Routes, routes...) } // cache the service to avoid duplicates in further loop iterations @@ -172,9 +169,6 @@ func generateKongRoutesFromHTTPRouteRule( // all matches to determine all the routes that will be needed for the services. var routes []kongstate.Route - // generate kong plugins from rule.filters - plugins := generatePluginsFromHTTPRouteFilters(rule.Filters, tags) - if len(rule.Matches) > 0 { for matchNumber := range rule.Matches { // determine the name of the route, identify it as a route that belongs @@ -187,12 +181,12 @@ func generateKongRoutesFromHTTPRouteRule( matchNumber, ) - r, err := generateKongRouteFromHTTPRouteMatches( + r, err := generateKongRoutesFromHTTPRouteMatches( routeName, rule.Matches[matchNumber:matchNumber+1], + rule.Filters, objectInfo, hostnames, - plugins, addRegexPrefix, tags, ) @@ -201,18 +195,23 @@ func generateKongRoutesFromHTTPRouteRule( } // add the route to the list of routes for the service(s) - routes = append(routes, r) + routes = append(routes, r...) } } else { routeName := fmt.Sprintf("httproute.%s.%s.0.0", httproute.Namespace, httproute.Name) - r, err := generateKongRouteFromHTTPRouteMatches(routeName, rule.Matches, objectInfo, hostnames, plugins, - addRegexPrefix, tags) + r, err := generateKongRoutesFromHTTPRouteMatches(routeName, + rule.Matches, + rule.Filters, + objectInfo, + hostnames, + addRegexPrefix, + tags) if err != nil { return nil, err } // add the route to the list of routes for the service(s) - routes = append(routes, r) + routes = append(routes, r...) } return routes, nil @@ -222,7 +221,7 @@ func generateKongRouteFromTranslation( httproute *gatewayv1beta1.HTTPRoute, translation translators.KongRouteTranslation, addRegexPrefix bool, -) (kongstate.Route, error) { +) ([]kongstate.Route, error) { // gather the k8s object information and hostnames from the httproute objectInfo := util.FromK8sObject(httproute) tags := util.GenerateTagsForObject(httproute) @@ -230,31 +229,28 @@ func generateKongRouteFromTranslation( // get the hostnames from the HTTPRoute hostnames := getHTTPRouteHostnamesAsSliceOfStringPointers(httproute) - // generate kong plugins from rule.filters - plugins := generatePluginsFromHTTPRouteFilters(translation.Filters, tags) - - return generateKongRouteFromHTTPRouteMatches( + return generateKongRoutesFromHTTPRouteMatches( translation.Name, translation.Matches, + translation.Filters, objectInfo, hostnames, - plugins, addRegexPrefix, tags, ) } -// generateKongRouteFromHTTPRouteMatches converts an HTTPRouteMatches to a Kong Route object. +// generateKongRoutesFromHTTPRouteMatches converts an HTTPRouteMatches to a slice of Kong Route objects. // This function assumes that the HTTPRouteMatches share the query params, headers and methods. -func generateKongRouteFromHTTPRouteMatches( +func generateKongRoutesFromHTTPRouteMatches( routeName string, matches []gatewayv1beta1.HTTPRouteMatch, + filters []gatewayv1beta1.HTTPRouteFilter, ingressObjectInfo util.K8sObjectInfo, hostnames []*string, - plugins []kong.Plugin, addRegexPrefix bool, tags []*string, -) (kongstate.Route, error) { +) ([]kongstate.Route, error) { if len(matches) == 0 { // it's acceptable for an HTTPRoute to have no matches in the rulesets, // but only backends as long as there are hostnames. In this case, we @@ -273,21 +269,18 @@ func generateKongRouteFromHTTPRouteMatches( // however in this case there must actually be some present hostnames // configured for the HTTPRoute or else it's not valid. if len(hostnames) == 0 { - return kongstate.Route{}, errRouteValidationNoMatchRulesOrHostnamesSpecified + return []kongstate.Route{}, errRouteValidationNoMatchRulesOrHostnamesSpecified } // otherwise apply the hostnames to the route r.Hosts = append(r.Hosts, hostnames...) - // attach the plugins to be applied to the given route - r.Plugins = append(r.Plugins, plugins...) - - return r, nil + return []kongstate.Route{r}, nil } // TODO: implement query param matches (https://github.com/Kong/kubernetes-ingress-controller/issues/2778) if len(matches[0].QueryParams) > 0 { - return kongstate.Route{}, errRouteValidationQueryParamMatchesUnsupported + return []kongstate.Route{}, errRouteValidationQueryParamMatchesUnsupported } r := generateKongstateHTTPRoute(routeName, ingressObjectInfo, hostnames) @@ -296,7 +289,7 @@ func generateKongRouteFromHTTPRouteMatches( // convert header matching from HTTPRoute to Route format headers, err := convertGatewayMatchHeadersToKongRouteMatchHeaders(matches[0].Headers) if err != nil { - return kongstate.Route{}, err + return []kongstate.Route{}, err } if len(headers) > 0 { r.Route.Headers = headers @@ -305,40 +298,90 @@ func generateKongRouteFromHTTPRouteMatches( // stripPath needs to be disabled by default to be conformant with the Gateway API r.StripPath = kong.Bool(false) - // attach the plugins to be applied to the given route - if len(plugins) != 0 { - if r.Plugins == nil { - r.Plugins = make([]kong.Plugin, 0, len(plugins)) - } + _, hasRedirectFilter := lo.Find(filters, func(filter gatewayv1beta1.HTTPRouteFilter) bool { + return filter.Type == gatewayv1beta1.HTTPRouteFilterRequestRedirect + }) + + routes := getRoutesFromMatches(matches, &r, filters, tags, hasRedirectFilter, addRegexPrefix) + + // if the redirect filter has not been set, we still need to set the route plugins + if !hasRedirectFilter { + plugins := generatePluginsFromHTTPRouteFilters(filters, "", tags) r.Plugins = append(r.Plugins, plugins...) + routes = []kongstate.Route{r} } + return routes, nil +} + +// getRoutesFromMatches converts all the httpRoute matches to the proper set of kong routes. +func getRoutesFromMatches(matches []gatewayv1beta1.HTTPRouteMatch, + route *kongstate.Route, + filters []gatewayv1beta1.HTTPRouteFilter, + tags []*string, + hasRedirectFilter bool, + addRegexPrefix bool, +) []kongstate.Route { seenMethods := make(map[string]struct{}) + routes := make([]kongstate.Route, 0) for _, match := range matches { - // configure path matching information about the route if paths matching was defined - // Kong automatically infers whether or not a path is a regular expression and uses a prefix match by - // default if it is not. For those types, we use the path value as-is and let Kong determine the type. - // For exact matches, we transform the path into a regular expression that terminates after the value - if match.Path != nil { - paths := generateKongRoutePathFromHTTPRouteMatch(match, addRegexPrefix) - for _, p := range paths { - r.Route.Paths = append(r.Route.Paths, kong.String(p)) + // if the rule specifies the redirectFilter, we cannot put all the paths under the same route, + // as the kong plugin needs to know the exact path to use to perform redirection. + if hasRedirectFilter { + matchRoute := route + // configure path matching information about the route if paths matching was defined + // Kong automatically infers whether or not a path is a regular expression and uses a prefix match by + // default if it is not. For those types, we use the path value as-is and let Kong determine the type. + // For exact matches, we transform the path into a regular expression that terminates after the value + if match.Path != nil { + paths := generateKongRoutePathFromHTTPRouteMatch(match, addRegexPrefix) + for _, p := range paths { + matchRoute.Route.Paths = append(matchRoute.Route.Paths, kong.String(p)) + } } - } - // configure method matching information about the route if method - // matching was defined. - if match.Method != nil { - method := string(*match.Method) - if _, ok := seenMethods[method]; !ok { - r.Route.Methods = append(r.Route.Methods, kong.String(string(*match.Method))) - seenMethods[method] = struct{}{} + // configure method matching information about the route if method + // matching was defined. + if match.Method != nil { + method := string(*match.Method) + if _, ok := seenMethods[method]; !ok { + matchRoute.Route.Methods = append(matchRoute.Route.Methods, kong.String(string(*match.Method))) + seenMethods[method] = struct{}{} + } + } + path := "" + if match.Path.Value != nil { + path = *match.Path.Value + } + + // generate kong plugins from rule.filters + plugins := generatePluginsFromHTTPRouteFilters(filters, path, tags) + matchRoute.Plugins = append(matchRoute.Plugins, plugins...) + + routes = append(routes, *route) + } else { + // configure path matching information about the route if paths matching was defined + // Kong automatically infers whether or not a path is a regular expression and uses a prefix match by + // default if it is not. For those types, we use the path value as-is and let Kong determine the type. + // For exact matches, we transform the path into a regular expression that terminates after the value + if match.Path != nil { + paths := generateKongRoutePathFromHTTPRouteMatch(match, addRegexPrefix) + for _, p := range paths { + route.Route.Paths = append(route.Route.Paths, kong.String(p)) + } + } + + if match.Method != nil { + method := string(*match.Method) + if _, ok := seenMethods[method]; !ok { + route.Route.Methods = append(route.Route.Methods, kong.String(string(*match.Method))) + seenMethods[method] = struct{}{} + } } } } - - return r, nil + return routes } func generateKongRoutePathFromHTTPRouteMatch(match gatewayv1beta1.HTTPRouteMatch, addRegexPrefix bool) []string { @@ -392,26 +435,79 @@ func generateKongstateHTTPRoute(routeName string, ingressObjectInfo util.K8sObje return r } -// generatePluginsFromHTTPRouteFilters converts HTTPRouteFilter into Kong filters. -func generatePluginsFromHTTPRouteFilters(filters []gatewayv1beta1.HTTPRouteFilter, tags []*string) []kong.Plugin { +// generatePluginsFromHTTPRouteFilters converts HTTPRouteFilter into Kong plugins. +// path is the parameter to be used by the redirect plugin, to perform redirection. +func generatePluginsFromHTTPRouteFilters(filters []gatewayv1beta1.HTTPRouteFilter, path string, tags []*string) []kong.Plugin { kongPlugins := make([]kong.Plugin, 0) if len(filters) == 0 { return kongPlugins } for _, filter := range filters { - if filter.Type == gatewayv1beta1.HTTPRouteFilterRequestHeaderModifier { + switch filter.Type { + case gatewayv1beta1.HTTPRouteFilterRequestHeaderModifier: kongPlugins = append(kongPlugins, generateRequestHeaderModifierKongPlugin(filter.RequestHeaderModifier)) + + case gatewayv1beta1.HTTPRouteFilterRequestRedirect: + kongPlugins = append(kongPlugins, generateRequestRedirectKongPlugin(filter.RequestRedirect, path)...) + + case gatewayv1beta1.HTTPRouteFilterExtensionRef, + gatewayv1beta1.HTTPRouteFilterRequestMirror, + gatewayv1beta1.HTTPRouteFilterResponseHeaderModifier, + gatewayv1beta1.HTTPRouteFilterURLRewrite: + // not supported } - // TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/2793 } for _, p := range kongPlugins { + // This plugin is derived from an HTTPRoute filter, not a KongPlugin, so we apply tags indicating that + // HTTPRoute as the parent Kubernetes resource for these generated plugins. p.Tags = tags } return kongPlugins } +func generateRequestRedirectKongPlugin(modifier *gatewayv1beta1.HTTPRequestRedirectFilter, path string) []kong.Plugin { + plugins := make([]kong.Plugin, 2) + plugins[0] = kong.Plugin{ + Name: kong.String("request-termination"), + Config: kong.Configuration{ + "status_code": modifier.StatusCode, + }, + } + + var locationHeader string + scheme := "http" + port := 80 + + if modifier.Scheme != nil { + scheme = *modifier.Scheme + } + if modifier.Port != nil { + port = int(*modifier.Port) + } + if modifier.Path != nil && modifier.Path.Type == gatewayv1beta1.FullPathHTTPPathModifier && modifier.Path.ReplaceFullPath != nil { + // only ReplaceFullPath currently supported + path = *modifier.Path.ReplaceFullPath + } + if modifier.Hostname != nil { + locationHeader = fmt.Sprintf("Location: %s://%s", scheme, pathlib.Join(fmt.Sprintf("%s:%d", *modifier.Hostname, port), path)) + } else { + locationHeader = fmt.Sprintf("Location: %s", path) + } + + plugins[1] = kong.Plugin{ + Name: kong.String("response-transformer"), + Config: kong.Configuration{ + "add": map[string][]string{ + "headers": {locationHeader}, + }, + }, + } + + return plugins +} + // generateRequestHeaderModifierKongPlugin converts a gatewayv1beta1.HTTPRequestHeaderFilter into a // kong.Plugin of type request-transformer. func generateRequestHeaderModifierKongPlugin(modifier *gatewayv1beta1.HTTPHeaderFilter) kong.Plugin { diff --git a/internal/dataplane/parser/translate_httproute_test.go b/internal/dataplane/parser/translate_httproute_test.go index 1687efdc40..4cf7b9859e 100644 --- a/internal/dataplane/parser/translate_httproute_test.go +++ b/internal/dataplane/parser/translate_httproute_test.go @@ -1352,6 +1352,98 @@ func TestGetHTTPRouteHostnamesAsSliceOfStringPointers(t *testing.T) { } } +func TestGeneratePluginsFromHTTPRouteFilters(t *testing.T) { + testCases := []struct { + name string + filters []gatewayv1beta1.HTTPRouteFilter + path string + expectedPlugins []kong.Plugin + }{ + { + name: "no filters", + filters: []gatewayv1beta1.HTTPRouteFilter{}, + expectedPlugins: []kong.Plugin{}, + }, + { + name: "request header modifier", + filters: []gatewayv1beta1.HTTPRouteFilter{ + { + Type: gatewayv1beta1.HTTPRouteFilterRequestHeaderModifier, + RequestHeaderModifier: &gatewayv1beta1.HTTPHeaderFilter{ + Set: []gatewayv1beta1.HTTPHeader{ + { + Name: "header-to-set", + Value: "bar", + }, + }, + Remove: []string{"header-to-remove"}, + }, + }, + }, + expectedPlugins: []kong.Plugin{ + { + Name: kong.String("request-transformer"), + Config: kong.Configuration{ + "add": map[string][]string{ + "headers": { + "header-to-set:bar", + }, + }, + "remove": map[string][]string{ + "headers": { + "header-to-remove", + }, + }, + "replace": map[string][]string{ + "headers": { + "header-to-set:bar", + }, + }, + }, + }, + }, + }, + { + name: "request redirect modifier", + filters: []gatewayv1beta1.HTTPRouteFilter{ + { + Type: gatewayv1beta1.HTTPRouteFilterRequestRedirect, + RequestRedirect: &gatewayv1beta1.HTTPRequestRedirectFilter{ + Hostname: (*gatewayv1beta1.PreciseHostname)(lo.ToPtr("example.org")), + StatusCode: lo.ToPtr(302), + }, + }, + }, + path: "/test", + expectedPlugins: []kong.Plugin{ + { + Name: kong.String("request-termination"), + Config: kong.Configuration{ + "status_code": lo.ToPtr(302), + }, + }, + { + Name: kong.String("response-transformer"), + Config: kong.Configuration{ + "add": map[string][]string{ + "headers": { + "Location: http://example.org:80/test", + }, + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + plugins := generatePluginsFromHTTPRouteFilters(tc.filters, tc.path, nil) + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expectedPlugins, plugins) + }) + } +} + func TestIngressRulesFromHTTPRoutes_RegexPrefix(t *testing.T) { fakestore, err := store.NewFakeStore(store.FakeObjects{}) require.NoError(t, err) diff --git a/internal/dataplane/parser/translate_utils.go b/internal/dataplane/parser/translate_utils.go index ec386ca94d..fe118fc256 100644 --- a/internal/dataplane/parser/translate_utils.go +++ b/internal/dataplane/parser/translate_utils.go @@ -83,10 +83,6 @@ func generateKongServiceFromBackendRefWithName( ) (kongstate.Service, error) { objName := fmt.Sprintf("%s %s/%s", route.GetObjectKind().GroupVersionKind().String(), route.GetNamespace(), route.GetName()) - if len(backendRefs) == 0 { - return kongstate.Service{}, fmt.Errorf("no backendRefs present for %s, cannot build Kong service", objName) - } - grants, err := storer.ListReferenceGrants() if err != nil { return kongstate.Service{}, fmt.Errorf("could not retrieve ReferenceGrants for %s: %w", objName, err) @@ -126,7 +122,7 @@ func generateKongServiceFromBackendRefWithName( // the response must have a status code of 500. Since The default behavior of Kong is returning 503 // if there is no backend for a service, we inject a plugin that terminates all requests with 500 // as status code - if len(service.Backends) == 0 { + if len(service.Backends) == 0 && len(backendRefs) != 0 { if service.Plugins == nil { service.Plugins = make([]kong.Plugin, 0) } diff --git a/internal/dataplane/parser/translate_utils_test.go b/internal/dataplane/parser/translate_utils_test.go index 6175b4fd4f..c332f8080f 100644 --- a/internal/dataplane/parser/translate_utils_test.go +++ b/internal/dataplane/parser/translate_utils_test.go @@ -411,13 +411,6 @@ func TestGenerateKongServiceFromBackendRef(t *testing.T) { result kongstate.Service wantErr bool }{ - { - msg: "empty backend list", - route: &gatewayv1beta1.HTTPRoute{}, - refs: []gatewayv1beta1.BackendRef{}, - result: kongstate.Service{}, - wantErr: true, - }, { msg: "all backends in route namespace", route: &gatewayv1beta1.HTTPRoute{ diff --git a/test/conformance/gateway_conformance_test.go b/test/conformance/gateway_conformance_test.go index 01bd7777f6..ec349f7dd3 100644 --- a/test/conformance/gateway_conformance_test.go +++ b/test/conformance/gateway_conformance_test.go @@ -84,7 +84,6 @@ func TestGatewayConformance(t *testing.T) { // standard conformance tests.HTTPRouteHeaderMatching.ShortName, - tests.HTTPRouteRedirectHostAndStatus.ShortName, // extended conformance // https://github.com/Kong/kubernetes-ingress-controller/issues/3680 diff --git a/test/integration/translation_failures_test.go b/test/integration/translation_failures_test.go index e406c7073c..36f60f0c42 100644 --- a/test/integration/translation_failures_test.go +++ b/test/integration/translation_failures_test.go @@ -249,35 +249,6 @@ func TestTranslationFailures(t *testing.T) { } }, }, - { - name: "httproute rule has no backendRefs defined", - translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) expectedTranslationFailure { - gatewayClient, err := gatewayclient.NewForConfig(env.Cluster().Config()) - require.NoError(t, err) - - gatewayClassName := testutils.RandomName(testTranslationFailuresObjectsPrefix) - gwc, err := DeployGatewayClass(ctx, gatewayClient, gatewayClassName) - require.NoError(t, err) - cleaner.Add(gwc) - - gatewayName := testutils.RandomName(testTranslationFailuresObjectsPrefix) - gateway, err := DeployGateway(ctx, gatewayClient, ns, gatewayClassName, func(gw *gatewayv1beta1.Gateway) { - gw.Name = gatewayName - }) - require.NoError(t, err) - cleaner.Add(gateway) - - httpRoute := httpRouteWithBackends(gatewayName) - httpRoute, err = gatewayClient.GatewayV1beta1().HTTPRoutes(ns).Create(ctx, httpRoute, metav1.CreateOptions{}) - require.NoError(t, err) - cleaner.Add(httpRoute) - - return expectedTranslationFailure{ - causingObjects: []client.Object{httpRoute}, - reasonContains: "missing backendRef in rule", - } - }, - }, } for _, tt := range testCases {