Skip to content

Commit

Permalink
Stringmatcher: factory context for aws signer (envoyproxy#32965)
Browse files Browse the repository at this point in the history
Signed-off-by: Greg Greenway <[email protected]>
Signed-off-by: Antonio Leonti <[email protected]>
  • Loading branch information
ggreenway authored and antoniovleonti committed Mar 20, 2024
1 parent fd17153 commit e7e46dd
Show file tree
Hide file tree
Showing 44 changed files with 227 additions and 185 deletions.
4 changes: 2 additions & 2 deletions envoy/grpc/google_grpc_creds.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

#include <memory>

#include "envoy/api/api.h"
#include "envoy/common/pure.h"
#include "envoy/config/core/v3/grpc_service.pb.h"
#include "envoy/config/typed_config.h"
#include "envoy/server/factory_context.h"

#include "grpcpp/grpcpp.h"

Expand Down Expand Up @@ -33,7 +33,7 @@ class GoogleGrpcCredentialsFactory : public Config::UntypedFactory {
*/
virtual std::shared_ptr<grpc::ChannelCredentials>
getChannelCredentials(const envoy::config::core::v3::GrpcService& grpc_service_config,
Api::Api& api) PURE;
Server::Configuration::CommonFactoryContext& context) PURE;

std::string category() const override { return "envoy.grpc_credentials"; }
};
Expand Down
26 changes: 13 additions & 13 deletions source/common/grpc/async_client_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ AsyncClientFactoryImpl::AsyncClientFactoryImpl(Upstream::ClusterManager& cm,
}

AsyncClientManagerImpl::AsyncClientManagerImpl(
Upstream::ClusterManager& cm, ThreadLocal::Instance& tls, TimeSource& time_source,
Api::Api& api, const StatNames& stat_names,
Upstream::ClusterManager& cm, ThreadLocal::Instance& tls,
Server::Configuration::CommonFactoryContext& context, const StatNames& stat_names,
const envoy::config::bootstrap::v3::Bootstrap::GrpcAsyncClientManagerConfig& config)
: cm_(cm), tls_(tls), time_source_(time_source), api_(api), stat_names_(stat_names),
raw_async_client_cache_(tls_) {
: tls_(tls), cm_(cm), context_(context), stat_names_(stat_names),
raw_async_client_cache_(context.threadLocal()) {

const auto max_cached_entry_idle_duration = std::chrono::milliseconds(
PROTOBUF_GET_MS_OR_DEFAULT(config, max_cached_entry_idle_duration, DefaultEntryIdleDuration));
Expand All @@ -69,11 +69,10 @@ AsyncClientManagerImpl::AsyncClientManagerImpl(
return std::make_shared<RawAsyncClientCache>(dispatcher, max_cached_entry_idle_duration);
});
#ifdef ENVOY_GOOGLE_GRPC
google_tls_slot_ = tls.allocateSlot();
google_tls_slot_ = context_.threadLocal().allocateSlot();
Api::Api& api = context_.api();
google_tls_slot_->set(
[&api](Event::Dispatcher&) { return std::make_shared<GoogleAsyncClientThreadLocal>(api); });
#else
UNREFERENCED_PARAMETER(api_);
#endif
}

Expand All @@ -83,17 +82,18 @@ RawAsyncClientPtr AsyncClientFactoryImpl::createUncachedRawAsyncClient() {

GoogleAsyncClientFactoryImpl::GoogleAsyncClientFactoryImpl(
ThreadLocal::Instance& tls, ThreadLocal::Slot* google_tls_slot, Stats::Scope& scope,
const envoy::config::core::v3::GrpcService& config, Api::Api& api, const StatNames& stat_names,
const envoy::config::core::v3::GrpcService& config,
Server::Configuration::CommonFactoryContext& context, const StatNames& stat_names,
absl::Status& creation_status)
: tls_(tls), google_tls_slot_(google_tls_slot),
scope_(scope.createScope(fmt::format("grpc.{}.", config.google_grpc().stat_prefix()))),
config_(config), api_(api), stat_names_(stat_names) {
config_(config), factory_context_(context), stat_names_(stat_names) {
#ifndef ENVOY_GOOGLE_GRPC
UNREFERENCED_PARAMETER(tls_);
UNREFERENCED_PARAMETER(google_tls_slot_);
UNREFERENCED_PARAMETER(scope_);
UNREFERENCED_PARAMETER(config_);
UNREFERENCED_PARAMETER(api_);
UNREFERENCED_PARAMETER(factory_context_);
UNREFERENCED_PARAMETER(stat_names_);
creation_status = absl::InvalidArgumentError("Google C++ gRPC client is not linked");
return;
Expand Down Expand Up @@ -128,7 +128,7 @@ RawAsyncClientPtr GoogleAsyncClientFactoryImpl::createUncachedRawAsyncClient() {
GoogleGenericStubFactory stub_factory;
return std::make_unique<GoogleAsyncClientImpl>(
tls_.dispatcher(), google_tls_slot_->getTyped<GoogleAsyncClientThreadLocal>(), stub_factory,
scope_, config_, api_, stat_names_);
scope_, config_, factory_context_, stat_names_);
#else
return nullptr;
#endif
Expand All @@ -142,11 +142,11 @@ AsyncClientManagerImpl::factoryForGrpcService(const envoy::config::core::v3::Grp
switch (config.target_specifier_case()) {
case envoy::config::core::v3::GrpcService::TargetSpecifierCase::kEnvoyGrpc:
factory = std::make_unique<AsyncClientFactoryImpl>(cm_, config, skip_cluster_check,
time_source_, creation_status);
context_.timeSource(), creation_status);
break;
case envoy::config::core::v3::GrpcService::TargetSpecifierCase::kGoogleGrpc:
factory = std::make_unique<GoogleAsyncClientFactoryImpl>(
tls_, google_tls_slot_.get(), scope, config, api_, stat_names_, creation_status);
tls_, google_tls_slot_.get(), scope, config, context_, stat_names_, creation_status);
break;
case envoy::config::core::v3::GrpcService::TargetSpecifierCase::TARGET_SPECIFIER_NOT_SET:
PANIC_DUE_TO_PROTO_UNSET;
Expand Down
14 changes: 7 additions & 7 deletions source/common/grpc/async_client_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ class GoogleAsyncClientFactoryImpl : public AsyncClientFactory {
public:
GoogleAsyncClientFactoryImpl(ThreadLocal::Instance& tls, ThreadLocal::Slot* google_tls_slot,
Stats::Scope& scope,
const envoy::config::core::v3::GrpcService& config, Api::Api& api,
const envoy::config::core::v3::GrpcService& config,
Server::Configuration::CommonFactoryContext& context,
const StatNames& stat_names, absl::Status& creation_status);
RawAsyncClientPtr createUncachedRawAsyncClient() override;

Expand All @@ -42,15 +43,15 @@ class GoogleAsyncClientFactoryImpl : public AsyncClientFactory {
ThreadLocal::Slot* google_tls_slot_;
Stats::ScopeSharedPtr scope_;
const envoy::config::core::v3::GrpcService config_;
Api::Api& api_;
Server::Configuration::CommonFactoryContext& factory_context_;
const StatNames& stat_names_;
};

class AsyncClientManagerImpl : public AsyncClientManager {
public:
AsyncClientManagerImpl(
Upstream::ClusterManager& cm, ThreadLocal::Instance& tls, TimeSource& time_source,
Api::Api& api, const StatNames& stat_names,
Upstream::ClusterManager& cm, ThreadLocal::Instance& tls,
Server::Configuration::CommonFactoryContext& context, const StatNames& stat_names,
const envoy::config::bootstrap::v3::Bootstrap::GrpcAsyncClientManagerConfig& config);
absl::StatusOr<RawAsyncClientSharedPtr>
getOrCreateRawAsyncClient(const envoy::config::core::v3::GrpcService& config, Stats::Scope& scope,
Expand Down Expand Up @@ -92,11 +93,10 @@ class AsyncClientManagerImpl : public AsyncClientManager {
};

private:
Upstream::ClusterManager& cm_;
ThreadLocal::Instance& tls_;
Upstream::ClusterManager& cm_; // Need to track outside of `context_` due to startup ordering.
Server::Configuration::CommonFactoryContext& context_;
ThreadLocal::SlotPtr google_tls_slot_;
TimeSource& time_source_;
Api::Api& api_;
const StatNames& stat_names_;
ThreadLocal::TypedSlot<RawAsyncClientCache> raw_async_client_cache_;
};
Expand Down
5 changes: 3 additions & 2 deletions source/common/grpc/google_async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ GoogleAsyncClientImpl::GoogleAsyncClientImpl(Event::Dispatcher& dispatcher,
GoogleStubFactory& stub_factory,
Stats::ScopeSharedPtr scope,
const envoy::config::core::v3::GrpcService& config,
Api::Api& api, const StatNames& stat_names)
Server::Configuration::CommonFactoryContext& context,
const StatNames& stat_names)
: dispatcher_(dispatcher), tls_(tls), stat_prefix_(config.google_grpc().stat_prefix()),
target_uri_(config.google_grpc().target_uri()), scope_(scope),
per_stream_buffer_limit_bytes_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(
Expand All @@ -94,7 +95,7 @@ GoogleAsyncClientImpl::GoogleAsyncClientImpl(Event::Dispatcher& dispatcher,
// 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
// new connection implied.
std::shared_ptr<grpc::Channel> channel = GoogleGrpcUtils::createChannel(config, api);
std::shared_ptr<grpc::Channel> channel = GoogleGrpcUtils::createChannel(config, context);
// Get state with try_to_connect = true to try connection at channel creation.
// This is for initializing gRPC channel at channel creation. This GetState(true) is used to poke
// the gRPC lb at channel creation, it doesn't have any effect no matter it succeeds or fails. But
Expand Down
3 changes: 2 additions & 1 deletion source/common/grpc/google_async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ class GoogleAsyncClientImpl final : public RawAsyncClient, Logger::Loggable<Logg
public:
GoogleAsyncClientImpl(Event::Dispatcher& dispatcher, GoogleAsyncClientThreadLocal& tls,
GoogleStubFactory& stub_factory, Stats::ScopeSharedPtr scope,
const envoy::config::core::v3::GrpcService& config, Api::Api& api,
const envoy::config::core::v3::GrpcService& config,
Server::Configuration::CommonFactoryContext& context,
const StatNames& stat_names);
~GoogleAsyncClientImpl() override;

Expand Down
4 changes: 2 additions & 2 deletions source/common/grpc/google_grpc_creds_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ class DefaultGoogleGrpcCredentialsFactory : public GoogleGrpcCredentialsFactory
public:
std::shared_ptr<grpc::ChannelCredentials>
getChannelCredentials(const envoy::config::core::v3::GrpcService& grpc_service_config,
Api::Api& api) override {
return CredsUtility::defaultChannelCredentials(grpc_service_config, api);
Server::Configuration::CommonFactoryContext& context) override {
return CredsUtility::defaultChannelCredentials(grpc_service_config, context.api());
}

std::string name() const override { return "envoy.grpc_credentials.default"; }
Expand Down
10 changes: 6 additions & 4 deletions source/common/grpc/google_grpc_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace {

std::shared_ptr<grpc::ChannelCredentials>
getGoogleGrpcChannelCredentials(const envoy::config::core::v3::GrpcService& grpc_service,
Api::Api& api) {
Server::Configuration::CommonFactoryContext& context) {
GoogleGrpcCredentialsFactory* credentials_factory = nullptr;
const std::string& google_grpc_credentials_factory_name =
grpc_service.google_grpc().credentials_factory_name();
Expand All @@ -41,7 +41,7 @@ getGoogleGrpcChannelCredentials(const envoy::config::core::v3::GrpcService& grpc
throw EnvoyException(absl::StrCat("Unknown google grpc credentials factory: ",
google_grpc_credentials_factory_name));
}
return credentials_factory->getChannelCredentials(grpc_service, api);
return credentials_factory->getChannelCredentials(grpc_service, context);
}

} // namespace
Expand Down Expand Up @@ -133,8 +133,10 @@ GoogleGrpcUtils::channelArgsFromConfig(const envoy::config::core::v3::GrpcServic
}

std::shared_ptr<grpc::Channel>
GoogleGrpcUtils::createChannel(const envoy::config::core::v3::GrpcService& config, Api::Api& api) {
std::shared_ptr<grpc::ChannelCredentials> creds = getGoogleGrpcChannelCredentials(config, api);
GoogleGrpcUtils::createChannel(const envoy::config::core::v3::GrpcService& config,
Server::Configuration::CommonFactoryContext& context) {
std::shared_ptr<grpc::ChannelCredentials> creds =
getGoogleGrpcChannelCredentials(config, context);
const grpc::ChannelArguments args = channelArgsFromConfig(config);
return CreateCustomChannel(config.google_grpc().target_uri(), creds, args);
}
Expand Down
5 changes: 3 additions & 2 deletions source/common/grpc/google_grpc_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
#include <cstdint>
#include <string>

#include "envoy/api/api.h"
#include "envoy/buffer/buffer.h"
#include "envoy/common/platform.h"
#include "envoy/config/core/v3/grpc_service.pb.h"
#include "envoy/server/factory_context.h"

#include "grpcpp/grpcpp.h"

Expand Down Expand Up @@ -46,7 +46,8 @@ class GoogleGrpcUtils {
* @return static std::shared_ptr<grpc::Channel> a gRPC channel.
*/
static std::shared_ptr<grpc::Channel>
createChannel(const envoy::config::core::v3::GrpcService& config, Api::Api& api);
createChannel(const envoy::config::core::v3::GrpcService& config,
Server::Configuration::CommonFactoryContext& context);
};

} // namespace Grpc
Expand Down
17 changes: 8 additions & 9 deletions source/common/upstream/cluster_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -295,12 +295,12 @@ void ClusterManagerInitHelper::setPrimaryClustersInitializedCb(

ClusterManagerImpl::ClusterManagerImpl(
const envoy::config::bootstrap::v3::Bootstrap& bootstrap, ClusterManagerFactory& factory,
Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime,
const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager,
Event::Dispatcher& main_thread_dispatcher, OptRef<Server::Admin> admin,
ProtobufMessage::ValidationContext& validation_context, Api::Api& api,
Http::Context& http_context, Grpc::Context& grpc_context, Router::Context& router_context,
const Server::Instance& server)
Server::Configuration::CommonFactoryContext& context, Stats::Store& stats,
ThreadLocal::Instance& tls, Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info,
AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher,
OptRef<Server::Admin> admin, ProtobufMessage::ValidationContext& validation_context,
Api::Api& api, Http::Context& http_context, Grpc::Context& grpc_context,
Router::Context& router_context, const Server::Instance& server)
: server_(server), factory_(factory), runtime_(runtime), stats_(stats), tls_(tls),
random_(api.randomGenerator()),
deferred_cluster_creation_(bootstrap.cluster_manager().enable_deferred_cluster_creation()),
Expand Down Expand Up @@ -331,8 +331,7 @@ ClusterManagerImpl::ClusterManagerImpl(
});
}
async_client_manager_ = std::make_unique<Grpc::AsyncClientManagerImpl>(
*this, tls, time_source_, api, grpc_context.statNames(),
bootstrap.grpc_async_client_manager_config());
*this, tls, context, grpc_context.statNames(), bootstrap.grpc_async_client_manager_config());
const auto& cm_config = bootstrap.cluster_manager();
if (cm_config.has_outlier_detection()) {
const std::string event_log_file_path = cm_config.outlier_detection().event_log_path();
Expand Down Expand Up @@ -2222,7 +2221,7 @@ void ClusterManagerImpl::ThreadLocalClusterManagerImpl::tcpConnPoolIsIdle(
ClusterManagerPtr ProdClusterManagerFactory::clusterManagerFromProto(
const envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
auto cluster_manager_impl = std::unique_ptr<ClusterManagerImpl>{new ClusterManagerImpl(
bootstrap, *this, stats_, tls_, context_.runtime(), context_.localInfo(),
bootstrap, *this, context_, stats_, tls_, context_.runtime(), context_.localInfo(),
context_.accessLogManager(), context_.mainThreadDispatcher(), context_.admin(),
context_.messageValidationContext(), context_.api(), http_context_, context_.grpcContext(),
context_.routerContext(), server_)};
Expand Down
3 changes: 2 additions & 1 deletion source/common/upstream/cluster_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,8 @@ class ClusterManagerImpl : public ClusterManager,
// ClusterManagerImpl's constructor should not be invoked directly; create instances from the
// clusterManagerFromProto() static method. The init() method must be called after construction.
ClusterManagerImpl(const envoy::config::bootstrap::v3::Bootstrap& bootstrap,
ClusterManagerFactory& factory, Stats::Store& stats,
ClusterManagerFactory& factory,
Server::Configuration::CommonFactoryContext& context, Stats::Store& stats,
ThreadLocal::Instance& tls, Runtime::Loader& runtime,
const LocalInfo::LocalInfo& local_info,
AccessLog::AccessLogManager& log_manager,
Expand Down
24 changes: 15 additions & 9 deletions source/extensions/common/aws/signer_base_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,22 @@ using AwsSigningHeaderExclusionVector = std::vector<envoy::type::matcher::v3::St
class SignerBaseImpl : public Signer, public Logger::Loggable<Logger::Id::aws> {
public:
SignerBaseImpl(absl::string_view service_name, absl::string_view region,
const CredentialsProviderSharedPtr& credentials_provider, TimeSource& time_source,
const CredentialsProviderSharedPtr& credentials_provider,
Server::Configuration::CommonFactoryContext& context,
const AwsSigningHeaderExclusionVector& matcher_config,
const bool query_string = false,
const uint16_t expiration_time = SignatureQueryParameterValues::DefaultExpiration)
: service_name_(service_name), region_(region), credentials_provider_(credentials_provider),
query_string_(query_string), expiration_time_(expiration_time), time_source_(time_source),
: service_name_(service_name), region_(region),
excluded_header_matchers_(defaultMatchers(context)),
credentials_provider_(credentials_provider), query_string_(query_string),
expiration_time_(expiration_time), time_source_(context.timeSource()),
long_date_formatter_(std::string(SignatureConstants::LongDateFormat)),
short_date_formatter_(std::string(SignatureConstants::ShortDateFormat)) {
for (const auto& matcher : matcher_config) {
excluded_header_matchers_.emplace_back(
std::make_unique<Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>>(
matcher));
std::make_unique<
Matchers::StringMatcherImplWithContext<envoy::type::matcher::v3::StringMatcher>>(
matcher, context));
}
}

Expand Down Expand Up @@ -128,14 +132,16 @@ class SignerBaseImpl : public Signer, public Logger::Loggable<Logger::Id::aws> {
const std::map<std::string, std::string>& signed_headers,
const uint16_t expiration_time) const;

std::vector<Matchers::StringMatcherPtr> defaultMatchers() const {
std::vector<Matchers::StringMatcherPtr>
defaultMatchers(Server::Configuration::CommonFactoryContext& context) const {
std::vector<Matchers::StringMatcherPtr> matcher_ptrs{};
for (const auto& header : default_excluded_headers_) {
envoy::type::matcher::v3::StringMatcher m;
m.set_exact(header);
matcher_ptrs.emplace_back(
std::make_unique<Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>>(
m));
std::make_unique<
Matchers::StringMatcherImplWithContext<envoy::type::matcher::v3::StringMatcher>>(
m, context));
}
return matcher_ptrs;
}
Expand All @@ -145,7 +151,7 @@ class SignerBaseImpl : public Signer, public Logger::Loggable<Logger::Id::aws> {
const std::vector<std::string> default_excluded_headers_ = {
Http::Headers::get().ForwardedFor.get(), Http::Headers::get().ForwardedProto.get(),
"x-amzn-trace-id"};
std::vector<Matchers::StringMatcherPtr> excluded_header_matchers_ = defaultMatchers();
std::vector<Matchers::StringMatcherPtr> excluded_header_matchers_;
CredentialsProviderSharedPtr credentials_provider_;
const bool query_string_;
const uint16_t expiration_time_;
Expand Down
5 changes: 3 additions & 2 deletions source/extensions/common/aws/sigv4_signer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ using AwsSigningHeaderExclusionVector = std::vector<envoy::type::matcher::v3::St
class SigV4SignerImpl : public SignerBaseImpl {
public:
SigV4SignerImpl(absl::string_view service_name, absl::string_view region,
const CredentialsProviderSharedPtr& credentials_provider, TimeSource& time_source,
const CredentialsProviderSharedPtr& credentials_provider,
Server::Configuration::CommonFactoryContext& context,
const AwsSigningHeaderExclusionVector& matcher_config,
const bool query_string = false,
const uint16_t expiration_time = SignatureQueryParameterValues::DefaultExpiration)
: SignerBaseImpl(service_name, region, credentials_provider, time_source, matcher_config,
: SignerBaseImpl(service_name, region, credentials_provider, context, matcher_config,
query_string, expiration_time) {}

private:
Expand Down
Loading

0 comments on commit e7e46dd

Please sign in to comment.