From 4b3cc40b2600939b81ce2c8c6a523fa8de99d50e Mon Sep 17 00:00:00 2001 From: xiada Date: Fri, 25 Mar 2022 15:49:29 +0800 Subject: [PATCH 1/5] ratelimit add support for custom http response code Signed-off-by: xiada --- .../extensions/filters/http/ratelimit/v3/BUILD | 1 + .../filters/http/ratelimit/v3/rate_limit.proto | 11 ++++++++++- .../extensions/filters/http/ratelimit/ratelimit.cc | 4 ++-- .../extensions/filters/http/ratelimit/ratelimit.h | 13 ++++++++++++- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/api/envoy/extensions/filters/http/ratelimit/v3/BUILD b/api/envoy/extensions/filters/http/ratelimit/v3/BUILD index 5b2bddfabb819..77ed9cc649472 100644 --- a/api/envoy/extensions/filters/http/ratelimit/v3/BUILD +++ b/api/envoy/extensions/filters/http/ratelimit/v3/BUILD @@ -10,6 +10,7 @@ api_proto_package( "//envoy/config/ratelimit/v3:pkg", "//envoy/config/route/v3:pkg", "//envoy/type/metadata/v3:pkg", + "//envoy/type/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", ], ) diff --git a/api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto b/api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto index a0b30661db45a..829e102b3017f 100644 --- a/api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto +++ b/api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto @@ -6,6 +6,7 @@ import "envoy/config/core/v3/extension.proto"; import "envoy/config/ratelimit/v3/rls.proto"; import "envoy/config/route/v3/route_components.proto"; import "envoy/type/metadata/v3/metadata.proto"; +import "envoy/type/v3/http_status.proto"; import "google/protobuf/duration.proto"; @@ -23,7 +24,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Rate limit :ref:`configuration overview `. // [#extension: envoy.filters.http.ratelimit] -// [#next-free-field: 10] +// [#next-free-field: 11] message RateLimit { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.rate_limit.v2.RateLimit"; @@ -111,6 +112,14 @@ message RateLimit { // in case of rate limiting (i.e. 429 responses). // Having this header not present potentially makes the request retriable. bool disable_x_envoy_ratelimited_header = 9; + + // This field allows for a custom HTTP response status code to the downstream client when + // the request has been rate limited. + // Defaults to 429 (TooManyRequests). + // + // .. note:: + // If this is set to < 400, 429 will be used instead. + type.v3.HttpStatus status = 10; } // Global rate limiting :ref:`architecture overview `. diff --git a/source/extensions/filters/http/ratelimit/ratelimit.cc b/source/extensions/filters/http/ratelimit/ratelimit.cc index 61bb6d698b2d9..9d7c51f78ce33 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.cc +++ b/source/extensions/filters/http/ratelimit/ratelimit.cc @@ -166,7 +166,7 @@ void Filter::complete(Filters::Common::RateLimit::LimitStatus status, Http::CodeStats::ResponseStatInfo info{config_->scope(), cluster_->statsScope(), empty_stat_name, - enumToInt(Http::Code::TooManyRequests), + enumToInt(config_->status()), true, empty_stat_name, empty_stat_name, @@ -200,7 +200,7 @@ void Filter::complete(Filters::Common::RateLimit::LimitStatus status, state_ = State::Responded; callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::RateLimited); callbacks_->sendLocalReply( - Http::Code::TooManyRequests, response_body, + config_->status(), response_body, [this](Http::HeaderMap& headers) { populateResponseHeaders(headers, /*from_local_reply=*/true); }, diff --git a/source/extensions/filters/http/ratelimit/ratelimit.h b/source/extensions/filters/http/ratelimit/ratelimit.h index 07dc086fd1a3a..3729c2f451603 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.h +++ b/source/extensions/filters/http/ratelimit/ratelimit.h @@ -55,7 +55,8 @@ class FilterConfig { config.rate_limited_as_resource_exhausted() ? absl::make_optional(Grpc::Status::WellKnownGrpcStatus::ResourceExhausted) : absl::nullopt), - http_context_(http_context), stat_names_(scope.symbolTable()) {} + http_context_(http_context), stat_names_(scope.symbolTable()), + status_(toErrorCode(config.status().code())) {} const std::string& domain() const { return domain_; } const LocalInfo::LocalInfo& localInfo() const { return local_info_; } uint64_t stage() const { return stage_; } @@ -70,6 +71,7 @@ class FilterConfig { } Http::Context& httpContext() { return http_context_; } Filters::Common::RateLimit::StatNames& statNames() { return stat_names_; } + Http::Code status() { return status_; } private: static FilterRequestType stringToType(const std::string& request_type) { @@ -83,6 +85,14 @@ class FilterConfig { } } + static Http::Code toErrorCode(uint64_t status) { + const auto code = static_cast(status); + if (code >= Http::Code::BadRequest) { + return code; + } + return Http::Code::TooManyRequests; + } + const std::string domain_; const uint64_t stage_; const FilterRequestType request_type_; @@ -95,6 +105,7 @@ class FilterConfig { const absl::optional rate_limited_grpc_status_; Http::Context& http_context_; Filters::Common::RateLimit::StatNames stat_names_; + const Http::Code status_; }; using FilterConfigSharedPtr = std::shared_ptr; From cd8f3c0a5e7ef18c4bc458ebf72f91d6e3c67a1a Mon Sep 17 00:00:00 2001 From: xiada Date: Fri, 1 Apr 2022 19:56:13 +0800 Subject: [PATCH 2/5] ratelimit:rename status to rate_limited_status Signed-off-by: xiada --- .../extensions/filters/http/ratelimit/v3/rate_limit.proto | 2 +- source/extensions/filters/http/ratelimit/ratelimit.cc | 4 ++-- source/extensions/filters/http/ratelimit/ratelimit.h | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto b/api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto index 829e102b3017f..daaae6a7309b4 100644 --- a/api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto +++ b/api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto @@ -119,7 +119,7 @@ message RateLimit { // // .. note:: // If this is set to < 400, 429 will be used instead. - type.v3.HttpStatus status = 10; + type.v3.HttpStatus rate_limited_status = 10; } // Global rate limiting :ref:`architecture overview `. diff --git a/source/extensions/filters/http/ratelimit/ratelimit.cc b/source/extensions/filters/http/ratelimit/ratelimit.cc index 9d7c51f78ce33..e6f56c6872263 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.cc +++ b/source/extensions/filters/http/ratelimit/ratelimit.cc @@ -166,7 +166,7 @@ void Filter::complete(Filters::Common::RateLimit::LimitStatus status, Http::CodeStats::ResponseStatInfo info{config_->scope(), cluster_->statsScope(), empty_stat_name, - enumToInt(config_->status()), + enumToInt(config_->rateLimitedStatus()), true, empty_stat_name, empty_stat_name, @@ -200,7 +200,7 @@ void Filter::complete(Filters::Common::RateLimit::LimitStatus status, state_ = State::Responded; callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::RateLimited); callbacks_->sendLocalReply( - config_->status(), response_body, + config_->rateLimitedStatus(), response_body, [this](Http::HeaderMap& headers) { populateResponseHeaders(headers, /*from_local_reply=*/true); }, diff --git a/source/extensions/filters/http/ratelimit/ratelimit.h b/source/extensions/filters/http/ratelimit/ratelimit.h index 3729c2f451603..8ff9e9f43b45b 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.h +++ b/source/extensions/filters/http/ratelimit/ratelimit.h @@ -56,7 +56,7 @@ class FilterConfig { ? absl::make_optional(Grpc::Status::WellKnownGrpcStatus::ResourceExhausted) : absl::nullopt), http_context_(http_context), stat_names_(scope.symbolTable()), - status_(toErrorCode(config.status().code())) {} + rate_limited_status_(toErrorCode(config.rate_limited_status().code())) {} const std::string& domain() const { return domain_; } const LocalInfo::LocalInfo& localInfo() const { return local_info_; } uint64_t stage() const { return stage_; } @@ -71,7 +71,7 @@ class FilterConfig { } Http::Context& httpContext() { return http_context_; } Filters::Common::RateLimit::StatNames& statNames() { return stat_names_; } - Http::Code status() { return status_; } + Http::Code rateLimitedStatus() { return rate_limited_status_; } private: static FilterRequestType stringToType(const std::string& request_type) { @@ -105,7 +105,7 @@ class FilterConfig { const absl::optional rate_limited_grpc_status_; Http::Context& http_context_; Filters::Common::RateLimit::StatNames stat_names_; - const Http::Code status_; + const Http::Code rate_limited_status_; }; using FilterConfigSharedPtr = std::shared_ptr; From eed47006b772633c53aaa59110e11da71021edb5 Mon Sep 17 00:00:00 2001 From: xiada Date: Fri, 1 Apr 2022 19:56:51 +0800 Subject: [PATCH 3/5] ratelimit:add test for rate_limited_status Signed-off-by: xiada --- .../filters/http/ratelimit/ratelimit_test.cc | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/test/extensions/filters/http/ratelimit/ratelimit_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_test.cc index 9ce82f406696a..ca671a9ea043e 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/http/ratelimit/ratelimit_test.cc @@ -87,6 +87,12 @@ class HttpRateLimitFilterTest : public testing::Test { domain: foo )EOF"; + const std::string rate_limited_status_config_ = R"EOF( + domain: foo + rate_limited_status: + code: 503 + )EOF"; + Filters::Common::RateLimit::MockClient* client_; NiceMock filter_callbacks_; Stats::StatNamePool pool_{filter_callbacks_.clusterInfo()->statsScope().symbolTable()}; @@ -96,6 +102,8 @@ class HttpRateLimitFilterTest : public testing::Test { Stats::StatName ratelimit_over_limit_{pool_.add("ratelimit.over_limit")}; Stats::StatName upstream_rq_4xx_{pool_.add("upstream_rq_4xx")}; Stats::StatName upstream_rq_429_{pool_.add("upstream_rq_429")}; + Stats::StatName upstream_rq_5xx_{pool_.add("upstream_rq_5xx")}; + Stats::StatName upstream_rq_503_{pool_.add("upstream_rq_503")}; Filters::Common::RateLimit::RequestCallbacks* request_callbacks_{}; Http::TestRequestHeaderMapImpl request_headers_; Http::TestRequestTrailerMapImpl request_trailers_; @@ -886,6 +894,47 @@ TEST_F(HttpRateLimitFilterTest, LimitResponseRuntimeDisabled) { filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(upstream_rq_429_).value()); } +TEST_F(HttpRateLimitFilterTest, LimitResponseWithRateLimitedStatus) { + SetUpTest(rate_limited_status_config_); + InSequence s; + + EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _)) + .WillOnce(SetArgReferee<0>(descriptor_)); + EXPECT_CALL(*client_, limit(_, _, _, _, _)) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + + EXPECT_CALL(filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::RateLimited)); + + Http::ResponseHeaderMapPtr h{new Http::TestResponseHeaderMapImpl()}; + Http::TestResponseHeaderMapImpl response_headers{ + {":status", "503"}, + {"x-envoy-ratelimited", Http::Headers::get().EnvoyRateLimitedValues.True}}; + EXPECT_CALL(filter_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); + + request_callbacks_->complete(Filters::Common::RateLimit::LimitStatus::OverLimit, nullptr, + std::move(h), nullptr, "", nullptr); + + EXPECT_EQ(1U, filter_callbacks_.clusterInfo() + ->statsScope() + .counterFromStatName(ratelimit_over_limit_) + .value()); + EXPECT_EQ( + 1U, + filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(upstream_rq_5xx_).value()); + EXPECT_EQ( + 1U, + filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(upstream_rq_503_).value()); + EXPECT_EQ("request_rate_limited", filter_callbacks_.details()); +} + TEST_F(HttpRateLimitFilterTest, ResetDuringCall) { SetUpTest(filter_config_); InSequence s; From 654e246f03705aa86ff12faa1f1a4cac83ece3ed Mon Sep 17 00:00:00 2001 From: xiada Date: Sat, 2 Apr 2022 14:50:48 +0800 Subject: [PATCH 4/5] ratelimit:add test for case of invalid rate_limited_status Signed-off-by: xiada --- .../filters/http/ratelimit/ratelimit_test.cc | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/extensions/filters/http/ratelimit/ratelimit_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_test.cc index ca671a9ea043e..3fa875c79f041 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/http/ratelimit/ratelimit_test.cc @@ -93,6 +93,12 @@ class HttpRateLimitFilterTest : public testing::Test { code: 503 )EOF"; + const std::string invalid_rate_limited_status_config_ = R"EOF( + domain: foo + rate_limited_status: + code: 200 + )EOF"; + Filters::Common::RateLimit::MockClient* client_; NiceMock filter_callbacks_; Stats::StatNamePool pool_{filter_callbacks_.clusterInfo()->statsScope().symbolTable()}; @@ -935,6 +941,47 @@ TEST_F(HttpRateLimitFilterTest, LimitResponseWithRateLimitedStatus) { EXPECT_EQ("request_rate_limited", filter_callbacks_.details()); } +TEST_F(HttpRateLimitFilterTest, LimitResponseWithInvalidRateLimitedStatus) { + SetUpTest(invalid_rate_limited_status_config_); + InSequence s; + + EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _)) + .WillOnce(SetArgReferee<0>(descriptor_)); + EXPECT_CALL(*client_, limit(_, _, _, _, _)) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + + EXPECT_CALL(filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::RateLimited)); + + Http::ResponseHeaderMapPtr h{new Http::TestResponseHeaderMapImpl()}; + Http::TestResponseHeaderMapImpl response_headers{ + {":status", "429"}, + {"x-envoy-ratelimited", Http::Headers::get().EnvoyRateLimitedValues.True}}; + EXPECT_CALL(filter_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); + + request_callbacks_->complete(Filters::Common::RateLimit::LimitStatus::OverLimit, nullptr, + std::move(h), nullptr, "", nullptr); + + EXPECT_EQ(1U, filter_callbacks_.clusterInfo() + ->statsScope() + .counterFromStatName(ratelimit_over_limit_) + .value()); + EXPECT_EQ( + 1U, + filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(upstream_rq_4xx_).value()); + EXPECT_EQ( + 1U, + filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(upstream_rq_429_).value()); + EXPECT_EQ("request_rate_limited", filter_callbacks_.details()); +} + TEST_F(HttpRateLimitFilterTest, ResetDuringCall) { SetUpTest(filter_config_); InSequence s; From 53d9217e5fe08c739c32e597d8f0b0754098a2e6 Mon Sep 17 00:00:00 2001 From: xiada Date: Fri, 8 Apr 2022 17:03:11 +0800 Subject: [PATCH 5/5] ratelimit: update version history and rate_limit_filter doc Signed-off-by: xiada --- .../configuration/http/http_filters/rate_limit_filter.rst | 4 ++-- docs/root/version_history/current.rst | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/root/configuration/http/http_filters/rate_limit_filter.rst b/docs/root/configuration/http/http_filters/rate_limit_filter.rst index 8e41021d30083..03b5e939630ab 100644 --- a/docs/root/configuration/http/http_filters/rate_limit_filter.rst +++ b/docs/root/configuration/http/http_filters/rate_limit_filter.rst @@ -14,7 +14,7 @@ can optionally include the virtual host rate limit configurations. More than one apply to a request. Each configuration results in a descriptor being sent to the rate limit service. If the rate limit service is called, and the response for any of the descriptors is over limit, a -429 response is returned. The rate limit filter also sets the :ref:`x-envoy-ratelimited` header, +429 response is returned (the response is configurable via :ref:`rate_limited_status `). The rate limit filter also sets the :ref:`x-envoy-ratelimited` header, unless :ref:`disable_x_envoy_ratelimited_header ` is set to true. @@ -142,7 +142,7 @@ Statistics ---------- The rate limit filter outputs statistics in the *cluster..ratelimit.* namespace. -429 responses are emitted to the normal cluster :ref:`dynamic HTTP statistics +429 responses or the configured :ref:`rate_limited_status ` are emitted to the normal cluster :ref:`dynamic HTTP statistics `. .. csv-table:: diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index a2430ea02104b..fae506ed8ed20 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -102,6 +102,7 @@ New Features * local_ratelimit: added support for X-RateLimit-* headers as defined in `draft RFC `_. * matching: the matching API can now express a match tree that will always match by omitting a matcher at the top level. * outlier_detection: :ref:`max_ejection_time_jitter` configuration added to allow adding a random value to the ejection time to prevent 'thundering herd' scenarios. Defaults to 0 so as to not break or change the behavior of existing deployments. +* ratelimit: added :ref:`rate_limited_status ` to support return a custom HTTP response status code to the downstream client when the request has been rate limited. * redis: support for hostnames returned in `cluster slots` response is now available. * schema_validator_tool: added ``bootstrap`` checking to the :ref:`schema validator check tool `.