Skip to content

Listener ECDS: Adding config provider manager implemantion for listener filter #20560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 27 commits into from
May 2, 2022
Merged

Listener ECDS: Adding config provider manager implemantion for listener filter #20560

Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
42591c4
Listener ECDS: Adding FilterConfigProviderManagerImpl class implemant…
yanjunxiang-google Mar 28, 2022
c97c715
fixing format error
yanjunxiang-google Mar 28, 2022
c473f29
switching to use tls_inspector listener filter in the test
yanjunxiang-google Mar 29, 2022
9b50205
addressing comments
yanjunxiang-google Mar 29, 2022
7829692
fixing test code
yanjunxiang-google Mar 30, 2022
5df2374
Merge branch 'main' of https://github.com/envoyproxy/envoy into ecds_…
yanjunxiang-google Apr 1, 2022
e7a217b
adding UDP listener filter support also refactoring ECDS tests
yanjunxiang-google Apr 3, 2022
39f434b
fixing format
yanjunxiang-google Apr 3, 2022
fe77b12
removing unused validate.proto
yanjunxiang-google Apr 3, 2022
4fd9312
fixing CI error
yanjunxiang-google Apr 3, 2022
cea0a65
abstract the FilterConfigProviderManagerImpl class
yanjunxiang-google Apr 7, 2022
817c9dd
fixing CI error
yanjunxiang-google Apr 8, 2022
4266bea
using string_view reference instead of enum as non-typed template pa…
yanjunxiang-google Apr 8, 2022
35cca65
address comments: adding typed test
yanjunxiang-google Apr 13, 2022
026f16e
fix CI error
yanjunxiang-google Apr 13, 2022
ca4c8a7
fixing CI GCC anonymous namespace error
yanjunxiang-google Apr 13, 2022
46e45c7
fixing clang tidy issue
yanjunxiang-google Apr 13, 2022
56fb733
addressing comments, moving statPrefix out of the template parameters
yanjunxiang-google Apr 14, 2022
1a8dd74
addressing comments
yanjunxiang-google Apr 14, 2022
99a590a
clean up test code
yanjunxiang-google Apr 21, 2022
7dfbd75
fix format
yanjunxiang-google Apr 21, 2022
8931395
addressing comments
yanjunxiang-google Apr 24, 2022
743de2a
adding a TODO comments
yanjunxiang-google Apr 24, 2022
ad043b8
fixing CI error
yanjunxiang-google Apr 24, 2022
105153a
reduce duplicated code
yanjunxiang-google Apr 24, 2022
eeff8c0
addressing comments
yanjunxiang-google Apr 25, 2022
2de5f43
addressing comments
yanjunxiang-google Apr 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions envoy/filter/config_provider_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ using DynamicFilterConfigProviderPtr = std::unique_ptr<DynamicFilterConfigProvid
* The FilterConfigProviderManager exposes the ability to get an FilterConfigProvider
* for both static and dynamic filter config providers.
*/
template <class FactoryCb> class FilterConfigProviderManager {
template <class FactoryCb, class FactoryCtx> class FilterConfigProviderManager {
public:
virtual ~FilterConfigProviderManager() = default;

Expand All @@ -40,7 +40,7 @@ template <class FactoryCb> class FilterConfigProviderManager {
*/
virtual DynamicFilterConfigProviderPtr<FactoryCb> createDynamicFilterConfigProvider(
const envoy::config::core::v3::ExtensionConfigSource& config_source,
const std::string& filter_config_name, Server::Configuration::FactoryContext& factory_context,
const std::string& filter_config_name, FactoryCtx& factory_context,
const std::string& stat_prefix, bool last_filter_in_filter_chain,
const std::string& filter_chain_type) PURE;

Expand Down
44 changes: 10 additions & 34 deletions source/common/filter/config_discovery_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
namespace Envoy {
namespace Filter {

constexpr absl::string_view HttpStatPrefix = "http_filter.";

namespace {
void validateTypeUrlHelper(const std::string& type_url,
const absl::flat_hash_set<std::string> require_type_urls) {
Expand Down Expand Up @@ -232,41 +230,19 @@ void FilterConfigProviderManagerImplBase::applyLastOrDefaultConfig(
}
}

std::tuple<ProtobufTypes::MessagePtr, std::string> HttpFilterConfigProviderManagerImpl::getMessage(
const envoy::config::core::v3::TypedExtensionConfig& filter_config,
Server::Configuration::ServerFactoryContext& factory_context) const {
auto& factory =
Config::Utility::getAndCheckFactory<Server::Configuration::NamedHttpFilterConfigFactory>(
filter_config);
ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig(
filter_config.typed_config(),
factory_context.messageValidationContext().dynamicValidationVisitor(), factory);
return {std::move(message), factory.name()};
}

absl::string_view HttpFilterConfigProviderManagerImpl::statPrefix() const { return HttpStatPrefix; }

ProtobufTypes::MessagePtr HttpFilterConfigProviderManagerImpl::getDefaultConfig(
const ProtobufWkt::Any& proto_config, const std::string& filter_config_name,
Server::Configuration::FactoryContext& factory_context, bool last_filter_in_filter_chain,
const std::string& filter_chain_type,
const absl::flat_hash_set<std::string>& require_type_urls) const {
auto* default_factory =
Config::Utility::getFactoryByType<Server::Configuration::NamedHttpFilterConfigFactory>(
proto_config);
if (default_factory == nullptr) {
void FilterConfigProviderManagerImplBase::validateProtoConfigDefaultFactory(

@adisuissa adisuissa May 2, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIUC both this method and validateProtoConfigTypeUrl are only called once.
Is there a plan to call them in other places in later PRs? If not, then we should just inline their contents in the call place.

@yanjunxiang-google yanjunxiang-google May 2, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comments.

This method throw exception. Envoy CI scripts doesn't allow throw exception in .h files.

const bool null_default_factory, const std::string& filter_config_name,
const std::string& type_url) const {
if (null_default_factory) {
throw EnvoyException(fmt::format("Error: cannot find filter factory {} for default filter "
"configuration with type URL {}.",
filter_config_name, proto_config.type_url()));
filter_config_name, type_url));
}
validateTypeUrlHelper(Config::Utility::getFactoryType(proto_config), require_type_urls);
ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig(
proto_config, factory_context.messageValidationVisitor(), *default_factory);
Config::Utility::validateTerminalFilters(
filter_config_name, default_factory->name(), filter_chain_type,
default_factory->isTerminalFilterByProto(*message, factory_context),
last_filter_in_filter_chain);
return message;
}

void FilterConfigProviderManagerImplBase::validateProtoConfigTypeUrl(
const std::string& type_url, const absl::flat_hash_set<std::string>& require_type_urls) const {
validateTypeUrlHelper(type_url, require_type_urls);
}

} // namespace Filter
Expand Down
196 changes: 150 additions & 46 deletions source/common/filter/config_discovery_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa
absl::string_view stat_prefix)
: DynamicFilterConfigProviderImplBase(subscription, require_type_urls,
last_filter_in_filter_chain, filter_chain_type),
stat_prefix_(stat_prefix), factory_context_(factory_context),
main_config_(std::make_shared<MainConfig>()),
stat_prefix_(stat_prefix), main_config_(std::make_shared<MainConfig>()),
default_configuration_(std::move(default_config)), tls_(factory_context.threadLocal()) {
tls_.set([](Event::Dispatcher&) { return std::make_shared<ThreadLocalConfig>(); });
};
Expand Down Expand Up @@ -119,7 +118,6 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa

protected:
const std::string& getStatPrefix() const { return stat_prefix_; }
Server::Configuration::FactoryContext& getFactoryContext() const { return factory_context_; }

private:
virtual FactoryCb instantiateFilterFactory(const Protobuf::Message& message) const PURE;
Expand Down Expand Up @@ -151,7 +149,6 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa
};

const std::string stat_prefix_;
Server::Configuration::FactoryContext& factory_context_;
std::shared_ptr<MainConfig> main_config_;
const ProtobufTypes::MessagePtr default_configuration_;
ThreadLocal::TypedSlot<ThreadLocalConfig> tls_;
Expand All @@ -161,13 +158,23 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa
class HttpDynamicFilterConfigProviderImpl
: public DynamicFilterConfigProviderImpl<Http::FilterFactoryCb> {
public:
using DynamicFilterConfigProviderImpl::DynamicFilterConfigProviderImpl;
HttpDynamicFilterConfigProviderImpl(FilterConfigSubscriptionSharedPtr& subscription,
const absl::flat_hash_set<std::string>& require_type_urls,
Server::Configuration::FactoryContext& factory_context,
ProtobufTypes::MessagePtr&& default_config,
bool last_filter_in_filter_chain,
const std::string& filter_chain_type,
absl::string_view stat_prefix)
: DynamicFilterConfigProviderImpl(subscription, require_type_urls, factory_context,
std::move(default_config), last_filter_in_filter_chain,
filter_chain_type, stat_prefix),
factory_context_(factory_context) {}
void validateMessage(const std::string& config_name, const Protobuf::Message& message,
const std::string& factory_name) const override {
auto* factory =
Registry::FactoryRegistry<Server::Configuration::NamedHttpFilterConfigFactory>::getFactory(
factory_name);
const bool is_terminal_filter = factory->isTerminalFilterByProto(message, getFactoryContext());
const bool is_terminal_filter = factory->isTerminalFilterByProto(message, factory_context_);
Config::Utility::validateTerminalFilters(config_name, factory_name, filter_chain_type_,
is_terminal_filter, last_filter_in_filter_chain_);
}
Expand All @@ -176,7 +183,62 @@ class HttpDynamicFilterConfigProviderImpl
Http::FilterFactoryCb instantiateFilterFactory(const Protobuf::Message& message) const override {
auto* factory = Registry::FactoryRegistry<Server::Configuration::NamedHttpFilterConfigFactory>::
getFactoryByType(message.GetTypeName());
return factory->createFilterFactoryFromProto(message, getStatPrefix(), getFactoryContext());
return factory->createFilterFactoryFromProto(message, getStatPrefix(), factory_context_);
}

Server::Configuration::FactoryContext& factory_context_;
};

// Implementation of a listener dynamic filter config provider.
template <class FactoryCb>
class ListenerDynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImpl<FactoryCb> {
public:
ListenerDynamicFilterConfigProviderImpl(
FilterConfigSubscriptionSharedPtr& subscription,
const absl::flat_hash_set<std::string>& require_type_urls,
Server::Configuration::ListenerFactoryContext& factory_context,
ProtobufTypes::MessagePtr&& default_config, bool last_filter_in_filter_chain,
const std::string& filter_chain_type, absl::string_view stat_prefix)
: DynamicFilterConfigProviderImpl<FactoryCb>(
subscription, require_type_urls, factory_context, std::move(default_config),
last_filter_in_filter_chain, filter_chain_type, stat_prefix),
factory_context_(factory_context) {}

void validateMessage(const std::string&, const Protobuf::Message&,
const std::string&) const override {}

protected:
Server::Configuration::ListenerFactoryContext& factory_context_;
};

class TcpListenerDynamicFilterConfigProviderImpl
: public ListenerDynamicFilterConfigProviderImpl<Network::ListenerFilterFactoryCb> {
public:
using ListenerDynamicFilterConfigProviderImpl::ListenerDynamicFilterConfigProviderImpl;

private:
Network::ListenerFilterFactoryCb
instantiateFilterFactory(const Protobuf::Message& message) const override {
auto* factory =
Registry::FactoryRegistry<Server::Configuration::NamedListenerFilterConfigFactory>::
getFactoryByType(message.GetTypeName());
// TODO(yanjunxiang): Change nullptr to actual listener filter matcher.
return factory->createListenerFilterFactoryFromProto(message, nullptr, factory_context_);
Comment thread
yanjunxiang-google marked this conversation as resolved.
}
};

class UdpListenerDynamicFilterConfigProviderImpl
: public ListenerDynamicFilterConfigProviderImpl<Network::UdpListenerFilterFactoryCb> {
public:
using ListenerDynamicFilterConfigProviderImpl::ListenerDynamicFilterConfigProviderImpl;

private:
Network::UdpListenerFilterFactoryCb
instantiateFilterFactory(const Protobuf::Message& message) const override {
auto* factory =
Registry::FactoryRegistry<Server::Configuration::NamedUdpListenerFilterConfigFactory>::
getFactoryByType(message.GetTypeName());
return factory->createFilterFactoryFromProto(message, factory_context_);
}
};

Expand Down Expand Up @@ -295,6 +357,11 @@ class FilterConfigProviderManagerImplBase : Logger::Loggable<Logger::Id::filter>
void applyLastOrDefaultConfig(std::shared_ptr<FilterConfigSubscription>& subscription,
DynamicFilterConfigProviderImplBase& provider,
const std::string& filter_config_name);
void validateProtoConfigDefaultFactory(const bool null_default_factory,
const std::string& filter_config_name,
const std::string& type_url) const;
void validateProtoConfigTypeUrl(const std::string& type_url,
const absl::flat_hash_set<std::string>& require_type_urls) const;

private:
absl::flat_hash_map<std::string, std::weak_ptr<FilterConfigSubscription>> subscriptions_;
Expand All @@ -304,14 +371,14 @@ class FilterConfigProviderManagerImplBase : Logger::Loggable<Logger::Id::filter>
/**
* An implementation of FilterConfigProviderManager.
*/
template <class FactoryCb>
template <class Factory, class FactoryCb, class FactoryCtx, class DynamicFilterConfigImpl>
class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBase,
public FilterConfigProviderManager<FactoryCb>,
public FilterConfigProviderManager<FactoryCb, FactoryCtx>,
public Singleton::Instance {
public:
DynamicFilterConfigProviderPtr<FactoryCb> createDynamicFilterConfigProvider(
const envoy::config::core::v3::ExtensionConfigSource& config_source,
const std::string& filter_config_name, Server::Configuration::FactoryContext& factory_context,
const std::string& filter_config_name, FactoryCtx& factory_context,
const std::string& stat_prefix, bool last_filter_in_filter_chain,
const std::string& filter_chain_type) override {
std::string subscription_stat_prefix;
Expand Down Expand Up @@ -367,54 +434,91 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa

absl::string_view statPrefix() const override PURE;

protected:
virtual ProtobufTypes::MessagePtr
getDefaultConfig(const ProtobufWkt::Any& proto_config, const std::string& filter_config_name,
Server::Configuration::FactoryContext& factory_context,
bool last_filter_in_filter_chain, const std::string& filter_chain_type,
const absl::flat_hash_set<std::string>& require_type_urls) const PURE;

private:
virtual std::unique_ptr<DynamicFilterConfigProviderImpl<FactoryCb>>
createFilterConfigProviderImpl(FilterConfigSubscriptionSharedPtr& subscription,
const absl::flat_hash_set<std::string>& require_type_urls,
Server::Configuration::FactoryContext& factory_context,
ProtobufTypes::MessagePtr&& default_config,
bool last_filter_in_filter_chain,
const std::string& filter_chain_type,
absl::string_view stat_prefix) PURE;
};

class HttpFilterConfigProviderManagerImpl
: public FilterConfigProviderManagerImpl<Http::FilterFactoryCb> {
public:
std::tuple<ProtobufTypes::MessagePtr, std::string>
getMessage(const envoy::config::core::v3::TypedExtensionConfig& filter_config,
Server::Configuration::ServerFactoryContext& factory_context) const override;

absl::string_view statPrefix() const override;
Server::Configuration::ServerFactoryContext& factory_context) const override {
auto& factory = Config::Utility::getAndCheckFactory<Factory>(filter_config);
ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig(
filter_config.typed_config(),
factory_context.messageValidationContext().dynamicValidationVisitor(), factory);
return {std::move(message), factory.name()};
}

protected:
virtual void validateFilters(const std::string&, const std::string&, const std::string&, bool,
bool) const {};
virtual bool isTerminalFilter(Factory*, Protobuf::Message&, FactoryCtx&) const { return false; }

ProtobufTypes::MessagePtr
getDefaultConfig(const ProtobufWkt::Any& proto_config, const std::string& filter_config_name,
Server::Configuration::FactoryContext& factory_context,
bool last_filter_in_filter_chain, const std::string& filter_chain_type,
const absl::flat_hash_set<std::string>& require_type_urls) const override;
FactoryCtx& factory_context, bool last_filter_in_filter_chain,
const std::string& filter_chain_type,
const absl::flat_hash_set<std::string>& require_type_urls) const {
auto* default_factory = Config::Utility::getFactoryByType<Factory>(proto_config);
validateProtoConfigDefaultFactory(default_factory == nullptr, filter_config_name,
proto_config.type_url());
validateProtoConfigTypeUrl(Config::Utility::getFactoryType(proto_config), require_type_urls);
ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig(
proto_config, factory_context.messageValidationVisitor(), *default_factory);
validateFilters(filter_config_name, default_factory->name(), filter_chain_type,
isTerminalFilter(default_factory, *message, factory_context),
last_filter_in_filter_chain);
return message;
}

private:
std::unique_ptr<DynamicFilterConfigProviderImpl<Http::FilterFactoryCb>>
createFilterConfigProviderImpl(FilterConfigSubscriptionSharedPtr& subscription,
const absl::flat_hash_set<std::string>& require_type_urls,
Server::Configuration::FactoryContext& factory_context,
ProtobufTypes::MessagePtr&& default_config,
bool last_filter_in_filter_chain,
const std::string& filter_chain_type,
absl::string_view stat_prefix) override {
return std::make_unique<HttpDynamicFilterConfigProviderImpl>(
std::unique_ptr<DynamicFilterConfigProviderImpl<FactoryCb>> createFilterConfigProviderImpl(
FilterConfigSubscriptionSharedPtr& subscription,
const absl::flat_hash_set<std::string>& require_type_urls, FactoryCtx& factory_context,
ProtobufTypes::MessagePtr&& default_config, bool last_filter_in_filter_chain,
const std::string& filter_chain_type, absl::string_view stat_prefix) {
return std::make_unique<DynamicFilterConfigImpl>(
subscription, require_type_urls, factory_context, std::move(default_config),
last_filter_in_filter_chain, filter_chain_type, stat_prefix);
}
};

// HTTP filter
class HttpFilterConfigProviderManagerImpl
: public FilterConfigProviderManagerImpl<
Server::Configuration::NamedHttpFilterConfigFactory, Http::FilterFactoryCb,
Server::Configuration::FactoryContext, HttpDynamicFilterConfigProviderImpl> {
public:
absl::string_view statPrefix() const override { return "http_filter."; }

protected:
bool isTerminalFilter(Server::Configuration::NamedHttpFilterConfigFactory* default_factory,
Protobuf::Message& message,
Server::Configuration::FactoryContext& factory_context) const override {
return default_factory->isTerminalFilterByProto(message, factory_context);
}
void validateFilters(const std::string& filter_config_name, const std::string& filter_type,
const std::string& filter_chain_type, bool is_terminal_filter,
bool last_filter_in_filter_chain) const override {
Config::Utility::validateTerminalFilters(filter_config_name, filter_type, filter_chain_type,
is_terminal_filter, last_filter_in_filter_chain);
}
};

// TCP listener filter
class TcpListenerFilterConfigProviderManagerImpl
Comment thread
yanjunxiang-google marked this conversation as resolved.
: public FilterConfigProviderManagerImpl<
Server::Configuration::NamedListenerFilterConfigFactory, Network::ListenerFilterFactoryCb,
Server::Configuration::ListenerFactoryContext,
TcpListenerDynamicFilterConfigProviderImpl> {
public:
absl::string_view statPrefix() const override { return "tcp_listener_filter."; }
};

// UDP listener filter
class UdpListenerFilterConfigProviderManagerImpl
: public FilterConfigProviderManagerImpl<
Server::Configuration::NamedUdpListenerFilterConfigFactory,
Network::UdpListenerFilterFactoryCb, Server::Configuration::ListenerFactoryContext,
UdpListenerDynamicFilterConfigProviderImpl> {
public:
absl::string_view statPrefix() const override { return "udp_listener_filter."; }
};

} // namespace Filter
} // namespace Envoy
4 changes: 3 additions & 1 deletion source/extensions/filters/network/http_connection_manager/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ namespace Extensions {
namespace NetworkFilters {
namespace HttpConnectionManager {

using FilterConfigProviderManager = Filter::FilterConfigProviderManager<Http::FilterFactoryCb>;
using FilterConfigProviderManager =
Filter::FilterConfigProviderManager<Http::FilterFactoryCb,
Server::Configuration::FactoryContext>;

/**
* Config registration for the HTTP connection manager filter. @see NamedNetworkFilterConfigFactory.
Expand Down
Loading