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: add dynamic metadata to ratelimit response #14508

Merged
merged 11 commits into from
Jan 7, 2021
12 changes: 11 additions & 1 deletion api/envoy/service/ratelimit/v3/rls.proto
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "envoy/config/core/v3/base.proto";
import "envoy/extensions/common/ratelimit/v3/ratelimit.proto";

import "google/protobuf/duration.proto";
import "google/protobuf/struct.proto";

import "udpa/annotations/status.proto";
import "udpa/annotations/versioning.proto";
Expand Down Expand Up @@ -51,7 +52,7 @@ message RateLimitRequest {
}

// A response from a ShouldRateLimit call.
// [#next-free-field: 6]
// [#next-free-field: 7]
message RateLimitResponse {
option (udpa.annotations.versioning).previous_message_type =
"envoy.service.ratelimit.v2.RateLimitResponse";
Expand Down Expand Up @@ -135,4 +136,13 @@ message RateLimitResponse {

// A response body to send to the downstream client when the response code is not OK.
bytes raw_body = 5;

// Optional response metadata that will be emitted as dynamic metadata to be consumed by the next
// filter. This metadata lives in a namespace specified by the canonical name of extension filter
// that requires it:
//
// - :ref:`envoy.filters.http.ratelimit <config_http_filters_ratelimit_dynamic_metadata>` for HTTP filter.
// - :ref:`envoy.filters.network.ratelimit <config_network_filters_ratelimit_dynamic_metadata>` for network filter.
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
// - :ref:`envoy.filters.thrift.rate_limit <config_thrift_filters_rate_limit_dynamic_metadata>` for Thrift filter.
google.protobuf.Struct dynamic_metadata = 6;
}
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,15 @@ The rate limit filter outputs statistics in the *cluster.<route target cluster>.
failure_mode_allowed, Counter, "Total requests that were error(s) but were allowed through because
of :ref:`failure_mode_deny <envoy_v3_api_field_extensions.filters.http.ratelimit.v3.RateLimit.failure_mode_deny>` set to false."

Dynamic Metadata
----------------
.. _config_http_filters_ratelimit_dynamic_metadata:

The ratelimit filter emits dynamic metadata as an opaque ``google.protobuf.Struct``
*only* when the gRPC ratelimit service returns a :ref:`RateLimitResponse
<envoy_v3_api_msg_service.ratelimit.v3.RateLimitResponse>` with a filled :ref:`dynamic_metadata
<envoy_v3_api_field_service.ratelimit.v3.RateLimitResponse.dynamic_metadata>` field.

Runtime
-------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,12 @@ ratelimit.tcp_filter_enabled
ratelimit.tcp_filter_enforcing
% of connections that will call the rate limit service and enforce the decision. Defaults to 100.
This can be used to test what would happen before fully enforcing the outcome.

Dynamic Metadata
----------------
.. _config_network_filters_ratelimit_dynamic_metadata:

The ratelimit filter emits dynamic metadata as an opaque ``google.protobuf.Struct``
*only* when the gRPC ratelimit service returns a :ref:`CheckResponse
<envoy_v3_api_msg_service.ratelimit.v3.RateLimitResponse>` with a filled :ref:`dynamic_metadata
<envoy_v3_api_field_service.ratelimit.v3.RateLimitResponse.dynamic_metadata>` field.
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,12 @@ The filter outputs statistics in the *cluster.<route target cluster>.ratelimit.*
of :ref:`failure_mode_deny
<envoy_v3_api_field_extensions.filters.network.thrift_proxy.filters.ratelimit.v3.RateLimit.failure_mode_deny>` set to
false."

Dynamic Metadata
----------------
.. _config_thrift_filters_rate_limit_dynamic_metadata:

The ratelimit filter emits dynamic metadata as an opaque ``google.protobuf.Struct``
*only* when the gRPC ratelimit service returns a :ref:`CheckResponse
<envoy_v3_api_msg_service.ratelimit.v3.RateLimitResponse>` with a filled :ref:`dynamic_metadata
<envoy_v3_api_field_service.ratelimit.v3.RateLimitResponse.dynamic_metadata>` field.
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ New Features
* ratelimit: added support for use of various :ref:`metadata <envoy_v3_api_field_config.route.v3.RateLimit.Action.metadata>` as a ratelimit action.
* 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.
* ratelimit: added :ref:`body <envoy_v3_api_field_service.ratelimit.v3.RateLimitResponse.raw_body>` field to support custom response bodies for non-OK responses from the external ratelimit service.
* ratelimit: added :ref:`dynamic_metadata <envoy_v3_api_field_service.ratelimit.v3.RateLimitResponse.dynamic_metadata>` field to support setting dynamic metadata from the ratelimit service.
* router: added support for regex rewrites during HTTP redirects using :ref:`regex_rewrite <envoy_v3_api_field_config.route.v3.RedirectAction.regex_rewrite>`.
* sds: improved support for atomic :ref:`key rotations <xds_certificate_rotation>` and added configurable rotation triggers for
:ref:`TlsCertificate <envoy_v3_api_field_extensions.transport_sockets.tls.v3.TlsCertificate.watched_directory>` and
Expand Down
12 changes: 11 additions & 1 deletion generated_api_shadow/envoy/service/ratelimit/v3/rls.proto

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

4 changes: 3 additions & 1 deletion source/extensions/filters/common/ratelimit/ratelimit.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ enum class LimitStatus {
using DescriptorStatusList =
std::vector<envoy::service::ratelimit::v3::RateLimitResponse_DescriptorStatus>;
using DescriptorStatusListPtr = std::unique_ptr<DescriptorStatusList>;
using DynamicMetadataPtr = std::unique_ptr<ProtobufWkt::Struct>;

/**
* Async callbacks used during limit() calls.
Expand All @@ -57,7 +58,8 @@ class RequestCallbacks {
virtual void complete(LimitStatus status, DescriptorStatusListPtr&& descriptor_statuses,
Http::ResponseHeaderMapPtr&& response_headers_to_add,
Http::RequestHeaderMapPtr&& request_headers_to_add,
const std::string& response_body) PURE;
const std::string& response_body,
DynamicMetadataPtr&& dynamic_metadata) PURE;
};

/**
Expand Down
9 changes: 7 additions & 2 deletions source/extensions/filters/common/ratelimit/ratelimit_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,20 @@ void GrpcClientImpl::onSuccess(

DescriptorStatusListPtr descriptor_statuses = std::make_unique<DescriptorStatusList>(
response->statuses().begin(), response->statuses().end());
// TODO(esmet): This dynamic metadata copy is probably unnecessary, but let's just following the
// existing pattern of copying parameters over as unique pointers for now.
DynamicMetadataPtr dynamic_metadata =
std::make_unique<ProtobufWkt::Struct>(response->dynamic_metadata());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid allocating here if there is no metadata which will be most cases. Can you guard this with has_dynamic_metadata()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's fair. By doing that, we probably don't even need to optimize away the copy when there is metadata, since the common case is still zero copy/alloc.

So, I'll add the guard and remove the comment.

callbacks_->complete(status, std::move(descriptor_statuses), std::move(response_headers_to_add),
std::move(request_headers_to_add), response->raw_body());
std::move(request_headers_to_add), response->raw_body(),
std::move(dynamic_metadata));
callbacks_ = nullptr;
}

void GrpcClientImpl::onFailure(Grpc::Status::GrpcStatus status, const std::string&,
Tracing::Span&) {
ASSERT(status != Grpc::Status::WellKnownGrpcStatus::Ok);
callbacks_->complete(LimitStatus::Error, nullptr, nullptr, nullptr, EMPTY_STRING);
callbacks_->complete(LimitStatus::Error, nullptr, nullptr, nullptr, EMPTY_STRING, nullptr);
callbacks_ = nullptr;
}

Expand Down
8 changes: 7 additions & 1 deletion source/extensions/filters/http/ratelimit/ratelimit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,19 @@ void Filter::complete(Filters::Common::RateLimit::LimitStatus status,
Filters::Common::RateLimit::DescriptorStatusListPtr&& descriptor_statuses,
Http::ResponseHeaderMapPtr&& response_headers_to_add,
Http::RequestHeaderMapPtr&& request_headers_to_add,
const std::string& response_body) {
const std::string& response_body,
Filters::Common::RateLimit::DynamicMetadataPtr&& dynamic_metadata) {
state_ = State::Complete;
response_headers_to_add_ = std::move(response_headers_to_add);
Http::HeaderMapPtr req_headers_to_add = std::move(request_headers_to_add);
Stats::StatName empty_stat_name;
Filters::Common::RateLimit::StatNames& stat_names = config_->statNames();

if (dynamic_metadata != nullptr && !dynamic_metadata->fields().empty()) {
callbacks_->streamInfo().setDynamicMetadata(HttpFilterNames::get().RateLimit,
*dynamic_metadata);
}

switch (status) {
case Filters::Common::RateLimit::LimitStatus::OK:
cluster_->statsScope().counterFromStatName(stat_names.ok_).inc();
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/filters/http/ratelimit/ratelimit.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ class Filter : public Http::StreamFilter, public Filters::Common::RateLimit::Req
Filters::Common::RateLimit::DescriptorStatusListPtr&& descriptor_statuses,
Http::ResponseHeaderMapPtr&& response_headers_to_add,
Http::RequestHeaderMapPtr&& request_headers_to_add,
const std::string& response_body) override;
const std::string& response_body,
Filters::Common::RateLimit::DynamicMetadataPtr&& dynamic_metadata) override;

private:
void initiateCall(const Http::RequestHeaderMap& headers);
Expand Down
1 change: 1 addition & 0 deletions source/extensions/filters/network/ratelimit/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ envoy_cc_library(
"//include/envoy/stats:stats_macros",
"//source/common/tracing:http_tracer_lib",
"//source/extensions/filters/common/ratelimit:ratelimit_client_interface",
"//source/extensions/filters/network:well_known_names",
"@envoy_api//envoy/extensions/filters/network/ratelimit/v3:pkg_cc_proto",
],
)
Expand Down
11 changes: 9 additions & 2 deletions source/extensions/filters/network/ratelimit/ratelimit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "common/common/fmt.h"
#include "common/tracing/http_tracer_impl.h"

#include "extensions/filters/network/well_known_names.h"

namespace Envoy {
namespace Extensions {
namespace NetworkFilters {
Expand Down Expand Up @@ -72,8 +74,13 @@ void Filter::onEvent(Network::ConnectionEvent event) {

void Filter::complete(Filters::Common::RateLimit::LimitStatus status,
Filters::Common::RateLimit::DescriptorStatusListPtr&&,
Http::ResponseHeaderMapPtr&&, Http::RequestHeaderMapPtr&&,
const std::string&) {
Http::ResponseHeaderMapPtr&&, Http::RequestHeaderMapPtr&&, const std::string&,
Filters::Common::RateLimit::DynamicMetadataPtr&& dynamic_metadata) {
if (dynamic_metadata != nullptr && !dynamic_metadata->fields().empty()) {
filter_callbacks_->connection().streamInfo().setDynamicMetadata(
NetworkFilterNames::get().RateLimit, *dynamic_metadata);
}

status_ = Status::Complete;
config_->stats().active_.dec();

Expand Down
3 changes: 2 additions & 1 deletion source/extensions/filters/network/ratelimit/ratelimit.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ class Filter : public Network::ReadFilter,
Filters::Common::RateLimit::DescriptorStatusListPtr&& descriptor_statuses,
Http::ResponseHeaderMapPtr&& response_headers_to_add,
Http::RequestHeaderMapPtr&& request_headers_to_add,
const std::string& response_body) override;
const std::string& response_body,
Filters::Common::RateLimit::DynamicMetadataPtr&& dynamic_metadata) override;

private:
enum class Status { NotStarted, Calling, Complete };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ envoy_cc_library(
"//source/extensions/filters/network/thrift_proxy:decoder_events_lib",
"//source/extensions/filters/network/thrift_proxy:protocol_interface",
"//source/extensions/filters/network/thrift_proxy:thrift_lib",
"//source/extensions/filters/network/thrift_proxy/filters:well_known_names",
"//source/extensions/filters/network/thrift_proxy/router:router_interface",
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ envoy_cc_library(
"//source/extensions/filters/common/ratelimit:stat_names_lib",
"//source/extensions/filters/network/thrift_proxy:app_exception_lib",
"//source/extensions/filters/network/thrift_proxy/filters:pass_through_filter_lib",
"//source/extensions/filters/network/thrift_proxy/filters:well_known_names",
"//source/extensions/filters/network/thrift_proxy/router:router_ratelimit_interface",
"@envoy_api//envoy/extensions/filters/network/thrift_proxy/filters/ratelimit/v3:pkg_cc_proto",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "common/tracing/http_tracer_impl.h"

#include "extensions/filters/network/thrift_proxy/app_exception_impl.h"
#include "extensions/filters/network/thrift_proxy/filters/well_known_names.h"
#include "extensions/filters/network/thrift_proxy/router/router.h"
#include "extensions/filters/network/thrift_proxy/router/router_ratelimit.h"

Expand Down Expand Up @@ -62,14 +63,20 @@ void Filter::onDestroy() {
void Filter::complete(Filters::Common::RateLimit::LimitStatus status,
Filters::Common::RateLimit::DescriptorStatusListPtr&& descriptor_statuses,
Http::ResponseHeaderMapPtr&& response_headers_to_add,
Http::RequestHeaderMapPtr&& request_headers_to_add, const std::string&) {
Http::RequestHeaderMapPtr&& request_headers_to_add, const std::string&,
Filters::Common::RateLimit::DynamicMetadataPtr&& dynamic_metadata) {
// TODO(zuercher): Store headers to append to a response. Adding them to a local reply (over
// limit or error) is a matter of modifying the callbacks to allow it. Adding them to an upstream
// response requires either response (aka encoder) filters or some other mechanism.
UNREFERENCED_PARAMETER(descriptor_statuses);
UNREFERENCED_PARAMETER(response_headers_to_add);
UNREFERENCED_PARAMETER(request_headers_to_add);

if (dynamic_metadata != nullptr && !dynamic_metadata->fields().empty()) {
decoder_callbacks_->streamInfo().setDynamicMetadata(
ThriftProxy::ThriftFilters::ThriftFilterNames::get().RATE_LIMIT, *dynamic_metadata);
}

state_ = State::Complete;
Filters::Common::RateLimit::StatNames& stat_names = config_->statNames();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ class Filter : public ThriftProxy::ThriftFilters::PassThroughDecoderFilter,
Filters::Common::RateLimit::DescriptorStatusListPtr&& descriptor_statuses,
Http::ResponseHeaderMapPtr&& response_headers_to_add,
Http::RequestHeaderMapPtr&& request_headers_to_add,
const std::string& response_body) override;
const std::string& response_body,
Filters::Common::RateLimit::DynamicMetadataPtr&& dynamic_metadata) override;

private:
void initiateCall(const ThriftProxy::MessageMetadata& metadata);
Expand Down
2 changes: 1 addition & 1 deletion test/common/network/filter_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ stat_prefix: name
.WillOnce(Return(&conn_pool));

request_callbacks->complete(Extensions::Filters::Common::RateLimit::LimitStatus::OK, nullptr,
nullptr, nullptr, "");
nullptr, nullptr, "", nullptr);

conn_pool.poolReady(upstream_connection);

Expand Down
14 changes: 7 additions & 7 deletions test/extensions/filters/common/ratelimit/ratelimit_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,16 @@ class MockRequestCallbacks : public RequestCallbacks {
void complete(LimitStatus status, DescriptorStatusListPtr&& descriptor_statuses,
Http::ResponseHeaderMapPtr&& response_headers_to_add,
Http::RequestHeaderMapPtr&& request_headers_to_add,
const std::string& response_body) override {
const std::string& response_body, DynamicMetadataPtr&& dynamic_metadata) override {
complete_(status, descriptor_statuses.get(), response_headers_to_add.get(),
request_headers_to_add.get(), response_body);
request_headers_to_add.get(), response_body, dynamic_metadata.get());
}

MOCK_METHOD(void, complete_,
(LimitStatus status, const DescriptorStatusList* descriptor_statuses,
const Http::ResponseHeaderMap* response_headers_to_add,
const Http::RequestHeaderMap* request_headers_to_add,
const std::string& response_body));
const std::string& response_body, const ProtobufWkt::Struct* dynamic_metadata));
};

class RateLimitGrpcClientTest : public testing::Test {
Expand Down Expand Up @@ -93,7 +93,7 @@ TEST_F(RateLimitGrpcClientTest, Basic) {
response = std::make_unique<envoy::service::ratelimit::v3::RateLimitResponse>();
response->set_overall_code(envoy::service::ratelimit::v3::RateLimitResponse::OVER_LIMIT);
EXPECT_CALL(span_, setTag(Eq("ratelimit_status"), Eq("over_limit")));
EXPECT_CALL(request_callbacks_, complete_(LimitStatus::OverLimit, _, _, _, _));
EXPECT_CALL(request_callbacks_, complete_(LimitStatus::OverLimit, _, _, _, _, _));
client_.onSuccess(std::move(response), span_);
}

Expand All @@ -112,7 +112,7 @@ TEST_F(RateLimitGrpcClientTest, Basic) {
response = std::make_unique<envoy::service::ratelimit::v3::RateLimitResponse>();
response->set_overall_code(envoy::service::ratelimit::v3::RateLimitResponse::OK);
EXPECT_CALL(span_, setTag(Eq("ratelimit_status"), Eq("ok")));
EXPECT_CALL(request_callbacks_, complete_(LimitStatus::OK, _, _, _, _));
EXPECT_CALL(request_callbacks_, complete_(LimitStatus::OK, _, _, _, _, _));
client_.onSuccess(std::move(response), span_);
}

Expand All @@ -129,7 +129,7 @@ TEST_F(RateLimitGrpcClientTest, Basic) {
Tracing::NullSpan::instance(), stream_info_);

response = std::make_unique<envoy::service::ratelimit::v3::RateLimitResponse>();
EXPECT_CALL(request_callbacks_, complete_(LimitStatus::Error, _, _, _, _));
EXPECT_CALL(request_callbacks_, complete_(LimitStatus::Error, _, _, _, _, _));
client_.onFailure(Grpc::Status::Unknown, "", span_);
}

Expand All @@ -152,7 +152,7 @@ TEST_F(RateLimitGrpcClientTest, Basic) {
response = std::make_unique<envoy::service::ratelimit::v3::RateLimitResponse>();
response->set_overall_code(envoy::service::ratelimit::v3::RateLimitResponse::OK);
EXPECT_CALL(span_, setTag(Eq("ratelimit_status"), Eq("ok")));
EXPECT_CALL(request_callbacks_, complete_(LimitStatus::OK, _, _, _, _));
EXPECT_CALL(request_callbacks_, complete_(LimitStatus::OK, _, _, _, _, _));
client_.onSuccess(std::move(response), span_);
}
}
Expand Down
Loading