ratelimit: add support for failure_mode_allow configuration

api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto

@@ -4,6 +4,7 @@ package envoy.config.filter.http.rate_limit.v2;
 option go_package = "v2";
 
 import "google/protobuf/duration.proto";
+import "google/protobuf/wrappers.proto";
 
 import "validate/validate.proto";
 import "gogoproto/gogo.proto";
@@ -34,4 +35,10 @@ message RateLimit {
   // The timeout in milliseconds for the rate limit service RPC. If not
   // set, this defaults to 20ms.
   google.protobuf.Duration timeout = 4 [(gogoproto.stdduration) = true];
+
+  // The filter's behaviour in case the rate limiting service does
+  // not respond back. When it is set to true, Envoy will also allow traffic in case of
+  // communication failure between rate limiting service and the proxy.
+  // Defaults to true.
+  google.protobuf.BoolValue failure_mode_allow = 5;
 }

api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto

@@ -5,6 +5,7 @@ option go_package = "v2";
 
 import "envoy/api/v2/ratelimit/ratelimit.proto";
 import "google/protobuf/duration.proto";
+import "google/protobuf/wrappers.proto";
 
 import "validate/validate.proto";
 import "gogoproto/gogo.proto";
@@ -26,4 +27,10 @@ message RateLimit {
   // The timeout in milliseconds for the rate limit service RPC. If not
   // set, this defaults to 20ms.
   google.protobuf.Duration timeout = 4 [(gogoproto.stdduration) = true];
+
+  // The filter's behaviour in case the rate limiting service does
+  // not respond back. When it is set to true, Envoy will also allow traffic in case of
+  // communication failure between rate limiting service and the proxy.
+  // Defaults to true.
+  google.protobuf.BoolValue failure_mode_allow = 5;
 }

docs/root/configuration/http_filters/rate_limit_filter.rst

@@ -16,6 +16,9 @@ apply to a request. Each configuration results in a descriptor being sent to the
 If the rate limit service is called, and the response for any of the descriptors is over limit, a
 429 response is returned.
 
+If there is an error in calling rate limit service or rate limit service returns an error and :ref:`failure_mode_allow <envoy_api_msg_config.filter.http.rate_limit.v2.RateLimit>` is 
+set to false, a 500 response is returned.
+
 .. _config_http_filters_rate_limit_composing_actions:
 
 Composing Actions
@@ -108,6 +111,8 @@ The buffer filter outputs statistics in the *cluster.<route target cluster>.rate
   ok, Counter, Total under limit responses from the rate limit service
   error, Counter, Total errors contacting the rate limit service
   over_limit, Counter, total over limit responses from the rate limit service
+  failure_mode_allowed, Counter, "Total requests that were error(s) but were allowed through because
+  of :ref:`failure_mode_allow <envoy_api_msg_config.filter.http.rate_limit.v2.RateLimit>` set to true."
 
 Runtime
 -------

docs/root/configuration/network_filters/rate_limit_filter.rst

@@ -25,6 +25,8 @@ following statistics:
   ok, Counter, Total under limit responses from the rate limit service
   cx_closed, Counter, Total connections closed due to an over limit response from the rate limit service
   active, Gauge, Total active requests to the rate limit service
+  failure_mode_allowed, Counter, "Total requests that were error(s) but were allowed through because
+  of :ref:`failure_mode_allow <envoy_api_msg_config.filter.http.rate_limit.v2.RateLimit>` set to true."
 
 Runtime
 -------

docs/root/intro/version_history.rst

@@ -58,6 +58,8 @@ Version history
 * upstream: require opt-in to use the :ref:`x-envoy-orignal-dst-host <config_http_conn_man_headers_x-envoy-original-dst-host>` header
   for overriding destination address when using the :ref:`Original Destination <arch_overview_load_balancing_types_original_destination>`
   load balancing policy.
+* ratelimit: added :ref:`failure_mode_allow <envoy_api_msg_config.filter.http.rate_limit.v2.RateLimit>` option to control traffic flow in 
+  case of rate limit service error.
 
 1.7.0
 ===============

include/envoy/request_info/request_info.h

@@ -48,8 +48,10 @@ enum ResponseFlag {
   RateLimited = 0x800,
   // Request was unauthorized by external authorization service.
   UnauthorizedExternalService = 0x1000,
+  // Unable to call Ratelimiting service.
+  RateLimitingServiceError = 0x1200,
   // ATTENTION: MAKE SURE THIS REMAINS EQUAL TO THE LAST FLAG.
-  LastFlag = UnauthorizedExternalService
+  LastFlag = RateLimitingServiceError
 };
 
 /**

source/common/request_info/utility.cc

@@ -31,7 +31,7 @@ void ResponseFlagUtils::appendString(std::string& result, const std::string& app
 const std::string ResponseFlagUtils::toShortString(const RequestInfo& request_info) {
   std::string result;
 
-  static_assert(ResponseFlag::LastFlag == 0x1000, "A flag has been added. Fix this code.");
+  static_assert(ResponseFlag::LastFlag == 0x1200, "A flag has been added. Fix this code.");
 
   if (request_info.hasResponseFlag(ResponseFlag::FailedLocalHealthCheck)) {
     appendString(result, FAILED_LOCAL_HEALTH_CHECK);

source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc

@@ -85,7 +85,7 @@ void HttpGrpcAccessLog::responseFlagsToAccessLogResponseFlags(
     envoy::data::accesslog::v2::AccessLogCommon& common_access_log,
     const RequestInfo::RequestInfo& request_info) {
 
-  static_assert(RequestInfo::ResponseFlag::LastFlag == 0x1000,
+  static_assert(RequestInfo::ResponseFlag::LastFlag == 0x1200,
                 "A flag has been added. Fix this code.");
 
   if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::FailedLocalHealthCheck)) {

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

@@ -125,6 +125,16 @@ void Filter::complete(RateLimit::LimitStatus status) {
     state_ = State::Responded;
     callbacks_->sendLocalReply(Http::Code::TooManyRequests, "", nullptr);
     callbacks_->requestInfo().setResponseFlag(RequestInfo::ResponseFlag::RateLimited);
+  } else if (status == RateLimit::LimitStatus::Error) {
+    if (config_->failureModeAllow()) {
+      cluster_->statsScope().counter("ratelimit.failure_mode_allowed").inc();
+      callbacks_->continueDecoding();
+    } else {
+      state_ = State::Responded;
+      callbacks_->sendLocalReply(Http::Code::InternalServerError, "", nullptr);
+      callbacks_->requestInfo().setResponseFlag(
+          RequestInfo::ResponseFlag::RateLimitingServiceError);
+    }
   } else if (!initiating_call_) {
     callbacks_->continueDecoding();
   }

source/extensions/filters/http/ratelimit/ratelimit.h

@@ -37,8 +37,8 @@ class FilterConfig {
       : domain_(config.domain()), stage_(static_cast<uint64_t>(config.stage())),
         request_type_(config.request_type().empty() ? stringToType("both")
                                                     : stringToType(config.request_type())),
-        local_info_(local_info), scope_(scope), runtime_(runtime), cm_(cm) {}
-
+        local_info_(local_info), scope_(scope), runtime_(runtime), cm_(cm),
+        failure_mode_allow_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, failure_mode_allow, true)) {}
   const std::string& domain() const { return domain_; }
   const LocalInfo::LocalInfo& localInfo() const { return local_info_; }
   uint64_t stage() const { return stage_; }
@@ -47,6 +47,8 @@ class FilterConfig {
   Upstream::ClusterManager& cm() { return cm_; }
   FilterRequestType requestType() const { return request_type_; }
 
+  bool failureModeAllow() const { return failure_mode_allow_; }
+
 private:
   static FilterRequestType stringToType(const std::string& request_type) {
     if (request_type == "internal") {
@@ -66,6 +68,7 @@ class FilterConfig {
   Stats::Scope& scope_;
   Runtime::Loader& runtime_;
   Upstream::ClusterManager& cm_;
+  bool failure_mode_allow_;
 };
 
 typedef std::shared_ptr<FilterConfig> FilterConfigSharedPtr;

source/extensions/filters/network/ratelimit/ratelimit.cc

@@ -16,8 +16,8 @@ namespace RateLimitFilter {
 Config::Config(const envoy::config::filter::network::rate_limit::v2::RateLimit& config,
                Stats::Scope& scope, Runtime::Loader& runtime)
     : domain_(config.domain()), stats_(generateStats(config.stat_prefix(), scope)),
-      runtime_(runtime) {
-
+      runtime_(runtime),
+      failure_mode_allow_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, failure_mode_allow, true)) {
   for (const auto& descriptor : config.descriptors()) {
     RateLimit::Descriptor new_descriptor;
     for (const auto& entry : descriptor.entries()) {
@@ -85,11 +85,18 @@ void Filter::complete(RateLimit::LimitStatus status) {
     break;
   }
 
-  // We fail open if there is an error contacting the service.
   if (status == RateLimit::LimitStatus::OverLimit &&
       config_->runtime().snapshot().featureEnabled("ratelimit.tcp_filter_enforcing", 100)) {
     config_->stats().cx_closed_.inc();
     filter_callbacks_->connection().close(Network::ConnectionCloseType::NoFlush);
+  } else if (status == RateLimit::LimitStatus::Error) {
+    if (config_->failureModeAllow()) {
+      config_->stats().failure_mode_allowed_.inc();
+      filter_callbacks_->continueReading();
+    } else {
+      config_->stats().cx_closed_.inc();
+      filter_callbacks_->connection().close(Network::ConnectionCloseType::NoFlush);
+    }
   } else {
     // We can get completion inline, so only call continue if that isn't happening.
     if (!calling_limit_) {

source/extensions/filters/network/ratelimit/ratelimit.h

@@ -27,6 +27,7 @@ namespace RateLimitFilter {
   COUNTER(error)                                                                                   \
   COUNTER(over_limit)                                                                              \
   COUNTER(ok)                                                                                      \
+  COUNTER(failure_mode_allowed)                                                                    \
   COUNTER(cx_closed)                                                                               \
   GAUGE  (active)
 // clang-format on
@@ -49,6 +50,7 @@ class Config {
   const std::vector<RateLimit::Descriptor>& descriptors() { return descriptors_; }
   Runtime::Loader& runtime() { return runtime_; }
   const InstanceStats& stats() { return stats_; }
+  bool failureModeAllow() const { return failure_mode_allow_; };
 
 private:
   static InstanceStats generateStats(const std::string& name, Stats::Scope& scope);
@@ -57,6 +59,7 @@ class Config {
   std::vector<RateLimit::Descriptor> descriptors_;
   const InstanceStats stats_;
   Runtime::Loader& runtime_;
+  bool failure_mode_allow_;
 };
 
 typedef std::shared_ptr<Config> ConfigSharedPtr;

test/common/access_log/access_log_impl_test.cc

@@ -817,7 +817,7 @@ name: envoy.file_access_log
   path: /dev/null
   )EOF";
 
-  static_assert(RequestInfo::ResponseFlag::LastFlag == 0x1000,
+  static_assert(RequestInfo::ResponseFlag::LastFlag == 0x1200,
                 "A flag has been added. Fix this code.");
 
   std::vector<RequestInfo::ResponseFlag> all_response_flags = {

test/common/request_info/utility_test.cc

@@ -14,7 +14,7 @@ namespace Envoy {
 namespace RequestInfo {
 
 TEST(ResponseFlagUtilsTest, toShortStringConversion) {
-  static_assert(ResponseFlag::LastFlag == 0x1000, "A flag has been added. Fix this code.");
+  static_assert(ResponseFlag::LastFlag == 0x1200, "A flag has been added. Fix this code.");
 
   std::vector<std::pair<ResponseFlag, std::string>> expected = {
       std::make_pair(ResponseFlag::FailedLocalHealthCheck, "LH"),
@@ -58,7 +58,7 @@ TEST(ResponseFlagUtilsTest, toShortStringConversion) {
 }
 
 TEST(ResponseFlagsUtilsTest, toResponseFlagConversion) {
-  static_assert(ResponseFlag::LastFlag == 0x1000, "A flag has been added. Fix this code.");
+  static_assert(ResponseFlag::LastFlag == 0x1200, "A flag has been added. Fix this code.");
 
   std::vector<std::pair<std::string, ResponseFlag>> expected = {
       std::make_pair("LH", ResponseFlag::FailedLocalHealthCheck),

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

@@ -46,10 +46,13 @@ class HttpRateLimitFilterTest : public testing::Test {
         .WillByDefault(Return(true));
   }
 
-  void SetUpTest(const std::string json) {
+  void SetUpTest(const std::string json, const bool failure_mode = true) {
     Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json);
     envoy::config::filter::http::rate_limit::v2::RateLimit proto_config{};
     Config::FilterJson::translateHttpRateLimitFilter(*json_config, proto_config);
+    // TODO(ramaraochavali): move filter_config_ to yaml and use a different config for
+    // failure_mode.
+    proto_config.mutable_failure_mode_allow()->set_value(failure_mode);
     config_.reset(new FilterConfig(proto_config, local_info_, stats_store_, runtime_, cm_));
 
     client_ = new RateLimit::MockClient();
@@ -239,6 +242,37 @@ TEST_F(HttpRateLimitFilterTest, ErrorResponse) {
   EXPECT_EQ(
       1U,
       cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("ratelimit.error").value());
+  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
+                    .counter("ratelimit.failure_mode_allowed")
+                    .value());
+}
+
+TEST_F(HttpRateLimitFilterTest, ErrorResponseWithFailureAllowModeOff) {
+  SetUpTest(filter_config_, false);
+  InSequence s;
+
+  EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _))
+      .WillOnce(SetArgReferee<1>(descriptor_));
+  EXPECT_CALL(*client_, limit(_, _, _, _))
+      .WillOnce(WithArgs<0>(Invoke([&](RateLimit::RequestCallbacks& callbacks) -> void {
+        request_callbacks_ = &callbacks;
+      })));
+
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(request_headers_, false));
+
+  request_callbacks_->complete(RateLimit::LimitStatus::Error);
+
+  EXPECT_CALL(filter_callbacks_.request_info_,
+              setResponseFlag(RequestInfo::ResponseFlag::RateLimitingServiceError))
+      .Times(0);
+
+  EXPECT_EQ(
+      1U,
+      cm_.thread_local_cluster_.cluster_.info_->stats_store_.counter("ratelimit.error").value());
+  EXPECT_EQ(0U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
+                    .counter("ratelimit.failure_mode_allowed")
+                    .value());
 }
 
 TEST_F(HttpRateLimitFilterTest, LimitResponse) {

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

@@ -33,7 +33,9 @@ namespace RateLimitFilter {
 
 class RateLimitFilterTest : public testing::Test {
 public:
-  RateLimitFilterTest() {
+  RateLimitFilterTest() {}
+
+  void SetUp() override {
     std::string json = R"EOF(
     {
       "domain": "foo",
@@ -53,6 +55,8 @@ class RateLimitFilterTest : public testing::Test {
     Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json);
     envoy::config::filter::network::rate_limit::v2::RateLimit proto_config{};
     Envoy::Config::FilterJson::translateTcpRateLimitFilter(*json_config, proto_config);
+    // TODO(ramaraochavali): move the config to yaml and use a different config for failure_mode.
+    proto_config.mutable_failure_mode_allow()->set_value(failure_mode_);
     config_.reset(new Config(proto_config, stats_store_, runtime_));
     client_ = new RateLimit::MockClient();
     filter_.reset(new Filter(config_, RateLimit::ClientPtr{client_}));
@@ -76,6 +80,16 @@ class RateLimitFilterTest : public testing::Test {
   std::unique_ptr<Filter> filter_;
   NiceMock<Network::MockReadFilterCallbacks> filter_callbacks_;
   RateLimit::RequestCallbacks* request_callbacks_{};
+  bool failure_mode_{true};
+};
+
+class RateLimitFilterFailureModeTest : public RateLimitFilterTest {
+public:
+  RateLimitFilterFailureModeTest() {
+    failure_mode_ = false;
+    std::cout << "setting failure mode to false..."
+              << "\n";
+  }
 };
 
 TEST_F(RateLimitFilterTest, BadRatelimitConfig) {
@@ -194,6 +208,7 @@ TEST_F(RateLimitFilterTest, Error) {
 
   EXPECT_EQ(1U, stats_store_.counter("ratelimit.name.total").value());
   EXPECT_EQ(1U, stats_store_.counter("ratelimit.name.error").value());
+  EXPECT_EQ(1U, stats_store_.counter("ratelimit.name.failure_mode_allowed").value());
 }
 
 TEST_F(RateLimitFilterTest, Disconnect) {
@@ -247,6 +262,29 @@ TEST_F(RateLimitFilterTest, RuntimeDisable) {
   EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data, false));
 }
 
+TEST_F(RateLimitFilterFailureModeTest, ErrorResponseWithFailureAllowModeOff) {
+  InSequence s;
+
+  EXPECT_CALL(*client_, limit(_, "foo", _, _))
+      .WillOnce(WithArgs<0>(Invoke([&](RateLimit::RequestCallbacks& callbacks) -> void {
+        request_callbacks_ = &callbacks;
+      })));
+
+  EXPECT_EQ(Network::FilterStatus::StopIteration, filter_->onNewConnection());
+  Buffer::OwnedImpl data("hello");
+  EXPECT_EQ(Network::FilterStatus::StopIteration, filter_->onData(data, false));
+  request_callbacks_->complete(RateLimit::LimitStatus::Error);
+
+  EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data, false));
+
+  EXPECT_CALL(*client_, cancel()).Times(0);
+  filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose);
+
+  EXPECT_EQ(1U, stats_store_.counter("ratelimit.name.total").value());
+  EXPECT_EQ(1U, stats_store_.counter("ratelimit.name.error").value());
+  EXPECT_EQ(0U, stats_store_.counter("ratelimit.name.failure_mode_allowed").value());
+}
+
 } // namespace RateLimitFilter
 } // namespace NetworkFilters
 } // namespace Extensions