Skip to content

Commit

Permalink
ecds: make onConfigUpdate generic over filter type (#18061)
Browse files Browse the repository at this point in the history 
Additional Description: Part of #14696 (comment). The goal is to templatize DynamicFilterConfigProviderImpl while having DynamicFilterConfigProviderImplBase, FilterConfigProviderManagerImplBase, and FilterConfigSubscription agnostic to HTTP vs. network filters. This is a step in that direction, following the approach of #14717. Because extra validation was added since that PR, the filter factory object must be created before onConfigUpdate in addition to during

Risk Level: Low
Testing: Existing (refactoring)
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A
#14696

Signed-off-by: Taylor Barrella <[email protected]>
  • Loading branch information
@tbarrella
tbarrella authored Oct 28, 2021
1 parent 98d0394 commit 4a2b01f
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 52 deletions.
13 changes: 8 additions & 5 deletions envoy/config/extension_config_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,20 @@ template <class Factory, class FactoryCallback> class ExtensionConfigProvider {
virtual absl::optional<FactoryCallback> config() PURE;
};

template <class FactoryCallback> class DynamicExtensionConfigProviderBase {
class DynamicExtensionConfigProviderBase {
public:
virtual ~DynamicExtensionConfigProviderBase() = default;

/**
* Update the provider with a new configuration.
* @param config is an extension factory callback to replace the existing configuration.
* Update the provider with a new configuration. This interface accepts proto rather than a
* factory callback so that it can be generic over factory types. If instantiating the factory
* throws, it should only do so on the main thread, before any changes are applied to workers.
* @param config is the new configuration. It is expected that the configuration has already been
* validated.
* @param version_info is the version of the new extension configuration.
* @param cb the continuation callback for a completed configuration application on all threads.
*/
virtual void onConfigUpdate(FactoryCallback config, const std::string& version_info,
virtual void onConfigUpdate(const Protobuf::Message& config, const std::string& version_info,
ConfigAppliedCb applied_on_all_threads) PURE;

/**
Expand All @@ -61,7 +64,7 @@ template <class FactoryCallback> class DynamicExtensionConfigProviderBase {
};

template <class Factory, class FactoryCallback>
class DynamicExtensionConfigProvider : public DynamicExtensionConfigProviderBase<FactoryCallback>,
class DynamicExtensionConfigProvider : public DynamicExtensionConfigProviderBase,
public ExtensionConfigProvider<Factory, FactoryCallback> {};

} // namespace Config
Expand Down
50 changes: 30 additions & 20 deletions source/common/filter/config_discovery_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,19 +132,17 @@ void FilterConfigSubscription::onConfigUpdate(
for (auto* provider : filter_config_providers_) {
provider->validateTerminalFilter(filter_config_name_, factory.name(), is_terminal_filter);
}
Envoy::Http::FilterFactoryCb factory_callback =
factory.createFilterFactoryFromProto(*message, stat_prefix_, factory_context_);
ENVOY_LOG(debug, "Updating filter config {}", filter_config_name_);

Common::applyToAllWithCleanup<DynamicFilterConfigProviderImplBase*>(
filter_config_providers_,
[&factory_callback, &version_info](DynamicFilterConfigProviderImplBase* provider,
std::shared_ptr<Cleanup> cleanup) {
provider->onConfigUpdate(factory_callback, version_info, [cleanup] {});
[&message, &version_info](DynamicFilterConfigProviderImplBase* provider,
std::shared_ptr<Cleanup> cleanup) {
provider->onConfigUpdate(*message, version_info, [cleanup] {});
},
[this]() { stats_.config_reload_.inc(); });
last_config_hash_ = new_hash;
last_config_ = factory_callback;
last_config_ = std::move(message);
last_type_url_ = type_url;
last_version_info_ = version_info;
last_filter_name_ = factory.name();
Expand All @@ -165,7 +163,7 @@ void FilterConfigSubscription::onConfigUpdate(
[this]() { stats_.config_reload_.inc(); });

last_config_hash_ = 0;
last_config_ = absl::nullopt;
last_config_ = nullptr;
last_type_url_ = "";
last_filter_is_terminal_ = false;
last_filter_name_ = "";
Expand Down Expand Up @@ -222,7 +220,7 @@ void FilterConfigProviderManagerImplBase::applyLastOrDefaultConfig(
// update arrives first. In this case, use the default config, increment a metric,
// and the applied config eventually converges once ECDS update arrives.
bool last_config_valid = false;
if (subscription->lastConfig().has_value()) {
if (subscription->lastConfig()) {
TRY_ASSERT_MAIN_THREAD {
provider.validateTypeUrl(subscription->lastTypeUrl());
provider.validateTerminalFilter(filter_config_name, subscription->lastFilterName(),
Expand All @@ -235,7 +233,7 @@ void FilterConfigProviderManagerImplBase::applyLastOrDefaultConfig(
subscription->incrementConflictCounter();
}
if (last_config_valid) {
provider.onConfigUpdate(subscription->lastConfig().value(), subscription->lastVersionInfo(),
provider.onConfigUpdate(*subscription->lastConfig(), subscription->lastVersionInfo(),
nullptr);
}
}
Expand Down Expand Up @@ -265,16 +263,20 @@ DynamicFilterConfigProviderPtr FilterConfigProviderManagerImpl::createDynamicFil
require_type_urls.emplace(factory_type_url);
}

Envoy::Http::FilterFactoryCb default_config = nullptr;
ProtobufTypes::MessagePtr default_config;
if (config_source.has_default_config()) {
default_config = getDefaultConfig(config_source.default_config(), filter_config_name,
factory_context, stat_prefix, last_filter_in_filter_chain,
filter_chain_type, require_type_urls);
default_config =
getDefaultConfig(config_source.default_config(), filter_config_name, factory_context,
last_filter_in_filter_chain, filter_chain_type, require_type_urls);
}

auto provider = std::make_unique<DynamicFilterConfigProviderImpl>(
subscription, require_type_urls, factory_context, default_config, last_filter_in_filter_chain,
filter_chain_type);
subscription, require_type_urls, factory_context, std::move(default_config),
last_filter_in_filter_chain, filter_chain_type,
[this, stat_prefix,
&factory_context](const Protobuf::Message& message) -> Envoy::Http::FilterFactoryCb {
return instantiateFilterFactory(message, stat_prefix, factory_context);
});

// Ensure the subscription starts if it has not already.
if (config_source.apply_default_config_without_warming()) {
Expand All @@ -284,11 +286,11 @@ DynamicFilterConfigProviderPtr FilterConfigProviderManagerImpl::createDynamicFil
return provider;
}

Http::FilterFactoryCb HttpFilterConfigProviderManagerImpl::getDefaultConfig(
ProtobufTypes::MessagePtr HttpFilterConfigProviderManagerImpl::getDefaultConfig(
const ProtobufWkt::Any& proto_config, const std::string& filter_config_name,
Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix,
bool last_filter_in_filter_chain, const std::string& filter_chain_type,
const absl::flat_hash_set<std::string> require_type_urls) const {
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);
Expand All @@ -304,7 +306,15 @@ Http::FilterFactoryCb HttpFilterConfigProviderManagerImpl::getDefaultConfig(
filter_config_name, default_factory->name(), filter_chain_type,
default_factory->isTerminalFilterByProto(*message, factory_context),
last_filter_in_filter_chain);
return default_factory->createFilterFactoryFromProto(*message, stat_prefix, factory_context);
return message;
}

Http::FilterFactoryCb HttpFilterConfigProviderManagerImpl::instantiateFilterFactory(
const Protobuf::Message& message, const std::string& stat_prefix,
Server::Configuration::FactoryContext& factory_context) const {
auto* factory = Registry::FactoryRegistry<
Server::Configuration::NamedHttpFilterConfigFactory>::getFactoryByType(message.GetTypeName());
return factory->createFilterFactoryFromProto(message, stat_prefix, factory_context);
}

} // namespace Filter
Expand Down
63 changes: 36 additions & 27 deletions source/common/filter/config_discovery_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ using FilterConfigSubscriptionSharedPtr = std::shared_ptr<FilterConfigSubscripti
/**
* Base class for a filter config provider using discovery subscriptions.
**/
class DynamicFilterConfigProviderImplBase
: public Config::DynamicExtensionConfigProviderBase<Envoy::Http::FilterFactoryCb> {
class DynamicFilterConfigProviderImplBase : public Config::DynamicExtensionConfigProviderBase {
public:
DynamicFilterConfigProviderImplBase(FilterConfigSubscriptionSharedPtr& subscription,
const absl::flat_hash_set<std::string>& require_type_urls,
Expand Down Expand Up @@ -66,27 +65,28 @@ class DynamicFilterConfigProviderImplBase
class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBase,
public DynamicFilterConfigProvider {
public:
DynamicFilterConfigProviderImpl(FilterConfigSubscriptionSharedPtr& subscription,
const absl::flat_hash_set<std::string>& require_type_urls,
Server::Configuration::FactoryContext& factory_context,
Envoy::Http::FilterFactoryCb default_config,
bool last_filter_in_filter_chain,
const std::string& filter_chain_type)
DynamicFilterConfigProviderImpl(
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,
std::function<Envoy::Http::FilterFactoryCb(const Protobuf::Message&)> factory_cb_fn)
: DynamicFilterConfigProviderImplBase(subscription, require_type_urls,
last_filter_in_filter_chain, filter_chain_type),
default_configuration_(default_config ? absl::make_optional(default_config)
: absl::nullopt),
tls_(factory_context.threadLocal()) {
default_configuration_(std::move(default_config)), tls_(factory_context.threadLocal()),
factory_cb_fn_(factory_cb_fn) {
tls_.set([](Event::Dispatcher&) { return std::make_shared<ThreadLocalConfig>(); });
};

// Config::ExtensionConfigProvider
const std::string& name() override { return DynamicFilterConfigProviderImplBase::name(); }
absl::optional<Envoy::Http::FilterFactoryCb> config() override { return tls_->config_; }

// Config::DynamicExtensionConfigProvider
void onConfigUpdate(Envoy::Http::FilterFactoryCb config, const std::string&,
// Config::DynamicExtensionConfigProviderBase
void onConfigUpdate(const Protobuf::Message& message, const std::string&,
Config::ConfigAppliedCb cb) override {
const Envoy::Http::FilterFactoryCb config = factory_cb_fn_(message);
tls_.runOnAllThreads(
[config, cb](OptRef<ThreadLocalConfig> tls) {
tls->config_ = config;
Expand All @@ -102,12 +102,15 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa
}

void onConfigRemoved(Config::ConfigAppliedCb applied_on_all_threads) override {
const absl::optional<Envoy::Http::FilterFactoryCb> default_config =
default_configuration_ ? absl::make_optional(factory_cb_fn_(*default_configuration_))
: absl::nullopt;
tls_.runOnAllThreads(
[config = default_configuration_](OptRef<ThreadLocalConfig> tls) { tls->config_ = config; },
[this, applied_on_all_threads]() {
[config = default_config](OptRef<ThreadLocalConfig> tls) { tls->config_ = config; },
[this, default_config, applied_on_all_threads]() {
// This happens after all workers have discarded the previous config so it can be safely
// deleted on the main thread by an update with the new config.
this->current_config_ = default_configuration_;
this->current_config_ = default_config;
if (applied_on_all_threads) {
applied_on_all_threads();
}
Expand All @@ -129,8 +132,9 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa
// Currently applied configuration to ensure that the main thread deletes the last reference to
// it.
absl::optional<Envoy::Http::FilterFactoryCb> current_config_{absl::nullopt};
const absl::optional<Envoy::Http::FilterFactoryCb> default_configuration_;
const ProtobufTypes::MessagePtr default_configuration_;
ThreadLocal::TypedSlot<ThreadLocalConfig> tls_;
const std::function<Envoy::Http::FilterFactoryCb(const Protobuf::Message&)> factory_cb_fn_;
};

/**
Expand Down Expand Up @@ -168,7 +172,7 @@ class FilterConfigSubscription

const Init::SharedTargetImpl& initTarget() { return init_target_; }
const std::string& name() { return filter_config_name_; }
const absl::optional<Envoy::Http::FilterFactoryCb>& lastConfig() { return last_config_; }
const Protobuf::Message* lastConfig() { return last_config_.get(); }
const std::string& lastTypeUrl() { return last_type_url_; }
const std::string& lastVersionInfo() { return last_version_info_; }
const std::string& lastFilterName() { return last_filter_name_; }
Expand All @@ -189,7 +193,7 @@ class FilterConfigSubscription

const std::string filter_config_name_;
uint64_t last_config_hash_{0ul};
absl::optional<Envoy::Http::FilterFactoryCb> last_config_{absl::nullopt};
ProtobufTypes::MessagePtr last_config_;
std::string last_type_url_;
std::string last_version_info_;
std::string last_filter_name_;
Expand Down Expand Up @@ -271,22 +275,27 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa
}

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

virtual Http::FilterFactoryCb
instantiateFilterFactory(const Protobuf::Message& message, const std::string& stat_prefix,
Server::Configuration::FactoryContext& factory_context) const PURE;
};

class HttpFilterConfigProviderManagerImpl : public FilterConfigProviderManagerImpl {
protected:
Http::FilterFactoryCb
ProtobufTypes::MessagePtr
getDefaultConfig(const ProtobufWkt::Any& proto_config, const std::string& filter_config_name,
Server::Configuration::FactoryContext& factory_context,
const std::string& stat_prefix, bool last_filter_in_filter_chain,
const std::string& filter_chain_type,
const absl::flat_hash_set<std::string> require_type_urls) const override;
bool last_filter_in_filter_chain, const std::string& filter_chain_type,
const absl::flat_hash_set<std::string>& require_type_urls) const override;
Http::FilterFactoryCb
instantiateFilterFactory(const Protobuf::Message& message, const std::string& stat_prefix,
Server::Configuration::FactoryContext& factory_context) const override;
};

} // namespace Filter
Expand Down

0 comments on commit 4a2b01f

Please sign in to comment.