From 05b12037a18ec9136fa6504c3cff2feedfb85803 Mon Sep 17 00:00:00 2001 From: zirain Date: Wed, 14 May 2025 09:37:25 +0800 Subject: [PATCH] local ratelimit support XRateLimitHeaders Signed-off-by: zirain --- internal/xds/translator/api_key_auth.go | 2 +- internal/xds/translator/authorization.go | 2 +- internal/xds/translator/basicauth.go | 2 +- internal/xds/translator/compressor.go | 2 +- internal/xds/translator/cors.go | 2 +- internal/xds/translator/credentialInjector.go | 2 +- internal/xds/translator/custom_response.go | 2 +- internal/xds/translator/extauth.go | 2 +- internal/xds/translator/extproc.go | 2 +- internal/xds/translator/fault.go | 2 +- internal/xds/translator/healthcheck.go | 2 +- internal/xds/translator/httpfilters.go | 9 +- internal/xds/translator/jwt.go | 2 +- internal/xds/translator/local_ratelimit.go | 8 +- internal/xds/translator/lua.go | 2 +- internal/xds/translator/oidc.go | 2 +- internal/xds/translator/request_buffer.go | 2 +- internal/xds/translator/route.go | 4 +- .../xds/translator/session_persistence.go | 2 +- .../local-ratelimit-distinct.routes.yaml | 2 + .../out/xds-ir/local-ratelimit.routes.yaml | 3 + internal/xds/translator/translator.go | 2 +- internal/xds/translator/wasm.go | 2 +- release-notes/current.yaml | 1 + test/e2e/testdata/local-ratelimit.yaml | 17 + test/e2e/tests/local_ratelimit.go | 523 ++++++++++-------- .../tests/local_ratelimit_distinct_cidr.go | 132 ++--- .../tests/local_ratelimit_distinct_header.go | 126 ++--- test/e2e/utils/http.go | 252 +++++++++ tools/make/golang.mk | 2 +- 30 files changed, 727 insertions(+), 388 deletions(-) create mode 100644 test/e2e/utils/http.go diff --git a/internal/xds/translator/api_key_auth.go b/internal/xds/translator/api_key_auth.go index 0c017f6d6c..2260980535 100644 --- a/internal/xds/translator/api_key_auth.go +++ b/internal/xds/translator/api_key_auth.go @@ -89,7 +89,7 @@ func (*apiKeyAuth) patchResources(*types.ResourceVersionTable, []*ir.HTTPRoute) // patchRoute patches the provided route with the apiKeyAuth config if applicable. // Note: this method overwrites the HCM level filter config with the per route filter config. -func (*apiKeyAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*apiKeyAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/authorization.go b/internal/xds/translator/authorization.go index 7d4b3af5c8..7a94453fb3 100644 --- a/internal/xds/translator/authorization.go +++ b/internal/xds/translator/authorization.go @@ -107,7 +107,7 @@ func listenerContainsRBAC(irListener *ir.HTTPListener) bool { } // patchRoute patches the provided route with the RBAC config if applicable. -func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*rbac) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/basicauth.go b/internal/xds/translator/basicauth.go index 502f15e59c..bbf4b3f511 100644 --- a/internal/xds/translator/basicauth.go +++ b/internal/xds/translator/basicauth.go @@ -115,7 +115,7 @@ func (*basicAuth) patchResources(*types.ResourceVersionTable, []*ir.HTTPRoute) e // patchRoute patches the provided route with the basicAuth config if applicable. // Note: this method overwrites the HCM level filter config with the per route filter config. -func (*basicAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*basicAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/compressor.go b/internal/xds/translator/compressor.go index 56dec069ad..fdf1a38cc6 100644 --- a/internal/xds/translator/compressor.go +++ b/internal/xds/translator/compressor.go @@ -143,7 +143,7 @@ func (*compressor) patchResources(*types.ResourceVersionTable, []*ir.HTTPRoute) // patchRoute patches the provided route with the compressor config if applicable. // Note: this method overwrites the HCM level filter config with the per route filter config. -func (*compressor) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*compressor) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/cors.go b/internal/xds/translator/cors.go index 5dadeaadaf..b53a120027 100644 --- a/internal/xds/translator/cors.go +++ b/internal/xds/translator/cors.go @@ -105,7 +105,7 @@ func listenerContainsCORS(irListener *ir.HTTPListener) bool { } // patchRoute patches the provided route with the CORS config if applicable. -func (*cors) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*cors) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/credentialInjector.go b/internal/xds/translator/credentialInjector.go index 1df31c913a..a7ffe5c333 100644 --- a/internal/xds/translator/credentialInjector.go +++ b/internal/xds/translator/credentialInjector.go @@ -141,7 +141,7 @@ func buildCredentialSecret(credentialInjection *ir.CredentialInjection) *tlsv3.S // patchRoute patches the provided route with the credential injector filter if applicable. // Note: this method enables the corresponding credential injector filter for the provided route. -func (*credentialInjector) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*credentialInjector) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/custom_response.go b/internal/xds/translator/custom_response.go index 259144e6be..14645ec82f 100644 --- a/internal/xds/translator/custom_response.go +++ b/internal/xds/translator/custom_response.go @@ -426,7 +426,7 @@ func (c *customResponse) patchResources(tCtx *types.ResourceVersionTable, // patchRoute patches the provided route with the customResponse config if applicable. // Note: this method enables the corresponding customResponse filter for the provided route. -func (c *customResponse) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (c *customResponse) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/extauth.go b/internal/xds/translator/extauth.go index 954d674636..51973fb4c9 100644 --- a/internal/xds/translator/extauth.go +++ b/internal/xds/translator/extauth.go @@ -244,7 +244,7 @@ func (*extAuth) patchResources(tCtx *types.ResourceVersionTable, // patchRoute patches the provided route with the extAuth config if applicable. // Note: this method enables the corresponding extAuth filter for the provided route. -func (*extAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*extAuth) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/extproc.go b/internal/xds/translator/extproc.go index b9452b6731..1aca83fdbd 100644 --- a/internal/xds/translator/extproc.go +++ b/internal/xds/translator/extproc.go @@ -212,7 +212,7 @@ func (*extProc) patchResources(tCtx *types.ResourceVersionTable, // patchRoute patches the provided route with the extProc config if applicable. // Note: this method enables the corresponding extProc filter for the provided route. -func (*extProc) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*extProc) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/fault.go b/internal/xds/translator/fault.go index 977113f9db..e017d00395 100644 --- a/internal/xds/translator/fault.go +++ b/internal/xds/translator/fault.go @@ -106,7 +106,7 @@ func (*fault) patchResources(*types.ResourceVersionTable, []*ir.HTTPRoute) error // patchRoute patches the provided route with the fault config if applicable. // Note: this method enables the corresponding fault filter for the provided route. -func (*fault) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*fault) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/healthcheck.go b/internal/xds/translator/healthcheck.go index c44484a6ad..560a9e1537 100644 --- a/internal/xds/translator/healthcheck.go +++ b/internal/xds/translator/healthcheck.go @@ -98,6 +98,6 @@ func (*healthCheck) patchResources(*types.ResourceVersionTable, []*ir.HTTPRoute) return nil } -func (*healthCheck) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*healthCheck) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { return nil } diff --git a/internal/xds/translator/httpfilters.go b/internal/xds/translator/httpfilters.go index 35439a678c..2f413cfaf1 100644 --- a/internal/xds/translator/httpfilters.go +++ b/internal/xds/translator/httpfilters.go @@ -56,7 +56,7 @@ type httpFilter interface { patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error // patchRoute patches the provide Route with a filter's Route level configuration. - patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error + patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, httpListener *ir.HTTPListener) error // patchResources adds all the other needed resources referenced by this // filter to the resource version table. @@ -296,12 +296,9 @@ func (t *Translator) patchHCMWithFilters( // patchRouteWithPerRouteConfig appends per-route filter configuration to the // provided route. -func patchRouteWithPerRouteConfig( - route *routev3.Route, - irRoute *ir.HTTPRoute, -) error { +func patchRouteWithPerRouteConfig(route *routev3.Route, irRoute *ir.HTTPRoute, httpListener *ir.HTTPListener) error { for _, filter := range httpFilters { - if err := filter.patchRoute(route, irRoute); err != nil { + if err := filter.patchRoute(route, irRoute, httpListener); err != nil { return err } } diff --git a/internal/xds/translator/jwt.go b/internal/xds/translator/jwt.go index 28276aa0fc..69fde2798f 100644 --- a/internal/xds/translator/jwt.go +++ b/internal/xds/translator/jwt.go @@ -255,7 +255,7 @@ func buildXdsUpstreamTLSSocket(sni string) (*corev3.TransportSocket, error) { // patchRoute patches the provided route with a JWT PerRouteConfig, if the route // doesn't contain it. -func (*jwt) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*jwt) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/local_ratelimit.go b/internal/xds/translator/local_ratelimit.go index 1fb7272f78..e988099284 100644 --- a/internal/xds/translator/local_ratelimit.go +++ b/internal/xds/translator/local_ratelimit.go @@ -117,7 +117,7 @@ func (*localRateLimit) patchResources(*types.ResourceVersionTable, return nil } -func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, httpListener *ir.HTTPListener) error { routeAction := route.GetRoute() // Return early if no rate limit config exists. @@ -168,7 +168,8 @@ func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) e Denominator: typev3.FractionalPercent_HUNDRED, }, }, - Descriptors: descriptors, + EnableXRatelimitHeaders: rlv3.XRateLimitHeadersRFCVersion_DRAFT_VERSION_03, + Descriptors: descriptors, // By setting AlwaysConsumeDefaultTokenBucket to false, the descriptors // won't consume the default token bucket. This means that a request only // counts towards the default token bucket if it does not match any of the @@ -177,6 +178,9 @@ func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) e Value: false, }, } + if httpListener.Headers != nil && httpListener.Headers.DisableRateLimitHeaders { + localRl.EnableXRatelimitHeaders = rlv3.XRateLimitHeadersRFCVersion_OFF + } localRlAny, err := anypb.New(localRl) if err != nil { diff --git a/internal/xds/translator/lua.go b/internal/xds/translator/lua.go index f4444a97dc..77a251fdff 100644 --- a/internal/xds/translator/lua.go +++ b/internal/xds/translator/lua.go @@ -107,7 +107,7 @@ func (*lua) patchResources(_ *types.ResourceVersionTable, _ []*ir.HTTPRoute) err } // patchRoute patches the provided route so Lua filters are enabled if applicable. -func (*lua) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*lua) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/oidc.go b/internal/xds/translator/oidc.go index b5082361e6..4265026107 100644 --- a/internal/xds/translator/oidc.go +++ b/internal/xds/translator/oidc.go @@ -476,7 +476,7 @@ func oauth2HMACSecretName(oidc *ir.OIDC) string { // patchRoute patches the provided route with the oauth2 config if applicable. // Note: this method enables the corresponding oauth2 filter for the provided route. -func (*oidc) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*oidc) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/request_buffer.go b/internal/xds/translator/request_buffer.go index 83b56658c6..b6ef831bc5 100644 --- a/internal/xds/translator/request_buffer.go +++ b/internal/xds/translator/request_buffer.go @@ -101,7 +101,7 @@ func (r *requestBuffer) patchResources(tCtx *types.ResourceVersionTable, routes } // patchRoute will add a BufferPerRoute filter for a particular route -func (r *requestBuffer) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (r *requestBuffer) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if !routeContainsRequestBuffer(irRoute) { return nil } diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go index b6df51ddaf..5298b9da97 100644 --- a/internal/xds/translator/route.go +++ b/internal/xds/translator/route.go @@ -40,7 +40,7 @@ var defaultUpgradeConfig = []*routev3.RouteAction_UpgradeConfig{ }, } -func buildXdsRoute(httpRoute *ir.HTTPRoute) (*routev3.Route, error) { +func buildXdsRoute(httpRoute *ir.HTTPRoute, httpListener *ir.HTTPListener) (*routev3.Route, error) { router := &routev3.Route{ Name: httpRoute.Name, Match: buildXdsRouteMatch(httpRoute.PathMatch, httpRoute.HeaderMatches, httpRoute.QueryParamMatches), @@ -124,7 +124,7 @@ func buildXdsRoute(httpRoute *ir.HTTPRoute) (*routev3.Route, error) { } // Add per route filter configs to the route, if needed. - if err := patchRouteWithPerRouteConfig(router, httpRoute); err != nil { + if err := patchRouteWithPerRouteConfig(router, httpRoute, httpListener); err != nil { return nil, err } diff --git a/internal/xds/translator/session_persistence.go b/internal/xds/translator/session_persistence.go index c43cd0c9bb..c8e64e4ec8 100644 --- a/internal/xds/translator/session_persistence.go +++ b/internal/xds/translator/session_persistence.go @@ -149,7 +149,7 @@ func getLongestNonRegexPrefix(path string) string { } // patchRoute patches the provide Route with a filter's Route level configuration. -func (s *sessionPersistence) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (s *sessionPersistence) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit-distinct.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit-distinct.routes.yaml index 02f71fc1db..65cce8400e 100644 --- a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit-distinct.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit-distinct.routes.yaml @@ -28,6 +28,7 @@ fillInterval: 0s maxTokens: 5 tokensPerFill: 5 + enableXRatelimitHeaders: DRAFT_VERSION_03 filterEnabled: defaultValue: numerator: 100 @@ -110,6 +111,7 @@ fillInterval: 60s maxTokens: 10 tokensPerFill: 10 + enableXRatelimitHeaders: DRAFT_VERSION_03 filterEnabled: defaultValue: numerator: 100 diff --git a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml index 9b5dd3f7d0..7fd4979238 100644 --- a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml @@ -44,6 +44,7 @@ fillInterval: 3600s maxTokens: 10 tokensPerFill: 10 + enableXRatelimitHeaders: DRAFT_VERSION_03 filterEnabled: defaultValue: numerator: 100 @@ -124,6 +125,7 @@ fillInterval: 60s maxTokens: 10 tokensPerFill: 10 + enableXRatelimitHeaders: DRAFT_VERSION_03 filterEnabled: defaultValue: numerator: 100 @@ -146,6 +148,7 @@ envoy.filters.http.local_ratelimit: '@type': type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit alwaysConsumeDefaultTokenBucket: false + enableXRatelimitHeaders: DRAFT_VERSION_03 filterEnabled: defaultValue: numerator: 100 diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go index bbe28719d3..5ccda1bfd0 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -480,7 +480,7 @@ func (t *Translator) addRouteToRouteConfig( var xdsRoute *routev3.Route // 1:1 between IR HTTPRoute and xDS config.route.v3.Route - xdsRoute, err = buildXdsRoute(httpRoute) + xdsRoute, err = buildXdsRoute(httpRoute, httpListener) if err != nil { // skip this route if failed to build xds route errs = errors.Join(errs, err) diff --git a/internal/xds/translator/wasm.go b/internal/xds/translator/wasm.go index b4fb84dbb1..87bf412dc7 100644 --- a/internal/xds/translator/wasm.go +++ b/internal/xds/translator/wasm.go @@ -179,7 +179,7 @@ func (*wasm) patchResources(_ *types.ResourceVersionTable, _ []*ir.HTTPRoute) er // patchRoute patches the provided route with the wasm config if applicable. // Note: this method enables the corresponding wasm filter for the provided route. -func (*wasm) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { +func (*wasm) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute, _ *ir.HTTPListener) error { if route == nil { return errors.New("xds route is nil") } diff --git a/release-notes/current.yaml b/release-notes/current.yaml index d7a4f65860..1fe5d4424e 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -11,6 +11,7 @@ new features: | Added support for percentage-based request mirroring Add an option to OIDC authentication to bypass it and defer to JWT when the request contains an "Authorization: Bearer ..." header. Added support for configuring Subject Alternative Names (SANs) for upstream TLS validation via `BackendTLSPolicy.validation.subjectAltNames`. + Added support for local rate limit header. bug fixes: | diff --git a/test/e2e/testdata/local-ratelimit.yaml b/test/e2e/testdata/local-ratelimit.yaml index dd819d6ca8..2840eabb23 100644 --- a/test/e2e/testdata/local-ratelimit.yaml +++ b/test/e2e/testdata/local-ratelimit.yaml @@ -1,4 +1,17 @@ apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: ClientTrafficPolicy +metadata: + name: disable-ratelimit-header + namespace: gateway-conformance-infra +spec: + headers: + disableRateLimitHeaders: true + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: all-namespaces # use different gatway +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 kind: BackendTrafficPolicy metadata: name: ratelimit-specific-user @@ -49,6 +62,7 @@ metadata: spec: parentRefs: - name: same-namespace + - name: all-namespaces rules: - backendRefs: - name: infra-backend-v1 @@ -66,6 +80,7 @@ metadata: spec: parentRefs: - name: same-namespace + - name: all-namespaces rules: - backendRefs: - name: infra-backend-v1 @@ -83,6 +98,7 @@ metadata: spec: parentRefs: - name: same-namespace + - name: all-namespaces rules: - backendRefs: - name: infra-backend-v1 @@ -125,6 +141,7 @@ metadata: spec: parentRefs: - name: same-namespace + - name: all-namespaces rules: - backendRefs: - name: infra-backend-v1 diff --git a/test/e2e/tests/local_ratelimit.go b/test/e2e/tests/local_ratelimit.go index 1c6d79a8ae..18fa6e4ab5 100644 --- a/test/e2e/tests/local_ratelimit.go +++ b/test/e2e/tests/local_ratelimit.go @@ -8,6 +8,7 @@ package tests import ( + "fmt" "testing" "k8s.io/apimachinery/pkg/types" @@ -19,239 +20,323 @@ import ( "github.com/envoyproxy/gateway/internal/gatewayapi" "github.com/envoyproxy/gateway/internal/gatewayapi/resource" + "github.com/envoyproxy/gateway/test/e2e/utils" ) func init() { ConformanceTests = append(ConformanceTests, LocalRateLimitTest) } +const ( + RatelimitLimitHeaderName = "x-ratelimit-limit" + RatelimitRemainingHeaderName = "x-ratelimit-remaining" +) + var LocalRateLimitTest = suite.ConformanceTest{ ShortName: "LocalRateLimit", - Description: "Make sure local rate limit works", + Description: "Make sure local rate limit work", Manifests: []string{"testdata/local-ratelimit.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { - // let make sure the gateway and http route are accepted - // and there's no rate limit on this route - ns := "gateway-conformance-infra" - routeNN := types.NamespacedName{Name: "http-no-ratelimit", Namespace: ns} - gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} - gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) - - expectOkResp := http.ExpectedResponse{ - Request: http.Request{ - Path: "/no-ratelimit", - }, - Response: http.Response{ - StatusCode: 200, - }, - Namespace: ns, + for _, disableHeader := range []bool{true, false} { + runNoRateLimitTest(t, suite, disableHeader) + + t.Run(fmt.Sprintf("SpecificUser-%t", disableHeader), func(t *testing.T) { + runSpecificUserRateLimitTest(t, suite, disableHeader) + }) + + t.Run(fmt.Sprintf("AllTraffic-%t", disableHeader), func(t *testing.T) { + runAllTrafficRateLimitTest(t, suite, disableHeader) + }) + + t.Run(fmt.Sprintf("HeaderInvertMatch-%t", disableHeader), func(t *testing.T) { + runHeaderInvertMatchRateLimitTest(t, suite, disableHeader) + }) } - expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") + }, +} +// gatewayNN return the gateway namespace name when disabled header or not +// All the HTTPRoute attached to the two gateways, the different is that we +// disabled rate limit headers on all-namespace gateway +func gatewayNN(disableHeader bool) types.NamespacedName { + if disableHeader { + return types.NamespacedName{Name: "all-namespaces", Namespace: "gateway-conformance-infra"} + } + return types.NamespacedName{Name: "same-namespace", Namespace: "gateway-conformance-infra"} +} + +func gatewayAndHTTPRoutesMustBeAccepted(t *testing.T, suite *suite.ConformanceTestSuite, gwNN types.NamespacedName) string { + gwRefs := []kubernetes.GatewayRef{ + kubernetes.NewGatewayRef(gatewayNN(true)), + kubernetes.NewGatewayRef(gatewayNN(false)), + } + gwAddrMap := utils.GatewaysMustBeAccepted(t, suite, gwRefs) + return gwAddrMap[gwNN] +} + +func runNoRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuite, disableHeader bool) { + // let make sure the gateway and http route are accepted + // and there's no rate limit on this route + ns := "gateway-conformance-infra" + gwNN := gatewayNN(disableHeader) + gwAddr := gatewayAndHTTPRoutesMustBeAccepted(t, suite, gwNN) + + expectOkResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/no-ratelimit", + }, + Response: http.Response{ + StatusCode: 200, + AbsentHeaders: []string{RatelimitLimitHeaderName, RatelimitRemainingHeaderName}, + }, + Namespace: ns, + } + + // keep sending requests till get 200 first, that will cost one 200 + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, expectOkResp) + + // send 10+ more + total := 10 + for total > 0 { // keep sending requests till get 200 first, that will cost one 200 - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, expectOkResp) + total-- + } +} - // the requests should not be limited because there is no rate limit on this route - if err := GotExactExpectedResponse(t, 10, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at last fourth request: %v", err) +func runSpecificUserRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuite, disableHeader bool) { + ns := "gateway-conformance-infra" + gwNN := gatewayNN(disableHeader) + gwAddr := gatewayAndHTTPRoutesMustBeAccepted(t, suite, gwNN) + + ancestorRef := gwapiv1a2.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), + Name: gwapiv1.ObjectName(gwNN.Name), + } + BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-specific-user", Namespace: ns}, suite.ControllerName, ancestorRef) + + // keep sending requests till get 200 first, that will cost one 200 + // use EG forked function to check the existence of the header + okResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-specific-user", + Headers: map[string]string{ + "x-user-id": "john", + }, + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + if !disableHeader { + okResponse.Response.Headers = map[string]string{ + RatelimitLimitHeaderName: "3", + RatelimitRemainingHeaderName: "", // empty string means we don't care about the value + } + } + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, okResponse) + + // this request should be limited because the user is john + limitResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-specific-user", + Headers: map[string]string{ + "x-user-id": "john", + }, + }, + Response: http.Response{ + StatusCode: 429, + }, + Namespace: ns, + } + if !disableHeader { + limitResponse.Response.Headers = map[string]string{ + RatelimitLimitHeaderName: "4", + RatelimitRemainingHeaderName: "0", // at the end the remaining should be 0 } + } + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, limitResponse) + + // this request should not be limited because the user is not john hit default bucket + notJohnResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-specific-user", + Headers: map[string]string{ + "x-user-id": "mike", + }, + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + if !disableHeader { + notJohnResponse.Response.Headers = map[string]string{ + RatelimitLimitHeaderName: "10", + RatelimitRemainingHeaderName: "2", // there almost 8 requests before reach this + } + } + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, notJohnResponse) + + // In the end it will hit the limit + notJohnLimitResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-specific-user", + Headers: map[string]string{ + "x-user-id": "mike", + }, + }, + Response: http.Response{ + Headers: map[string]string{ + RatelimitLimitHeaderName: "10", + RatelimitRemainingHeaderName: "0", // it will be limited at the end + }, + StatusCode: 429, + }, + Namespace: ns, + } + if !disableHeader { + notJohnLimitResponse.Response.Headers = map[string]string{ + RatelimitLimitHeaderName: "10", + RatelimitRemainingHeaderName: "0", // it will be limited at the end + } + } + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, notJohnLimitResponse) +} - t.Run("SpecificUser", func(t *testing.T) { - ns := "gateway-conformance-infra" - routeNN := types.NamespacedName{Name: "http-ratelimit-specific-user", Namespace: ns} - gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} - gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) - - ancestorRef := gwapiv1a2.ParentReference{ - Group: gatewayapi.GroupPtr(gwapiv1.GroupName), - Kind: gatewayapi.KindPtr(resource.KindGateway), - Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), - Name: gwapiv1.ObjectName(gwNN.Name), - } - BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-specific-user", Namespace: ns}, suite.ControllerName, ancestorRef) - - expectOkResp := http.ExpectedResponse{ - Request: http.Request{ - Path: "/ratelimit-specific-user", - Headers: map[string]string{ - "x-user-id": "john", - }, - }, - Response: http.Response{ - StatusCode: 200, - }, - Namespace: ns, - } - expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") - - // should just send exactly 4 requests, and expect 429 - - // keep sending requests till get 200 first, that will cost one 200 - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) - - // fire the rest request - if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at first three request: %v", err) - } - - // this request should be limited because the user is john - expectLimitResp := http.ExpectedResponse{ - Request: http.Request{ - Path: "/ratelimit-specific-user", - Headers: map[string]string{ - "x-user-id": "john", - }, - }, - Response: http.Response{ - StatusCode: 429, - }, - Namespace: ns, - } - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectLimitResp) - - // this request should not be limited because the user is not john - expectOkResp = http.ExpectedResponse{ - Request: http.Request{ - Path: "/ratelimit-specific-user", - Headers: map[string]string{ - "x-user-id": "mike", - }, - }, - Response: http.Response{ - StatusCode: 200, - }, - Namespace: ns, - } - expectOkReq = http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") - // the requests should not be limited because the user is mike - if err := GotExactExpectedResponse(t, 4, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at first three request: %v", err) - } - }) - - t.Run("AllTraffic", func(t *testing.T) { - ns := "gateway-conformance-infra" - routeNN := types.NamespacedName{Name: "http-ratelimit-all-traffic", Namespace: ns} - gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} - gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) - - ancestorRef := gwapiv1a2.ParentReference{ - Group: gatewayapi.GroupPtr(gwapiv1.GroupName), - Kind: gatewayapi.KindPtr(resource.KindGateway), - Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), - Name: gwapiv1.ObjectName(gwNN.Name), - } - BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-all-traffic", Namespace: ns}, suite.ControllerName, ancestorRef) - - expectOkResp := http.ExpectedResponse{ - Request: http.Request{ - Path: "/ratelimit-all-traffic", - }, - Response: http.Response{ - StatusCode: 200, - }, - Namespace: ns, - } - expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") - - // should just send exactly 4 requests, and expect 429 - - // keep sending requests till get 200 first, that will cost one 200 - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) - - // fire the rest request - if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at first three request: %v", err) - } - - // this request should be limited at the end - expectLimitResp := http.ExpectedResponse{ - Request: http.Request{ - Path: "/ratelimit-all-traffic", - }, - Response: http.Response{ - StatusCode: 429, - }, - Namespace: ns, - } - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectLimitResp) - }) - - t.Run("HeaderInvertMatch", func(t *testing.T) { - ns := "gateway-conformance-infra" - routeNN := types.NamespacedName{Name: "http-ratelimit-invert-match", Namespace: ns} - gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} - gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) - - ancestorRef := gwapiv1a2.ParentReference{ - Group: gatewayapi.GroupPtr(gwapiv1.GroupName), - Kind: gatewayapi.KindPtr(resource.KindGateway), - Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), - Name: gwapiv1.ObjectName(gwNN.Name), - } - BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-invert-match", Namespace: ns}, suite.ControllerName, ancestorRef) - - expectOkResp := http.ExpectedResponse{ - Request: http.Request{ - Path: "/ratelimit-invert-match", - Headers: map[string]string{ - "x-user-id": "one", - "x-org-id": "org1", - }, - }, - Response: http.Response{ - StatusCode: 200, - }, - Namespace: ns, - } - - expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") - - // should just send exactly 4 requests, and expect 429 - - // keep sending requests till get 200 first, that will cost one 200 - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) - - // fire the rest request - if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at first three request: %v", err) - } - - // this request should be limited because the user is one and org is not test and the limit is 3 - expectLimitResp := http.ExpectedResponse{ - Request: http.Request{ - Path: "/ratelimit-invert-match", - Headers: map[string]string{ - "x-user-id": "one", - "x-org-id": "org1", - }, - }, - Response: http.Response{ - StatusCode: 429, - }, - Namespace: ns, - } - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectLimitResp) - - // with test org - expectOkResp = http.ExpectedResponse{ - Request: http.Request{ - Path: "/ratelimit-invert-match", - Headers: map[string]string{ - "x-user-id": "one", - "x-org-id": "test", - }, - }, - Response: http.Response{ - StatusCode: 200, - }, - Namespace: ns, - } - expectOkReq = http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") - // the requests should not be limited because the user is one but org is test - if err := GotExactExpectedResponse(t, 4, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at first three request: %v", err) - } - }) - }, +func runAllTrafficRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuite, disableHeader bool) { + ns := "gateway-conformance-infra" + gwNN := gatewayNN(disableHeader) + gwAddr := gatewayAndHTTPRoutesMustBeAccepted(t, suite, gwNN) + + ancestorRef := gwapiv1a2.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), + Name: gwapiv1.ObjectName(gwNN.Name), + } + BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-all-traffic", Namespace: ns}, suite.ControllerName, ancestorRef) + + okResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-all-traffic", + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + if !disableHeader { + okResponse.Response.Headers = map[string]string{ + RatelimitLimitHeaderName: "3", + RatelimitRemainingHeaderName: "", // empty string means we don't care about the value + } + } + // keep sending requests till get 200 first, that will cost one 200 + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, okResponse) + + limitResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-all-traffic", + }, + Response: http.Response{ + StatusCode: 429, + }, + Namespace: ns, + } + if !disableHeader { + limitResponse.Response.Headers = map[string]string{ + RatelimitLimitHeaderName: "3", + RatelimitRemainingHeaderName: "0", // at the end the remaining should be 0 + } + } + // this request should be limited at the end + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, limitResponse) +} + +func runHeaderInvertMatchRateLimitTest(t *testing.T, suite *suite.ConformanceTestSuite, disableHeader bool) { + ns := "gateway-conformance-infra" + gwNN := gatewayNN(disableHeader) + gwAddr := gatewayAndHTTPRoutesMustBeAccepted(t, suite, gwNN) + + ancestorRef := gwapiv1a2.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), + Name: gwapiv1.ObjectName(gwNN.Name), + } + BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-invert-match", Namespace: ns}, suite.ControllerName, ancestorRef) + + // keep sending requests till get 200 first, that will cost one 200 + okResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-invert-match", + Headers: map[string]string{ + "x-user-id": "one", + "x-org-id": "org1", + }, + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + if !disableHeader { + okResponse.Response.Headers = map[string]string{ + RatelimitLimitHeaderName: "3", + RatelimitRemainingHeaderName: "", // empty string means we don't care about the value + } + } + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, okResponse) + + // this request should be limited because the user is one and org is not test and the limit is 3 + limitResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-invert-match", + Headers: map[string]string{ + "x-user-id": "one", + "x-org-id": "org1", + }, + }, + Response: http.Response{ + Headers: map[string]string{ + RatelimitLimitHeaderName: "3", + RatelimitRemainingHeaderName: "0", // at the end the remaining should be 0 + }, + StatusCode: 429, + }, + Namespace: ns, + } + if !disableHeader { + limitResponse.Response.Headers = map[string]string{ + RatelimitLimitHeaderName: "3", + RatelimitRemainingHeaderName: "0", // empty string means we don't care about the value + } + } + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, limitResponse) + + // with test org + testOrgResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-invert-match", + Headers: map[string]string{ + "x-user-id": "one", + "x-org-id": "test", + }, + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + if !disableHeader { + testOrgResponse.Response.Headers = map[string]string{ + RatelimitLimitHeaderName: "", // we don't care the real number + RatelimitRemainingHeaderName: "", + } + } + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, testOrgResponse) } diff --git a/test/e2e/tests/local_ratelimit_distinct_cidr.go b/test/e2e/tests/local_ratelimit_distinct_cidr.go index b82650fd64..2b943e5e1d 100644 --- a/test/e2e/tests/local_ratelimit_distinct_cidr.go +++ b/test/e2e/tests/local_ratelimit_distinct_cidr.go @@ -19,6 +19,7 @@ import ( "github.com/envoyproxy/gateway/internal/gatewayapi" "github.com/envoyproxy/gateway/internal/gatewayapi/resource" + "github.com/envoyproxy/gateway/test/e2e/utils" ) func init() { @@ -34,55 +35,75 @@ var LocalRateLimitDistinctCIDRTest = suite.ConformanceTest{ routeNN := types.NamespacedName{Name: "http-ratelimit-distinct-cidr", Namespace: ns} gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + ancestorRef := gwapiv1a2.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), + Name: gwapiv1.ObjectName(gwNN.Name), + } t.Run("requests with x-forwarded-for header should be limited per IP", func(t *testing.T) { - ancestorRef := gwapiv1a2.ParentReference{ - Group: gatewayapi.GroupPtr(gwapiv1.GroupName), - Kind: gatewayapi.KindPtr(resource.KindGateway), - Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), - Name: gwapiv1.ObjectName(gwNN.Name), - } BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-distinct-cidr", Namespace: ns}, suite.ControllerName, ancestorRef) path := "/ratelimit-distinct-cidr" - testDistinctCIDRRatelimit(t, "192.168.1.1", "", ns, gwAddr, path, true, suite) - testDistinctCIDRRatelimit(t, "192.168.1.2", "", ns, gwAddr, path, true, suite) + testRatelimit(t, suite, map[string]string{ + "X-Forwarded-For": "192.168.1.1", + "x-org-id": "", + }, ns, gwAddr, path) + testRatelimit(t, suite, map[string]string{ + "X-Forwarded-For": "192.168.1.2", + "x-org-id": "", + }, ns, gwAddr, path) }) t.Run("requests with x-forwarded-for header and matching x-org-id header should be limited per IP", func(t *testing.T) { - ancestorRef := gwapiv1a2.ParentReference{ - Group: gatewayapi.GroupPtr(gwapiv1.GroupName), - Kind: gatewayapi.KindPtr(resource.KindGateway), - Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), - Name: gwapiv1.ObjectName(gwNN.Name), - } BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-distinct-cidr-and-exact-header", Namespace: ns}, suite.ControllerName, ancestorRef) path := "/ratelimit-distinct-cidr-and-exact-header" - testDistinctCIDRRatelimit(t, "192.168.1.1", "foo", ns, gwAddr, path, true, suite) - testDistinctCIDRRatelimit(t, "192.168.1.2", "foo", ns, gwAddr, path, true, suite) + testRatelimit(t, suite, map[string]string{ + "X-Forwarded-For": "192.168.1.1", + "x-org-id": "foo", + }, ns, gwAddr, path) + testRatelimit(t, suite, map[string]string{ + "X-Forwarded-For": "192.168.1.2", + "x-org-id": "foo", + }, ns, gwAddr, path) }) - t.Run("requests with with x-forwarded-for header but no matching x-org-id header should not be limited", func(t *testing.T) { - ancestorRef := gwapiv1a2.ParentReference{ - Group: gatewayapi.GroupPtr(gwapiv1.GroupName), - Kind: gatewayapi.KindPtr(resource.KindGateway), - Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), - Name: gwapiv1.ObjectName(gwNN.Name), - } + t.Run("requests with with x-forwarded-for header but no matching x-org-id header will hit default bucket", func(t *testing.T) { BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-distinct-cidr-and-exact-header", Namespace: ns}, suite.ControllerName, ancestorRef) path := "/ratelimit-distinct-cidr-and-exact-header" - testDistinctCIDRRatelimit(t, "192.168.1.1", "bar", ns, gwAddr, path, false, suite) + + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, http.ExpectedResponse{ + Request: http.Request{ + Path: path, + Headers: map[string]string{ + "X-Forwarded-For": "192.168.1.1", + "x-org-id": "bar", + }, + }, + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Path: path, + Headers: nil, // don't check headers since Envoy will append the client IP to the X-Forwarded-For header + }, + }, + Response: http.Response{ + StatusCode: 429, + Headers: map[string]string{ + RatelimitLimitHeaderName: "10", // this means it hit the default bucket + RatelimitRemainingHeaderName: "0", + }, + }, + Namespace: ns, + }) }) }, } -func testDistinctCIDRRatelimit(t *testing.T, clientIP, org, ns, gwAddr, path string, limited bool, suite *suite.ConformanceTestSuite) { - expectOkResp := http.ExpectedResponse{ +func testRatelimit(t *testing.T, suite *suite.ConformanceTestSuite, headers map[string]string, ns, gwAddr, path string) { + utils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite, gwAddr, http.ExpectedResponse{ Request: http.Request{ - Path: path, - Headers: map[string]string{ - "X-Forwarded-For": clientIP, - "x-org-id": org, - }, + Path: path, + Headers: headers, }, ExpectedRequest: &http.ExpectedRequest{ Request: http.Request{ @@ -92,45 +113,32 @@ func testDistinctCIDRRatelimit(t *testing.T, clientIP, org, ns, gwAddr, path str }, Response: http.Response{ StatusCode: 200, + Headers: map[string]string{ + RatelimitLimitHeaderName: "3", + RatelimitRemainingHeaderName: "", // empty string means we don't care about the value + }, }, Namespace: ns, - } + }) - expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") - - expectLimitResp := http.ExpectedResponse{ + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, http.ExpectedResponse{ Request: http.Request{ - Path: path, - Headers: map[string]string{ - "X-Forwarded-For": clientIP, - "x-org-id": org, + Path: path, + Headers: headers, + }, + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Path: path, + Headers: nil, // don't check headers since Envoy will append the client IP to the X-Forwarded-For header }, }, Response: http.Response{ StatusCode: 429, + Headers: map[string]string{ + RatelimitLimitHeaderName: "3", + RatelimitRemainingHeaderName: "0", // at the end the remaining should be 0 + }, }, Namespace: ns, - } - expectLimitReq := http.MakeRequest(t, &expectLimitResp, gwAddr, "HTTP", "http") - - // should just send exactly 4 requests, and expect 429 - - // keep sending requests till get 200 first, that will cost one 200 - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) - - // fire the rest request - if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at first three request: %v", err) - } - - if limited { - // this request should be limited because the limit is 3 - if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil { - t.Errorf("fail to get expected response at the fourth request: %v", err) - } - } else { - if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at the fourth request: %v", err) - } - } + }) } diff --git a/test/e2e/tests/local_ratelimit_distinct_header.go b/test/e2e/tests/local_ratelimit_distinct_header.go index 2c032c6551..3eac372926 100644 --- a/test/e2e/tests/local_ratelimit_distinct_header.go +++ b/test/e2e/tests/local_ratelimit_distinct_header.go @@ -34,97 +34,67 @@ var LocalRateLimitDistinctHeaderTest = suite.ConformanceTest{ routeNN := types.NamespacedName{Name: "http-ratelimit-distinct-header", Namespace: ns} gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + ancestorRef := gwapiv1a2.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), + Name: gwapiv1.ObjectName(gwNN.Name), + } t.Run("requests with x-user-id header should be limited per user", func(t *testing.T) { - ancestorRef := gwapiv1a2.ParentReference{ - Group: gatewayapi.GroupPtr(gwapiv1.GroupName), - Kind: gatewayapi.KindPtr(resource.KindGateway), - Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), - Name: gwapiv1.ObjectName(gwNN.Name), - } BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-distinct-header", Namespace: ns}, suite.ControllerName, ancestorRef) path := "/ratelimit-distinct-header" - testDistinctHeaderRatelimit(t, "john", "", ns, gwAddr, path, true, suite) - testDistinctHeaderRatelimit(t, "alice", "", ns, gwAddr, path, true, suite) + + testRatelimit(t, suite, map[string]string{ + "x-user-id": "john", + "x-org-id": "", + }, ns, gwAddr, path) + testRatelimit(t, suite, map[string]string{ + "x-user-id": "alice", + "x-org-id": "", + }, ns, gwAddr, path) }) t.Run("requests with x-user-id header and matching x-org-id header should be limited per user", func(t *testing.T) { - ancestorRef := gwapiv1a2.ParentReference{ - Group: gatewayapi.GroupPtr(gwapiv1.GroupName), - Kind: gatewayapi.KindPtr(resource.KindGateway), - Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), - Name: gwapiv1.ObjectName(gwNN.Name), - } BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-distinct-header-and-exact-header", Namespace: ns}, suite.ControllerName, ancestorRef) path := "/ratelimit-distinct-header-and-exact-header" - testDistinctHeaderRatelimit(t, "john", "foo", ns, gwAddr, path, true, suite) - testDistinctHeaderRatelimit(t, "alice", "foo", ns, gwAddr, path, true, suite) + + testRatelimit(t, suite, map[string]string{ + "x-user-id": "john", + "x-org-id": "foo", + }, ns, gwAddr, path) + testRatelimit(t, suite, map[string]string{ + "x-user-id": "alice", + "x-org-id": "foo", + }, ns, gwAddr, path) }) - t.Run("requests with x-user-id header but no matching x-org-id header should not be limited", func(t *testing.T) { - ancestorRef := gwapiv1a2.ParentReference{ - Group: gatewayapi.GroupPtr(gwapiv1.GroupName), - Kind: gatewayapi.KindPtr(resource.KindGateway), - Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), - Name: gwapiv1.ObjectName(gwNN.Name), - } + t.Run("requests with x-user-id header but no matching x-org-id header will hit default bucket", func(t *testing.T) { BackendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-distinct-header-and-exact-header", Namespace: ns}, suite.ControllerName, ancestorRef) path := "/ratelimit-distinct-header-and-exact-header" - testDistinctHeaderRatelimit(t, "john", "bar", ns, gwAddr, path, false, suite) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, http.ExpectedResponse{ + Request: http.Request{ + Path: path, + Headers: map[string]string{ + "x-user-id": "john", + "x-org-id": "bar", + }, + }, + ExpectedRequest: &http.ExpectedRequest{ + Request: http.Request{ + Path: path, + Headers: nil, // don't check headers since Envoy will append the client IP to the X-Forwarded-For header + }, + }, + Response: http.Response{ + StatusCode: 200, + Headers: map[string]string{ + RatelimitLimitHeaderName: "10", // this means it hit the default bucket + RatelimitRemainingHeaderName: "4", + }, + }, + Namespace: ns, + }) }) }, } - -func testDistinctHeaderRatelimit(t *testing.T, user, org, ns, gwAddr, path string, limited bool, suite *suite.ConformanceTestSuite) { - expectOkResp := http.ExpectedResponse{ - Request: http.Request{ - Path: path, - Headers: map[string]string{ - "x-user-id": user, - "x-org-id": org, - }, - }, - Response: http.Response{ - StatusCode: 200, - }, - Namespace: ns, - } - - expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") - - expectLimitResp := http.ExpectedResponse{ - Request: http.Request{ - Path: path, - Headers: map[string]string{ - "x-user-id": user, - "x-org-id": org, - }, - }, - Response: http.Response{ - StatusCode: 429, - }, - Namespace: ns, - } - expectLimitReq := http.MakeRequest(t, &expectLimitResp, gwAddr, "HTTP", "http") - - // should just send exactly 4 requests, and expect 429 - - // keep sending requests till get 200 first, that will cost one 200 - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) - - // fire the rest request - if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at first three request: %v", err) - } - - if limited { - // this request should be limited because the limit is 3 - if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil { - t.Errorf("fail to get expected response at the fourth request: %v", err) - } - } else { - if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectOkResp); err != nil { - t.Errorf("fail to get expected response at the fourth request: %v", err) - } - } -} diff --git a/test/e2e/utils/http.go b/test/e2e/utils/http.go new file mode 100644 index 0000000000..41338f58cd --- /dev/null +++ b/test/e2e/utils/http.go @@ -0,0 +1,252 @@ +// 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 utils + +import ( + "fmt" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "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/roundtripper" + "sigs.k8s.io/gateway-api/conformance/utils/suite" + "sigs.k8s.io/gateway-api/conformance/utils/tlog" +) + +// MakeRequestAndExpectEventuallyConsistentResponse sends a request to the gateway and waits for an eventually consistent response. +// This's a fork of upstream unless https://github.com/kubernetes-sigs/gateway-api/issues/3794 fixed. +func MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, suite *suite.ConformanceTestSuite, gwAddr string, expected http.ExpectedResponse) { + t.Helper() + + req := http.MakeRequest(t, &expected, gwAddr, "HTTP", "http") + + http.AwaitConvergence(t, suite.TimeoutConfig.RequiredConsecutiveSuccesses, suite.TimeoutConfig.MaxTimeToConsistency, func(elapsed time.Duration) bool { + cReq, cRes, err := suite.RoundTripper.CaptureRoundTrip(req) + if err != nil { + tlog.Logf(t, "Request failed, not ready yet: %v (after %v)", err.Error(), elapsed) + return false + } + + if err := compareRequest(t, &req, cReq, cRes, expected); err != nil { + tlog.Logf(t, "Response expectation failed for request: %+v not ready yet: %v (after %v)", req, err, elapsed) + return false + } + + return true + }) + tlog.Logf(t, "Request passed") +} + +// GatewaysMustBeAccepted waits for the Gateways to be accepted and returns the address of the Gateways. +// This is used when a HTTPRoute referenced by multiple Gateways. +// Warning: we didn't check the status of HTTPRoute. +func GatewaysMustBeAccepted(t *testing.T, suite *suite.ConformanceTestSuite, gwRefs []kubernetes.GatewayRef) map[types.NamespacedName]string { + t.Helper() + + gwAddress := make(map[types.NamespacedName]string) + requiredListenerConditions := []metav1.Condition{ + { + Type: string(gwapiv1.ListenerConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "", // any reason + }, + { + Type: string(gwapiv1.ListenerConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "", // any reason + }, + { + Type: string(gwapiv1.ListenerConditionProgrammed), + Status: metav1.ConditionTrue, + Reason: "", // any reason + }, + } + + for _, gw := range gwRefs { + gwAddr, err := kubernetes.WaitForGatewayAddress(t, suite.Client, suite.TimeoutConfig, gw) + require.NoErrorf(t, err, "timed out waiting for Gateway %s address to be assigned", gw.NamespacedName) + gwAddress[gw.NamespacedName] = gwAddr + + kubernetes.GatewayListenersMustHaveConditions(t, suite.Client, suite.TimeoutConfig, gw.NamespacedName, requiredListenerConditions) + } + + return gwAddress +} + +func compareRequest(t *testing.T, req *roundtripper.Request, cReq *roundtripper.CapturedRequest, cRes *roundtripper.CapturedResponse, expected http.ExpectedResponse) error { + if roundtripper.IsTimeoutError(cRes.StatusCode) { + if roundtripper.IsTimeoutError(expected.Response.StatusCode) { + return nil + } + } + if expected.Response.StatusCode != cRes.StatusCode { + return fmt.Errorf("expected status code to be %d, got %d", expected.Response.StatusCode, cRes.StatusCode) + } + if cRes.StatusCode == 200 { + // The request expected to arrive at the backend is + // the same as the request made, unless otherwise + // specified. + if expected.ExpectedRequest == nil { + expected.ExpectedRequest = &http.ExpectedRequest{Request: expected.Request} + } + + if expected.ExpectedRequest.Method == "" { + expected.ExpectedRequest.Method = "GET" + } + + if expected.ExpectedRequest.Host != "" && expected.ExpectedRequest.Host != cReq.Host { + return fmt.Errorf("expected host to be %s, got %s", expected.ExpectedRequest.Host, cReq.Host) + } + + if expected.ExpectedRequest.Path != cReq.Path { + return fmt.Errorf("expected path to be %s, got %s", expected.ExpectedRequest.Path, cReq.Path) + } + if expected.ExpectedRequest.Method != cReq.Method { + return fmt.Errorf("expected method to be %s, got %s", expected.ExpectedRequest.Method, cReq.Method) + } + if expected.Namespace != cReq.Namespace { + return fmt.Errorf("expected namespace to be %s, got %s", expected.Namespace, cReq.Namespace) + } + if expected.ExpectedRequest.Headers != nil { + if cReq.Headers == nil { + return fmt.Errorf("no headers captured, expected %v", len(expected.ExpectedRequest.Headers)) + } + for name, val := range cReq.Headers { + cReq.Headers[strings.ToLower(name)] = val + } + for name, expectedVal := range expected.ExpectedRequest.Headers { + actualVal, ok := cReq.Headers[strings.ToLower(name)] + if !ok { + return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cReq.Headers) + } else if strings.Join(actualVal, ",") != expectedVal { + return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, strings.Join(actualVal, ",")) + } + } + } + + if expected.Response.Headers != nil { + if cRes.Headers == nil { + return fmt.Errorf("no headers captured, expected %v", len(expected.ExpectedRequest.Headers)) + } + for name, val := range cRes.Headers { + cRes.Headers[strings.ToLower(name)] = val + } + + for name, expectedVal := range expected.Response.Headers { + actualVal, ok := cRes.Headers[strings.ToLower(name)] + if !ok { + return fmt.Errorf("expected %s header to be set, actual headers: %v", name, cRes.Headers) + } + + if expectedVal == "" { + // If the expected value is empty, we don't care about the actual value. + // This is useful for headers that are set by the backend, and we don't + // care about their values. + continue + } + + if strings.Join(actualVal, ",") != expectedVal { + return fmt.Errorf("expected %s header to be set to %s, got %s", name, expectedVal, strings.Join(actualVal, ",")) + } + } + } + + if len(expected.Response.AbsentHeaders) > 0 { + for name, val := range cRes.Headers { + cRes.Headers[strings.ToLower(name)] = val + } + + for _, name := range expected.Response.AbsentHeaders { + val, ok := cRes.Headers[strings.ToLower(name)] + if ok { + return fmt.Errorf("expected %s header to not be set, got %s", name, val) + } + } + } + + // Verify that headers expected *not* to be present on the + // request are actually not present. + if len(expected.ExpectedRequest.AbsentHeaders) > 0 { + for name, val := range cReq.Headers { + cReq.Headers[strings.ToLower(name)] = val + } + + for _, name := range expected.ExpectedRequest.AbsentHeaders { + val, ok := cReq.Headers[strings.ToLower(name)] + if ok { + return fmt.Errorf("expected %s header to not be set, got %s", name, val) + } + } + } + + if !strings.HasPrefix(cReq.Pod, expected.Backend) { + return fmt.Errorf("expected pod name to start with %s, got %s", expected.Backend, cReq.Pod) + } + } else if roundtripper.IsRedirect(cRes.StatusCode) { + if expected.RedirectRequest == nil { + return nil + } + + setRedirectRequestDefaults(req, cRes, &expected) + + if expected.RedirectRequest.Host != cRes.RedirectRequest.Host { + return fmt.Errorf("expected redirected hostname to be %q, got %q", expected.RedirectRequest.Host, cRes.RedirectRequest.Host) + } + + gotPort := cRes.RedirectRequest.Port + if expected.RedirectRequest.Port == "" { + // If the test didn't specify any expected redirect port, we'll try to use + // the scheme to determine sensible defaults for the port. Well known + // schemes like "http" and "https" MAY skip setting any port. + if strings.ToLower(cRes.RedirectRequest.Scheme) == "http" && gotPort != "80" && gotPort != "" { + return fmt.Errorf("for http scheme, expected redirected port to be 80 or not set, got %q", gotPort) + } + if strings.ToLower(cRes.RedirectRequest.Scheme) == "https" && gotPort != "443" && gotPort != "" { + return fmt.Errorf("for https scheme, expected redirected port to be 443 or not set, got %q", gotPort) + } + if strings.ToLower(cRes.RedirectRequest.Scheme) != "http" || strings.ToLower(cRes.RedirectRequest.Scheme) != "https" { + tlog.Logf(t, "Can't validate redirectPort for unrecognized scheme %v", cRes.RedirectRequest.Scheme) + } + } else if expected.RedirectRequest.Port != gotPort { + // An expected port was specified in the tests but it didn't match with + // gotPort. + return fmt.Errorf("expected redirected port to be %q, got %q", expected.RedirectRequest.Port, gotPort) + } + + if expected.RedirectRequest.Scheme != cRes.RedirectRequest.Scheme { + return fmt.Errorf("expected redirected scheme to be %q, got %q", expected.RedirectRequest.Scheme, cRes.RedirectRequest.Scheme) + } + + if expected.RedirectRequest.Path != cRes.RedirectRequest.Path { + return fmt.Errorf("expected redirected path to be %q, got %q", expected.RedirectRequest.Path, cRes.RedirectRequest.Path) + } + } + return nil +} + +func setRedirectRequestDefaults(req *roundtripper.Request, cRes *roundtripper.CapturedResponse, expected *http.ExpectedResponse) { + // If the expected host is nil it means we do not test host redirect. + // In that case we are setting it to the one we got from the response because we do not know the ip/host of the gateway. + if expected.RedirectRequest.Host == "" { + expected.RedirectRequest.Host = cRes.RedirectRequest.Host + } + + if expected.RedirectRequest.Scheme == "" { + expected.RedirectRequest.Scheme = req.URL.Scheme + } + + if expected.RedirectRequest.Path == "" { + expected.RedirectRequest.Path = req.URL.Path + } +} diff --git a/tools/make/golang.mk b/tools/make/golang.mk index 7255afa64e..70ca58a019 100644 --- a/tools/make/golang.mk +++ b/tools/make/golang.mk @@ -107,7 +107,7 @@ go.mod.lint: go.mod.tidy go.mod.tidy.examples ## Check if go.mod is clean .PHONY: go.lint.fmt go.lint.fmt: @$(LOG_TARGET) - @go tool golangci-lint fmt --build-tags=$(LINT_BUILD_TAGS) --config=tools/linter/golangci-lint/.golangci.yml + @go tool golangci-lint fmt --config=tools/linter/golangci-lint/.golangci.yml .PHONY: go.generate go.generate: ## Generate code from templates