fix: ReplaceFullPath not working for root path (/) (#3530)

arkodg · web-flow · commit 8f83c3deef13 · 2024-06-05T13:37:59.000-07:00
* fix: ReplaceFullPath not working for root path (/) Takes #2817 forward Signed-off-by: Arko Dasgupta <arko@tetrate.io>
diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go
@@ -386,7 +386,7 @@ func buildXdsURLRewriteAction(destName string, urlRewrite *ir.URLRewrite, pathMa
 		if urlRewrite.Path.FullReplace != nil {
 			routeAction.RegexRewrite = &matcherv3.RegexMatchAndSubstitute{
 				Pattern: &matcherv3.RegexMatcher{
-					Regex: "/.+",
+					Regex: "^/.*$",
 				},
 				Substitution: *urlRewrite.Path.FullReplace,
 			}
diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-url-fullpath.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-url-fullpath.routes.yaml
@@ -12,7 +12,7 @@
         cluster: rewrite-route
         regexRewrite:
           pattern:
-            regex: /.+
+            regex: ^/.*$
           substitution: /rewrite
         upgradeConfigs:
         - upgradeType: websocket
diff --git a/test/e2e/testdata/httproute-rewrite-full-path.yaml b/test/e2e/testdata/httproute-rewrite-full-path.yaml
@@ -0,0 +1,22 @@
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: rewrite-full-path
+  namespace: gateway-conformance-infra
+spec:
+  parentRefs:
+  - name: same-namespace
+  rules:
+  - matches:
+    - path:
+        type: PathPrefix
+        value: /
+    filters:
+    - type: URLRewrite
+      urlRewrite:
+        path:
+          type: ReplaceFullPath
+          replaceFullPath: /full-replace
+    backendRefs:
+    - name: infra-backend-v1
+      port: 8080
diff --git a/test/e2e/tests/httproute-rewrite-full-path.go b/test/e2e/tests/httproute-rewrite-full-path.go
@@ -0,0 +1,59 @@
+// Copyright Envoy Gateway Authors
+// SPDX-License-Identifier: Apache-2.0
+// The full text of the Apache license is available in the LICENSE file at
+// the root of the repo.
+
+//go:build e2e
+// +build e2e
+
+package tests
+
+import (
+	"testing"
+
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/gateway-api/conformance/utils/http"
+	"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
+	"sigs.k8s.io/gateway-api/conformance/utils/suite"
+)
+
+func init() {
+	ConformanceTests = append(ConformanceTests, HTTPRouteRewriteFullPath)
+}
+
+var HTTPRouteRewriteFullPath = suite.ConformanceTest{
+	ShortName:   "HTTPRouteRewriteFullPath",
+	Description: "An HTTPRoute with path rewrite filter to replace full path",
+	Manifests:   []string{"testdata/httproute-rewrite-full-path.yaml"},
+	Test: func(t *testing.T, suite *suite.ConformanceTestSuite) {
+		ns := "gateway-conformance-infra"
+		routeNN := types.NamespacedName{Name: "rewrite-full-path", Namespace: ns}
+		gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns}
+		gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN)
+		kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN, gwNN)
+
+		testCases := []http.ExpectedResponse{
+			{
+				Request: http.Request{
+					Path: "/",
+				},
+				ExpectedRequest: &http.ExpectedRequest{
+					Request: http.Request{
+						Path: "/full-replace",
+					},
+				},
+				Backend:   "infra-backend-v1",
+				Namespace: ns,
+			},
+		}
+		for i := range testCases {
+			// Declare tc here to avoid loop variable
+			// reuse issues across parallel tests.
+			tc := testCases[i]
+			t.Run(tc.GetTestCaseName(i), func(t *testing.T) {
+				t.Parallel()
+				http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, tc)
+			})
+		}
+	},
+}

Original file line number	Diff line number	Diff line change
`@@ -386,7 +386,7 @@ func buildXdsURLRewriteAction(destName string, urlRewrite *ir.URLRewrite, pathMa`
`386`	`386`	`if urlRewrite.Path.FullReplace != nil {`
`387`	`387`	`routeAction.RegexRewrite = &matcherv3.RegexMatchAndSubstitute{`
`388`	`388`	`Pattern: &matcherv3.RegexMatcher{`
`389`		`- Regex: "/.+",`
	`389`	`+ Regex: "^/.*$",`
`390`	`390`	`},`
`391`	`391`	`Substitution: *urlRewrite.Path.FullReplace,`
`392`	`392`	`}`