diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go index 4426dc2efe..fe2d516a27 100644 --- a/internal/xds/translator/route.go +++ b/internal/xds/translator/route.go @@ -520,6 +520,10 @@ func useRegexRewriteForPrefixMatchReplace(pathMatch *ir.StringMatch, prefixMatch (prefixMatchReplace == "" || prefixMatchReplace == "/") } +func prefixPathMatchAll(pathMatch *ir.StringMatch) bool { + return pathMatch != nil && pathMatch.Prefix != nil && *pathMatch.Prefix == "/" +} + func prefix2RegexRewrite(prefix string) *matcherv3.RegexMatchAndSubstitute { return &matcherv3.RegexMatchAndSubstitute{ Pattern: &matcherv3.RegexMatcher{ @@ -559,9 +563,17 @@ func buildXdsURLRewriteAction(route *ir.HTTPRoute, urlRewrite *ir.URLRewrite, pa // An empty replace string does not seem to solve the issue so we are using // a regex match and replace instead // Remove this workaround once https://github.com/envoyproxy/envoy/issues/26055 is fixed - if useRegexRewriteForPrefixMatchReplace(pathMatch, *urlRewrite.Path.PrefixMatchReplace) { + switch { + case useRegexRewriteForPrefixMatchReplace(pathMatch, *urlRewrite.Path.PrefixMatchReplace): routeAction.RegexRewrite = prefix2RegexRewrite(*pathMatch.Prefix) - } else { + case prefixPathMatchAll(pathMatch): + routeAction.RegexRewrite = &matcherv3.RegexMatchAndSubstitute{ + Pattern: &matcherv3.RegexMatcher{ + Regex: "(^/$)|(.*)", + }, + Substitution: strings.TrimSuffix(*urlRewrite.Path.PrefixMatchReplace, "/") + "\\2", + } + default: // remove trailing / to fix #3989 // when the pathMath.Prefix has suffix / but EG has removed it, // and the urlRewrite.Path.PrefixMatchReplace suffix with / the upstream will get unwanted / diff --git a/internal/xds/translator/testdata/in/xds-ir/http-route-rewrite-match-all.yaml b/internal/xds/translator/testdata/in/xds-ir/http-route-rewrite-match-all.yaml new file mode 100644 index 0000000000..893a2ac8b6 --- /dev/null +++ b/internal/xds/translator/testdata/in/xds-ir/http-route-rewrite-match-all.yaml @@ -0,0 +1,27 @@ +http: +- name: "first-listener" + address: "::" + port: 10080 + hostnames: + - "*" + path: + mergeSlashes: true + escapedSlashesAction: UnescapeAndRedirect + routes: + - name: "rewrite-route" + pathMatch: + prefix: "/" + hostname: gateway.envoyproxy.io + headerMatches: + - name: ":authority" + exact: gateway.envoyproxy.io + destination: + name: "rewrite-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 + name: "rewrite-route-dest/backend/0" + urlRewrite: + path: + prefixMatchReplace: /docs/ diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.clusters.yaml new file mode 100644 index 0000000000..ff534d4ac2 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.clusters.yaml @@ -0,0 +1,23 @@ +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_PREFERRED + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: rewrite-route-dest + ignoreHealthOnHostRemoval: true + loadBalancingPolicy: + policies: + - typedExtensionConfig: + name: envoy.load_balancing_policies.least_request + typedConfig: + '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest + localityLbConfig: + localityWeightedLbConfig: {} + name: rewrite-route-dest + perConnectionBufferLimitBytes: 32768 + type: EDS diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.endpoints.yaml new file mode 100644 index 0000000000..256dda0908 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.endpoints.yaml @@ -0,0 +1,12 @@ +- clusterName: rewrite-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: rewrite-route-dest/backend/0 diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.listeners.yaml new file mode 100644 index 0000000000..5dd5e46e3c --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.listeners.yaml @@ -0,0 +1,35 @@ +- address: + socketAddress: + address: '::' + portValue: 10080 + defaultFilterChain: + filters: + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + commonHttpProtocolOptions: + headersWithUnderscoresAction: REJECT_REQUEST + http2ProtocolOptions: + initialConnectionWindowSize: 1048576 + initialStreamWindowSize: 65536 + maxConcurrentStreams: 100 + httpFilters: + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + suppressEnvoyHeaders: true + mergeSlashes: true + normalizePath: true + pathWithEscapedSlashesAction: UNESCAPE_AND_REDIRECT + rds: + configSource: + ads: {} + resourceApiVersion: V3 + routeConfigName: first-listener + serverHeaderTransformation: PASS_THROUGH + statPrefix: http-10080 + useRemoteAddress: true + name: first-listener + maxConnectionsToAcceptPerSocketEvent: 1 + name: first-listener + perConnectionBufferLimitBytes: 32768 diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.routes.yaml new file mode 100644 index 0000000000..7a9ae6c8fa --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-match-all.routes.yaml @@ -0,0 +1,22 @@ +- ignorePortInHostMatching: true + name: first-listener + virtualHosts: + - domains: + - gateway.envoyproxy.io + name: first-listener/gateway_envoyproxy_io + routes: + - match: + headers: + - name: :authority + stringMatch: + exact: gateway.envoyproxy.io + prefix: / + name: rewrite-route + route: + cluster: rewrite-route-dest + regexRewrite: + pattern: + regex: (^/$)|(.*) + substitution: /docs\2 + upgradeConfigs: + - upgradeType: websocket diff --git a/release-notes/current/breaking_changes/9415-root-prefix-urlrewrite-xds-shape.md b/release-notes/current/breaking_changes/9415-root-prefix-urlrewrite-xds-shape.md new file mode 100644 index 0000000000..26f166a573 --- /dev/null +++ b/release-notes/current/breaking_changes/9415-root-prefix-urlrewrite-xds-shape.md @@ -0,0 +1 @@ +Root `PathPrefix "/"` `HTTPRoute` URL rewrites with `replacePrefixMatch` now generate `regexRewrite` instead of `prefixRewrite` in the translated xDS `RouteAction` so the replacement prefix preserves the original path separator. EnvoyPatchPolicies or extension servers that patch or inspect the old `prefixRewrite` field for these routes must be updated before upgrade. diff --git a/release-notes/current/bug_fixes/6764-root-prefix-urlrewrite.md b/release-notes/current/bug_fixes/6764-root-prefix-urlrewrite.md new file mode 100644 index 0000000000..3e1c680083 --- /dev/null +++ b/release-notes/current/bug_fixes/6764-root-prefix-urlrewrite.md @@ -0,0 +1 @@ +Fixed `HTTPRoute` URL rewrites with `PathPrefix "/"` so `replacePrefixMatch` can prepend a new prefix without dropping the separator between the new prefix and the original request path. diff --git a/test/e2e/testdata/httproute-rewrite-root-prefix-path.yaml b/test/e2e/testdata/httproute-rewrite-root-prefix-path.yaml new file mode 100644 index 0000000000..097dc0a1a9 --- /dev/null +++ b/test/e2e/testdata/httproute-rewrite-root-prefix-path.yaml @@ -0,0 +1,22 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: rewrite-root-prefix-path + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + rules: + - matches: + - path: + type: PathPrefix + value: / + filters: + - type: URLRewrite + urlRewrite: + path: + type: ReplacePrefixMatch + replacePrefixMatch: /prefix-match/ + backendRefs: + - name: infra-backend-v1 + port: 8080 diff --git a/test/e2e/tests/httproute_rewrite_root_prefix_path.go b/test/e2e/tests/httproute_rewrite_root_prefix_path.go new file mode 100644 index 0000000000..b5443cf4f2 --- /dev/null +++ b/test/e2e/tests/httproute_rewrite_root_prefix_path.go @@ -0,0 +1,69 @@ +// 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 + +package tests + +import ( + "testing" + + "k8s.io/apimachinery/pkg/types" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + "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, HTTPRouteRewriteRootPrefixPath) +} + +var HTTPRouteRewriteRootPrefixPath = suite.ConformanceTest{ + ShortName: "HTTPRouteRewriteRootPrefixPath", + Description: "An HTTPRoute with a root prefix rewrite preserves the original request path", + Manifests: []string{"testdata/httproute-rewrite-root-prefix-path.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "rewrite-root-prefix-path", Namespace: ns} + gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + gwAddr := kubernetes.GatewayAndRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), &gwapiv1.HTTPRoute{}, false, routeNN) + kubernetes.HTTPRouteMustHaveResolvedRefsConditionsTrue(t, suite.Client, suite.TimeoutConfig, routeNN, gwNN) + + testCases := []http.ExpectedResponse{ + { + Request: http.Request{ + Path: "/doc", + }, + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Path: "/prefix-match/doc", + }, + }, + Backend: "infra-backend-v1", + Namespace: ns, + }, + { + Request: http.Request{ + Path: "/", + }, + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Path: "/prefix-match", + }, + }, + Backend: "infra-backend-v1", + Namespace: ns, + }, + } + for i := range testCases { + tc := testCases[i] + t.Run(tc.GetTestCaseName(i), func(t *testing.T) { + t.Parallel() + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, tc) + }) + } + }, +}