Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ratelimit: be able to disable x-envoy-ratelimited response header sent #13270

Merged
merged 11 commits into from
Oct 14, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// Rate limit :ref:`configuration overview <config_http_filters_rate_limit>`.
// [#extension: envoy.filters.http.ratelimit]

// [#next-free-field: 9]
// [#next-free-field: 10]
message RateLimit {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.filter.http.rate_limit.v2.RateLimit";
Expand Down Expand Up @@ -60,7 +60,6 @@ message RateLimit {
// The filter's behaviour in case the rate limiting service does
// 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 false.
bool failure_mode_deny = 5;

// Specifies whether a `RESOURCE_EXHAUSTED` gRPC code must be returned instead
Expand Down Expand Up @@ -99,6 +98,11 @@ message RateLimit {
// Disabled by default.
XRateLimitHeadersRFCVersion enable_x_ratelimit_headers = 8
[(validate.rules).enum = {defined_only: true}];

// Disables emitting the :ref:`x-envoy-ratelimited<config_http_filters_router_x-envoy-ratelimited>` header
// in case of rate limiting (i.e. 429 responses).
// Having this header not present potentially makes the request retriable.
bool disable_x_envoy_ratelimited_header = 9;
andrascz marked this conversation as resolved.
Show resolved Hide resolved
}

message RateLimitPerRoute {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ can optionally include the virtual host rate limit configurations. More than one
apply to a request. Each configuration results in a descriptor being sent to the rate limit service.

If the rate limit service is called, and the response for any of the descriptors is over limit, a
429 response is returned. The rate limit filter also sets the :ref:`x-envoy-ratelimited<config_http_filters_router_x-envoy-ratelimited>` header.
429 response is returned. The rate limit filter also sets the :ref:`x-envoy-ratelimited<config_http_filters_router_x-envoy-ratelimited>` header,
unless :ref:`disable_x_envoy_ratelimited_header <envoy_v3_api_field_extensions.filters.http.ratelimit.v3.RateLimit.disable_x_envoy_ratelimited_header>` is
set to true.

If there is an error in calling rate limit service or rate limit service returns an error and :ref:`failure_mode_deny <envoy_v3_api_field_extensions.filters.http.ratelimit.v3.RateLimit.failure_mode_deny>` is
set to true, a 500 response is returned.
Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ New Features
* grpc: implemented header value syntax support when defining :ref:`initial metadata <envoy_v3_api_field_config.core.v3.GrpcService.initial_metadata>` for gRPC-based `ext_authz` :ref:`HTTP <envoy_v3_api_field_extensions.filters.http.ext_authz.v3.ExtAuthz.grpc_service>` and :ref:`network <envoy_v3_api_field_extensions.filters.network.ext_authz.v3.ExtAuthz.grpc_service>` filters, and :ref:`ratelimit <envoy_v3_api_field_config.ratelimit.v3.RateLimitServiceConfig.grpc_service>` filters.
* health_check: added option to use :ref:`no_traffic_healthy_interval <envoy_v3_api_field_config.core.v3.HealthCheck.no_traffic_healthy_interval>` which allows a different no traffic interval when the host is healthy.
* mongo_proxy: the list of commands to produce metrics for is now :ref:`configurable <envoy_v3_api_field_extensions.filters.network.mongo_proxy.v3.MongoProxy.commands>`.
* ratelimit: added :ref:`disable_x_envoy_ratelimited_header <envoy_v3_api_msg_extensions.filters.http.ratelimit.v3.RateLimit>` option to disable `X-Envoy-RateLimited` header.

Deprecated
----------

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions source/extensions/filters/http/ratelimit/ratelimit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,13 @@ void Filter::complete(Filters::Common::RateLimit::LimitStatus status,
empty_stat_name,
false};
httpContext().codeStats().chargeResponseStat(info);
if (response_headers_to_add_ == nullptr) {
response_headers_to_add_ = Http::ResponseHeaderMapImpl::create();
if (config_->enableXEnvoyRateLimitedHeader()) {
if (response_headers_to_add_ == nullptr) {
response_headers_to_add_ = Http::ResponseHeaderMapImpl::create();
}
response_headers_to_add_->setReferenceEnvoyRateLimited(
Http::Headers::get().EnvoyRateLimitedValues.True);
}
response_headers_to_add_->setReferenceEnvoyRateLimited(
Http::Headers::get().EnvoyRateLimitedValues.True);
break;
}

Expand Down
3 changes: 3 additions & 0 deletions source/extensions/filters/http/ratelimit/ratelimit.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class FilterConfig {
enable_x_ratelimit_headers_(
config.enable_x_ratelimit_headers() ==
envoy::extensions::filters::http::ratelimit::v3::RateLimit::DRAFT_VERSION_03),
disable_x_envoy_ratelimited_header_(config.disable_x_envoy_ratelimited_header()),
rate_limited_grpc_status_(
config.rate_limited_as_resource_exhausted()
? absl::make_optional(Grpc::Status::WellKnownGrpcStatus::ResourceExhausted)
Expand All @@ -64,6 +65,7 @@ class FilterConfig {
FilterRequestType requestType() const { return request_type_; }
bool failureModeAllow() const { return !failure_mode_deny_; }
bool enableXRateLimitHeaders() const { return enable_x_ratelimit_headers_; }
bool enableXEnvoyRateLimitedHeader() const { return !disable_x_envoy_ratelimited_header_; }
const absl::optional<Grpc::Status::GrpcStatus> rateLimitedGrpcStatus() const {
return rate_limited_grpc_status_;
}
Expand All @@ -90,6 +92,7 @@ class FilterConfig {
Runtime::Loader& runtime_;
const bool failure_mode_deny_;
const bool enable_x_ratelimit_headers_;
const bool disable_x_envoy_ratelimited_header_;
const absl::optional<Grpc::Status::GrpcStatus> rate_limited_grpc_status_;
Http::Context& http_context_;
Filters::Common::RateLimit::StatNames stat_names_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class RatelimitIntegrationTest : public Grpc::VersionedGrpcClientIntegrationPara
TestUtility::loadFromYaml(base_filter_config_, proto_config_);
proto_config_.set_failure_mode_deny(failure_mode_deny_);
proto_config_.set_enable_x_ratelimit_headers(enable_x_ratelimit_headers_);
proto_config_.set_disable_x_envoy_ratelimited_header(disable_x_envoy_ratelimited_header_);
setGrpcService(*proto_config_.mutable_rate_limit_service()->mutable_grpc_service(),
"ratelimit", fake_upstreams_.back()->localAddress());
proto_config_.mutable_rate_limit_service()->set_transport_api_version(apiVersion());
Expand Down Expand Up @@ -192,6 +193,7 @@ class RatelimitIntegrationTest : public Grpc::VersionedGrpcClientIntegrationPara
bool failure_mode_deny_ = false;
envoy::extensions::filters::http::ratelimit::v3::RateLimit::XRateLimitHeadersRFCVersion
enable_x_ratelimit_headers_ = envoy::extensions::filters::http::ratelimit::v3::RateLimit::OFF;
bool disable_x_envoy_ratelimited_header_ = false;
envoy::extensions::filters::http::ratelimit::v3::RateLimit proto_config_{};
const std::string base_filter_config_ = R"EOF(
domain: some_domain
Expand All @@ -214,12 +216,24 @@ class RatelimitFilterHeadersEnabledIntegrationTest : public RatelimitIntegration
}
};

// Test verifies that disabling X-Envoy-RateLimited response header works.
class RatelimitFilterEnvoyRatelimitedHeaderDisabledIntegrationTest
: public RatelimitIntegrationTest {
public:
RatelimitFilterEnvoyRatelimitedHeaderDisabledIntegrationTest() {
disable_x_envoy_ratelimited_header_ = true;
}
};

INSTANTIATE_TEST_SUITE_P(IpVersionsClientType, RatelimitIntegrationTest,
VERSIONED_GRPC_CLIENT_INTEGRATION_PARAMS);
INSTANTIATE_TEST_SUITE_P(IpVersionsClientType, RatelimitFailureModeIntegrationTest,
VERSIONED_GRPC_CLIENT_INTEGRATION_PARAMS);
INSTANTIATE_TEST_SUITE_P(IpVersionsClientType, RatelimitFilterHeadersEnabledIntegrationTest,
VERSIONED_GRPC_CLIENT_INTEGRATION_PARAMS);
INSTANTIATE_TEST_SUITE_P(IpVersionsClientType,
RatelimitFilterEnvoyRatelimitedHeaderDisabledIntegrationTest,
VERSIONED_GRPC_CLIENT_INTEGRATION_PARAMS);

TEST_P(RatelimitIntegrationTest, Ok) { basicFlow(); }

Expand Down Expand Up @@ -261,6 +275,11 @@ TEST_P(RatelimitIntegrationTest, OverLimit) {
sendRateLimitResponse(envoy::service::ratelimit::v3::RateLimitResponse::OVER_LIMIT, {},
Http::TestResponseHeaderMapImpl{}, Http::TestRequestHeaderMapImpl{});
waitForFailedUpstreamResponse(429);

EXPECT_THAT(response_.get()->headers(),
Http::HeaderValueOf(Http::Headers::get().EnvoyRateLimited,
Http::Headers::get().EnvoyRateLimitedValues.True));

cleanup();

EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.ok"));
Expand All @@ -284,6 +303,10 @@ TEST_P(RatelimitIntegrationTest, OverLimitWithHeaders) {
return Http::HeaderMap::Iterate::Continue;
});

EXPECT_THAT(response_.get()->headers(),
Http::HeaderValueOf(Http::Headers::get().EnvoyRateLimited,
Http::Headers::get().EnvoyRateLimitedValues.True));

cleanup();

EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.ok"));
Expand Down Expand Up @@ -436,5 +459,23 @@ TEST_P(RatelimitFilterHeadersEnabledIntegrationTest, OverLimitWithFilterHeaders)
EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.error"));
}

TEST_P(RatelimitFilterEnvoyRatelimitedHeaderDisabledIntegrationTest,
OverLimitWithoutEnvoyRatelimitedHeader) {
initiateClientConnection();
waitForRatelimitRequest();
sendRateLimitResponse(envoy::service::ratelimit::v3::RateLimitResponse::OVER_LIMIT, {},
Http::TestResponseHeaderMapImpl{}, Http::TestRequestHeaderMapImpl{});
waitForFailedUpstreamResponse(429);

EXPECT_THAT(response_.get()->headers(),
::testing::Not(Http::HeaderValueOf(Http::Headers::get().EnvoyRateLimited, _)));

cleanup();

EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.ok"));
EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.ratelimit.over_limit")->value());
EXPECT_EQ(nullptr, test_server_->counter("cluster.cluster_0.ratelimit.error"));
}

} // namespace
} // namespace Envoy
43 changes: 43 additions & 0 deletions test/extensions/filters/http/ratelimit/ratelimit_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ class HttpRateLimitFilterTest : public testing::Test {
enable_x_ratelimit_headers: DRAFT_VERSION_03
)EOF";

const std::string disable_x_envoy_ratelimited_header_config_ = R"EOF(
domain: foo
disable_x_envoy_ratelimited_header: true
)EOF";

const std::string filter_config_ = R"EOF(
domain: foo
)EOF";
Expand Down Expand Up @@ -632,6 +637,44 @@ TEST_F(HttpRateLimitFilterTest, LimitResponseWithFilterHeaders) {
filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(upstream_rq_429_).value());
}

TEST_F(HttpRateLimitFilterTest, LimitResponseWithoutEnvoyRateLimitedHeader) {
SetUpTest(disable_x_envoy_ratelimited_header_config_);
InSequence s;

EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _))
.WillOnce(SetArgReferee<1>(descriptor_));
EXPECT_CALL(*client_, limit(_, _, _, _, _))
.WillOnce(
WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void {
request_callbacks_ = &callbacks;
})));

EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_->decodeHeaders(request_headers_, false));

Http::ResponseHeaderMapPtr h{new Http::TestResponseHeaderMapImpl()};
Http::TestResponseHeaderMapImpl response_headers{{":status", "429"}};
EXPECT_CALL(filter_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true));
EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
EXPECT_CALL(filter_callbacks_.stream_info_,
setResponseFlag(StreamInfo::ResponseFlag::RateLimited));

request_callbacks_->complete(Filters::Common::RateLimit::LimitStatus::OverLimit, nullptr,
std::move(h), nullptr);

EXPECT_EQ(1U, filter_callbacks_.clusterInfo()
->statsScope()
.counterFromStatName(ratelimit_over_limit_)
.value());
EXPECT_EQ(
1U,
filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(upstream_rq_4xx_).value());
EXPECT_EQ(
1U,
filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(upstream_rq_429_).value());
EXPECT_EQ("request_rate_limited", filter_callbacks_.details());
}

TEST_F(HttpRateLimitFilterTest, LimitResponseRuntimeDisabled) {
SetUpTest(filter_config_);
InSequence s;
Expand Down