From f0a6a887304c5117d6bb6fb2335a6ca2fdda20e5 Mon Sep 17 00:00:00 2001 From: Rama Date: Tue, 7 Aug 2018 13:35:56 +0530 Subject: [PATCH 01/11] support failure_mode_allow in ratelimit Signed-off-by: Rama --- .../http/rate_limit/v2/rate_limit.proto | 7 ++++ .../network/rate_limit/v2/rate_limit.proto | 7 ++++ .../http_filters/ext_authz_filter.rst | 2 +- .../http_filters/rate_limit_filter.rst | 5 +++ .../network_filters/rate_limit_filter.rst | 2 + docs/root/intro/version_history.rst | 1 + include/envoy/request_info/request_info.h | 4 +- source/common/request_info/utility.cc | 2 +- .../http_grpc/grpc_access_log_impl.cc | 2 +- .../filters/http/ratelimit/ratelimit.cc | 10 +++++ .../filters/http/ratelimit/ratelimit.h | 7 +++- .../filters/network/ratelimit/ratelimit.cc | 13 ++++-- .../filters/network/ratelimit/ratelimit.h | 3 ++ .../filters/http/ratelimit/ratelimit_test.cc | 36 ++++++++++++++++- .../network/ratelimit/ratelimit_test.cc | 40 ++++++++++++++++++- .../integration/ratelimit_integration_test.cc | 33 ++++++++++++++- 16 files changed, 161 insertions(+), 13 deletions(-) diff --git a/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto b/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto index 9d3f50e7542b0..279f5eedd58fd 100644 --- a/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto +++ b/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; } diff --git a/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto b/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto index 240129499ace4..e932e7440c96a 100644 --- a/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto +++ b/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; } diff --git a/docs/root/configuration/http_filters/ext_authz_filter.rst b/docs/root/configuration/http_filters/ext_authz_filter.rst index bd92156303970..88f6ed46f1938 100644 --- a/docs/root/configuration/http_filters/ext_authz_filter.rst +++ b/docs/root/configuration/http_filters/ext_authz_filter.rst @@ -82,5 +82,5 @@ The HTTP filter outputs statistics in the *cluster..ext_au ok, Counter, Total responses from the filter. error, Counter, Total errors contacting the external service. denied, Counter, Total responses from the authorizations service that were to deny the traffic. - failure_mode_allow, Counter, "Total requests that were error(s) but were allowed through because + failure_mode_allowed, Counter, "Total requests that were error(s) but were allowed through because of failure_mode_allow set to true." diff --git a/docs/root/configuration/http_filters/rate_limit_filter.rst b/docs/root/configuration/http_filters/rate_limit_filter.rst index dcac97e337cb3..d0da7a7d6cd40 100644 --- a/docs/root/configuration/http_filters/rate_limit_filter.rst +++ b/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 ` 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..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 ` set to true." Runtime ------- diff --git a/docs/root/configuration/network_filters/rate_limit_filter.rst b/docs/root/configuration/network_filters/rate_limit_filter.rst index 4cefd6813699a..3e5377948f9b9 100644 --- a/docs/root/configuration/network_filters/rate_limit_filter.rst +++ b/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 ` set to true." Runtime ------- diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 62f92f2070040..ade1944a4d2c9 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -54,6 +54,7 @@ Version history * thrift_proxy: introduced thrift routing, moved configuration to correct location * upstream: added configuration option to the subset load balancer to take locality weights into account when selecting a host from a subset. +* ratelimit: added :ref:`failure_mode_allow ` option to control traffic flow in case of rate limit error. 1.7.0 =============== diff --git a/include/envoy/request_info/request_info.h b/include/envoy/request_info/request_info.h index ee21a3ae7e8b1..6a6fa2f4b5bde 100644 --- a/include/envoy/request_info/request_info.h +++ b/include/envoy/request_info/request_info.h @@ -47,8 +47,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 }; /** diff --git a/source/common/request_info/utility.cc b/source/common/request_info/utility.cc index 27a01a6b24453..b7d6f4837980f 100644 --- a/source/common/request_info/utility.cc +++ b/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); diff --git a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc b/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc index 6804cdd9bd617..b586d9ce3f531 100644 --- a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc +++ b/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)) { diff --git a/source/extensions/filters/http/ratelimit/ratelimit.cc b/source/extensions/filters/http/ratelimit/ratelimit.cc index d63f0ca6cdd36..7788cf12a8da6 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.cc +++ b/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(); } diff --git a/source/extensions/filters/http/ratelimit/ratelimit.h b/source/extensions/filters/http/ratelimit/ratelimit.h index e121e383a9dec..ed0da1a006cc6 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.h +++ b/source/extensions/filters/http/ratelimit/ratelimit.h @@ -36,8 +36,8 @@ class FilterConfig { : domain_(config.domain()), stage_(static_cast(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_; } @@ -46,6 +46,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") { @@ -65,6 +67,7 @@ class FilterConfig { Stats::Scope& scope_; Runtime::Loader& runtime_; Upstream::ClusterManager& cm_; + bool failure_mode_allow_; }; typedef std::shared_ptr FilterConfigSharedPtr; diff --git a/source/extensions/filters/network/ratelimit/ratelimit.cc b/source/extensions/filters/network/ratelimit/ratelimit.cc index e677ae7677497..a4c6cc6229e40 100644 --- a/source/extensions/filters/network/ratelimit/ratelimit.cc +++ b/source/extensions/filters/network/ratelimit/ratelimit.cc @@ -14,8 +14,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()) { @@ -83,11 +83,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_) { diff --git a/source/extensions/filters/network/ratelimit/ratelimit.h b/source/extensions/filters/network/ratelimit/ratelimit.h index a8d1c6fb5f82c..b32253f64173d 100644 --- a/source/extensions/filters/network/ratelimit/ratelimit.h +++ b/source/extensions/filters/network/ratelimit/ratelimit.h @@ -26,6 +26,7 @@ namespace RateLimitFilter { COUNTER(error) \ COUNTER(over_limit) \ COUNTER(ok) \ + COUNTER(failure_mode_allowed) \ COUNTER(cx_closed) \ GAUGE (active) // clang-format on @@ -48,6 +49,7 @@ class Config { const std::vector& 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); @@ -56,6 +58,7 @@ class Config { std::vector descriptors_; const InstanceStats stats_; Runtime::Loader& runtime_; + bool failure_mode_allow_; }; typedef std::shared_ptr ConfigSharedPtr; diff --git a/test/extensions/filters/http/ratelimit/ratelimit_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_test.cc index cbcec14f27383..d7dfe080b63e2 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_test.cc +++ b/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) { diff --git a/test/extensions/filters/network/ratelimit/ratelimit_test.cc b/test/extensions/filters/network/ratelimit/ratelimit_test.cc index ceb163f0ccfff..993472895f2a4 100644 --- a/test/extensions/filters/network/ratelimit/ratelimit_test.cc +++ b/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_; NiceMock 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 diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index c45084e716fb2..cad1e2b3e90df 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -38,8 +38,13 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, } void initialize() override { - config_helper_.addFilter( - "{ name: envoy.rate_limit, config: { domain: some_domain, timeout: 0.5s } }"); + if (failure_mode_) { + config_helper_.addFilter("{ name: envoy.rate_limit, config: { domain: some_domain, " + "failure_mode_allow: false, timeout: 0.5s } }"); + } else { + config_helper_.addFilter( + "{ name: envoy.rate_limit, config: { domain: some_domain, timeout: 0.5s } }"); + } config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { auto* ratelimit_cluster = bootstrap.mutable_static_resources()->add_clusters(); ratelimit_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); @@ -157,10 +162,19 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, const uint64_t request_size_ = 1024; const uint64_t response_size_ = 512; + bool failure_mode_ = false; +}; + +// Test that verifies failure mode cases. +class RatelimitFailureModeIntegrationTest : public RatelimitIntegrationTest { +public: + RatelimitFailureModeIntegrationTest() { failure_mode_ = true; } }; INSTANTIATE_TEST_CASE_P(IpVersionsClientType, RatelimitIntegrationTest, RATELIMIT_GRPC_CLIENT_INTEGRATION_PARAMS); +INSTANTIATE_TEST_CASE_P(IpVersionsClientType, RatelimitFailureModeIntegrationTest, + RATELIMIT_GRPC_CLIENT_INTEGRATION_PARAMS); TEST_P(RatelimitIntegrationTest, Ok) { initiateClientConnection(); @@ -197,6 +211,7 @@ TEST_P(RatelimitIntegrationTest, Error) { EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.ok")); EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.over_limit")); EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.ratelimit.error")->value()); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.ratelimit.failure_mode_allowed")->value()); } TEST_P(RatelimitIntegrationTest, Timeout) { @@ -245,5 +260,19 @@ TEST_P(RatelimitIntegrationTest, FailedConnect) { cleanup(); } +TEST_P(RatelimitFailureModeIntegrationTest, ErrorWithFailureModeOff) { + initiateClientConnection(); + waitForRatelimitRequest(); + ratelimit_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, true); + // Rate limiter fail closed + waitForFailedUpstreamResponse(500); + cleanup(); + + EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.ok")); + EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.over_limit")); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.ratelimit.error")->value()); + EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.failure_mode_allowed")); +} + } // namespace } // namespace Envoy From 88e49e6a9e61075a6a9c517efe47d99d64eb0473 Mon Sep 17 00:00:00 2001 From: Rama Date: Tue, 7 Aug 2018 13:39:02 +0530 Subject: [PATCH 02/11] corrected version history Signed-off-by: Rama --- docs/root/intro/version_history.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index df6ea90df79b4..8b929c88a259b 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -55,10 +55,11 @@ Version history * thrift_proxy: introduced thrift routing, moved configuration to correct location * upstream: added configuration option to the subset load balancer to take locality weights into account when selecting a host from a subset. -* ratelimit: added :ref:`failure_mode_allow ` option to control traffic flow in case of rate limit error. * upstream: require opt-in to use the :ref:`x-envoy-orignal-dst-host ` header for overriding destination address when using the :ref:`Original Destination ` load balancing policy. +* ratelimit: added :ref:`failure_mode_allow ` option to control traffic flow in + case of rate limit service error. 1.7.0 =============== From 83e75ec8aa4a5616bc64651de8846bc09f4b9967 Mon Sep 17 00:00:00 2001 From: Rama Date: Tue, 7 Aug 2018 13:45:23 +0530 Subject: [PATCH 03/11] revert some changes Signed-off-by: Rama --- docs/root/configuration/http_filters/ext_authz_filter.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/configuration/http_filters/ext_authz_filter.rst b/docs/root/configuration/http_filters/ext_authz_filter.rst index 88f6ed46f1938..bd92156303970 100644 --- a/docs/root/configuration/http_filters/ext_authz_filter.rst +++ b/docs/root/configuration/http_filters/ext_authz_filter.rst @@ -82,5 +82,5 @@ The HTTP filter outputs statistics in the *cluster..ext_au ok, Counter, Total responses from the filter. error, Counter, Total errors contacting the external service. denied, Counter, Total responses from the authorizations service that were to deny the traffic. - failure_mode_allowed, Counter, "Total requests that were error(s) but were allowed through because + failure_mode_allow, Counter, "Total requests that were error(s) but were allowed through because of failure_mode_allow set to true." From ef281ade31e2a49279d82e14b89b022d1dc44095 Mon Sep 17 00:00:00 2001 From: Rama Date: Tue, 7 Aug 2018 15:03:17 +0530 Subject: [PATCH 04/11] fix test compilation issues Signed-off-by: Rama --- test/common/access_log/access_log_impl_test.cc | 2 +- test/common/request_info/utility_test.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/common/access_log/access_log_impl_test.cc b/test/common/access_log/access_log_impl_test.cc index cf9b939d825cd..9226bccdd7665 100644 --- a/test/common/access_log/access_log_impl_test.cc +++ b/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 all_response_flags = { diff --git a/test/common/request_info/utility_test.cc b/test/common/request_info/utility_test.cc index e9058a1f1ed5a..630d6e2ee029b 100644 --- a/test/common/request_info/utility_test.cc +++ b/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> 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> expected = { std::make_pair("LH", ResponseFlag::FailedLocalHealthCheck), From 91439ea14dfc598d8cd18e90e0223b367f8565e0 Mon Sep 17 00:00:00 2001 From: Rama Date: Sun, 12 Aug 2018 14:47:41 +0530 Subject: [PATCH 05/11] addressed review feedback Signed-off-by: Rama --- api/envoy/data/accesslog/v2/accesslog.proto | 3 + include/envoy/request_info/request_info.h | 6 +- source/common/request_info/utility.cc | 1 + source/common/request_info/utility.h | 1 + .../http_grpc/grpc_access_log_impl.cc | 4 ++ .../filters/http/ratelimit/ratelimit.cc | 3 +- .../filters/http/ratelimit/ratelimit.h | 2 +- .../filters/network/ratelimit/ratelimit.h | 2 +- .../common/access_log/access_log_impl_test.cc | 1 + test/common/request_info/utility_test.cc | 1 + .../filters/http/ratelimit/ratelimit_test.cc | 24 +++---- .../network/ratelimit/ratelimit_test.cc | 67 +++++++++++-------- .../integration/ratelimit_integration_test.cc | 10 +-- 13 files changed, 74 insertions(+), 51 deletions(-) diff --git a/api/envoy/data/accesslog/v2/accesslog.proto b/api/envoy/data/accesslog/v2/accesslog.proto index 129e996d0afc6..a289730b45877 100644 --- a/api/envoy/data/accesslog/v2/accesslog.proto +++ b/api/envoy/data/accesslog/v2/accesslog.proto @@ -184,6 +184,9 @@ message ResponseFlags { // Indicates if the request was deemed unauthorized and the reason for it. Unauthorized unauthorized_details = 13; + + // Indicates that the request was rejected because there was an error in rate limit service. + bool rate_limit_service_error = 14; } // [#not-implemented-hide:] diff --git a/include/envoy/request_info/request_info.h b/include/envoy/request_info/request_info.h index 7395c638b766d..ee0c8e7d8a98f 100644 --- a/include/envoy/request_info/request_info.h +++ b/include/envoy/request_info/request_info.h @@ -48,10 +48,10 @@ enum ResponseFlag { RateLimited = 0x800, // Request was unauthorized by external authorization service. UnauthorizedExternalService = 0x1000, - // Unable to call Ratelimiting service. - RateLimitingServiceError = 0x1200, + // Unable to call Ratelimit service. + RateLimitServiceError = 0x1200, // ATTENTION: MAKE SURE THIS REMAINS EQUAL TO THE LAST FLAG. - LastFlag = RateLimitingServiceError + LastFlag = RateLimitServiceError }; /** diff --git a/source/common/request_info/utility.cc b/source/common/request_info/utility.cc index b7d6f4837980f..b069c4d756ae1 100644 --- a/source/common/request_info/utility.cc +++ b/source/common/request_info/utility.cc @@ -19,6 +19,7 @@ const std::string ResponseFlagUtils::DELAY_INJECTED = "DI"; const std::string ResponseFlagUtils::FAULT_INJECTED = "FI"; const std::string ResponseFlagUtils::RATE_LIMITED = "RL"; const std::string ResponseFlagUtils::UNAUTHORIZED_EXTERNAL_SERVICE = "UAEX"; +const std::string ResponseFlagUtils::RATELIMIT_SERVICE_ERROR = "RLSE"; void ResponseFlagUtils::appendString(std::string& result, const std::string& append) { if (result.empty()) { diff --git a/source/common/request_info/utility.h b/source/common/request_info/utility.h index 879bccd196cff..209a8a963ddeb 100644 --- a/source/common/request_info/utility.h +++ b/source/common/request_info/utility.h @@ -34,6 +34,7 @@ class ResponseFlagUtils { const static std::string FAULT_INJECTED; const static std::string RATE_LIMITED; const static std::string UNAUTHORIZED_EXTERNAL_SERVICE; + const static std::string RATELIMIT_SERVICE_ERROR; }; /** diff --git a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc b/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc index b586d9ce3f531..c8f9ce7db0184 100644 --- a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc +++ b/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc @@ -141,6 +141,10 @@ void HttpGrpcAccessLog::responseFlagsToAccessLogResponseFlags( envoy::data::accesslog::v2::ResponseFlags_Unauthorized_Reason:: ResponseFlags_Unauthorized_Reason_EXTERNAL_SERVICE); } + + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::RateLimitServiceError)) { + common_access_log.mutable_response_flags()->set_rate_limit_service_error(true); + } } void HttpGrpcAccessLog::log(const Http::HeaderMap* request_headers, diff --git a/source/extensions/filters/http/ratelimit/ratelimit.cc b/source/extensions/filters/http/ratelimit/ratelimit.cc index 7788cf12a8da6..786f16b73bde1 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.cc +++ b/source/extensions/filters/http/ratelimit/ratelimit.cc @@ -132,8 +132,7 @@ void Filter::complete(RateLimit::LimitStatus status) { } else { state_ = State::Responded; callbacks_->sendLocalReply(Http::Code::InternalServerError, "", nullptr); - callbacks_->requestInfo().setResponseFlag( - RequestInfo::ResponseFlag::RateLimitingServiceError); + callbacks_->requestInfo().setResponseFlag(RequestInfo::ResponseFlag::RateLimitServiceError); } } else if (!initiating_call_) { callbacks_->continueDecoding(); diff --git a/source/extensions/filters/http/ratelimit/ratelimit.h b/source/extensions/filters/http/ratelimit/ratelimit.h index db2b840736b00..7a4cc109a1a33 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.h +++ b/source/extensions/filters/http/ratelimit/ratelimit.h @@ -68,7 +68,7 @@ class FilterConfig { Stats::Scope& scope_; Runtime::Loader& runtime_; Upstream::ClusterManager& cm_; - bool failure_mode_allow_; + const bool failure_mode_allow_; }; typedef std::shared_ptr FilterConfigSharedPtr; diff --git a/source/extensions/filters/network/ratelimit/ratelimit.h b/source/extensions/filters/network/ratelimit/ratelimit.h index 0529f99d8d622..516e30c60e4c7 100644 --- a/source/extensions/filters/network/ratelimit/ratelimit.h +++ b/source/extensions/filters/network/ratelimit/ratelimit.h @@ -59,7 +59,7 @@ class Config { std::vector descriptors_; const InstanceStats stats_; Runtime::Loader& runtime_; - bool failure_mode_allow_; + const bool failure_mode_allow_; }; typedef std::shared_ptr ConfigSharedPtr; diff --git a/test/common/access_log/access_log_impl_test.cc b/test/common/access_log/access_log_impl_test.cc index 9226bccdd7665..c3a166041c81c 100644 --- a/test/common/access_log/access_log_impl_test.cc +++ b/test/common/access_log/access_log_impl_test.cc @@ -834,6 +834,7 @@ name: envoy.file_access_log RequestInfo::ResponseFlag::FaultInjected, RequestInfo::ResponseFlag::RateLimited, RequestInfo::ResponseFlag::UnauthorizedExternalService, + RequestInfo::ResponseFlag::RateLimitServiceError, }; InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV2Yaml(yaml), context_); diff --git a/test/common/request_info/utility_test.cc b/test/common/request_info/utility_test.cc index 630d6e2ee029b..300c23ab509de 100644 --- a/test/common/request_info/utility_test.cc +++ b/test/common/request_info/utility_test.cc @@ -30,6 +30,7 @@ TEST(ResponseFlagUtilsTest, toShortStringConversion) { std::make_pair(ResponseFlag::FaultInjected, "FI"), std::make_pair(ResponseFlag::RateLimited, "RL"), std::make_pair(ResponseFlag::UnauthorizedExternalService, "UAEX"), + std::make_pair(ResponseFlag::RateLimitServiceError, "RLSE"), }; for (const auto& test_case : expected) { diff --git a/test/extensions/filters/http/ratelimit/ratelimit_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_test.cc index d7dfe080b63e2..8142c6abb1d20 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/http/ratelimit/ratelimit_test.cc @@ -46,13 +46,10 @@ class HttpRateLimitFilterTest : public testing::Test { .WillByDefault(Return(true)); } - void SetUpTest(const std::string json, const bool failure_mode = true) { - Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json); + void SetUpTest(const std::string& yaml) { 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); + MessageUtil::loadFromYaml(yaml, proto_config); + config_.reset(new FilterConfig(proto_config, local_info_, stats_store_, runtime_, cm_)); client_ = new RateLimit::MockClient(); @@ -67,10 +64,13 @@ class HttpRateLimitFilterTest : public testing::Test { .emplace_back(vh_rate_limit_); } + const std::string fail_close_config_ = R"EOF( + domain: foo + failure_mode_allow: false + )EOF"; + const std::string filter_config_ = R"EOF( - { - "domain": "foo" - } + domain: foo )EOF"; FilterConfigSharedPtr config_; @@ -247,8 +247,8 @@ TEST_F(HttpRateLimitFilterTest, ErrorResponse) { .value()); } -TEST_F(HttpRateLimitFilterTest, ErrorResponseWithFailureAllowModeOff) { - SetUpTest(filter_config_, false); +TEST_F(HttpRateLimitFilterTest, ErrorResponseWithFailureModeAllowOff) { + SetUpTest(fail_close_config_); InSequence s; EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)) @@ -264,7 +264,7 @@ TEST_F(HttpRateLimitFilterTest, ErrorResponseWithFailureAllowModeOff) { request_callbacks_->complete(RateLimit::LimitStatus::Error); EXPECT_CALL(filter_callbacks_.request_info_, - setResponseFlag(RequestInfo::ResponseFlag::RateLimitingServiceError)) + setResponseFlag(RequestInfo::ResponseFlag::RateLimitServiceError)) .Times(0); EXPECT_EQ( diff --git a/test/extensions/filters/network/ratelimit/ratelimit_test.cc b/test/extensions/filters/network/ratelimit/ratelimit_test.cc index 993472895f2a4..269a0a1bc4966 100644 --- a/test/extensions/filters/network/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/network/ratelimit/ratelimit_test.cc @@ -35,28 +35,14 @@ class RateLimitFilterTest : public testing::Test { public: RateLimitFilterTest() {} - void SetUp() override { - std::string json = R"EOF( - { - "domain": "foo", - "descriptors": [ - [{"key": "hello", "value": "world"}, {"key": "foo", "value": "bar"}], - [{"key": "foo2", "value": "bar2"}] - ], - "stat_prefix": "name" - } - )EOF"; - + void SetUpTest(const std::string& yaml) { ON_CALL(runtime_.snapshot_, featureEnabled("ratelimit.tcp_filter_enabled", 100)) .WillByDefault(Return(true)); ON_CALL(runtime_.snapshot_, featureEnabled("ratelimit.tcp_filter_enforcing", 100)) .WillByDefault(Return(true)); - 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_); + MessageUtil::loadFromYaml(yaml, proto_config); config_.reset(new Config(proto_config, stats_store_, runtime_)); client_ = new RateLimit::MockClient(); filter_.reset(new Filter(config_, RateLimit::ClientPtr{client_})); @@ -73,6 +59,35 @@ class RateLimitFilterTest : public testing::Test { } } + const std::string filter_config_ = R"EOF( +domain: foo +descriptors: +- entries: + - key: hello + value: world + - key: foo + value: bar +- entries: + - key: foo2 + value: bar2 +stat_prefix: name +)EOF"; + + const std::string fail_close_config_ = R"EOF( +domain: foo +descriptors: +- entries: + - key: hello + value: world + - key: foo + value: bar +- entries: + - key: foo2 + value: bar2 +stat_prefix: name +failure_mode_allow: false +)EOF"; + Stats::IsolatedStoreImpl stats_store_; NiceMock runtime_; ConfigSharedPtr config_; @@ -80,16 +95,6 @@ class RateLimitFilterTest : public testing::Test { std::unique_ptr filter_; NiceMock 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) { @@ -111,6 +116,7 @@ TEST_F(RateLimitFilterTest, BadRatelimitConfig) { TEST_F(RateLimitFilterTest, OK) { InSequence s; + SetUpTest(filter_config_); EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ @@ -139,6 +145,7 @@ TEST_F(RateLimitFilterTest, OK) { TEST_F(RateLimitFilterTest, OverLimit) { InSequence s; + SetUpTest(filter_config_); EXPECT_CALL(*client_, limit(_, "foo", _, _)) .WillOnce(WithArgs<0>(Invoke([&](RateLimit::RequestCallbacks& callbacks) -> void { @@ -162,6 +169,7 @@ TEST_F(RateLimitFilterTest, OverLimit) { TEST_F(RateLimitFilterTest, OverLimitNotEnforcing) { InSequence s; + SetUpTest(filter_config_); EXPECT_CALL(*client_, limit(_, "foo", _, _)) .WillOnce(WithArgs<0>(Invoke([&](RateLimit::RequestCallbacks& callbacks) -> void { @@ -188,6 +196,7 @@ TEST_F(RateLimitFilterTest, OverLimitNotEnforcing) { TEST_F(RateLimitFilterTest, Error) { InSequence s; + SetUpTest(filter_config_); EXPECT_CALL(*client_, limit(_, "foo", _, _)) .WillOnce(WithArgs<0>(Invoke([&](RateLimit::RequestCallbacks& callbacks) -> void { @@ -213,6 +222,7 @@ TEST_F(RateLimitFilterTest, Error) { TEST_F(RateLimitFilterTest, Disconnect) { InSequence s; + SetUpTest(filter_config_); EXPECT_CALL(*client_, limit(_, "foo", _, _)) .WillOnce(WithArgs<0>(Invoke([&](RateLimit::RequestCallbacks& callbacks) -> void { @@ -231,6 +241,7 @@ TEST_F(RateLimitFilterTest, Disconnect) { TEST_F(RateLimitFilterTest, ImmediateOK) { InSequence s; + SetUpTest(filter_config_); EXPECT_CALL(filter_callbacks_, continueReading()).Times(0); EXPECT_CALL(*client_, limit(_, "foo", _, _)) @@ -252,6 +263,7 @@ TEST_F(RateLimitFilterTest, ImmediateOK) { TEST_F(RateLimitFilterTest, RuntimeDisable) { InSequence s; + SetUpTest(filter_config_); EXPECT_CALL(runtime_.snapshot_, featureEnabled("ratelimit.tcp_filter_enabled", 100)) .WillOnce(Return(false)); @@ -262,8 +274,9 @@ TEST_F(RateLimitFilterTest, RuntimeDisable) { EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data, false)); } -TEST_F(RateLimitFilterFailureModeTest, ErrorResponseWithFailureAllowModeOff) { +TEST_F(RateLimitFilterTest, ErrorResponseWithFailureModeAllowOff) { InSequence s; + SetUpTest(fail_close_config_); EXPECT_CALL(*client_, limit(_, "foo", _, _)) .WillOnce(WithArgs<0>(Invoke([&](RateLimit::RequestCallbacks& callbacks) -> void { diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index cad1e2b3e90df..fd2d9ed45fdcb 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -39,11 +39,11 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, void initialize() override { if (failure_mode_) { - config_helper_.addFilter("{ name: envoy.rate_limit, config: { domain: some_domain, " - "failure_mode_allow: false, timeout: 0.5s } }"); - } else { config_helper_.addFilter( "{ name: envoy.rate_limit, config: { domain: some_domain, timeout: 0.5s } }"); + } else { + config_helper_.addFilter("{ name: envoy.rate_limit, config: { domain: some_domain, " + "failure_mode_allow: false, timeout: 0.5s } }"); } config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { auto* ratelimit_cluster = bootstrap.mutable_static_resources()->add_clusters(); @@ -162,13 +162,13 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, const uint64_t request_size_ = 1024; const uint64_t response_size_ = 512; - bool failure_mode_ = false; + bool failure_mode_ = true; }; // Test that verifies failure mode cases. class RatelimitFailureModeIntegrationTest : public RatelimitIntegrationTest { public: - RatelimitFailureModeIntegrationTest() { failure_mode_ = true; } + RatelimitFailureModeIntegrationTest() { failure_mode_ = false; } }; INSTANTIATE_TEST_CASE_P(IpVersionsClientType, RatelimitIntegrationTest, From 37b691369b0c2a0dc64ebd80aa878ac28ca72e38 Mon Sep 17 00:00:00 2001 From: Rama Date: Sun, 12 Aug 2018 17:40:10 +0530 Subject: [PATCH 06/11] fix test cases Signed-off-by: Rama --- source/common/request_info/utility.cc | 5 +++++ test/common/request_info/utility_test.cc | 1 + .../access_loggers/http_grpc/grpc_access_log_impl_test.cc | 1 + 3 files changed, 7 insertions(+) diff --git a/source/common/request_info/utility.cc b/source/common/request_info/utility.cc index b069c4d756ae1..b7278cbd74850 100644 --- a/source/common/request_info/utility.cc +++ b/source/common/request_info/utility.cc @@ -86,6 +86,10 @@ const std::string ResponseFlagUtils::toShortString(const RequestInfo& request_in appendString(result, UNAUTHORIZED_EXTERNAL_SERVICE); } + if (request_info.hasResponseFlag(ResponseFlag::RateLimitServiceError)) { + appendString(result, RATELIMIT_SERVICE_ERROR); + } + return result.empty() ? NONE : result; } @@ -105,6 +109,7 @@ absl::optional ResponseFlagUtils::toResponseFlag(const std::string {ResponseFlagUtils::FAULT_INJECTED, ResponseFlag::FaultInjected}, {ResponseFlagUtils::RATE_LIMITED, ResponseFlag::RateLimited}, {ResponseFlagUtils::UNAUTHORIZED_EXTERNAL_SERVICE, ResponseFlag::UnauthorizedExternalService}, + {ResponseFlagUtils::RATELIMIT_SERVICE_ERROR, ResponseFlag::RateLimitServiceError}, }; const auto& it = map.find(flag); if (it != map.end()) { diff --git a/test/common/request_info/utility_test.cc b/test/common/request_info/utility_test.cc index 300c23ab509de..64d836aa02ece 100644 --- a/test/common/request_info/utility_test.cc +++ b/test/common/request_info/utility_test.cc @@ -75,6 +75,7 @@ TEST(ResponseFlagsUtilsTest, toResponseFlagConversion) { std::make_pair("FI", ResponseFlag::FaultInjected), std::make_pair("RL", ResponseFlag::RateLimited), std::make_pair("UAEX", ResponseFlag::UnauthorizedExternalService), + std::make_pair("RLSE", ResponseFlag::RateLimitServiceError), }; EXPECT_FALSE(ResponseFlagUtils::toResponseFlag("NonExistentFlag").has_value()); diff --git a/test/extensions/access_loggers/http_grpc/grpc_access_log_impl_test.cc b/test/extensions/access_loggers/http_grpc/grpc_access_log_impl_test.cc index bfbb62bc7eeb6..31daa3341d6d8 100644 --- a/test/extensions/access_loggers/http_grpc/grpc_access_log_impl_test.cc +++ b/test/extensions/access_loggers/http_grpc/grpc_access_log_impl_test.cc @@ -434,6 +434,7 @@ TEST(responseFlagsToAccessLogResponseFlagsTest, All) { common_access_log_expected.mutable_response_flags()->mutable_unauthorized_details()->set_reason( envoy::data::accesslog::v2::ResponseFlags_Unauthorized_Reason:: ResponseFlags_Unauthorized_Reason_EXTERNAL_SERVICE); + common_access_log_expected.mutable_response_flags()->set_rate_limit_service_error(true); EXPECT_EQ(common_access_log_expected.DebugString(), common_access_log.DebugString()); } From a6ca282a3f836a01dd01a55d2c907330cd807092 Mon Sep 17 00:00:00 2001 From: Rama Date: Mon, 13 Aug 2018 12:28:36 +0530 Subject: [PATCH 07/11] fixed the test case Signed-off-by: Rama --- api/envoy/config/filter/accesslog/v2/accesslog.proto | 2 +- include/envoy/request_info/request_info.h | 2 +- source/common/request_info/utility.cc | 2 +- .../access_loggers/http_grpc/grpc_access_log_impl.cc | 2 +- test/common/access_log/access_log_impl_test.cc | 5 +++-- test/common/request_info/utility_test.cc | 4 ++-- test/integration/ratelimit_integration_test.cc | 6 +++--- 7 files changed, 12 insertions(+), 11 deletions(-) diff --git a/api/envoy/config/filter/accesslog/v2/accesslog.proto b/api/envoy/config/filter/accesslog/v2/accesslog.proto index 6642560694eaf..fb1ec8e71afda 100644 --- a/api/envoy/config/filter/accesslog/v2/accesslog.proto +++ b/api/envoy/config/filter/accesslog/v2/accesslog.proto @@ -162,6 +162,6 @@ message ResponseFlagFilter { // This field is optional. If it is not specified, then any response flag will pass // the filter check. repeated string flags = 1 [(validate.rules).repeated .items.string = { - in: ["LH", "UH", "UT", "LR", "UR", "UF", "UC", "UO", "NR", "DI", "FI", "RL", "UAEX"] + in: ["LH", "UH", "UT", "LR", "UR", "UF", "UC", "UO", "NR", "DI", "FI", "RL", "UAEX", "RLSE"] }]; } diff --git a/include/envoy/request_info/request_info.h b/include/envoy/request_info/request_info.h index ee0c8e7d8a98f..8af4c7e01cb83 100644 --- a/include/envoy/request_info/request_info.h +++ b/include/envoy/request_info/request_info.h @@ -49,7 +49,7 @@ enum ResponseFlag { // Request was unauthorized by external authorization service. UnauthorizedExternalService = 0x1000, // Unable to call Ratelimit service. - RateLimitServiceError = 0x1200, + RateLimitServiceError = 0x2000, // ATTENTION: MAKE SURE THIS REMAINS EQUAL TO THE LAST FLAG. LastFlag = RateLimitServiceError }; diff --git a/source/common/request_info/utility.cc b/source/common/request_info/utility.cc index b7278cbd74850..6d71182fff221 100644 --- a/source/common/request_info/utility.cc +++ b/source/common/request_info/utility.cc @@ -32,7 +32,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 == 0x1200, "A flag has been added. Fix this code."); + static_assert(ResponseFlag::LastFlag == 0x2000, "A flag has been added. Fix this code."); if (request_info.hasResponseFlag(ResponseFlag::FailedLocalHealthCheck)) { appendString(result, FAILED_LOCAL_HEALTH_CHECK); diff --git a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc b/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc index c8f9ce7db0184..2453280c35802 100644 --- a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc +++ b/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 == 0x1200, + static_assert(RequestInfo::ResponseFlag::LastFlag == 0x2000, "A flag has been added. Fix this code."); if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::FailedLocalHealthCheck)) { diff --git a/test/common/access_log/access_log_impl_test.cc b/test/common/access_log/access_log_impl_test.cc index c3a166041c81c..f5829a8fc6245 100644 --- a/test/common/access_log/access_log_impl_test.cc +++ b/test/common/access_log/access_log_impl_test.cc @@ -813,11 +813,12 @@ name: envoy.file_access_log - FI - RL - UAEX + - RLSE config: path: /dev/null )EOF"; - static_assert(RequestInfo::ResponseFlag::LastFlag == 0x1200, + static_assert(RequestInfo::ResponseFlag::LastFlag == 0x2000, "A flag has been added. Fix this code."); std::vector all_response_flags = { @@ -864,7 +865,7 @@ name: envoy.file_access_log "Proto constraint validation failed (AccessLogFilterValidationError.ResponseFlagFilter: " "[\"embedded message failed validation\"] | caused by " "ResponseFlagFilterValidationError.Flags[i]: [\"value must be in list \" [\"LH\" \"UH\" " - "\"UT\" \"LR\" \"UR\" \"UF\" \"UC\" \"UO\" \"NR\" \"DI\" \"FI\" \"RL\" \"UAEX\"]]): " + "\"UT\" \"LR\" \"UR\" \"UF\" \"UC\" \"UO\" \"NR\" \"DI\" \"FI\" \"RL\" \"UAEX\" \"RLSE\"]]): " "response_flag_filter {\n flags: \"UnsupportedFlag\"\n}\n"); } diff --git a/test/common/request_info/utility_test.cc b/test/common/request_info/utility_test.cc index 64d836aa02ece..90087bd67dd53 100644 --- a/test/common/request_info/utility_test.cc +++ b/test/common/request_info/utility_test.cc @@ -14,7 +14,7 @@ namespace Envoy { namespace RequestInfo { TEST(ResponseFlagUtilsTest, toShortStringConversion) { - static_assert(ResponseFlag::LastFlag == 0x1200, "A flag has been added. Fix this code."); + static_assert(ResponseFlag::LastFlag == 0x2000, "A flag has been added. Fix this code."); std::vector> expected = { std::make_pair(ResponseFlag::FailedLocalHealthCheck, "LH"), @@ -59,7 +59,7 @@ TEST(ResponseFlagUtilsTest, toShortStringConversion) { } TEST(ResponseFlagsUtilsTest, toResponseFlagConversion) { - static_assert(ResponseFlag::LastFlag == 0x1200, "A flag has been added. Fix this code."); + static_assert(ResponseFlag::LastFlag == 0x2000, "A flag has been added. Fix this code."); std::vector> expected = { std::make_pair("LH", ResponseFlag::FailedLocalHealthCheck), diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index fd2d9ed45fdcb..21a386500a050 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -38,7 +38,7 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, } void initialize() override { - if (failure_mode_) { + if (failure_mode_allow_) { config_helper_.addFilter( "{ name: envoy.rate_limit, config: { domain: some_domain, timeout: 0.5s } }"); } else { @@ -162,13 +162,13 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, const uint64_t request_size_ = 1024; const uint64_t response_size_ = 512; - bool failure_mode_ = true; + bool failure_mode_allow_ = true; }; // Test that verifies failure mode cases. class RatelimitFailureModeIntegrationTest : public RatelimitIntegrationTest { public: - RatelimitFailureModeIntegrationTest() { failure_mode_ = false; } + RatelimitFailureModeIntegrationTest() { failure_mode_allow_ = false; } }; INSTANTIATE_TEST_CASE_P(IpVersionsClientType, RatelimitIntegrationTest, From 3289db05ff98b9e4c3c9085e3dc7dccaf6dac75d Mon Sep 17 00:00:00 2001 From: Rama Date: Sat, 18 Aug 2018 17:26:56 +0530 Subject: [PATCH 08/11] fix tests Signed-off-by: Rama --- test/extensions/filters/http/ratelimit/ratelimit_test.cc | 2 +- test/extensions/filters/network/ratelimit/ratelimit_test.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/extensions/filters/http/ratelimit/ratelimit_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_test.cc index def8c427ec138..e694bba861d37 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/http/ratelimit/ratelimit_test.cc @@ -350,7 +350,7 @@ TEST_F(HttpRateLimitFilterTest, ErrorResponseWithFailureModeAllowOff) { EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, false)); - request_callbacks_->complete(RateLimit::LimitStatus::Error); + request_callbacks_->complete(RateLimit::LimitStatus::Error, nullptr); EXPECT_CALL(filter_callbacks_.request_info_, setResponseFlag(RequestInfo::ResponseFlag::RateLimitServiceError)) diff --git a/test/extensions/filters/network/ratelimit/ratelimit_test.cc b/test/extensions/filters/network/ratelimit/ratelimit_test.cc index 4d39fc16038e2..d286437537e61 100644 --- a/test/extensions/filters/network/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/network/ratelimit/ratelimit_test.cc @@ -286,7 +286,7 @@ TEST_F(RateLimitFilterTest, ErrorResponseWithFailureModeAllowOff) { 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); + request_callbacks_->complete(RateLimit::LimitStatus::Error, nullptr); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onData(data, false)); From cf357e2cbade55493af61037d9ec8acea033909f Mon Sep 17 00:00:00 2001 From: Rama Date: Tue, 21 Aug 2018 12:36:30 +0530 Subject: [PATCH 09/11] change failure_mode_allow to failure_mode_deny Signed-off-by: Rama --- api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto | 6 +++--- .../config/filter/network/rate_limit/v2/rate_limit.proto | 6 +++--- docs/root/intro/version_history.rst | 2 +- source/extensions/filters/http/ratelimit/ratelimit.h | 6 +++--- source/extensions/filters/network/ratelimit/ratelimit.cc | 3 +-- source/extensions/filters/network/ratelimit/ratelimit.h | 4 ++-- test/extensions/filters/http/ratelimit/ratelimit_test.cc | 2 +- test/extensions/filters/network/ratelimit/ratelimit_test.cc | 2 +- test/integration/ratelimit_integration_test.cc | 2 +- 9 files changed, 16 insertions(+), 17 deletions(-) diff --git a/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto b/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto index 279f5eedd58fd..cd9db057cc054 100644 --- a/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto +++ b/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto @@ -37,8 +37,8 @@ message RateLimit { 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 + // not respond back. When it is set to true, Envoy will not allow traffic in case of // communication failure between rate limiting service and the proxy. - // Defaults to true. - google.protobuf.BoolValue failure_mode_allow = 5; + // Defaults to false. + bool failure_mode_deny = 5; } diff --git a/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto b/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto index e932e7440c96a..08cd6185baf5e 100644 --- a/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto +++ b/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto @@ -29,8 +29,8 @@ message RateLimit { 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 + // not respond back. When it is set to true, Envoy will not allow traffic in case of // communication failure between rate limiting service and the proxy. - // Defaults to true. - google.protobuf.BoolValue failure_mode_allow = 5; + // Defaults to false. + bool failure_mode_deny = 5; } diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index f829a7d5a060e..ed462867c8e74 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -65,7 +65,7 @@ Version history * upstream: require opt-in to use the :ref:`x-envoy-orignal-dst-host ` header for overriding destination address when using the :ref:`Original Destination ` load balancing policy. -* ratelimit: added :ref:`failure_mode_allow ` option to control traffic flow in +* ratelimit: added :ref:`failure_mode_deny ` option to control traffic flow in case of rate limit service error. 1.7.0 diff --git a/source/extensions/filters/http/ratelimit/ratelimit.h b/source/extensions/filters/http/ratelimit/ratelimit.h index 17a9b230ebf2f..4f411a797b0a2 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.h +++ b/source/extensions/filters/http/ratelimit/ratelimit.h @@ -38,7 +38,7 @@ class FilterConfig { request_type_(config.request_type().empty() ? stringToType("both") : stringToType(config.request_type())), local_info_(local_info), scope_(scope), runtime_(runtime), cm_(cm), - failure_mode_allow_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, failure_mode_allow, true)) {} + failure_mode_deny_(config.failure_mode_deny()) {} const std::string& domain() const { return domain_; } const LocalInfo::LocalInfo& localInfo() const { return local_info_; } uint64_t stage() const { return stage_; } @@ -47,7 +47,7 @@ class FilterConfig { Upstream::ClusterManager& cm() { return cm_; } FilterRequestType requestType() const { return request_type_; } - bool failureModeAllow() const { return failure_mode_allow_; } + bool failureModeAllow() const { return !failure_mode_deny_; } private: static FilterRequestType stringToType(const std::string& request_type) { @@ -68,7 +68,7 @@ class FilterConfig { Stats::Scope& scope_; Runtime::Loader& runtime_; Upstream::ClusterManager& cm_; - const bool failure_mode_allow_; + const bool failure_mode_deny_; }; typedef std::shared_ptr FilterConfigSharedPtr; diff --git a/source/extensions/filters/network/ratelimit/ratelimit.cc b/source/extensions/filters/network/ratelimit/ratelimit.cc index 02553f8d16a51..b74f94d0e0fdc 100644 --- a/source/extensions/filters/network/ratelimit/ratelimit.cc +++ b/source/extensions/filters/network/ratelimit/ratelimit.cc @@ -16,8 +16,7 @@ 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), - failure_mode_allow_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, failure_mode_allow, true)) { + runtime_(runtime), failure_mode_deny_(config.failure_mode_deny()) { for (const auto& descriptor : config.descriptors()) { RateLimit::Descriptor new_descriptor; for (const auto& entry : descriptor.entries()) { diff --git a/source/extensions/filters/network/ratelimit/ratelimit.h b/source/extensions/filters/network/ratelimit/ratelimit.h index bff2ccd30859e..a19de07b8b416 100644 --- a/source/extensions/filters/network/ratelimit/ratelimit.h +++ b/source/extensions/filters/network/ratelimit/ratelimit.h @@ -50,7 +50,7 @@ class Config { const std::vector& descriptors() { return descriptors_; } Runtime::Loader& runtime() { return runtime_; } const InstanceStats& stats() { return stats_; } - bool failureModeAllow() const { return failure_mode_allow_; }; + bool failureModeAllow() const { return !failure_mode_deny_; }; private: static InstanceStats generateStats(const std::string& name, Stats::Scope& scope); @@ -59,7 +59,7 @@ class Config { std::vector descriptors_; const InstanceStats stats_; Runtime::Loader& runtime_; - const bool failure_mode_allow_; + const bool failure_mode_deny_; }; typedef std::shared_ptr ConfigSharedPtr; diff --git a/test/extensions/filters/http/ratelimit/ratelimit_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_test.cc index e694bba861d37..ed1c6cfb7ef0f 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/http/ratelimit/ratelimit_test.cc @@ -66,7 +66,7 @@ class HttpRateLimitFilterTest : public testing::Test { const std::string fail_close_config_ = R"EOF( domain: foo - failure_mode_allow: false + failure_mode_deny: true )EOF"; const std::string filter_config_ = R"EOF( diff --git a/test/extensions/filters/network/ratelimit/ratelimit_test.cc b/test/extensions/filters/network/ratelimit/ratelimit_test.cc index d286437537e61..6a1907dccbdc5 100644 --- a/test/extensions/filters/network/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/network/ratelimit/ratelimit_test.cc @@ -85,7 +85,7 @@ domain: foo - key: foo2 value: bar2 stat_prefix: name -failure_mode_allow: false +failure_mode_deny: true )EOF"; Stats::IsolatedStoreImpl stats_store_; diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index 049d3a8466de0..cc9bdc8f800f7 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -43,7 +43,7 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, "{ name: envoy.rate_limit, config: { domain: some_domain, timeout: 0.5s } }"); } else { config_helper_.addFilter("{ name: envoy.rate_limit, config: { domain: some_domain, " - "failure_mode_allow: false, timeout: 0.5s } }"); + "failure_mode_deny: true, timeout: 0.5s } }"); } config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { auto* ratelimit_cluster = bootstrap.mutable_static_resources()->add_clusters(); From d561b19581ec2c933b78c0351eae74bcd40a3400 Mon Sep 17 00:00:00 2001 From: Rama Date: Tue, 21 Aug 2018 12:45:48 +0530 Subject: [PATCH 10/11] change integration test to use failure_mode_deny for better clarity Signed-off-by: Rama --- test/integration/ratelimit_integration_test.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index cc9bdc8f800f7..67269b26a09e1 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -38,12 +38,13 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, } void initialize() override { - if (failure_mode_allow_) { - config_helper_.addFilter( - "{ name: envoy.rate_limit, config: { domain: some_domain, timeout: 0.5s } }"); - } else { + if (failure_mode_deny_) { config_helper_.addFilter("{ name: envoy.rate_limit, config: { domain: some_domain, " "failure_mode_deny: true, timeout: 0.5s } }"); + + } else { + config_helper_.addFilter( + "{ name: envoy.rate_limit, config: { domain: some_domain, timeout: 0.5s } }"); } config_helper_.addConfigModifier([this](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { auto* ratelimit_cluster = bootstrap.mutable_static_resources()->add_clusters(); @@ -175,13 +176,13 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, const uint64_t request_size_ = 1024; const uint64_t response_size_ = 512; - bool failure_mode_allow_ = true; + bool failure_mode_deny_ = false; }; // Test that verifies failure mode cases. class RatelimitFailureModeIntegrationTest : public RatelimitIntegrationTest { public: - RatelimitFailureModeIntegrationTest() { failure_mode_allow_ = false; } + RatelimitFailureModeIntegrationTest() { failure_mode_deny_ = true; } }; INSTANTIATE_TEST_CASE_P(IpVersionsClientType, RatelimitIntegrationTest, From 4404df229e66fbec969ad2e5e94108a242917c4a Mon Sep 17 00:00:00 2001 From: Rama Date: Tue, 21 Aug 2018 22:10:37 +0530 Subject: [PATCH 11/11] updated docs Signed-off-by: Rama --- api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto | 1 - .../config/filter/network/rate_limit/v2/rate_limit.proto | 1 - docs/root/configuration/http_filters/rate_limit_filter.rst | 6 +++--- .../configuration/network_filters/rate_limit_filter.rst | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto b/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto index cd9db057cc054..d79764745ab74 100644 --- a/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto +++ b/api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto @@ -4,7 +4,6 @@ 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"; diff --git a/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto b/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto index 08cd6185baf5e..fe579f01e444c 100644 --- a/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto +++ b/api/envoy/config/filter/network/rate_limit/v2/rate_limit.proto @@ -5,7 +5,6 @@ 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"; diff --git a/docs/root/configuration/http_filters/rate_limit_filter.rst b/docs/root/configuration/http_filters/rate_limit_filter.rst index d0da7a7d6cd40..52754a0186747 100644 --- a/docs/root/configuration/http_filters/rate_limit_filter.rst +++ b/docs/root/configuration/http_filters/rate_limit_filter.rst @@ -16,8 +16,8 @@ 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 ` is -set to false, a 500 response is returned. +If there is an error in calling rate limit service or rate limit service returns an error and :ref:`failure_mode_deny ` is +set to true, a 500 response is returned. .. _config_http_filters_rate_limit_composing_actions: @@ -112,7 +112,7 @@ The buffer filter outputs statistics in the *cluster..rate 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 ` set to true." + of :ref:`failure_mode_deny ` set to false." Runtime ------- diff --git a/docs/root/configuration/network_filters/rate_limit_filter.rst b/docs/root/configuration/network_filters/rate_limit_filter.rst index 3e5377948f9b9..ff9771bba8066 100644 --- a/docs/root/configuration/network_filters/rate_limit_filter.rst +++ b/docs/root/configuration/network_filters/rate_limit_filter.rst @@ -26,7 +26,7 @@ following statistics: 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 ` set to true." + of :ref:`failure_mode_deny ` set to false." Runtime -------