From aebef17f7db32055bac0944ed87cbb5774182e60 Mon Sep 17 00:00:00 2001 From: Eitan Yarmush Date: Thu, 1 Aug 2024 01:18:16 -0400 Subject: [PATCH] Add the ability to set the hits_addend for a given rate_limit request via a hardcoded dynamic metadata field (#34184) Commit Message: Adds the ability to set the hits_addend for a given rate_limit request via a hardcoded dynamic metadata field: envoy.ratelimit:hits_addend. Additional Description: Risk Level: Low Testing: Added unit test. I have also manually tested this using gloo-edge as the control-plane. Docs Changes: Release Notes: Platform Specific Features: N/A [Optional Runtime guard:] N/A [Optional Fixes #Issue] N/A [Optional Fixes commit #PR or SHA] N/A [Optional Deprecated:] N/A [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] N/A --------- Signed-off-by: Eitan Yarmush Signed-off-by: code Co-authored-by: code Signed-off-by: Martin Duke --- api/envoy/service/ratelimit/v3/rls.proto | 2 + changelogs/current.yaml | 4 ++ .../advanced/well_known_filter_state.rst | 5 ++ .../extensions/filters/http/ratelimit/BUILD | 2 + .../filters/http/ratelimit/ratelimit.cc | 32 +++++++++- .../filters/http/ratelimit/ratelimit_test.cc | 61 +++++++++++++++++++ 6 files changed, 105 insertions(+), 1 deletion(-) diff --git a/api/envoy/service/ratelimit/v3/rls.proto b/api/envoy/service/ratelimit/v3/rls.proto index 7375aceb5c2b8..d69a323d88b7a 100644 --- a/api/envoy/service/ratelimit/v3/rls.proto +++ b/api/envoy/service/ratelimit/v3/rls.proto @@ -49,6 +49,8 @@ message RateLimitRequest { // Rate limit requests can optionally specify the number of hits a request adds to the matched // limit. If the value is not set in the message, a request increases the matched limit by 1. + // This value can be overridden by setting filter state value ``envoy.ratelimit.hits_addend`` + // to the desired number. Invalid number (< 0) or number will be ignored. uint32 hits_addend = 3; } diff --git a/changelogs/current.yaml b/changelogs/current.yaml index ffe96071d8fc5..cf1201c0c0a97 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -115,6 +115,10 @@ new_features: ` to allow overriding TLS certificate selection behavior. An extension can select certificate base on the incoming SNI, in both sync and async mode. +- area: ratelimit + change: | + Added the ability to modify :ref:`hits_addend ` + by setting by setting filter state value ``envoy.ratelimit.hits_addend`` to the desired value. - area: access_log change: | Added new access log command operators ``%START_TIME_LOCAL%`` and ``%EMIT_TIME_LOCAL%``, diff --git a/docs/root/configuration/advanced/well_known_filter_state.rst b/docs/root/configuration/advanced/well_known_filter_state.rst index f238dd2078546..edcfc2e321ff8 100644 --- a/docs/root/configuration/advanced/well_known_filter_state.rst +++ b/docs/root/configuration/advanced/well_known_filter_state.rst @@ -68,6 +68,11 @@ The following lists the filter state object keys used by the Envoy extensions: ` override on a per-connection basis. Accepts a count of milliseconds number string as a constructor. +``envoy.ratelimit.hits_addend`` + :ref:`Rate Limit Hits Addend + ` override on a per-route basis. + Accepts a number string as a constructor. + Filter state object fields -------------------------- diff --git a/source/extensions/filters/http/ratelimit/BUILD b/source/extensions/filters/http/ratelimit/BUILD index fd4c15c81acee..6d56028db50db 100644 --- a/source/extensions/filters/http/ratelimit/BUILD +++ b/source/extensions/filters/http/ratelimit/BUILD @@ -20,11 +20,13 @@ envoy_cc_library( ":ratelimit_headers_lib", "//envoy/http:codes_interface", "//envoy/ratelimit:ratelimit_interface", + "//envoy/stream_info:uint32_accessor_interface", "//source/common/common:assert_lib", "//source/common/common:empty_string", "//source/common/common:enum_to_int", "//source/common/http:codes_lib", "//source/common/router:config_lib", + "//source/common/stream_info:uint32_accessor_lib", "//source/extensions/filters/common/ratelimit:ratelimit_client_interface", "//source/extensions/filters/common/ratelimit:stat_names_lib", "@envoy_api//envoy/extensions/filters/http/ratelimit/v3:pkg_cc_proto", diff --git a/source/extensions/filters/http/ratelimit/ratelimit.cc b/source/extensions/filters/http/ratelimit/ratelimit.cc index 382029e5f3dbc..7052f8f793edf 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.cc +++ b/source/extensions/filters/http/ratelimit/ratelimit.cc @@ -4,6 +4,7 @@ #include #include "envoy/http/codes.h" +#include "envoy/stream_info/stream_info.h" #include "source/common/common/assert.h" #include "source/common/common/enum_to_int.h" @@ -11,6 +12,7 @@ #include "source/common/http/codes.h" #include "source/common/http/header_utility.h" #include "source/common/router/config_impl.h" +#include "source/common/stream_info/uint32_accessor_impl.h" #include "source/extensions/filters/http/ratelimit/ratelimit_headers.h" namespace Envoy { @@ -18,6 +20,26 @@ namespace Extensions { namespace HttpFilters { namespace RateLimitFilter { +namespace { +constexpr absl::string_view HitsAddendFilterStateKey = "envoy.ratelimit.hits_addend"; + +class HitsAddendObjectFactory : public StreamInfo::FilterState::ObjectFactory { +public: + std::string name() const override { return std::string(HitsAddendFilterStateKey); } + std::unique_ptr + createFromBytes(absl::string_view data) const override { + uint32_t hits_addend = 0; + if (absl::SimpleAtoi(data, &hits_addend)) { + return std::make_unique(hits_addend); + } + return nullptr; + } +}; + +REGISTER_FACTORY(HitsAddendObjectFactory, StreamInfo::FilterState::ObjectFactory); + +} // namespace + struct RcDetailsValues { // This request went above the configured limits for the rate limit filter. const std::string RateLimited = "request_rate_limited"; @@ -64,11 +86,19 @@ void Filter::initiateCall(const Http::RequestHeaderMap& headers) { break; } + const StreamInfo::UInt32Accessor* hits_addend_filter_state = + callbacks_->streamInfo().filterState()->getDataReadOnly( + HitsAddendFilterStateKey); + double hits_addend = 0; + if (hits_addend_filter_state != nullptr) { + hits_addend = hits_addend_filter_state->value(); + } + if (!descriptors.empty()) { state_ = State::Calling; initiating_call_ = true; client_->limit(*this, getDomain(), descriptors, callbacks_->activeSpan(), - callbacks_->streamInfo(), 0); + callbacks_->streamInfo(), hits_addend); initiating_call_ = false; } } diff --git a/test/extensions/filters/http/ratelimit/ratelimit_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_test.cc index 8207ba087fab8..6caa81e8eb5f1 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/http/ratelimit/ratelimit_test.cc @@ -3,11 +3,13 @@ #include #include "envoy/extensions/filters/http/ratelimit/v3/rate_limit.pb.h" +#include "envoy/stream_info/stream_info.h" #include "source/common/buffer/buffer_impl.h" #include "source/common/common/empty_string.h" #include "source/common/http/context_impl.h" #include "source/common/http/headers.h" +#include "source/common/stream_info/uint32_accessor_impl.h" #include "source/extensions/filters/http/ratelimit/ratelimit.h" #include "test/extensions/filters/common/ratelimit/mocks.h" @@ -257,6 +259,53 @@ TEST_F(HttpRateLimitFilterTest, OkResponse) { 1U, filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(ratelimit_ok_).value()); } +TEST_F(HttpRateLimitFilterTest, OkResponseWithAdditionalHitsAddend) { + setUpTest(filter_config_); + InSequence s; + + filter_callbacks_.stream_info_.filter_state_->setData( + "envoy.ratelimit.hits_addend", std::make_unique(5), + StreamInfo::FilterState::StateType::ReadOnly); + EXPECT_CALL(filter_callbacks_.route_->route_entry_.rate_limit_policy_, getApplicableRateLimit(0)); + + EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _)) + .WillOnce(SetArgReferee<0>(descriptor_)); + + EXPECT_CALL(filter_callbacks_.route_->virtual_host_.rate_limit_policy_, + getApplicableRateLimit(0)); + + EXPECT_CALL(*client_, limit(_, "foo", + testing::ContainerEq(std::vector{ + {{{"descriptor_key", "descriptor_value"}}}}), + _, _, 5)) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + + request_headers_.addCopy(Http::Headers::get().RequestId, "requestid"); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + Http::MetadataMap metadata_map{{"metadata", "metadata"}}; + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->decodeMetadata(metadata_map)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_trailers_)); + EXPECT_EQ(Http::Filter1xxHeadersStatus::Continue, filter_->encode1xxHeaders(response_headers_)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers_, false)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers_)); + + EXPECT_CALL(filter_callbacks_, continueDecoding()); + EXPECT_CALL(filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::CoreResponseFlag::RateLimited)) + .Times(0); + request_callbacks_->complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, + nullptr, "", nullptr); + + EXPECT_EQ( + 1U, filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(ratelimit_ok_).value()); +} + TEST_F(HttpRateLimitFilterTest, OkResponseWithHeaders) { setUpTest(filter_config_); InSequence s; @@ -1667,6 +1716,18 @@ TEST_F(HttpRateLimitFilterTest, StatsWithPrefix) { EXPECT_EQ("request_rate_limited", filter_callbacks_.details()); } +TEST(ObjectFactory, HitsAddend) { + const std::string name = "envoy.ratelimit.hits_addend"; + auto* factory = + Registry::FactoryRegistry::getFactory(name); + ASSERT_NE(nullptr, factory); + EXPECT_EQ(name, factory->name()); + const std::string hits_addend = std::to_string(1234); + auto object = factory->createFromBytes(hits_addend); + ASSERT_NE(nullptr, object); + EXPECT_EQ(hits_addend, object->serializeAsString()); +} + } // namespace } // namespace RateLimitFilter } // namespace HttpFilters