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

Envoy rate limit descriptor fix #11153

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions api/envoy/config/route/v3/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1395,6 +1395,11 @@ message RateLimit {

// The key to use in the descriptor entry.
string descriptor_key = 2 [(validate.rules).string = {min_bytes: 1}];

// If set to true, Envoy skips the descriptor while calling rate limiting service
// when header is not present in the request. By default it skips calling the
// rate limiting service if this header is not present in the request.
bool skip_if_absent = 3;
}

// The following descriptor entry is appended to the descriptor and is populated using the
Expand Down
5 changes: 5 additions & 0 deletions api/envoy/config/route/v4alpha/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1376,6 +1376,11 @@ message RateLimit {

// The key to use in the descriptor entry.
string descriptor_key = 2 [(validate.rules).string = {min_bytes: 1}];

// If set to true, Envoy skips the descriptor while calling rate limiting service
// when header is not present in the request. By default it skips calling the
// rate limiting service if this header is not present in the request.
bool skip_if_absent = 3;
}

// The following descriptor entry is appended to the descriptor and is populated using the
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 @@ -75,6 +75,7 @@ New Features
tracing is not forced.
* router: add support for RESPONSE_FLAGS and RESPONSE_CODE_DETAILS :ref:`header formatters
<config_http_conn_man_headers_custom_request_headers>`.
* router: allow Rate Limiting Service to be called in case of missing request header for a descriptor if the :ref:`skip_if_absent <envoy_v3_api_field_config.route.v3.RateLimit.Action.RequestHeaders.skip_if_absent>` field is set to true.
* router: more fine grained internal redirect configs are added to the :ref`internal_redirect_policy
<envoy_api_field_router.RouterAction.internal_redirect_policy>` field.
* runtime: add new gauge :ref:`deprecated_feature_seen_since_process_start <runtime_stats>` that gets reset across hot restarts.
Expand Down

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

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

7 changes: 5 additions & 2 deletions source/common/router/router_ratelimit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,13 @@ bool RequestHeadersAction::populateDescriptor(const Router::RouteEntry&,
const Http::HeaderMap& headers,
const Network::Address::Instance&) const {
const Http::HeaderEntry* header_value = headers.get(header_name_);

// If header is not present in the request and if skip_if_absent is true skip this descriptor,
// while calling rate limiting service. If skip_if_absent is false, do not call rate limiting
// service.
if (!header_value) {
return false;
return skip_if_absent_;
}

descriptor.entries_.push_back(
{descriptor_key_, std::string(header_value->value().getStringView())});
return true;
Expand Down
4 changes: 3 additions & 1 deletion source/common/router/router_ratelimit.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ class DestinationClusterAction : public RateLimitAction {
class RequestHeadersAction : public RateLimitAction {
public:
RequestHeadersAction(const envoy::config::route::v3::RateLimit::Action::RequestHeaders& action)
: header_name_(action.header_name()), descriptor_key_(action.descriptor_key()) {}
: header_name_(action.header_name()), descriptor_key_(action.descriptor_key()),
skip_if_absent_(action.skip_if_absent()) {}

// Router::RateLimitAction
bool populateDescriptor(const Router::RouteEntry& route, RateLimit::Descriptor& descriptor,
Expand All @@ -52,6 +53,7 @@ class RequestHeadersAction : public RateLimitAction {
private:
const Http::LowerCaseString header_name_;
const std::string descriptor_key_;
const bool skip_if_absent_;
};

/**
Expand Down
47 changes: 47 additions & 0 deletions test/common/router/router_ratelimit_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,53 @@ TEST_F(RateLimitPolicyEntryTest, RequestHeaders) {
testing::ContainerEq(descriptors_));
}

// Validate that a descriptor is added if the missing request header
// has skip_if_absent set to true
TEST_F(RateLimitPolicyEntryTest, RequestHeadersWithSkipIfAbsent) {
const std::string yaml = R"EOF(
actions:
- request_headers:
header_name: x-header-name
descriptor_key: my_header_name
skip_if_absent: false
- request_headers:
header_name: x-header
descriptor_key: my_header
skip_if_absent: true
)EOF";

setupTest(yaml);
Http::TestHeaderMapImpl header{{"x-header-name", "test_value"}};

rate_limit_entry_->populateDescriptors(route_, descriptors_, "service_cluster", header,
default_remote_address_);
EXPECT_THAT(std::vector<Envoy::RateLimit::Descriptor>({{{{"my_header_name", "test_value"}}}}),
testing::ContainerEq(descriptors_));
}

// Tests if the descriptors are added if one of the headers is missing
// and skip_if_absent is set to default value which is false
TEST_F(RateLimitPolicyEntryTest, RequestHeadersWithDefaultSkipIfAbsent) {
const std::string yaml = R"EOF(
actions:
- request_headers:
header_name: x-header-name
descriptor_key: my_header_name
skip_if_absent: false
- request_headers:
header_name: x-header
descriptor_key: my_header
skip_if_absent: false
)EOF";

setupTest(yaml);
Http::TestHeaderMapImpl header{{"x-header-test", "test_value"}};

rate_limit_entry_->populateDescriptors(route_, descriptors_, "service_cluster", header,
default_remote_address_);
EXPECT_TRUE(descriptors_.empty());
}

TEST_F(RateLimitPolicyEntryTest, RequestHeadersNoMatch) {
const std::string yaml = R"EOF(
actions:
Expand Down