Add the ability to set the hits_addend for a given rate_limit request…

… via a hardcoded dynamic metadata field (envoyproxy#34184)

<!--
!!!ATTENTION!!!

If you are fixing *any* crash or *any* potential security issue, *do
not*
open a pull request in this repo. Please report the issue via emailing
envoy-security@googlegroups.com where the issue will be triaged
appropriately.
Thank you in advance for helping to keep Envoy secure.

!!!ATTENTION!!!

For an explanation of how to fill out the fields, please see the
relevant section
in
[PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/main/PULL_REQUESTS.md)
-->

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 <eitan.yarmush@solo.io>
Signed-off-by: code <wbphub@gmail.com>
Co-authored-by: code <wbphub@gmail.com>
Signed-off-by: Martin Duke <martin.h.duke@gmail.com>

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;
 }
 

changelogs/current.yaml

@@ -115,6 +115,10 @@ new_features:
     <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CommonTlsContext.custom_tls_certificate_selector>`
     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 <envoy_v3_api_field_service.ratelimit.v3.RateLimitRequest.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%``,

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:
   <envoy_v3_api_field_extensions.filters.network.tcp_proxy.v3.TcpProxy.idle_timeout>` 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
+  <envoy_v3_api_field_service.ratelimit.v3.RateLimitRequest.hits_addend>` override on a per-route basis.
+  Accepts a number string as a constructor.
+
 Filter state object fields
 --------------------------
 

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",

source/extensions/filters/http/ratelimit/ratelimit.cc

@@ -4,20 +4,42 @@
 #include <vector>
 
 #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"
 #include "source/common/common/fmt.h"
 #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 {
 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<StreamInfo::FilterState::Object>
+  createFromBytes(absl::string_view data) const override {
+    uint32_t hits_addend = 0;
+    if (absl::SimpleAtoi(data, &hits_addend)) {
+      return std::make_unique<StreamInfo::UInt32AccessorImpl>(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<StreamInfo::UInt32Accessor>(
+          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;
   }
 }

test/extensions/filters/http/ratelimit/ratelimit_test.cc

@@ -3,11 +3,13 @@
 #include <vector>
 
 #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<StreamInfo::UInt32AccessorImpl>(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<RateLimit::Descriptor>{
+                                  {{{"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<StreamInfo::FilterState::ObjectFactory>::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