diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index e3fa4843e5..2da35b5d81 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -1213,9 +1213,18 @@ func buildRateLimitRule(rule egv1a1.RateLimitRule) (*ir.RateLimitRule, error) { if match.Path != nil { switch ptr.Deref(match.Path.Type, gwapiv1.PathMatchPathPrefix) { case gwapiv1.PathMatchPathPrefix: - irRule.PathMatch = &ir.StringMatch{ - Prefix: ptr.To(match.Path.Value), - Invert: match.Path.Invert, + if match.Path.Value == "/" { + irRule.PathMatch = &ir.StringMatch{ + Prefix: ptr.To(match.Path.Value), + Invert: match.Path.Invert, + } + } else { + // envoy ratelimit HeaderMatcher doesn't support PathSeparatedPrefix like route matching, + // so we use regex to achieve the same path-separated prefix behavior. + irRule.PathMatch = &ir.StringMatch{ + SafeRegex: ptr.To(regex.PathSeparatedPrefixRegex(match.Path.Value)), + Invert: match.Path.Invert, + } } case gwapiv1.PathMatchExact: irRule.PathMatch = &ir.StringMatch{ diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.in.yaml index 74cae0fc65..ca70dbd117 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.in.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.in.yaml @@ -114,6 +114,9 @@ backendTrafficPolicies: - name: x-org-id value: admin invert: true + path: + type: PathPrefix + value: "/user" limit: requests: 10 unit: Hour @@ -135,6 +138,9 @@ backendTrafficPolicies: - sourceCIDR: type: "Distinct" value: 192.168.0.0/16 + path: + type: PathPrefix + value: "/" limit: requests: 20 unit: Hour diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml index ca29c7923e..b261a0dfd9 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml @@ -9,7 +9,10 @@ backendTrafficPolicies: global: rules: - clientSelectors: - - sourceCIDR: + - path: + type: PathPrefix + value: / + sourceCIDR: type: Distinct value: 192.168.0.0/16 cost: @@ -101,6 +104,9 @@ backendTrafficPolicies: - invert: true name: x-org-id value: admin + path: + type: PathPrefix + value: /user limit: requests: 10 unit: Hour @@ -490,6 +496,10 @@ xdsIR: requests: 10 unit: Hour name: envoy-gateway/policy-for-gateway/rule/0 + pathMatch: + distinct: false + name: "" + safeRegex: ^/user(/.*|\?.*|#.*|;.*|$) readyListener: address: 0.0.0.0 ipFamily: IPv4 @@ -583,6 +593,10 @@ xdsIR: requests: 20 unit: Hour name: default/policy-for-route/rule/0 + pathMatch: + distinct: false + name: "" + prefix: / requestCost: number: 1 responseCost: diff --git a/internal/utils/regex/regex.go b/internal/utils/regex/regex.go index e4c686a6ea..a3c6de269d 100644 --- a/internal/utils/regex/regex.go +++ b/internal/utils/regex/regex.go @@ -8,6 +8,7 @@ package regex import ( "fmt" "regexp" + "strings" ) // Validate validates a regex string. @@ -17,3 +18,19 @@ func Validate(regex string) error { } return nil } + +// PathSeparatedPrefixRegex creates a regex pattern that Envoy's PathSeparatedPrefix behavior. +// The pattern matches paths that either exactly match the prefix or have the prefix followed by "/". +// This ensures proper path separation (e.g., "/api" matches "/api" and "/api/v1" but not "/apiv1"). +// +// References: +// - Envoy 'path_separated_prefix' : https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#config-route-v3-routematch +func PathSeparatedPrefixRegex(prefix string) string { + // Remove trailing slash + trimmedPrefix := strings.TrimSuffix(prefix, "/") + + // Escape special regex characters in the prefix + escapedPrefix := regexp.QuoteMeta(trimmedPrefix) + + return "^" + escapedPrefix + "(/.*|\\?.*|#.*|;.*|$)" +} diff --git a/internal/utils/regex/regex_test.go b/internal/utils/regex/regex_test.go index b3dfaf4c4e..351a13ff5b 100644 --- a/internal/utils/regex/regex_test.go +++ b/internal/utils/regex/regex_test.go @@ -5,7 +5,10 @@ package regex -import "testing" +import ( + "regexp" + "testing" +) func TestValidate(t *testing.T) { tests := []struct { @@ -32,3 +35,129 @@ func TestValidate(t *testing.T) { }) } } + +func TestPathSeparatedPrefixRegex(t *testing.T) { + tests := []struct { + name string + prefix string + testPath string + want bool + }{ + // Tests for prefix "/api/v1" - what should match + { + name: "exact match", + prefix: "/api/v1", + testPath: "/api/v1", + want: true, + }, + { + name: "with trailing slash", + prefix: "/api/v1", + testPath: "/api/v1/", + want: true, + }, + { + name: "with sub-path", + prefix: "/api/v1", + testPath: "/api/v1/users", + want: true, + }, + { + name: "with deep sub-path", + prefix: "/api/v1", + testPath: "/api/v1/users/123/profile", + want: true, + }, + { + name: "with query params", + prefix: "/api/v1", + testPath: "/api/v1?version=latest", + want: true, + }, + { + name: "with complex query", + prefix: "/api/v1", + testPath: "/api/v1?param1=value1¶m2=value2", + want: true, + }, + { + name: "with fragment", + prefix: "/api/v1", + testPath: "/api/v1#section", + want: true, + }, + { + name: "with semicolon parameter", + prefix: "/api/v1", + testPath: "/api/v1;sessionid=123", + want: true, + }, + { + name: "with semicolon and sub-path", + prefix: "/api/v1", + testPath: "/api/v1;sessionid=123/profile", + want: true, + }, + + // Tests for prefix "/api/v1" - what should NOT match + { + name: "alphanumeric continuation", + prefix: "/api/v1", + testPath: "/api/v1abc", + want: false, + }, + { + name: "underscore continuation", + prefix: "/api/v1", + testPath: "/api/v1_test", + want: false, + }, + { + name: "dash continuation", + prefix: "/api/v1", + testPath: "/api/v1-beta", + want: false, + }, + { + name: "dot continuation", + prefix: "/api/v1", + testPath: "/api/v1.1", + want: false, + }, + { + name: "different path completely", + prefix: "/api/v1", + testPath: "/api/v2", + want: false, + }, + { + name: "prefix longer than path", + prefix: "/api/v1", + testPath: "/api", + want: false, + }, + { + name: "similar but different", + prefix: "/api/v1", + testPath: "/api/v10", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pattern := PathSeparatedPrefixRegex(tt.prefix) + + regex, err := regexp.Compile(pattern) + if err != nil { + t.Fatalf("Failed to compile regex pattern %q: %v", pattern, err) + } + + got := regex.MatchString(tt.testPath) + if got != tt.want { + t.Errorf("PathSeparatedPrefixRegex(%q).MatchString(%q) = %v, want %v (pattern: %q)", + tt.prefix, tt.testPath, got, tt.want, pattern) + } + }) + } +} diff --git a/test/e2e/tests/ratelimit.go b/test/e2e/tests/ratelimit.go index 8d5b1cefaa..12583031f3 100644 --- a/test/e2e/tests/ratelimit.go +++ b/test/e2e/tests/ratelimit.go @@ -258,6 +258,36 @@ var RateLimitPathMatchTest = suite.ConformanceTest{ if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil { t.Errorf("failed to get expected response for the last (fourth) request: %v", err) } + + // Subpath should be rate limited due to prefix matching. + expectLimitResp = http.ExpectedResponse{ + Request: http.Request{ + Path: "/get/specific-path/subpath", + }, + Response: http.Response{ + StatusCode: 429, + }, + Namespace: ns, + } + expectLimitReq = http.MakeRequest(t, &expectLimitResp, gwAddr, "HTTP", "http") + if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil { + t.Errorf("failed to get expected response for the last (fourth) request: %v", err) + } + + // Different path (contains the path prefix) should not be rate limited. + expectOkResp = http.ExpectedResponse{ + Request: http.Request{ + Path: "/get/specific-path2", + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + expectOkReq = http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") + if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { + t.Errorf("failed to get expected response for the last (fourth) request: %v", err) + } }) t.Run("not matched path cannot got limited", func(t *testing.T) {