Envoy Rate Limit: Enable stage setting matching. (#532)

docs/configuration/http_conn_man/route_config/rate_limits.rst

@@ -14,12 +14,10 @@ Global rate limiting :ref:`architecture overview <arch_overview_rate_limit>`.
   }
 
 stage
-  *(optional, integer)* Refers to the stage set in the filter. If set, the rate limit configuration
-  only applies to filters with the same stage number and for filters set to default. If not set,
-  the rate limit configuration will apply for all rate limit filters set to default. The default
-  value is 0.
+  *(optional, integer)* Refers to the stage set in the filter. The rate limit configuration
+  only applies to filters with the same stage number. The default stage number is 0.
 
-  **NOTE:** This functionality hasn't been implemented yet and stage values are currently ignored.
+  **NOTE:** The filter supports a range of 0 - 10 inclusively for stage numbers.
 
 disable_key
   *(optional, string)* The key to be set in runtime to disable this rate limit configuration.
@@ -57,7 +55,9 @@ Source Cluster
 
 The following descriptor entry is appended to the descriptor:
 
-  * ("source_cluster", "<local service cluster>")
+.. code-block:: cpp
+
+  ("source_cluster", "<local service cluster>")
 
 <local service cluster> is derived from the :option:`--service-cluster` option.
 
@@ -72,7 +72,9 @@ Destination Cluster
 
 The following descriptor entry is appended to the descriptor:
 
-  * ("destination_cluster", "<routed target cluster>")
+.. code-block:: cpp
+
+  ("destination_cluster", "<routed target cluster>")
 
 Once a request matches against a route table rule, a routed cluster is determined by one of the
 following :ref:`route table configuration <config_http_conn_man_route_table_route_cluster>`
@@ -106,7 +108,9 @@ descriptor_key
 The following descriptor entry is appended when a header contains a key that matches the
 *header_name*:
 
-  * ("<descriptor_key>", "<header_value_queried_from_header>")
+.. code-block:: cpp
+
+  ("<descriptor_key>", "<header_value_queried_from_header>")
 
 Remote Address
 ^^^^^^^^^^^^^^
@@ -120,8 +124,9 @@ Remote Address
 The following descriptor entry is appended to the descriptor and is populated using the trusted
 address from :ref:`x-forwarded-for <config_http_conn_man_headers_x-forwarded-for>`:
 
-    * ("remote_address", "<:ref:`trusted address from x-forwarded-for
-      <config_http_conn_man_headers_x-forwarded-for>`>")
+.. code-block:: cpp
+
+  ("remote_address", "<trusted address from x-forwarded-for>")
 
 Generic Key
 ^^^^^^^^^^^
@@ -139,7 +144,9 @@ descriptor_value
 
 The following descriptor entry is appended to the descriptor:
 
-    * ("generic_key", "<descriptor_value>")
+.. code-block:: cpp
+
+  ("generic_key", "<descriptor_value>")
 
 Header Value Match
 ^^^^^^^^^^^^^^^^^^
@@ -165,7 +172,9 @@ descriptor_value
 The following descriptor entry is appended to the descriptor if the request matches the headers
 specified in the action config:
 
-    * ("header_match", "<descriptor_value>")
+.. code-block:: cpp
+
+  ("header_match", "<descriptor_value>")
 
 .. _config_http_conn_man_route_table_rate_limit_composing_actions:
 
@@ -178,7 +187,9 @@ will be populated in the order the actions are specified in the configuration.
 
 For example, to generate the following descriptor:
 
-  * ("generic_key", "some_value"), ("source_cluster", "from_cluster")
+.. code-block:: cpp
+
+  ("generic_key", "some_value"), ("source_cluster", "from_cluster")
 
 The configuration would be:
 
@@ -220,10 +231,14 @@ generate a few possible descriptors depending on what is present in the request.
 For a request with :ref:`x-forwarded-for<config_http_conn_man_headers_x-forwarded-for>` set and the
 trusted address is for example *127.0.0.1*, the following descriptor would be generated:
 
-  * ("generic_key", "some_value"), ("remote_address", "127.0.0.1"), ("source_cluster",
+.. code-block:: cpp
+
+  ("generic_key", "some_value"), ("remote_address", "127.0.0.1"), ("source_cluster",
     "from_cluster")
 
 If a request did not set :ref:`x-forwarded-for<config_http_conn_man_headers_x-forwarded-for>`, the
 following descriptor would be generated:
 
-  * ("generic_key", "some_value"), ("source_cluster", "from_cluster")
+.. code-block:: cpp
+
+  ("generic_key", "some_value"), ("source_cluster", "from_cluster")

docs/configuration/http_filters/rate_limit_filter.rst

@@ -28,8 +28,10 @@ domain
   *(required, string)* The rate limit domain to use when calling the rate limit service.
 
 stage
-  *(optional, integer)* If set, rate limit configurations will only be applied with the same stage
-  number. If not set, all rate limit configurations will be applied.
+  *(optional, integer)* Specifies the rate limit configurations to be applied with the same stage
+  number. If not set, the default stage number is 0.
+
+  **NOTE:** The filter supports a range of 0 - 10 inclusively for stage numbers.
 
 Statistics
 ----------

include/envoy/router/router_ratelimit.h

@@ -38,7 +38,7 @@ class RateLimitPolicyEntry {
   /**
    * @return the stage value that the configuration is applicable to.
    */
-  virtual int64_t stage() const PURE;
+  virtual uint64_t stage() const PURE;
 
   /**
    * @return runtime key to be set to disable the configuration.
@@ -72,7 +72,7 @@ class RateLimitPolicy {
    * @return set of RateLimitPolicyEntry that are applicable for a stage.
    */
   virtual const std::vector<std::reference_wrapper<const RateLimitPolicyEntry>>&
-  getApplicableRateLimit(int64_t stage) const PURE;
+  getApplicableRateLimit(uint64_t stage) const PURE;
 };
 
 } // Router

source/common/http/async_client_impl.h

@@ -75,7 +75,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
   struct NullRateLimitPolicy : public Router::RateLimitPolicy {
     // Router::RateLimitPolicy
     const std::vector<std::reference_wrapper<const Router::RateLimitPolicyEntry>>&
-        getApplicableRateLimit(int64_t) const override {
+        getApplicableRateLimit(uint64_t) const override {
       return rate_limit_policy_entry_;
     }
 

source/common/http/filter/ratelimit.cc

@@ -6,20 +6,11 @@
 #include "common/common/empty_string.h"
 #include "common/common/enum_to_int.h"
 #include "common/http/codes.h"
-#include "common/json/config_schemas.h"
 #include "common/router/config_impl.h"
 
 namespace Http {
 namespace RateLimit {
 
-FilterConfig::FilterConfig(const Json::Object& config, const LocalInfo::LocalInfo& local_info,
-                           Stats::Store& global_store, Runtime::Loader& runtime,
-                           Upstream::ClusterManager& cm)
-    : domain_(config.getString("domain")), stage_(config.getInteger("stage", 0)),
-      local_info_(local_info), global_store_(global_store), runtime_(runtime), cm_(cm) {
-  config.validateSchema(Json::Schema::RATE_LIMIT_HTTP_FILTER_SCHEMA);
-}
-
 const Http::HeaderMapPtr Filter::TOO_MANY_REQUESTS_HEADER{new Http::HeaderMapImpl{
     {Http::Headers::get().Status, std::to_string(enumToInt(Code::TooManyRequests))}}};
 

source/common/http/filter/ratelimit.h

@@ -7,29 +7,35 @@
 #include "envoy/upstream/cluster_manager.h"
 
 #include "common/http/header_map_impl.h"
+#include "common/json/config_schemas.h"
 #include "common/json/json_loader.h"
+#include "common/json/json_validator.h"
 
 namespace Http {
 namespace RateLimit {
 
 /**
  * Global configuration for the HTTP rate limit filter.
  */
-class FilterConfig {
+class FilterConfig : Json::JsonValidator {
 public:
   FilterConfig(const Json::Object& config, const LocalInfo::LocalInfo& local_info,
-               Stats::Store& global_store, Runtime::Loader& runtime, Upstream::ClusterManager& cm);
+               Stats::Store& global_store, Runtime::Loader& runtime, Upstream::ClusterManager& cm)
+      : Json::JsonValidator(config, Json::Schema::RATE_LIMIT_HTTP_FILTER_SCHEMA),
+        domain_(config.getString("domain")),
+        stage_(static_cast<uint64_t>(config.getInteger("stage", 0))), local_info_(local_info),
+        global_store_(global_store), runtime_(runtime), cm_(cm) {}
 
   const std::string& domain() const { return domain_; }
   const LocalInfo::LocalInfo& localInfo() const { return local_info_; }
-  int64_t stage() const { return stage_; }
+  uint64_t stage() const { return stage_; }
   Runtime::Loader& runtime() { return runtime_; }
   Stats::Store& globalStore() { return global_store_; }
   Upstream::ClusterManager& cm() { return cm_; }
 
 private:
   const std::string domain_;
-  int64_t stage_;
+  uint64_t stage_;
   const LocalInfo::LocalInfo& local_info_;
   Stats::Store& global_store_;
   Runtime::Loader& runtime_;

source/common/json/config_schemas.cc

@@ -645,7 +645,11 @@ const std::string Json::Schema::HTTP_RATE_LIMITS_CONFIGURATION_SCHEMA(R"EOF(
     },
     "type" : "object",
     "properties" : {
-      "stage" : {"type" : "integer"},
+      "stage" : {
+        "type" : "integer",
+        "minimum" : 0,
+        "maximum" : 10
+      },
       "disable_key" : {"type" : "string"},
       "actions" : {
         "type" : "array",
@@ -756,7 +760,11 @@ const std::string Json::Schema::RATE_LIMIT_HTTP_FILTER_SCHEMA(R"EOF(
     "type" : "object",
     "properties" : {
       "domain" : {"type" : "string"},
-      "stage" : {"type" : "integer"}
+      "stage" : {
+        "type" : "integer",
+        "minimum" : 0,
+        "maximum" : 10
+      }
     },
     "required" : ["domain"],
     "additionalProperties" : false

source/common/router/router_ratelimit.cc

@@ -1,12 +1,12 @@
 #include "router_ratelimit.h"
 
+#include "common/common/assert.h"
 #include "common/common/empty_string.h"
 #include "common/json/config_schemas.h"
 
 namespace Router {
 
-const std::vector<std::reference_wrapper<const RateLimitPolicyEntry>>
-    RateLimitPolicyImpl::empty_rate_limit_;
+const uint64_t RateLimitPolicyImpl::MAX_STAGE_NUMBER = 10UL;
 
 void SourceClusterAction::populateDescriptor(const Router::RouteEntry&,
                                              ::RateLimit::Descriptor& descriptor,
@@ -69,10 +69,9 @@ void HeaderValueMatchAction::populateDescriptor(const Router::RouteEntry&,
 }
 
 RateLimitPolicyEntryImpl::RateLimitPolicyEntryImpl(const Json::Object& config)
-    : disable_key_(config.getString("disable_key", "")), stage_(config.getInteger("stage", 0)) {
-
-  config.validateSchema(Json::Schema::HTTP_RATE_LIMITS_CONFIGURATION_SCHEMA);
-
+    : Json::JsonValidator(config, Json::Schema::HTTP_RATE_LIMITS_CONFIGURATION_SCHEMA),
+      disable_key_(config.getString("disable_key", "")),
+      stage_(static_cast<uint64_t>(config.getInteger("stage", 0))) {
   for (const Json::ObjectPtr& action : config.getObjectArray("actions")) {
     std::string type = action->getString("type");
     if (type == "source_cluster") {
@@ -107,31 +106,24 @@ void RateLimitPolicyEntryImpl::populateDescriptors(
   }
 }
 
-RateLimitPolicyImpl::RateLimitPolicyImpl(const Json::Object& config) {
+RateLimitPolicyImpl::RateLimitPolicyImpl(const Json::Object& config)
+    : rate_limit_entries_reference_(RateLimitPolicyImpl::MAX_STAGE_NUMBER + 1) {
   if (config.hasObject("rate_limits")) {
-    std::vector<std::unique_ptr<RateLimitPolicyEntry>> rate_limit_policy;
-    std::vector<std::reference_wrapper<const RateLimitPolicyEntry>> rate_limit_policy_reference;
     for (const Json::ObjectPtr& rate_limit : config.getObjectArray("rate_limits")) {
       std::unique_ptr<RateLimitPolicyEntry> rate_limit_policy_entry(
           new RateLimitPolicyEntryImpl(*rate_limit));
-      rate_limit_policy_reference.emplace_back(*rate_limit_policy_entry);
-      rate_limit_policy.emplace_back(std::move(rate_limit_policy_entry));
+      uint64_t stage = rate_limit_policy_entry->stage();
+      ASSERT(stage < rate_limit_entries_reference_.size());
+      rate_limit_entries_reference_[stage].emplace_back(*rate_limit_policy_entry);
+      rate_limit_entries_.emplace_back(std::move(rate_limit_policy_entry));
     }
-    rate_limit_entries_.emplace_back(std::move(rate_limit_policy));
-    rate_limit_entries_reference_.emplace_back(rate_limit_policy_reference);
   }
 }
 
-const std::vector<std::reference_wrapper<const RateLimitPolicyEntry>>&
-    RateLimitPolicyImpl::getApplicableRateLimit(int64_t) const {
-  // Currently return all rate limit policy entries.
-  // TODO(mattklein123): Implement returning only rate limit policy entries that match the stage
-  // setting.
-  if (rate_limit_entries_.empty()) {
-    return empty_rate_limit_;
-  } else {
-    return rate_limit_entries_reference_[0];
-  }
+const std::vector<std::reference_wrapper<const Router::RateLimitPolicyEntry>>&
+RateLimitPolicyImpl::getApplicableRateLimit(uint64_t stage) const {
+  ASSERT(stage < rate_limit_entries_reference_.size());
+  return rate_limit_entries_reference_[stage];
 }
 
 } // Router

source/common/router/router_ratelimit.h

@@ -4,6 +4,7 @@
 #include "envoy/router/router_ratelimit.h"
 
 #include "common/http/filter/ratelimit.h"
+#include "common/json/json_validator.h"
 #include "common/router/config_utility.h"
 
 namespace Router {
@@ -97,23 +98,21 @@ class HeaderValueMatchAction : public RateLimitAction {
 /*
  * Implementation of RateLimitPolicyEntry that holds the action for the configuration.
  */
-class RateLimitPolicyEntryImpl : public RateLimitPolicyEntry {
+class RateLimitPolicyEntryImpl : public RateLimitPolicyEntry, Json::JsonValidator {
 public:
   RateLimitPolicyEntryImpl(const Json::Object& config);
 
   // Router::RateLimitPolicyEntry
-  int64_t stage() const override { return stage_; }
+  uint64_t stage() const override { return stage_; }
   const std::string& disableKey() const override { return disable_key_; }
-
-  // Router::RateLimitAction
   void populateDescriptors(const Router::RouteEntry& route,
                            std::vector<::RateLimit::Descriptor>& descriptors,
                            const std::string& local_service_cluster, const Http::HeaderMap&,
                            const std::string& remote_address) const override;
 
 private:
   const std::string disable_key_;
-  int64_t stage_;
+  uint64_t stage_;
   std::vector<RateLimitActionPtr> actions_;
 };
 
@@ -126,13 +125,16 @@ class RateLimitPolicyImpl : public RateLimitPolicy {
 
   // Router::RateLimitPolicy
   const std::vector<std::reference_wrapper<const RateLimitPolicyEntry>>&
-  getApplicableRateLimit(int64_t stage = 0) const override;
+  getApplicableRateLimit(uint64_t stage = 0) const override;
 
 private:
-  std::vector<std::vector<std::unique_ptr<RateLimitPolicyEntry>>> rate_limit_entries_;
+  std::vector<std::unique_ptr<RateLimitPolicyEntry>> rate_limit_entries_;
   std::vector<std::vector<std::reference_wrapper<const RateLimitPolicyEntry>>>
       rate_limit_entries_reference_;
-  static const std::vector<std::reference_wrapper<const RateLimitPolicyEntry>> empty_rate_limit_;
+  // The maximum stage number supported. This value should match the maximum stage number in
+  // Json::Schema::HTTP_RATE_LIMITS_CONFIGURATION_SCHEMA and
+  // Json::Schema::RATE_LIMIT_HTTP_FILTER_SCHEMA from common/json/config_schemas.cc.
+  static const uint64_t MAX_STAGE_NUMBER;
 };
 
-} // Router
+} // Router