Skip to content

Only generate a descriptor if all actions append an entry. #632

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.

Sign up for GitHub

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

Merged
ccaraman merged 3 commits into from
Mar 28, 2017
Merged

Only generate a descriptor if all actions append an entry. #632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
34ae14e
initial
ccaraman Mar 25, 2017
252304b
Document update
ccaraman Mar 27, 2017
81f1234
Update Documentation
ccaraman Mar 28, 2017
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
38 changes: 22 additions & 16 deletions docs/configuration/http_conn_man/route_config/rate_limits.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ disable_key

actions
*(required, array)* A list of actions that are to be applied for this rate limit configuration.
Order matters as the actions are processed sequentially and the descriptor will be composed by
appending decriptor entries in that sequence. See :ref:`composing actions
Order matters as the actions are processed sequentially and the descriptor is composed by
appending descriptor entries in that sequence. If an action cannot append a descriptor entry,
no descriptor is generated for the configuration. See :ref:`composing actions
<config_http_conn_man_route_table_rate_limit_composing_actions>` for additional documentation.

.. _config_http_conn_man_route_table_rate_limit_actions:
Expand Down Expand Up @@ -185,11 +186,15 @@ Each action populates a descriptor entry. A vector of descriptor entries compose
create more complex rate limit descriptors, actions can be composed in any order. The descriptor
will be populated in the order the actions are specified in the configuration.

Example 1
^^^^^^^^^

For example, to generate the following descriptor:

.. code-block:: cpp

("generic_key", "some_value"), ("source_cluster", "from_cluster")
("generic_key", "some_value")
("source_cluster", "from_cluster")

The configuration would be:

Expand All @@ -207,9 +212,13 @@ The configuration would be:
]
}

If an action doesn't append a descriptor entry, the next item in the action list will
be processed. For example given the following rate limit configuration, a request can
generate a few possible descriptors depending on what is present in the request.
Example 2
^^^^^^^^^

If an action doesn't append a descriptor entry, no descriptor is generated for
the configuration.

For the following configuration:

.. code-block:: json

Expand All @@ -228,17 +237,14 @@ generate a few possible descriptors depending on what is present in the request.
]
}

For a request with :ref:`x-forwarded-for<config_http_conn_man_headers_x-forwarded-for>` set and the
trusted address is for example *127.0.0.1*, the following descriptor would be generated:
If a request did not set :ref:`x-forwarded-for<config_http_conn_man_headers_x-forwarded-for>`,
no descriptor is generated.

.. code-block:: cpp

("generic_key", "some_value"), ("remote_address", "127.0.0.1"), ("source_cluster",
"from_cluster")

If a request did not set :ref:`x-forwarded-for<config_http_conn_man_headers_x-forwarded-for>`, the
following descriptor would be generated:
If a request sets :ref:`x-forwarded-for<config_http_conn_man_headers_x-forwarded-for>`, the
the following descriptor is generated:

.. code-block:: cpp

("generic_key", "some_value"), ("source_cluster", "from_cluster")
("generic_key", "some_value")
("remote_address", "<trusted address from x-forwarded-for>")
("source_cluster", "from_cluster")
3 changes: 2 additions & 1 deletion include/envoy/router/router_ratelimit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ class RateLimitAction {
* @param local_service_cluster supplies the name of the local service cluster.
* @param headers supplies the header for the request.
* @param remote_address supplies the trusted downstream address for the connection.
* @return true if the RateLimitAction populated the descriptor.
*/
virtual void populateDescriptor(const RouteEntry& route, ::RateLimit::Descriptor& descriptor,
virtual bool populateDescriptor(const RouteEntry& route, ::RateLimit::Descriptor& descriptor,
const std::string& local_service_cluster,
const Http::HeaderMap& headers,
const std::string& remote_address) const PURE;
Expand Down
37 changes: 25 additions & 12 deletions source/common/router/router_ratelimit.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,47 +8,52 @@ namespace Router {

const uint64_t RateLimitPolicyImpl::MAX_STAGE_NUMBER = 10UL;

void SourceClusterAction::populateDescriptor(const Router::RouteEntry&,
bool SourceClusterAction::populateDescriptor(const Router::RouteEntry&,
::RateLimit::Descriptor& descriptor,
const std::string& local_service_cluster,
const Http::HeaderMap&, const std::string&) const {
descriptor.entries_.push_back({"source_cluster", local_service_cluster});
return true;
}

void DestinationClusterAction::populateDescriptor(const Router::RouteEntry& route,
bool DestinationClusterAction::populateDescriptor(const Router::RouteEntry& route,
::RateLimit::Descriptor& descriptor,
const std::string&, const Http::HeaderMap&,
const std::string&) const {
descriptor.entries_.push_back({"destination_cluster", route.clusterName()});
return true;
}

void RequestHeadersAction::populateDescriptor(const Router::RouteEntry&,
bool RequestHeadersAction::populateDescriptor(const Router::RouteEntry&,
::RateLimit::Descriptor& descriptor,
const std::string&, const Http::HeaderMap& headers,
const std::string&) const {
const Http::HeaderEntry* header_value = headers.get(header_name_);
if (!header_value) {
return;
return false;
}

descriptor.entries_.push_back({descriptor_key_, header_value->value().c_str()});
return true;
}

void RemoteAddressAction::populateDescriptor(const Router::RouteEntry&,
bool RemoteAddressAction::populateDescriptor(const Router::RouteEntry&,
::RateLimit::Descriptor& descriptor,
const std::string&, const Http::HeaderMap&,
const std::string& remote_address) const {
if (remote_address.empty()) {
return;
return false;
}

descriptor.entries_.push_back({"remote_address", remote_address});
return true;
}

void GenericKeyAction::populateDescriptor(const Router::RouteEntry&,
bool GenericKeyAction::populateDescriptor(const Router::RouteEntry&,
::RateLimit::Descriptor& descriptor, const std::string&,
const Http::HeaderMap&, const std::string&) const {
descriptor.entries_.push_back({"generic_key", descriptor_value_});
return true;
}

HeaderValueMatchAction::HeaderValueMatchAction(const Json::Object& action)
Expand All @@ -59,12 +64,15 @@ HeaderValueMatchAction::HeaderValueMatchAction(const Json::Object& action)
}
}

void HeaderValueMatchAction::populateDescriptor(const Router::RouteEntry&,
bool HeaderValueMatchAction::populateDescriptor(const Router::RouteEntry&,
::RateLimit::Descriptor& descriptor,
const std::string&, const Http::HeaderMap& headers,
const std::string&) const {
if (ConfigUtility::matchHeaders(headers, action_headers_)) {
descriptor.entries_.push_back({"header_match", descriptor_value_});
return true;
} else {
return false;
}
}

Expand All @@ -85,9 +93,8 @@ RateLimitPolicyEntryImpl::RateLimitPolicyEntryImpl(const Json::Object& config)
} else if (type == "generic_key") {
actions_.emplace_back(new GenericKeyAction(*action));
} else if (type == "header_value_match") {
ASSERT(type == "header_value_match");
actions_.emplace_back(new HeaderValueMatchAction(*action));
} else {
throw EnvoyException(fmt::format("unknown http rate limit filter action '{}'", type));
}
}
}
Expand All @@ -97,11 +104,17 @@ void RateLimitPolicyEntryImpl::populateDescriptors(
const std::string& local_service_cluster, const Http::HeaderMap& headers,
const std::string& remote_address) const {
::RateLimit::Descriptor descriptor;
bool result = true;
for (const RateLimitActionPtr& action : actions_) {
action->populateDescriptor(route, descriptor, local_service_cluster, headers, remote_address);
result = result &&
action->populateDescriptor(route, descriptor, local_service_cluster, headers,
remote_address);
if (!result) {
break;
}
}

if (!descriptor.entries_.empty()) {
if (result) {
descriptors.emplace_back(descriptor);
}
}
Expand Down
12 changes: 6 additions & 6 deletions source/common/router/router_ratelimit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Router {
class SourceClusterAction : public RateLimitAction {
public:
// Router::RateLimitAction
void populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
bool populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
const std::string& local_service_cluster, const Http::HeaderMap& headers,
const std::string& remote_address) const override;
};
Expand All @@ -26,7 +26,7 @@ class SourceClusterAction : public RateLimitAction {
class DestinationClusterAction : public RateLimitAction {
public:
// Router::RateLimitAction
void populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
bool populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
const std::string& local_service_cluster, const Http::HeaderMap& headers,
const std::string& remote_address) const override;
};
Expand All @@ -41,7 +41,7 @@ class RequestHeadersAction : public RateLimitAction {
descriptor_key_(action.getString("descriptor_key")) {}

// Router::RateLimitAction
void populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
bool populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
const std::string& local_service_cluster, const Http::HeaderMap& headers,
const std::string& remote_address) const override;

Expand All @@ -56,7 +56,7 @@ class RequestHeadersAction : public RateLimitAction {
class RemoteAddressAction : public RateLimitAction {
public:
// Router::RateLimitAction
void populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
bool populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
const std::string& local_service_cluster, const Http::HeaderMap& headers,
const std::string& remote_address) const override;
};
Expand All @@ -70,7 +70,7 @@ class GenericKeyAction : public RateLimitAction {
: descriptor_value_(action.getString("descriptor_value")) {}

// Router::RateLimitAction
void populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
bool populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
const std::string& local_service_cluster, const Http::HeaderMap& headers,
const std::string& remote_address) const override;

Expand All @@ -86,7 +86,7 @@ class HeaderValueMatchAction : public RateLimitAction {
HeaderValueMatchAction(const Json::Object& action);

// Router::RateLimitAction
void populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
bool populateDescriptor(const Router::RouteEntry& route, ::RateLimit::Descriptor& descriptor,
const std::string& local_service_cluster, const Http::HeaderMap& headers,
const std::string& remote_address) const override;

Expand Down
28 changes: 28 additions & 0 deletions test/common/router/router_ratelimit_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,4 +567,32 @@ TEST_F(RateLimitPolicyEntryTest, CompoundActions) {
testing::ContainerEq(descriptors_));
}

TEST_F(RateLimitPolicyEntryTest, CompoundActionsNoDescriptor) {
std::string json = R"EOF(
{
"actions": [
{
"type": "destination_cluster"
},
{
"type": "header_value_match",
"descriptor_value": "fake_value",
"headers": [
{
"name": "x-header-name",
"value": "test_value",
"regex": false
}
]
}
]
}
)EOF";

SetUpTest(json);

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

} // Router