From e5aa69658c6182dd41b6217ec7f6c4c00cac84b4 Mon Sep 17 00:00:00 2001 From: Dhi Aurrahman Date: Fri, 9 Oct 2020 07:28:27 +0700 Subject: [PATCH] grpc: Allow to set parent context to a client to propagate stream info (#13356) This patch allows to set parent context which carries the current request stream info to a gRPC async client instance. Risk Level: Low Testing: Added Docs Changes: Updated Release Notes: Added Fixes #13345 Signed-off-by: Dhi Aurrahman --- api/envoy/config/core/v3/grpc_service.proto | 8 ++- .../config/core/v4alpha/grpc_service.proto | 8 ++- docs/root/version_history/current.rst | 1 + .../envoy/config/core/v3/grpc_service.proto | 8 ++- .../config/core/v4alpha/grpc_service.proto | 8 ++- include/envoy/http/async_client.h | 19 ++++++ source/common/grpc/async_client_impl.cc | 12 ++-- source/common/grpc/async_client_impl.h | 3 +- .../common/grpc/google_async_client_impl.cc | 13 ++-- source/common/grpc/google_async_client_impl.h | 3 +- source/common/router/header_parser.cc | 26 +++++--- source/common/router/header_parser.h | 8 ++- .../common/ext_authz/ext_authz_grpc_impl.cc | 8 ++- .../filters/common/ratelimit/ratelimit.h | 3 +- .../common/ratelimit/ratelimit_impl.cc | 10 +-- .../filters/common/ratelimit/ratelimit_impl.h | 2 +- .../filters/http/ratelimit/ratelimit.cc | 3 +- .../filters/network/ratelimit/ratelimit.cc | 3 +- .../filters/ratelimit/ratelimit.cc | 3 +- test/common/grpc/async_client_impl_test.cc | 44 +++++++++++++ .../grpc/google_async_client_impl_test.cc | 39 ++++++++++++ .../network/filter_manager_impl_test.cc | 2 +- test/common/router/header_formatter_test.cc | 63 +++++++++++++++++++ .../filters/common/ratelimit/mocks.h | 3 +- .../common/ratelimit/ratelimit_impl_test.cc | 14 +++-- .../filters/http/ratelimit/ratelimit_test.cc | 52 +++++++-------- .../network/ratelimit/ratelimit_test.cc | 18 +++--- .../filters/ratelimit/ratelimit_test.cc | 24 +++---- 28 files changed, 304 insertions(+), 104 deletions(-) diff --git a/api/envoy/config/core/v3/grpc_service.proto b/api/envoy/config/core/v3/grpc_service.proto index 8dae5633df54..e3730d017410 100644 --- a/api/envoy/config/core/v3/grpc_service.proto +++ b/api/envoy/config/core/v3/grpc_service.proto @@ -286,8 +286,10 @@ message GrpcService { // request. google.protobuf.Duration timeout = 3; - // Additional metadata to include in streams initiated to the GrpcService. - // This can be used for scenarios in which additional ad hoc authorization - // headers (e.g. ``x-foo-bar: baz-key``) are to be injected. + // Additional metadata to include in streams initiated to the GrpcService. This can be used for + // scenarios in which additional ad hoc authorization headers (e.g. ``x-foo-bar: baz-key``) are to + // be injected. For more information, including details on header value syntax, see the + // documentation on :ref:`custom request headers + // `. repeated HeaderValue initial_metadata = 5; } diff --git a/api/envoy/config/core/v4alpha/grpc_service.proto b/api/envoy/config/core/v4alpha/grpc_service.proto index 938beb917afe..9ea35b456470 100644 --- a/api/envoy/config/core/v4alpha/grpc_service.proto +++ b/api/envoy/config/core/v4alpha/grpc_service.proto @@ -292,8 +292,10 @@ message GrpcService { // request. google.protobuf.Duration timeout = 3; - // Additional metadata to include in streams initiated to the GrpcService. - // This can be used for scenarios in which additional ad hoc authorization - // headers (e.g. ``x-foo-bar: baz-key``) are to be injected. + // Additional metadata to include in streams initiated to the GrpcService. This can be used for + // scenarios in which additional ad hoc authorization headers (e.g. ``x-foo-bar: baz-key``) are to + // be injected. For more information, including details on header value syntax, see the + // documentation on :ref:`custom request headers + // `. repeated HeaderValue initial_metadata = 5; } diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 9f9ec8052c21..449c4373d6a0 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -42,6 +42,7 @@ New Features * ext_authz filter: added support for letting the authorization server instruct Envoy to remove headers from the original request by setting the new field :ref:`headers_to_remove ` before forwarding it to the upstream. * ext_authz filter: added support for sending :ref:`raw bytes as request body ` of a gRPC check request by setting :ref:`pack_as_bytes ` to true. * ext_authz_filter: added :ref:`disable_request_body_buffering ` to disable request data buffering per-route. +* grpc: implemented header value syntax support when defining :ref:`initial metadata ` for gRPC-based `ext_authz` :ref:`HTTP ` and :ref:`network ` filters, and :ref:`ratelimit ` filters. * grpc-json: support specifying `response_body` field in for `google.api.HttpBody` message. * hds: added :ref:`cluster_endpoints_health ` to HDS responses, keeping endpoints in the same groupings as they were configured in the HDS specifier by cluster and locality instead of as a flat list. * hds: added :ref:`transport_socket_matches ` to HDS cluster health check specifier, so the existing match filter :ref:`transport_socket_match_criteria ` in the repeated field :ref:`health_checks ` has context to match against. This unblocks support for health checks over HTTPS and HTTP/2. diff --git a/generated_api_shadow/envoy/config/core/v3/grpc_service.proto b/generated_api_shadow/envoy/config/core/v3/grpc_service.proto index df7ed194ce66..fb05f3b73a5f 100644 --- a/generated_api_shadow/envoy/config/core/v3/grpc_service.proto +++ b/generated_api_shadow/envoy/config/core/v3/grpc_service.proto @@ -284,8 +284,10 @@ message GrpcService { // request. google.protobuf.Duration timeout = 3; - // Additional metadata to include in streams initiated to the GrpcService. - // This can be used for scenarios in which additional ad hoc authorization - // headers (e.g. ``x-foo-bar: baz-key``) are to be injected. + // Additional metadata to include in streams initiated to the GrpcService. This can be used for + // scenarios in which additional ad hoc authorization headers (e.g. ``x-foo-bar: baz-key``) are to + // be injected. For more information, including details on header value syntax, see the + // documentation on :ref:`custom request headers + // `. repeated HeaderValue initial_metadata = 5; } diff --git a/generated_api_shadow/envoy/config/core/v4alpha/grpc_service.proto b/generated_api_shadow/envoy/config/core/v4alpha/grpc_service.proto index 938beb917afe..9ea35b456470 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/grpc_service.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/grpc_service.proto @@ -292,8 +292,10 @@ message GrpcService { // request. google.protobuf.Duration timeout = 3; - // Additional metadata to include in streams initiated to the GrpcService. - // This can be used for scenarios in which additional ad hoc authorization - // headers (e.g. ``x-foo-bar: baz-key``) are to be injected. + // Additional metadata to include in streams initiated to the GrpcService. This can be used for + // scenarios in which additional ad hoc authorization headers (e.g. ``x-foo-bar: baz-key``) are to + // be injected. For more information, including details on header value syntax, see the + // documentation on :ref:`custom request headers + // `. repeated HeaderValue initial_metadata = 5; } diff --git a/include/envoy/http/async_client.h b/include/envoy/http/async_client.h index 066ccb04e716..5121a3c7b969 100644 --- a/include/envoy/http/async_client.h +++ b/include/envoy/http/async_client.h @@ -6,6 +6,7 @@ #include "envoy/config/route/v3/route_components.pb.h" #include "envoy/event/dispatcher.h" #include "envoy/http/message.h" +#include "envoy/stream_info/stream_info.h" #include "envoy/tracing/http_tracer.h" #include "common/protobuf/protobuf.h" @@ -168,6 +169,13 @@ class AsyncClient { virtual ~AsyncClient() = default; + /** + * A context from the caller of an async client. + */ + struct ParentContext { + const StreamInfo::StreamInfo* stream_info; + }; + /** * A structure to hold the options for AsyncStream object. */ @@ -193,6 +201,10 @@ class AsyncClient { hash_policy = v; return *this; } + StreamOptions& setParentContext(const ParentContext& v) { + parent_context = v; + return *this; + } // For gmock test bool operator==(const StreamOptions& src) const { @@ -215,6 +227,9 @@ class AsyncClient { // Provides the hash policy for hashing load balancing strategies. Protobuf::RepeatedPtrField hash_policy; + + // Provides parent context. Currently, this holds stream info from the caller. + ParentContext parent_context; }; /** @@ -242,6 +257,10 @@ class AsyncClient { StreamOptions::setHashPolicy(v); return *this; } + RequestOptions& setParentContext(const ParentContext& v) { + StreamOptions::setParentContext(v); + return *this; + } RequestOptions& setParentSpan(Tracing::Span& parent_span) { parent_span_ = &parent_span; return *this; diff --git a/source/common/grpc/async_client_impl.cc b/source/common/grpc/async_client_impl.cc index 55e4fa75b23b..aaec44e0e03f 100644 --- a/source/common/grpc/async_client_impl.cc +++ b/source/common/grpc/async_client_impl.cc @@ -16,8 +16,9 @@ AsyncClientImpl::AsyncClientImpl(Upstream::ClusterManager& cm, const envoy::config::core::v3::GrpcService& config, TimeSource& time_source) : cm_(cm), remote_cluster_name_(config.envoy_grpc().cluster_name()), - host_name_(config.envoy_grpc().authority()), initial_metadata_(config.initial_metadata()), - time_source_(time_source) {} + host_name_(config.envoy_grpc().authority()), time_source_(time_source), + metadata_parser_( + Router::HeaderParser::configure(config.initial_metadata(), /*append=*/false)) {} AsyncClientImpl::~AsyncClientImpl() { while (!active_streams_.empty()) { @@ -88,10 +89,9 @@ void AsyncStreamImpl::initialize(bool buffer_body_for_retry) { parent_.host_name_.empty() ? parent_.remote_cluster_name_ : parent_.host_name_, service_full_name_, method_name_, options_.timeout); // Fill service-wide initial metadata. - for (const auto& header_value : parent_.initial_metadata_) { - headers_message_->headers().addCopy(Http::LowerCaseString(header_value.key()), - header_value.value()); - } + parent_.metadata_parser_->evaluateHeaders(headers_message_->headers(), + options_.parent_context.stream_info); + callbacks_.onCreateInitialMetadata(headers_message_->headers()); stream_->sendHeaders(headers_message_->headers(), false); } diff --git a/source/common/grpc/async_client_impl.h b/source/common/grpc/async_client_impl.h index ae0e2c7782ab..2e7139df209c 100644 --- a/source/common/grpc/async_client_impl.h +++ b/source/common/grpc/async_client_impl.h @@ -10,6 +10,7 @@ #include "common/grpc/codec.h" #include "common/grpc/typed_async_client.h" #include "common/http/async_client_impl.h" +#include "common/router/header_parser.h" namespace Envoy { namespace Grpc { @@ -39,9 +40,9 @@ class AsyncClientImpl final : public RawAsyncClient { const std::string remote_cluster_name_; // The host header value in the http transport. const std::string host_name_; - const Protobuf::RepeatedPtrField initial_metadata_; std::list active_streams_; TimeSource& time_source_; + Router::HeaderParserPtr metadata_parser_; friend class AsyncRequestImpl; friend class AsyncStreamImpl; diff --git a/source/common/grpc/google_async_client_impl.cc b/source/common/grpc/google_async_client_impl.cc index 319ee5be4693..9239fe4d74c5 100644 --- a/source/common/grpc/google_async_client_impl.cc +++ b/source/common/grpc/google_async_client_impl.cc @@ -10,6 +10,7 @@ #include "common/grpc/common.h" #include "common/grpc/google_grpc_creds_impl.h" #include "common/grpc/google_grpc_utils.h" +#include "common/router/header_parser.h" #include "common/tracing/http_tracer_impl.h" #include "grpcpp/support/proto_buffer_reader.h" @@ -79,9 +80,11 @@ GoogleAsyncClientImpl::GoogleAsyncClientImpl(Event::Dispatcher& dispatcher, const envoy::config::core::v3::GrpcService& config, Api::Api& api, const StatNames& stat_names) : dispatcher_(dispatcher), tls_(tls), stat_prefix_(config.google_grpc().stat_prefix()), - initial_metadata_(config.initial_metadata()), scope_(scope), + scope_(scope), per_stream_buffer_limit_bytes_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( - config.google_grpc(), per_stream_buffer_limit_bytes, DefaultBufferLimitBytes)) { + config.google_grpc(), per_stream_buffer_limit_bytes, DefaultBufferLimitBytes)), + metadata_parser_( + Router::HeaderParser::configure(config.initial_metadata(), /*append=*/false)) { // We rebuild the channel each time we construct the channel. It appears that the gRPC library is // smart enough to do connection pooling and reuse with identical channel args, so this should // have comparable overhead to what we are doing in Grpc::AsyncClientImpl, i.e. no expensive @@ -167,12 +170,8 @@ void GoogleAsyncStreamImpl::initialize(bool /*buffer_body_for_retry*/) { : gpr_inf_future(GPR_CLOCK_REALTIME); ctxt_.set_deadline(abs_deadline); // Fill service-wide initial metadata. - for (const auto& header_value : parent_.initial_metadata_) { - ctxt_.AddMetadata(header_value.key(), header_value.value()); - } - // Due to the different HTTP header implementations, we effectively double - // copy headers here. auto initial_metadata = Http::RequestHeaderMapImpl::create(); + parent_.metadata_parser_->evaluateHeaders(*initial_metadata, options_.parent_context.stream_info); callbacks_.onCreateInitialMetadata(*initial_metadata); initial_metadata->iterate([this](const Http::HeaderEntry& header) { ctxt_.AddMetadata(std::string(header.key().getStringView()), diff --git a/source/common/grpc/google_async_client_impl.h b/source/common/grpc/google_async_client_impl.h index 8e946ce5c0cb..a0e27c5e5efd 100644 --- a/source/common/grpc/google_async_client_impl.h +++ b/source/common/grpc/google_async_client_impl.h @@ -19,6 +19,7 @@ #include "common/grpc/google_grpc_context.h" #include "common/grpc/stat_names.h" #include "common/grpc/typed_async_client.h" +#include "common/router/header_parser.h" #include "common/tracing/http_tracer_impl.h" #include "absl/container/node_hash_set.h" @@ -197,10 +198,10 @@ class GoogleAsyncClientImpl final : public RawAsyncClient, Logger::Loggable active_streams_; const std::string stat_prefix_; - const Protobuf::RepeatedPtrField initial_metadata_; Stats::ScopeSharedPtr scope_; GoogleAsyncClientStats stats_; uint64_t per_stream_buffer_limit_bytes_; + Router::HeaderParserPtr metadata_parser_; friend class GoogleAsyncClientThreadLocal; friend class GoogleAsyncRequestImpl; diff --git a/source/common/router/header_parser.cc b/source/common/router/header_parser.cc index 758f89c86bc6..1e4a9f4e3098 100644 --- a/source/common/router/header_parser.cc +++ b/source/common/router/header_parser.cc @@ -226,9 +226,9 @@ HeaderParserPtr HeaderParser::configure( for (const auto& header_value_option : headers_to_add) { const bool append = PROTOBUF_GET_WRAPPED_OR_DEFAULT(header_value_option, append, true); HeaderFormatterPtr header_formatter = parseInternal(header_value_option.header(), append); - header_parser->headers_to_add_.emplace_back( - Http::LowerCaseString(header_value_option.header().key()), std::move(header_formatter)); + Http::LowerCaseString(header_value_option.header().key()), + HeadersToAddEntry{std::move(header_formatter), header_value_option.header().value()}); } return header_parser; @@ -241,9 +241,9 @@ HeaderParserPtr HeaderParser::configure( for (const auto& header_value : headers_to_add) { HeaderFormatterPtr header_formatter = parseInternal(header_value, append); - - header_parser->headers_to_add_.emplace_back(Http::LowerCaseString(header_value.key()), - std::move(header_formatter)); + header_parser->headers_to_add_.emplace_back( + Http::LowerCaseString(header_value.key()), + HeadersToAddEntry{std::move(header_formatter), header_value.value()}); } return header_parser; @@ -269,19 +269,25 @@ HeaderParserPtr HeaderParser::configure( void HeaderParser::evaluateHeaders(Http::HeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const { + evaluateHeaders(headers, &stream_info); +} + +void HeaderParser::evaluateHeaders(Http::HeaderMap& headers, + const StreamInfo::StreamInfo* stream_info) const { // Removing headers in the headers_to_remove_ list first makes // remove-before-add the default behavior as expected by users. for (const auto& header : headers_to_remove_) { headers.remove(header); } - for (const auto& formatter : headers_to_add_) { - const std::string value = formatter.second->format(stream_info); + for (const auto& [key, entry] : headers_to_add_) { + const std::string value = + stream_info != nullptr ? entry.formatter_->format(*stream_info) : entry.original_value_; if (!value.empty()) { - if (formatter.second->append()) { - headers.addReferenceKey(formatter.first, value); + if (entry.formatter_->append()) { + headers.addReferenceKey(key, value); } else { - headers.setReferenceKey(formatter.first, value); + headers.setReferenceKey(key, value); } } } diff --git a/source/common/router/header_parser.h b/source/common/router/header_parser.h index d32832f414b4..3b947a3d6e11 100644 --- a/source/common/router/header_parser.h +++ b/source/common/router/header_parser.h @@ -49,12 +49,18 @@ class HeaderParser { const Protobuf::RepeatedPtrField& headers_to_remove); void evaluateHeaders(Http::HeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const; + void evaluateHeaders(Http::HeaderMap& headers, const StreamInfo::StreamInfo* stream_info) const; protected: HeaderParser() = default; private: - std::vector> headers_to_add_; + struct HeadersToAddEntry { + HeaderFormatterPtr formatter_; + const std::string original_value_; + }; + + std::vector> headers_to_add_; std::vector headers_to_remove_; }; diff --git a/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc b/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc index f4b133e2b01a..d4450e09df58 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc +++ b/source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc @@ -39,7 +39,7 @@ void GrpcClientImpl::cancel() { void GrpcClientImpl::check(RequestCallbacks& callbacks, Event::Dispatcher& dispatcher, const envoy::service::auth::v3::CheckRequest& request, - Tracing::Span& parent_span, const StreamInfo::StreamInfo&) { + Tracing::Span& parent_span, const StreamInfo::StreamInfo& stream_info) { ASSERT(callbacks_ == nullptr); callbacks_ = &callbacks; @@ -47,8 +47,8 @@ void GrpcClientImpl::check(RequestCallbacks& callbacks, Event::Dispatcher& dispa if (timeout_.has_value()) { if (timeoutStartsAtCheckCreation()) { // TODO(yuval-k): We currently use dispatcher based timeout even if the underlying client is - // google gRPC client, which has it's own timeout mechanism. We may want to change that in - // the future if the implementations converge. + // Google gRPC client, which has its own timeout mechanism. We may want to change that in the + // future if the implementations converge. timeout_timer_ = dispatcher.createTimer([this]() -> void { onTimeout(); }); timeout_timer_->enableTimer(timeout_.value()); } else { @@ -57,6 +57,8 @@ void GrpcClientImpl::check(RequestCallbacks& callbacks, Event::Dispatcher& dispa } } + options.setParentContext(Http::AsyncClient::ParentContext{&stream_info}); + ENVOY_LOG(trace, "Sending CheckRequest: {}", request.DebugString()); request_ = async_client_->send(service_method_, request, *this, parent_span, options, transport_api_version_); diff --git a/source/extensions/filters/common/ratelimit/ratelimit.h b/source/extensions/filters/common/ratelimit/ratelimit.h index 4ad48e7a87ab..068cd369b643 100644 --- a/source/extensions/filters/common/ratelimit/ratelimit.h +++ b/source/extensions/filters/common/ratelimit/ratelimit.h @@ -9,6 +9,7 @@ #include "envoy/ratelimit/ratelimit.h" #include "envoy/service/ratelimit/v3/rls.pb.h" #include "envoy/singleton/manager.h" +#include "envoy/stream_info/stream_info.h" #include "envoy/tracing/http_tracer.h" #include "absl/types/optional.h" @@ -77,7 +78,7 @@ class Client { */ virtual void limit(RequestCallbacks& callbacks, const std::string& domain, const std::vector& descriptors, - Tracing::Span& parent_span) PURE; + Tracing::Span& parent_span, const StreamInfo::StreamInfo& stream_info) PURE; }; using ClientPtr = std::unique_ptr; diff --git a/source/extensions/filters/common/ratelimit/ratelimit_impl.cc b/source/extensions/filters/common/ratelimit/ratelimit_impl.cc index 5a93471af903..d4c3f5afdaa3 100644 --- a/source/extensions/filters/common/ratelimit/ratelimit_impl.cc +++ b/source/extensions/filters/common/ratelimit/ratelimit_impl.cc @@ -61,16 +61,18 @@ void GrpcClientImpl::createRequest(envoy::service::ratelimit::v3::RateLimitReque void GrpcClientImpl::limit(RequestCallbacks& callbacks, const std::string& domain, const std::vector& descriptors, - Tracing::Span& parent_span) { + Tracing::Span& parent_span, const StreamInfo::StreamInfo& stream_info) { ASSERT(callbacks_ == nullptr); callbacks_ = &callbacks; envoy::service::ratelimit::v3::RateLimitRequest request; createRequest(request, domain, descriptors); - request_ = async_client_->send(service_method_, request, *this, parent_span, - Http::AsyncClient::RequestOptions().setTimeout(timeout_), - transport_api_version_); + request_ = + async_client_->send(service_method_, request, *this, parent_span, + Http::AsyncClient::RequestOptions().setTimeout(timeout_).setParentContext( + Http::AsyncClient::ParentContext{&stream_info}), + transport_api_version_); } void GrpcClientImpl::onSuccess( diff --git a/source/extensions/filters/common/ratelimit/ratelimit_impl.h b/source/extensions/filters/common/ratelimit/ratelimit_impl.h index 4108ec2b45c0..4386102a21ca 100644 --- a/source/extensions/filters/common/ratelimit/ratelimit_impl.h +++ b/source/extensions/filters/common/ratelimit/ratelimit_impl.h @@ -58,7 +58,7 @@ class GrpcClientImpl : public Client, void cancel() override; void limit(RequestCallbacks& callbacks, const std::string& domain, const std::vector& descriptors, - Tracing::Span& parent_span) override; + Tracing::Span& parent_span, const StreamInfo::StreamInfo& stream_info) override; // Grpc::AsyncRequestCallbacks void onCreateInitialMetadata(Http::RequestHeaderMap&) override {} diff --git a/source/extensions/filters/http/ratelimit/ratelimit.cc b/source/extensions/filters/http/ratelimit/ratelimit.cc index dca76209a042..c5fb0d284a49 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.cc +++ b/source/extensions/filters/http/ratelimit/ratelimit.cc @@ -73,7 +73,8 @@ void Filter::initiateCall(const Http::RequestHeaderMap& headers) { if (!descriptors.empty()) { state_ = State::Calling; initiating_call_ = true; - client_->limit(*this, config_->domain(), descriptors, callbacks_->activeSpan()); + client_->limit(*this, config_->domain(), descriptors, callbacks_->activeSpan(), + callbacks_->streamInfo()); initiating_call_ = false; } } diff --git a/source/extensions/filters/network/ratelimit/ratelimit.cc b/source/extensions/filters/network/ratelimit/ratelimit.cc index 430508ce3b61..00ed50a9f60c 100644 --- a/source/extensions/filters/network/ratelimit/ratelimit.cc +++ b/source/extensions/filters/network/ratelimit/ratelimit.cc @@ -49,7 +49,8 @@ Network::FilterStatus Filter::onNewConnection() { config_->stats().active_.inc(); config_->stats().total_.inc(); calling_limit_ = true; - client_->limit(*this, config_->domain(), config_->descriptors(), Tracing::NullSpan::instance()); + client_->limit(*this, config_->domain(), config_->descriptors(), Tracing::NullSpan::instance(), + filter_callbacks_->connection().streamInfo()); calling_limit_ = false; } diff --git a/source/extensions/filters/network/thrift_proxy/filters/ratelimit/ratelimit.cc b/source/extensions/filters/network/thrift_proxy/filters/ratelimit/ratelimit.cc index e26a565f5856..45e025c04e34 100644 --- a/source/extensions/filters/network/thrift_proxy/filters/ratelimit/ratelimit.cc +++ b/source/extensions/filters/network/thrift_proxy/filters/ratelimit/ratelimit.cc @@ -45,7 +45,8 @@ void Filter::initiateCall(const ThriftProxy::MessageMetadata& metadata) { if (!descriptors.empty()) { state_ = State::Calling; initiating_call_ = true; - client_->limit(*this, config_->domain(), descriptors, Tracing::NullSpan::instance()); + client_->limit(*this, config_->domain(), descriptors, Tracing::NullSpan::instance(), + decoder_callbacks_->streamInfo()); initiating_call_ = false; } } diff --git a/test/common/grpc/async_client_impl_test.cc b/test/common/grpc/async_client_impl_test.cc index 6544c33bf952..8340b56aeda9 100644 --- a/test/common/grpc/async_client_impl_test.cc +++ b/test/common/grpc/async_client_impl_test.cc @@ -1,6 +1,7 @@ #include "envoy/config/core/v3/grpc_service.pb.h" #include "common/grpc/async_client_impl.h" +#include "common/network/address_impl.h" #include "test/mocks/http/mocks.h" #include "test/mocks/tracing/mocks.h" @@ -27,6 +28,11 @@ class EnvoyAsyncClientImplTest : public testing::Test { : method_descriptor_(helloworld::Greeter::descriptor()->FindMethodByName("SayHello")) { envoy::config::core::v3::GrpcService config; config.mutable_envoy_grpc()->set_cluster_name("test_cluster"); + + auto& initial_metadata_entry = *config.mutable_initial_metadata()->Add(); + initial_metadata_entry.set_key("downstream-local-address"); + initial_metadata_entry.set_value("%DOWNSTREAM_LOCAL_ADDRESS_WITHOUT_PORT%"); + grpc_client_ = std::make_unique(cm_, config, test_time_.timeSystem()); ON_CALL(cm_, httpAsyncClientForCluster("test_cluster")).WillByDefault(ReturnRef(http_client_)); } @@ -96,6 +102,44 @@ TEST_F(EnvoyAsyncClientImplTest, HostIsOverrideByConfig) { EXPECT_EQ(grpc_stream, nullptr); } +// Validate that the metadata header is the initial metadata in gRPC service config and the value is +// interpolated. +TEST_F(EnvoyAsyncClientImplTest, MetadataIsInitialized) { + NiceMock> grpc_callbacks; + Http::AsyncClient::StreamCallbacks* http_callbacks; + + Http::MockAsyncClientStream http_stream; + EXPECT_CALL(http_client_, start(_, _)) + .WillOnce( + Invoke([&http_callbacks, &http_stream](Http::AsyncClient::StreamCallbacks& callbacks, + const Http::AsyncClient::StreamOptions&) { + http_callbacks = &callbacks; + return &http_stream; + })); + + const std::string expected_downstream_local_address = "5.5.5.5"; + EXPECT_CALL(grpc_callbacks, + onCreateInitialMetadata(testing::Truly([&expected_downstream_local_address]( + Http::RequestHeaderMap& headers) { + return headers.get(Http::LowerCaseString("downstream-local-address"))->value() == + expected_downstream_local_address; + }))); + EXPECT_CALL(http_stream, sendHeaders(_, _)) + .WillOnce(Invoke([&http_callbacks](Http::HeaderMap&, bool) { http_callbacks->onReset(); })); + + // Prepare the parent context of this call. + StreamInfo::StreamInfoImpl stream_info{test_time_.timeSystem()}; + stream_info.setDownstreamLocalAddress( + std::make_shared(expected_downstream_local_address)); + Http::AsyncClient::ParentContext parent_context{&stream_info}; + + Http::AsyncClient::StreamOptions stream_options; + stream_options.setParentContext(parent_context); + + auto grpc_stream = grpc_client_->start(*method_descriptor_, grpc_callbacks, stream_options); + EXPECT_EQ(grpc_stream, nullptr); +} + // Validate that a failure in the HTTP client returns immediately with status // UNAVAILABLE. TEST_F(EnvoyAsyncClientImplTest, StreamHttpStartFail) { diff --git a/test/common/grpc/google_async_client_impl_test.cc b/test/common/grpc/google_async_client_impl_test.cc index 1474899621f1..7ecbf57bc5cf 100644 --- a/test/common/grpc/google_async_client_impl_test.cc +++ b/test/common/grpc/google_async_client_impl_test.cc @@ -4,7 +4,9 @@ #include "common/api/api_impl.h" #include "common/event/dispatcher_impl.h" #include "common/grpc/google_async_client_impl.h" +#include "common/network/address_impl.h" #include "common/stats/isolated_store_impl.h" +#include "common/stream_info/stream_info_impl.h" #include "test/mocks/grpc/mocks.h" #include "test/mocks/tracing/mocks.h" @@ -58,6 +60,11 @@ class EnvoyGoogleAsyncClientImplTest : public testing::Test { auto* google_grpc = config_.mutable_google_grpc(); google_grpc->set_target_uri("fake_address"); google_grpc->set_stat_prefix("test_cluster"); + + auto& initial_metadata_entry = *config_.mutable_initial_metadata()->Add(); + initial_metadata_entry.set_key("downstream-local-address"); + initial_metadata_entry.set_value("%DOWNSTREAM_LOCAL_ADDRESS_WITHOUT_PORT%"); + tls_ = std::make_unique(*api_); } @@ -94,6 +101,38 @@ TEST_F(EnvoyGoogleAsyncClientImplTest, StreamHttpStartFail) { EXPECT_TRUE(grpc_stream == nullptr); } +// Validate that the metadata header is the initial metadata in gRPC service config and the value is +// interpolated. +TEST_F(EnvoyGoogleAsyncClientImplTest, MetadataIsInitialized) { + initialize(); + + EXPECT_CALL(*stub_factory_.stub_, PrepareCall_(_, _, _)).WillOnce(Return(nullptr)); + MockAsyncStreamCallbacks grpc_callbacks; + + const std::string expected_downstream_local_address = "5.5.5.5"; + EXPECT_CALL(grpc_callbacks, + onCreateInitialMetadata(testing::Truly([&expected_downstream_local_address]( + Http::RequestHeaderMap& headers) { + return headers.get(Http::LowerCaseString("downstream-local-address"))->value() == + expected_downstream_local_address; + }))); + + EXPECT_CALL(grpc_callbacks, onReceiveTrailingMetadata_(_)); + EXPECT_CALL(grpc_callbacks, onRemoteClose(Status::WellKnownGrpcStatus::Unavailable, "")); + + // Prepare the parent context of this call. + StreamInfo::StreamInfoImpl stream_info{test_time_.timeSystem()}; + stream_info.setDownstreamLocalAddress( + std::make_shared(expected_downstream_local_address)); + Http::AsyncClient::ParentContext parent_context{&stream_info}; + + Http::AsyncClient::StreamOptions stream_options; + stream_options.setParentContext(parent_context); + + auto grpc_stream = grpc_client_->start(*method_descriptor_, grpc_callbacks, stream_options); + EXPECT_TRUE(grpc_stream == nullptr); +} + // Validate that a failure in gRPC stub call creation returns immediately with // status UNAVAILABLE. TEST_F(EnvoyGoogleAsyncClientImplTest, RequestHttpStartFail) { diff --git a/test/common/network/filter_manager_impl_test.cc b/test/common/network/filter_manager_impl_test.cc index 705f1370e01c..c40e2804ed0c 100644 --- a/test/common/network/filter_manager_impl_test.cc +++ b/test/common/network/filter_manager_impl_test.cc @@ -403,7 +403,7 @@ stat_prefix: name EXPECT_CALL(*rl_client, limit(_, "foo", testing::ContainerEq( std::vector{{{{"hello", "world"}}}}), - testing::A())) + testing::A(), _)) .WillOnce(WithArgs<0>( Invoke([&](Extensions::Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks = &callbacks; diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index 93f626fc86e2..d56a890ed427 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -982,6 +982,69 @@ match: { prefix: "/new_endpoint" } EXPECT_TRUE(header_map.has("x-client-ip-port")); } +TEST(HeaderParserTest, EvaluateHeadersWithNullStreamInfo) { + const std::string yaml = R"EOF( +match: { prefix: "/new_endpoint" } +route: + cluster: "www2" + prefix_rewrite: "/api/new_endpoint" +request_headers_to_add: + - header: + key: "x-client-ip" + value: "%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%" + append: true + - header: + key: "x-client-ip-port" + value: "%DOWNSTREAM_REMOTE_ADDRESS%" + append: true +)EOF"; + + HeaderParserPtr req_header_parser = + HeaderParser::configure(parseRouteFromV3Yaml(yaml).request_headers_to_add()); + Http::TestRequestHeaderMapImpl header_map{{":method", "POST"}}; + req_header_parser->evaluateHeaders(header_map, nullptr); + EXPECT_TRUE(header_map.has("x-client-ip")); + EXPECT_TRUE(header_map.has("x-client-ip-port")); + EXPECT_EQ("%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%", header_map.get_("x-client-ip")); + EXPECT_EQ("%DOWNSTREAM_REMOTE_ADDRESS%", header_map.get_("x-client-ip-port")); +} + +TEST(HeaderParserTest, EvaluateHeaderValuesWithNullStreamInfo) { + Http::TestRequestHeaderMapImpl header_map{{":method", "POST"}}; + Protobuf::RepeatedPtrField headers_values; + + auto& first_entry = *headers_values.Add(); + first_entry.set_key("key"); + + // This tests when we have "StreamInfoHeaderFormatter", but stream info is null. + first_entry.set_value("%DOWNSTREAM_REMOTE_ADDRESS%"); + + HeaderParserPtr req_header_parser_add = HeaderParser::configure(headers_values, /*append=*/true); + req_header_parser_add->evaluateHeaders(header_map, nullptr); + EXPECT_TRUE(header_map.has("key")); + EXPECT_EQ("%DOWNSTREAM_REMOTE_ADDRESS%", header_map.get_("key")); + + headers_values.Clear(); + auto& set_entry = *headers_values.Add(); + set_entry.set_key("key"); + set_entry.set_value("great"); + + HeaderParserPtr req_header_parser_set = HeaderParser::configure(headers_values, /*append=*/false); + req_header_parser_set->evaluateHeaders(header_map, nullptr); + EXPECT_TRUE(header_map.has("key")); + EXPECT_EQ("great", header_map.get_("key")); + + headers_values.Clear(); + auto& empty_entry = *headers_values.Add(); + empty_entry.set_key("empty"); + empty_entry.set_value(""); + + HeaderParserPtr req_header_parser_empty = + HeaderParser::configure(headers_values, /*append=*/false); + req_header_parser_empty->evaluateHeaders(header_map, nullptr); + EXPECT_FALSE(header_map.has("empty")); +} + TEST(HeaderParserTest, EvaluateEmptyHeaders) { const std::string yaml = R"EOF( match: { prefix: "/new_endpoint" } diff --git a/test/extensions/filters/common/ratelimit/mocks.h b/test/extensions/filters/common/ratelimit/mocks.h index f04b1582da68..325f61cc79d7 100644 --- a/test/extensions/filters/common/ratelimit/mocks.h +++ b/test/extensions/filters/common/ratelimit/mocks.h @@ -4,6 +4,7 @@ #include #include "envoy/ratelimit/ratelimit.h" +#include "envoy/stream_info/stream_info.h" #include "extensions/filters/common/ratelimit/ratelimit.h" @@ -25,7 +26,7 @@ class MockClient : public Client { MOCK_METHOD(void, limit, (RequestCallbacks & callbacks, const std::string& domain, const std::vector& descriptors, - Tracing::Span& parent_span)); + Tracing::Span& parent_span, const StreamInfo::StreamInfo& stream_info)); }; } // namespace RateLimit diff --git a/test/extensions/filters/common/ratelimit/ratelimit_impl_test.cc b/test/extensions/filters/common/ratelimit/ratelimit_impl_test.cc index 65242eb83c1f..319596f436a9 100644 --- a/test/extensions/filters/common/ratelimit/ratelimit_impl_test.cc +++ b/test/extensions/filters/common/ratelimit/ratelimit_impl_test.cc @@ -13,6 +13,7 @@ #include "extensions/filters/common/ratelimit/ratelimit_impl.h" #include "test/mocks/grpc/mocks.h" +#include "test/mocks/stream_info/mocks.h" #include "test/mocks/tracing/mocks.h" #include "test/test_common/printers.h" #include "test/test_common/utility.h" @@ -60,6 +61,7 @@ class RateLimitGrpcClientTest : public testing::Test { GrpcClientImpl client_; MockRequestCallbacks request_callbacks_; Tracing::MockSpan span_; + StreamInfo::MockStreamInfo stream_info_; }; TEST_F(RateLimitGrpcClientTest, Basic) { @@ -80,7 +82,8 @@ TEST_F(RateLimitGrpcClientTest, Basic) { return &async_request_; })); - client_.limit(request_callbacks_, "foo", {{{{"foo", "bar"}}}}, Tracing::NullSpan::instance()); + client_.limit(request_callbacks_, "foo", {{{{"foo", "bar"}}}}, Tracing::NullSpan::instance(), + stream_info_); client_.onCreateInitialMetadata(headers); EXPECT_EQ(nullptr, headers.RequestId()); @@ -100,7 +103,7 @@ TEST_F(RateLimitGrpcClientTest, Basic) { .WillOnce(Return(&async_request_)); client_.limit(request_callbacks_, "foo", {{{{"foo", "bar"}, {"bar", "baz"}}}}, - Tracing::NullSpan::instance()); + Tracing::NullSpan::instance(), stream_info_); client_.onCreateInitialMetadata(headers); @@ -121,7 +124,7 @@ TEST_F(RateLimitGrpcClientTest, Basic) { client_.limit(request_callbacks_, "foo", {{{{"foo", "bar"}, {"bar", "baz"}}}, {{{"foo2", "bar2"}, {"bar2", "baz2"}}}}, - Tracing::NullSpan::instance()); + Tracing::NullSpan::instance(), stream_info_); response = std::make_unique(); EXPECT_CALL(request_callbacks_, complete_(LimitStatus::Error, _, _, _)); @@ -140,7 +143,7 @@ TEST_F(RateLimitGrpcClientTest, Basic) { client_.limit( request_callbacks_, "foo", {{{{"foo", "bar"}, {"bar", "baz"}}, {{42, envoy::type::v3::RateLimitUnit::MINUTE}}}}, - Tracing::NullSpan::instance()); + Tracing::NullSpan::instance(), stream_info_); client_.onCreateInitialMetadata(headers); @@ -157,7 +160,8 @@ TEST_F(RateLimitGrpcClientTest, Cancel) { EXPECT_CALL(*async_client_, sendRaw(_, _, _, _, _, _)).WillOnce(Return(&async_request_)); - client_.limit(request_callbacks_, "foo", {{{{"foo", "bar"}}}}, Tracing::NullSpan::instance()); + client_.limit(request_callbacks_, "foo", {{{{"foo", "bar"}}}}, Tracing::NullSpan::instance(), + stream_info_); EXPECT_CALL(async_request_, cancel()); client_.cancel(); diff --git a/test/extensions/filters/http/ratelimit/ratelimit_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_test.cc index 39d4fe6f9384..c7ab957ada5e 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/http/ratelimit/ratelimit_test.cc @@ -148,7 +148,7 @@ TEST_F(HttpRateLimitFilterTest, NoApplicableRateLimit) { SetUpTest(filter_config_); filter_callbacks_.route_->route_entry_.rate_limit_policy_.rate_limit_policy_entry_.clear(); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_)); @@ -164,7 +164,7 @@ TEST_F(HttpRateLimitFilterTest, NoDescriptor) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)).Times(1); EXPECT_CALL(vh_rate_limit_, populateDescriptors(_, _, _, _, _, _)).Times(1); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_)); @@ -207,7 +207,7 @@ TEST_F(HttpRateLimitFilterTest, OkResponse) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -254,7 +254,7 @@ TEST_F(HttpRateLimitFilterTest, OkResponseWithHeaders) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -312,7 +312,7 @@ TEST_F(HttpRateLimitFilterTest, OkResponseWithFilterHeaders) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -365,7 +365,7 @@ TEST_F(HttpRateLimitFilterTest, ImmediateOkResponse) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -396,7 +396,7 @@ TEST_F(HttpRateLimitFilterTest, ImmediateErrorResponse) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::Error, nullptr, nullptr, @@ -428,7 +428,7 @@ TEST_F(HttpRateLimitFilterTest, ErrorResponse) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -462,7 +462,7 @@ TEST_F(HttpRateLimitFilterTest, ErrorResponseWithFailureModeAllowOff) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -494,7 +494,7 @@ TEST_F(HttpRateLimitFilterTest, LimitResponse) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -534,7 +534,7 @@ TEST_F(HttpRateLimitFilterTest, LimitResponseWithHeaders) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -586,7 +586,7 @@ TEST_F(HttpRateLimitFilterTest, LimitResponseWithFilterHeaders) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -638,7 +638,7 @@ TEST_F(HttpRateLimitFilterTest, LimitResponseRuntimeDisabled) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -680,7 +680,7 @@ TEST_F(HttpRateLimitFilterTest, ResetDuringCall) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -701,7 +701,7 @@ TEST_F(HttpRateLimitFilterTest, RouteRateLimitDisabledForRouteKey) { .WillByDefault(Return(false)); EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)).Times(0); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); @@ -721,7 +721,7 @@ TEST_F(HttpRateLimitFilterTest, VirtualHostRateLimitDisabledForRouteKey) { .WillByDefault(Return(false)); EXPECT_CALL(vh_rate_limit_, populateDescriptors(_, _, _, _, _, _)).Times(0); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); @@ -744,7 +744,7 @@ TEST_F(HttpRateLimitFilterTest, IncorrectRequestType) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)).Times(0); EXPECT_CALL(vh_rate_limit_, populateDescriptors(_, _, _, _, _, _)).Times(0); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_)); @@ -765,7 +765,7 @@ TEST_F(HttpRateLimitFilterTest, IncorrectRequestType) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _, _)).Times(0); EXPECT_CALL(vh_rate_limit_, populateDescriptors(_, _, _, _, _, _)).Times(0); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_)); @@ -800,7 +800,7 @@ TEST_F(HttpRateLimitFilterTest, InternalRequestType) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -846,7 +846,7 @@ TEST_F(HttpRateLimitFilterTest, ExternalRequestType) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -900,7 +900,7 @@ TEST_F(HttpRateLimitFilterTest, DEPRECATED_FEATURE_TEST(ExcludeVirtualHost)) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -951,7 +951,7 @@ TEST_F(HttpRateLimitFilterTest, OverrideVHRateLimitOptionWithRouteRateLimitSet) EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -1003,7 +1003,7 @@ TEST_F(HttpRateLimitFilterTest, OverrideVHRateLimitOptionWithoutRouteRateLimit) EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -1052,7 +1052,7 @@ TEST_F(HttpRateLimitFilterTest, IncludeVHRateLimitOptionWithOnlyVHRateLimitSet) EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{{{{"key", "value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -1103,7 +1103,7 @@ TEST_F(HttpRateLimitFilterTest, IncludeVHRateLimitOptionWithRouteAndVHRateLimitS EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{{{{"key", "value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -1151,7 +1151,7 @@ TEST_F(HttpRateLimitFilterTest, IgnoreVHRateLimitOptionWithRouteRateLimitSet) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, diff --git a/test/extensions/filters/network/ratelimit/ratelimit_test.cc b/test/extensions/filters/network/ratelimit/ratelimit_test.cc index ac64a1d6d108..f2255e356cd2 100644 --- a/test/extensions/filters/network/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/network/ratelimit/ratelimit_test.cc @@ -102,7 +102,7 @@ TEST_F(RateLimitFilterTest, OK) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"hello", "world"}, {"foo", "bar"}}}, {{{"foo2", "bar2"}}}}), - testing::A())) + testing::A(), _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -130,7 +130,7 @@ TEST_F(RateLimitFilterTest, OverLimit) { InSequence s; SetUpTest(filter_config_); - EXPECT_CALL(*client_, limit(_, "foo", _, _)) + EXPECT_CALL(*client_, limit(_, "foo", _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -156,7 +156,7 @@ TEST_F(RateLimitFilterTest, OverLimitNotEnforcing) { InSequence s; SetUpTest(filter_config_); - EXPECT_CALL(*client_, limit(_, "foo", _, _)) + EXPECT_CALL(*client_, limit(_, "foo", _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -185,7 +185,7 @@ TEST_F(RateLimitFilterTest, Error) { InSequence s; SetUpTest(filter_config_); - EXPECT_CALL(*client_, limit(_, "foo", _, _)) + EXPECT_CALL(*client_, limit(_, "foo", _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -213,7 +213,7 @@ TEST_F(RateLimitFilterTest, Disconnect) { InSequence s; SetUpTest(filter_config_); - EXPECT_CALL(*client_, limit(_, "foo", _, _)) + EXPECT_CALL(*client_, limit(_, "foo", _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -234,7 +234,7 @@ TEST_F(RateLimitFilterTest, ImmediateOK) { SetUpTest(filter_config_); EXPECT_CALL(filter_callbacks_, continueReading()).Times(0); - EXPECT_CALL(*client_, limit(_, "foo", _, _)) + EXPECT_CALL(*client_, limit(_, "foo", _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -258,7 +258,7 @@ TEST_F(RateLimitFilterTest, ImmediateError) { SetUpTest(filter_config_); EXPECT_CALL(filter_callbacks_, continueReading()).Times(0); - EXPECT_CALL(*client_, limit(_, "foo", _, _)) + EXPECT_CALL(*client_, limit(_, "foo", _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::Error, nullptr, nullptr, @@ -284,7 +284,7 @@ TEST_F(RateLimitFilterTest, RuntimeDisable) { EXPECT_CALL(runtime_.snapshot_, featureEnabled("ratelimit.tcp_filter_enabled", 100)) .WillOnce(Return(false)); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(Network::FilterStatus::Continue, filter_->onNewConnection()); Buffer::OwnedImpl data("hello"); @@ -295,7 +295,7 @@ TEST_F(RateLimitFilterTest, ErrorResponseWithFailureModeAllowOff) { InSequence s; SetUpTest(fail_close_config_); - EXPECT_CALL(*client_, limit(_, "foo", _, _)) + EXPECT_CALL(*client_, limit(_, "foo", _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; diff --git a/test/extensions/filters/network/thrift_proxy/filters/ratelimit/ratelimit_test.cc b/test/extensions/filters/network/thrift_proxy/filters/ratelimit/ratelimit_test.cc index d4862b1a378d..d0b8031938ab 100644 --- a/test/extensions/filters/network/thrift_proxy/filters/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/network/thrift_proxy/filters/ratelimit/ratelimit_test.cc @@ -176,7 +176,7 @@ TEST_F(ThriftRateLimitFilterTest, NoApplicableRateLimit) { setupTest(filter_config_); filter_callbacks_.route_->route_entry_.rate_limit_policy_.rate_limit_policy_entry_.clear(); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(ThriftProxy::FilterStatus::Continue, filter_->messageBegin(request_metadata_)); } @@ -185,7 +185,7 @@ TEST_F(ThriftRateLimitFilterTest, NoDescriptor) { setupTest(filter_config_); EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)).Times(1); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(ThriftProxy::FilterStatus::Continue, filter_->messageBegin(request_metadata_)); } @@ -212,7 +212,7 @@ TEST_F(ThriftRateLimitFilterTest, OkResponse) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -243,7 +243,7 @@ TEST_F(ThriftRateLimitFilterTest, ImmediateOkResponse) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, @@ -267,7 +267,7 @@ TEST_F(ThriftRateLimitFilterTest, ImmediateErrorResponse) { EXPECT_CALL(*client_, limit(_, "foo", testing::ContainerEq(std::vector{ {{{"descriptor_key", "descriptor_value"}}}}), - _)) + _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { callbacks.complete(Filters::Common::RateLimit::LimitStatus::Error, nullptr, nullptr, @@ -291,7 +291,7 @@ TEST_F(ThriftRateLimitFilterTest, ErrorResponse) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -322,7 +322,7 @@ TEST_F(ThriftRateLimitFilterTest, ErrorResponseWithFailureModeAllowOff) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -355,7 +355,7 @@ TEST_F(ThriftRateLimitFilterTest, LimitResponse) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -386,7 +386,7 @@ TEST_F(ThriftRateLimitFilterTest, LimitResponseWithHeaders) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -419,7 +419,7 @@ TEST_F(ThriftRateLimitFilterTest, LimitResponseRuntimeDisabled) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -444,7 +444,7 @@ TEST_F(ThriftRateLimitFilterTest, ResetDuringCall) { EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)) .WillOnce(SetArgReferee<1>(descriptor_)); - EXPECT_CALL(*client_, limit(_, _, _, _)) + EXPECT_CALL(*client_, limit(_, _, _, _, _)) .WillOnce( WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { request_callbacks_ = &callbacks; @@ -464,7 +464,7 @@ TEST_F(ThriftRateLimitFilterTest, RouteRateLimitDisabledForRouteKey) { .WillByDefault(Return(false)); EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _, _)).Times(0); - EXPECT_CALL(*client_, limit(_, _, _, _)).Times(0); + EXPECT_CALL(*client_, limit(_, _, _, _, _)).Times(0); EXPECT_EQ(ThriftProxy::FilterStatus::Continue, filter_->messageBegin(request_metadata_)); }