Gateway API double slash when stripping path prefix fix

Path prefixes could not be stripped which lead to double slashes. Fixes: #28270 Signed-off-by: Dawid Danieluk <lolnoxy@gmail.com>
cilium · Oct 3, 2023 · 65c1a2c · 65c1a2c
1 parent 79f07df
commit 65c1a2c
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 13 deletions.
diff --git a/operator/pkg/model/translation/envoy_virtual_host.go b/operator/pkg/model/translation/envoy_virtual_host.go
@@ -246,7 +246,7 @@ func envoyHTTPRoutes(httpRoutes []model.HTTPRoute, hostnames []string, hostNameS
 		if hRoutes[0].RequestRedirect != nil {
 			route.Action = getRouteRedirect(hRoutes[0].RequestRedirect, listenerPort)
 		} else {
-			route.Action = getRouteAction(backends, r.Rewrite, r.RequestMirrors)
+			route.Action = getRouteAction(&r, backends, r.Rewrite, r.RequestMirrors)
 		}
 		routes = append(routes, &route)
 		delete(matchBackendMap, r.GetMatchKey())
@@ -268,15 +268,23 @@ func hostRewriteMutation(rewrite *model.HTTPURLRewriteFilter) routeActionMutatio
 	}
 }
 
-func pathPrefixMutation(rewrite *model.HTTPURLRewriteFilter) routeActionMutation {
+func pathPrefixMutation(rewrite *model.HTTPURLRewriteFilter, httpRoute *model.HTTPRoute) routeActionMutation {
 	return func(route *envoy_config_route_v3.Route_Route) *envoy_config_route_v3.Route_Route {
-		if rewrite == nil || rewrite.Path == nil || len(rewrite.Path.Exact) != 0 || len(rewrite.Path.Regex) != 0 {
+		if rewrite == nil || rewrite.Path == nil || httpRoute == nil || len(rewrite.Path.Exact) != 0 || len(rewrite.Path.Regex) != 0 {
 			return route
 		}
-		if len(rewrite.Path.Prefix) == 0 {
-			// Refer to: https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io%2fv1beta1.HTTPPathModifier
-			// ReplacePrefix is allowed to be empty.
-			route.Route.PrefixRewrite = "/"
+
+		// Refer to: https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io%2fv1beta1.HTTPPathModifier
+		// ReplacePrefix is allowed to be empty.
+		if len(rewrite.Path.Prefix) == 0 || rewrite.Path.Prefix == "/" {
+
+			route.Route.RegexRewrite = &envoy_type_matcher_v3.RegexMatchAndSubstitute{
+				Pattern: &envoy_type_matcher_v3.RegexMatcher{
+					Regex: "^" + httpRoute.PathMatch.Prefix,
+				},
+				Substitution: "",
+			}
+
 		} else {
 			route.Route.PrefixRewrite = rewrite.Path.Prefix
 		}
@@ -318,12 +326,12 @@ func requestMirrorMutation(mirrors []*model.HTTPRequestMirror) routeActionMutati
 	}
 }
 
-func getRouteAction(backends []model.Backend, rewrite *model.HTTPURLRewriteFilter, mirrors []*model.HTTPRequestMirror) *envoy_config_route_v3.Route_Route {
+func getRouteAction(route *model.HTTPRoute, backends []model.Backend, rewrite *model.HTTPURLRewriteFilter, mirrors []*model.HTTPRequestMirror) *envoy_config_route_v3.Route_Route {
 	var routeAction *envoy_config_route_v3.Route_Route
 
 	var mutators = []routeActionMutation{
 		hostRewriteMutation(rewrite),
-		pathPrefixMutation(rewrite),
+		pathPrefixMutation(rewrite, route),
 		pathFullReplaceMutation(rewrite),
 		requestMirrorMutation(mirrors),
 	}

diff --git a/operator/pkg/model/translation/envoy_virtual_host_test.go b/operator/pkg/model/translation/envoy_virtual_host_test.go
@@ -153,11 +153,13 @@ func Test_pathPrefixMutation(t *testing.T) {
 		route := &envoy_config_route_v3.Route_Route{
 			Route: &envoy_config_route_v3.RouteAction{},
 		}
-		res := pathPrefixMutation(nil)(route)
+		res := pathPrefixMutation(nil, nil)(route)
 		require.Equal(t, route, res)
 	})
 
 	t.Run("with prefix rewrite", func(t *testing.T) {
+		httpRoute := model.HTTPRoute{}
+		httpRoute.PathMatch.Prefix = "/strip-prefix"
 		route := &envoy_config_route_v3.Route_Route{
 			Route: &envoy_config_route_v3.RouteAction{},
 		}
@@ -167,10 +169,12 @@ func Test_pathPrefixMutation(t *testing.T) {
 			},
 		}
 
-		res := pathPrefixMutation(rewrite)(route)
+		res := pathPrefixMutation(rewrite, &httpRoute)(route)
 		require.Equal(t, res.Route.PrefixRewrite, "/prefix")
 	})
 	t.Run("with empty prefix rewrite", func(t *testing.T) {
+		httpRoute := model.HTTPRoute{}
+		httpRoute.PathMatch.Prefix = "/strip-prefix"
 		route := &envoy_config_route_v3.Route_Route{
 			Route: &envoy_config_route_v3.RouteAction{},
 		}
@@ -180,8 +184,33 @@ func Test_pathPrefixMutation(t *testing.T) {
 			},
 		}
 
-		res := pathPrefixMutation(rewrite)(route)
-		require.Equal(t, res.Route.PrefixRewrite, "/")
+		res := pathPrefixMutation(rewrite, &httpRoute)(route)
+		require.EqualValues(t, &envoy_type_matcher_v3.RegexMatchAndSubstitute{
+			Pattern: &envoy_type_matcher_v3.RegexMatcher{
+				Regex: "^" + httpRoute.PathMatch.Prefix,
+			},
+			Substitution: "",
+		}, res.Route.RegexRewrite)
+	})
+	t.Run("with slash prefix rewrite", func(t *testing.T) {
+		httpRoute := model.HTTPRoute{}
+		httpRoute.PathMatch.Prefix = "/strip-prefix"
+		route := &envoy_config_route_v3.Route_Route{
+			Route: &envoy_config_route_v3.RouteAction{},
+		}
+		rewrite := &model.HTTPURLRewriteFilter{
+			Path: &model.StringMatch{
+				Prefix: "/",
+			},
+		}
+
+		res := pathPrefixMutation(rewrite, &httpRoute)(route)
+		require.EqualValues(t, &envoy_type_matcher_v3.RegexMatchAndSubstitute{
+			Pattern: &envoy_type_matcher_v3.RegexMatcher{
+				Regex: "^" + httpRoute.PathMatch.Prefix,
+			},
+			Substitution: "",
+		}, res.Route.RegexRewrite)
 	})
 }