Skip to content

Commit

Permalink
feat: implement rewrite path prefix filter for HTTPRoute in expressio…
Browse files Browse the repository at this point in the history
…ns router mode
  • Loading branch information
czeslavo committed May 6, 2024
1 parent 5687b74 commit 7090f7b
Show file tree
Hide file tree
Showing 7 changed files with 183 additions and 23 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ Adding a new version? You'll need three changes:
[#5894](https://github.com/Kong/kubernetes-ingress-controller/pull/5894)
- Add support in `HTTPRoute`s for `URLRewrite`:
- `FullPathRewrite` [#5855](https://github.com/Kong/kubernetes-ingress-controller/pull/5855)
- `ReplacePrefixMatch` [#5895](https://github.com/Kong/kubernetes-ingress-controller/pull/5895)
- `ReplacePrefixMatch` for both router modes:
- `traditional_compatible`: [#5895](https://github.com/Kong/kubernetes-ingress-controller/pull/5895)
- `expressions`: [#5940](https://github.com/Kong/kubernetes-ingress-controller/pull/5940)
- DB mode now supports Event reporting for resources that failed to apply.
[#5785](https://github.com/Kong/kubernetes-ingress-controller/pull/5785)

Expand Down
58 changes: 44 additions & 14 deletions internal/dataplane/translator/subtranslator/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/kongstate"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator/atc"
"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
)

Expand Down Expand Up @@ -650,12 +651,7 @@ func generateRequestTransformerForURLRewrite(
return plugin, nil, nil

case gatewayapi.PrefixMatchHTTPPathModifier:
if expressionsRouterEnabled {
// TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/3686
return kong.Plugin{}, nil, fmt.Errorf("%s unsupported with expressions router", gatewayapi.PrefixMatchHTTPPathModifier)
}

plugin, routeModifier := generateRequestTransformerForURLRewritePrefixMatch(filter, path)
plugin, routeModifier := generateRequestTransformerForURLRewritePrefixMatch(filter, path, expressionsRouterEnabled)
return plugin, routeModifier, nil
}
}
Expand Down Expand Up @@ -687,7 +683,14 @@ func generateRequestTransformerForURLRewriteFullPath(filter *gatewayapi.HTTPURLR

// generateRequestTransformerForURLRewritePrefixMatch generates a request-transformer plugin and a route modifier
// for the URLRewrite filter with a PrefixMatchHTTPPathModifier.
func generateRequestTransformerForURLRewritePrefixMatch(filter *gatewayapi.HTTPURLRewriteFilter, path string) (kong.Plugin, kongRouteModifier) {
func generateRequestTransformerForURLRewritePrefixMatch(
filter *gatewayapi.HTTPURLRewriteFilter,
path string,
expressionsRouterEnabled bool,
) (kong.Plugin, kongRouteModifier) {
// Normalize the path before passing it down.
path = normalizePath(path)

return kong.Plugin{
Name: kong.String("request-transformer"),
Config: kong.Configuration{
Expand All @@ -698,7 +701,7 @@ func generateRequestTransformerForURLRewritePrefixMatch(filter *gatewayapi.HTTPU
),
},
},
}, generateKongRouteModifierForURLRewritePrefixMatch(path)
}, generateKongRouteModifierForURLRewritePrefixMatch(path, expressionsRouterEnabled)
}

// generateRequestTransformerReplaceURIForURLRewritePrefixMatch generates the replacement URI for the request-transformer
Expand Down Expand Up @@ -757,10 +760,28 @@ func generateRequestTransformerReplaceURIForURLRewritePrefixMatch(
// Please note that depending on the path, the capture group will:
// - Exclude the leading slash if the path is root.
// - Include the leading slash otherwise.
func generateKongRouteModifierForURLRewritePrefixMatch(path string) func(route *kongstate.Route) {
func generateKongRouteModifierForURLRewritePrefixMatch(path string, expressionsRouterEnabled bool) func(route *kongstate.Route) {
pathIsRoot := isPathRoot(path)
// We need to change the paths in the Kong route to a regex with a capture group that can be used
// in the request-transformer plugin.

// If expressions router is enabled, we need to set the expression on the Kong Route.
if expressionsRouterEnabled {
return func(route *kongstate.Route) {
exactPrefixPredicate := atc.NewPredicateHTTPPath(atc.OpEqual, path)
subpathsPredicate := func() atc.Predicate {
if pathIsRoot {
// If the path is "/", we don't capture the slash as Kong Route's path has to begin with a slash.
// If we captured the slash, we'd generate "(/.*)", and it'd be rejected by Kong.
return atc.NewPredicateHTTPPath(atc.OpRegexMatch, "^/(.*)")
}
// If the path is not "/", i.e. it has a prefix, we capture the slash to make it possible to
// route "/prefix" to "/replacement" and "/prefix/" to "/replacement/" correctly.
return atc.NewPredicateHTTPPath(atc.OpRegexMatch, fmt.Sprintf("^%s(/.*)", path))
}()
route.Route.Expression = lo.ToPtr(atc.Or(exactPrefixPredicate, subpathsPredicate).Expression())
}
}

// Otherwise, we set the Kong Route's paths.
return func(route *kongstate.Route) {
paths := make([]*string, 0, 2)
// The first path matches the exact path.
Expand All @@ -774,14 +795,23 @@ func generateKongRouteModifierForURLRewritePrefixMatch(path string) func(route *
// If the path is not "/", i.e. it has a prefix, we capture the slash to make it possible to
// route "/prefix" to "/replacement" and "/prefix/" to "/replacement/" correctly.
paths = append(paths, lo.ToPtr(
fmt.Sprintf("%s%s(/.*)", KongPathRegexPrefix, strings.TrimSuffix(path, "/"))),
fmt.Sprintf("%s%s(/.*)", KongPathRegexPrefix, path)),
)
}
route.Paths = paths
}
}

// normalizePath normalizes the path by making it:
// - a slash if it's empty or "/",
// - a path without trailing slash otherwise.
func normalizePath(path string) string {
if path == "/" || path == "" {
return "/"
}
return strings.TrimSuffix(path, "/")
}

func isPathRoot(path string) bool {
// Path is considered root if it's "/" or empty.
return path == "/" || path == ""
return path == "/"
}
88 changes: 88 additions & 0 deletions internal/dataplane/translator/subtranslator/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,32 @@ func TestGenerateRequestTransformerForURLRewrite(t *testing.T) {
},
},
},
{
name: "URLRewriteFilter with empty firstMatchPath",
modifier: &gatewayapi.HTTPURLRewriteFilter{
Path: &gatewayapi.HTTPPathModifier{
Type: gatewayapi.PrefixMatchHTTPPathModifier,
ReplacePrefixMatch: lo.ToPtr("/prefix"),
},
},
firstMatchPath: "",
expected: kong.Plugin{
Name: lo.ToPtr("request-transformer"),
Config: kong.Configuration{
"replace": map[string]string{
"uri": `/prefix$(uri_captures[1] == nil and "" or "/" .. uri_captures[1])`,
},
},
},
expectedKongRouteModification: kongstate.Route{
Route: kong.Route{
Paths: []*string{
lo.ToPtr("~/$"),
lo.ToPtr("~/(.*)"),
},
},
},
},
{
name: "URLRewriteFilter with '/' ReplacePrefixPatch",
modifier: &gatewayapi.HTTPURLRewriteFilter{
Expand Down Expand Up @@ -443,6 +469,32 @@ func TestGenerateRequestTransformerForURLRewrite(t *testing.T) {
},
},
},
{
name: "URLRewriteFilter with firstMatchPath with a trailing slash",
modifier: &gatewayapi.HTTPURLRewriteFilter{
Path: &gatewayapi.HTTPPathModifier{
Type: gatewayapi.PrefixMatchHTTPPathModifier,
ReplacePrefixMatch: lo.ToPtr("/new-prefix"),
},
},
firstMatchPath: "/prefix/",
expected: kong.Plugin{
Name: lo.ToPtr("request-transformer"),
Config: kong.Configuration{
"replace": map[string]string{
"uri": `/new-prefix$(uri_captures[1])`,
},
},
},
expectedKongRouteModification: kongstate.Route{
Route: kong.Route{
Paths: []*string{
lo.ToPtr("~/prefix$"),
lo.ToPtr("~/prefix(/.*)"),
},
},
},
},
// TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/3685
{
name: "valid URLRewriteFilter with unsupported",
Expand Down Expand Up @@ -475,6 +527,42 @@ func TestGenerateRequestTransformerForURLRewrite(t *testing.T) {
}
}

func TestGenerateKongRouteModifierForURLRewritePrefixMatch_ExpressionsRouter(t *testing.T) {
testCases := []struct {
name string
path string
expectedRouteModification kongstate.Route
}{
{
name: "root path",
path: "/",
expectedRouteModification: kongstate.Route{
Route: kong.Route{
Expression: lo.ToPtr(`(http.path == "/") || (http.path ~ "^/(.*)")`),
},
},
},
{
name: "prefix path",
path: "/prefix",
expectedRouteModification: kongstate.Route{
Route: kong.Route{
Expression: lo.ToPtr(`(http.path == "/prefix") || (http.path ~ "^/prefix(/.*)")`),
},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
modifier := generateKongRouteModifierForURLRewritePrefixMatch(tc.path, true)
route := kongstate.Route{}
modifier(&route)
require.Equal(t, tc.expectedRouteModification, route)
})
}
}

func TestMergePluginsOfTheSameType(t *testing.T) {
testCases := []struct {
name string
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feature_flags:
ExpressionRoutes: true
1 change: 1 addition & 0 deletions test/conformance/gateway_conformance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ var expressionRoutesSupportedFeatures = []suite.SupportedFeature{
suite.SupportHTTPRouteQueryParamMatching,
suite.SupportHTTPRouteMethodMatching,
suite.SupportHTTPRouteResponseHeaderModification,
suite.SupportHTTPRoutePathRewrite,
// TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/5868
// Temporarily disabled and tracking through the following issue.
// suite.SupportHTTPRouteBackendTimeout,
Expand Down
8 changes: 0 additions & 8 deletions test/integration/isolated/examples_httproute_rewrite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ import (
"sigs.k8s.io/e2e-framework/pkg/envconf"
"sigs.k8s.io/e2e-framework/pkg/features"

dpconf "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/config"
"github.com/kong/kubernetes-ingress-controller/v3/test/integration/consts"
"github.com/kong/kubernetes-ingress-controller/v3/test/internal/helpers"
"github.com/kong/kubernetes-ingress-controller/v3/test/internal/testenv"
"github.com/kong/kubernetes-ingress-controller/v3/test/internal/testlabels"
)

Expand Down Expand Up @@ -58,12 +56,6 @@ func TestHTTPRouteRewriteExample(t *testing.T) {
consts.WaitTick,
)

if testenv.KongRouterFlavor() != dpconf.RouterFlavorTraditional {
// TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/3686
t.Log("skipping prefix rewrite test for expressions router - to be implemented")
return ctx
}

t.Logf("asserting /old-prefix?msg=hello path is redirected to /echo?msg=hello replacing the prefix")
helpers.EventuallyGETPath(
t,
Expand Down

0 comments on commit 7090f7b

Please sign in to comment.