Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a33db77
imporve doc
qinggniq Mar 30, 2021
e93cfb4
Update README.md
qinggniq Mar 30, 2021
418554b
fix typo
qinggniq Mar 31, 2021
ed05ce5
Merge branch 'main' of https://github.com/envoyproxy/envoy into main
qinggniq Apr 1, 2021
bcf50b2
change signature of isTermianlFilter
qinggniq Apr 1, 2021
e89113f
remove code
qinggniq Apr 1, 2021
3bebde5
cover more cases to validate config
qinggniq Apr 1, 2021
18bf6ed
fix test flakes
qinggniq Apr 1, 2021
3d81b83
fix build flakes
qinggniq Apr 2, 2021
55dba70
fix typo
qinggniq Apr 2, 2021
af0ecf2
fix build flakes
qinggniq Apr 2, 2021
7b2214d
fix: prevent Werror=overloaded-virtual warnig
qinggniq Apr 2, 2021
f5c5371
merge from master
qinggniq Apr 6, 2021
a6dee8f
fix test flakes
qinggniq Apr 6, 2021
0e56490
refactor the signature of isTerminalFilter
qinggniq Apr 6, 2021
e7075fa
fix extension_discovery test
qinggniq Apr 9, 2021
c98dd0e
Merge branch 'main' of https://github.com/envoyproxy/envoy into main
qinggniq Apr 9, 2021
3b3884e
fix extension_discovery_integration_test flakes
qinggniq Apr 9, 2021
123ac59
kick ci
qinggniq Apr 9, 2021
244ebb4
fix doc format
qinggniq Apr 9, 2021
b9dfe5c
Merge branch 'main' of https://github.com/envoyproxy/envoy into main
qinggniq Apr 13, 2021
d72f95f
fix review comment
qinggniq Apr 14, 2021
f3dde43
fix: envoy-filter-example sha
qinggniq Apr 15, 2021
dbff93b
fix: change envoy-filter-exmaple url
qinggniq Apr 15, 2021
953f9b2
Merge branch 'main' of https://github.com/envoyproxy/envoy into main
qinggniq Apr 15, 2021
ff8a04b
merge from upstream
qinggniq Apr 16, 2021
3f9afe6
fix: change sha to latest
qinggniq Apr 16, 2021
23aade7
fix: build flakes
qinggniq Apr 16, 2021
af1a9b2
fix: change envoy-filter-example sha to latest commit
qinggniq Apr 19, 2021
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
2 changes: 1 addition & 1 deletion ci/filter_example_setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
set -e

# This is the hash on https://github.com/envoyproxy/envoy-filter-example.git we pin to.
ENVOY_FILTER_EXAMPLE_GITSHA="d135632a6e96268ab2c6a62e1cceec25c728ccf9"
ENVOY_FILTER_EXAMPLE_GITSHA="dfdc226d44d1b7c300e6e691e2e8ada98b045edb"
ENVOY_FILTER_EXAMPLE_SRCDIR="${BUILD_DIR}/envoy-filter-example"

# shellcheck disable=SC2034
Expand Down
2 changes: 1 addition & 1 deletion docs/root/intro/arch_overview/http/http_filters.rst
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ decoder/encoder filters):
- A
- B
# The last configured filter has to be a terminal filter, as determined by the
# NamedHttpFilterConfigFactory::isTerminalFilter() function. This is most likely the router
# NamedHttpFilterConfigFactory::isTerminalFilterByProto(config, context) function. This is most likely the router
# filter.
- C

Expand Down
6 changes: 5 additions & 1 deletion include/envoy/filter/http/filter_config_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ class FilterConfigProviderManager {
* @param filter_config_name the filter config resource name.
* @param factory_context is the context to use for the filter config provider.
* @param stat_prefix supplies the stat_prefix to use for the provider stats.
* @param last_filter_in_filter_chain indicates whether this filter is the last filter in the
* configured chain
* @param filter_chain_type is the filter chain type
*/
virtual DynamicFilterConfigProviderPtr createDynamicFilterConfigProvider(
const envoy::config::core::v3::ExtensionConfigSource& config_source,
const std::string& filter_config_name, Server::Configuration::FactoryContext& factory_context,
const std::string& stat_prefix) PURE;
const std::string& stat_prefix, bool last_filter_in_filter_chain,
const std::string& filter_chain_type) PURE;

/**
* Get an FilterConfigProviderPtr for a statically inlined filter config.
Expand Down
4 changes: 2 additions & 2 deletions include/envoy/server/filter_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class NamedNetworkFilterConfigFactory : public ProtocolOptionsFactory {
/**
* @return bool true if this filter must be the last filter in a filter chain, false otherwise.
*/
virtual bool isTerminalFilter() { return false; }
virtual bool isTerminalFilterByProto(const Protobuf::Message&, FactoryContext&) { return false; }
};

/**
Expand Down Expand Up @@ -206,7 +206,7 @@ class NamedHttpFilterConfigFactory : public ProtocolOptionsFactory {
/**
* @return bool true if this filter must be the last filter in a filter chain, false otherwise.
*/
virtual bool isTerminalFilter() { return false; }
virtual bool isTerminalFilterByProto(const Protobuf::Message&, FactoryContext&) { return false; }

/**
* @return FilterDependenciesPtr specification of dependencies required or
Expand Down
2 changes: 1 addition & 1 deletion source/common/config/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ class Utility {
* @throws EnvoyException if there is a mismatch between design and configuration.
*/
static void validateTerminalFilters(const std::string& name, const std::string& filter_type,
const char* filter_chain_type, bool is_terminal_filter,
const std::string& filter_chain_type, bool is_terminal_filter,
bool last_filter_in_current_config) {
if (is_terminal_filter && !last_filter_in_current_config) {
ExceptionUtil::throwEnvoyException(
Expand Down
53 changes: 43 additions & 10 deletions source/common/filter/http/filter_config_discovery_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,25 @@ void validateTypeUrlHelper(const std::string& type_url,
DynamicFilterConfigProviderImpl::DynamicFilterConfigProviderImpl(
FilterConfigSubscriptionSharedPtr& subscription,
const absl::flat_hash_set<std::string>& require_type_urls,

Server::Configuration::FactoryContext& factory_context,
Envoy::Http::FilterFactoryCb default_config)
Envoy::Http::FilterFactoryCb default_config, bool last_filter_in_filter_chain,
const std::string& filter_chain_type)
: subscription_(subscription), require_type_urls_(require_type_urls),
default_configuration_(default_config ? absl::make_optional(default_config) : absl::nullopt),
tls_(factory_context.threadLocal()),
init_target_("DynamicFilterConfigProviderImpl", [this]() {
subscription_->start();
// This init target is used to activate the subscription but not wait for a response. It is
// used whenever a default config is provided to be used while waiting for a response.
init_target_.ready();
}) {
tls_(factory_context.threadLocal()), init_target_("DynamicFilterConfigProviderImpl",
[this]() {
subscription_->start();
// This init target is used to activate
// the subscription but not wait for a
// response. It is used whenever a default
// config is provided to be used while
// waiting for a response.
init_target_.ready();
}),
last_filter_in_filter_chain_(last_filter_in_filter_chain),
filter_chain_type_(filter_chain_type) {

subscription_->filter_config_providers_.insert(this);
tls_.set([](Event::Dispatcher&) { return std::make_shared<ThreadLocalConfig>(); });
}
Expand Down Expand Up @@ -78,6 +86,13 @@ void DynamicFilterConfigProviderImpl::onConfigUpdate(Envoy::Http::FilterFactoryC
});
}

void DynamicFilterConfigProviderImpl::validateTerminalFilter(const std::string& name,
const std::string& filter_type,
bool is_terminal_filter) {
Config::Utility::validateTerminalFilters(name, filter_type, filter_chain_type_,
is_terminal_filter, last_filter_in_filter_chain_);
}

void DynamicFilterConfigProviderImpl::onConfigRemoved(
Config::ConfigAppliedCb applied_on_all_threads) {
tls_.runOnAllThreads(
Expand Down Expand Up @@ -156,9 +171,14 @@ void FilterConfigSubscription::onConfigUpdate(
}
ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig(
filter_config.typed_config(), validator_, factory);
bool is_terminal_filter = factory.isTerminalFilterByProto(*message, factory_context_);
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<DynamicFilterConfigProviderImpl*>(
filter_config_providers_,
[&factory_callback, &version_info](DynamicFilterConfigProviderImpl* provider,
Expand All @@ -170,6 +190,8 @@ void FilterConfigSubscription::onConfigUpdate(
last_config_ = factory_callback;
last_type_url_ = type_url;
last_version_info_ = version_info;
last_filter_name_ = factory.name();
last_filter_is_terminal_ = is_terminal_filter;
}

void FilterConfigSubscription::onConfigUpdate(
Expand All @@ -188,6 +210,8 @@ void FilterConfigSubscription::onConfigUpdate(
last_config_hash_ = 0;
last_config_ = absl::nullopt;
last_type_url_ = "";
last_filter_is_terminal_ = false;
last_filter_name_ = "";
} else if (!added_resources.empty()) {
onConfigUpdate(added_resources, added_resources[0].get().version());
}
Expand Down Expand Up @@ -235,7 +259,8 @@ std::shared_ptr<FilterConfigSubscription> FilterConfigProviderManagerImpl::getSu
DynamicFilterConfigProviderPtr FilterConfigProviderManagerImpl::createDynamicFilterConfigProvider(
const envoy::config::core::v3::ExtensionConfigSource& config_source,
const std::string& filter_config_name, Server::Configuration::FactoryContext& factory_context,
const std::string& stat_prefix) {
const std::string& stat_prefix, bool last_filter_in_filter_config,
const std::string& filter_chain_type) {
auto subscription = getSubscription(config_source.config_source(), filter_config_name,
factory_context, stat_prefix);
// For warming, wait until the subscription receives the first response to indicate readiness.
Expand All @@ -249,6 +274,7 @@ DynamicFilterConfigProviderPtr FilterConfigProviderManagerImpl::createDynamicFil
auto factory_type_url = TypeUtil::typeUrlToDescriptorFullName(type_url);
require_type_urls.emplace(factory_type_url);
}

Envoy::Http::FilterFactoryCb default_config = nullptr;
if (config_source.has_default_config()) {
auto* default_factory =
Expand All @@ -265,12 +291,17 @@ DynamicFilterConfigProviderPtr FilterConfigProviderManagerImpl::createDynamicFil
ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig(
config_source.default_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_config);
default_config =
default_factory->createFilterFactoryFromProto(*message, stat_prefix, factory_context);
}

auto provider = std::make_unique<DynamicFilterConfigProviderImpl>(
subscription, require_type_urls, factory_context, default_config);
subscription, require_type_urls, factory_context, default_config,
last_filter_in_filter_config, filter_chain_type);

// Ensure the subscription starts if it has not already.
if (config_source.apply_default_config_without_warming()) {
Expand All @@ -286,6 +317,8 @@ DynamicFilterConfigProviderPtr FilterConfigProviderManagerImpl::createDynamicFil
if (subscription->lastConfig().has_value()) {
TRY_ASSERT_MAIN_THREAD {
provider->validateTypeUrl(subscription->lastTypeUrl());
provider->validateTerminalFilter(filter_config_name, subscription->lastFilterName(),
subscription->isLastFilterTerminal());
last_config_valid = true;
}
END_TRY catch (const EnvoyException& e) {
Expand Down
17 changes: 14 additions & 3 deletions source/common/filter/http/filter_config_discovery_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,15 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProvider {
DynamicFilterConfigProviderImpl(FilterConfigSubscriptionSharedPtr& subscription,
const absl::flat_hash_set<std::string>& require_type_urls,
Server::Configuration::FactoryContext& factory_context,
Envoy::Http::FilterFactoryCb default_config);
Envoy::Http::FilterFactoryCb default_config,
bool last_filter_in_filter_chain,
const std::string& filter_chain_type);

~DynamicFilterConfigProviderImpl() override;

void validateTypeUrl(const std::string& type_url) const;

void validateTerminalFilter(const std::string& name, const std::string& filter_type,
bool is_terminal_filter);
// Config::ExtensionConfigProvider
const std::string& name() override;
absl::optional<Envoy::Http::FilterFactoryCb> config() override;
Expand Down Expand Up @@ -74,6 +78,8 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProvider {
// case no warming is requested by any other filter config provider.
Init::TargetImpl init_target_;

const bool last_filter_in_filter_chain_;
const std::string filter_chain_type_;
friend class FilterConfigProviderManagerImpl;
};

Expand Down Expand Up @@ -115,6 +121,8 @@ class FilterConfigSubscription
const absl::optional<Envoy::Http::FilterFactoryCb>& lastConfig() { return last_config_; }
const std::string& lastTypeUrl() { return last_type_url_; }
const std::string& lastVersionInfo() { return last_version_info_; }
const std::string& lastFilterName() { return last_filter_name_; }
bool isLastFilterTerminal() { return last_filter_is_terminal_; }
void incrementConflictCounter();

private:
Expand All @@ -134,6 +142,8 @@ class FilterConfigSubscription
absl::optional<Envoy::Http::FilterFactoryCb> last_config_{absl::nullopt};
std::string last_type_url_;
std::string last_version_info_;
std::string last_filter_name_;
bool last_filter_is_terminal_;
Server::Configuration::FactoryContext& factory_context_;
ProtobufMessage::ValidationVisitor& validator_;

Expand Down Expand Up @@ -183,7 +193,8 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManager,
DynamicFilterConfigProviderPtr createDynamicFilterConfigProvider(
const envoy::config::core::v3::ExtensionConfigSource& config_source,
const std::string& filter_config_name, Server::Configuration::FactoryContext& factory_context,
const std::string& stat_prefix) override;
const std::string& stat_prefix, bool last_filter_in_filter_chain,
const std::string& filter_chain_type) override;

FilterConfigProviderPtr
createStaticFilterConfigProvider(const Envoy::Http::FilterFactoryCb& config,
Expand Down
11 changes: 11 additions & 0 deletions source/extensions/filters/http/common/factory_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,21 @@ class FactoryBase : public Server::Configuration::NamedHttpFilterConfigFactory {

std::string name() const override { return name_; }

bool isTerminalFilterByProto(const Protobuf::Message& proto_config,
Server::Configuration::FactoryContext& context) override {
return isTerminalFilterByProtoTyped(MessageUtil::downcastAndValidate<const ConfigProto&>(
proto_config, context.messageValidationVisitor()),
context);
}

protected:
FactoryBase(const std::string& name) : name_(name) {}

private:
virtual bool isTerminalFilterByProtoTyped(const ConfigProto&,
Server::Configuration::FactoryContext&) {
return false;
}
virtual Http::FilterFactoryCb
createFilterFactoryFromProtoTyped(const ConfigProto& proto_config,
const std::string& stats_prefix,
Expand Down
6 changes: 4 additions & 2 deletions source/extensions/filters/http/router/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ class RouterFilterConfig
public:
RouterFilterConfig() : FactoryBase(HttpFilterNames::get().Router) {}

bool isTerminalFilter() override { return true; }

private:
bool isTerminalFilterByProtoTyped(const envoy::extensions::filters::http::router::v3::Router&,
Server::Configuration::FactoryContext&) override {
return true;
}
Http::FilterFactoryCb createFilterFactoryFromProtoTyped(
const envoy::extensions::filters::http::router::v3::Router& proto_config,
const std::string& stat_prefix, Server::Configuration::FactoryContext& context) override;
Expand Down
11 changes: 10 additions & 1 deletion source/extensions/filters/network/common/factory_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,22 @@ class FactoryBase : public Server::Configuration::NamedNetworkFilterConfigFactor

std::string name() const override { return name_; }

bool isTerminalFilter() override { return is_terminal_filter_; }
bool isTerminalFilterByProto(const Protobuf::Message& proto_config,
Server::Configuration::FactoryContext& context) override {
return isTerminalFilterByProtoTyped(MessageUtil::downcastAndValidate<const ConfigProto&>(
proto_config, context.messageValidationVisitor()),
context);
}

protected:
FactoryBase(const std::string& name, bool is_terminal = false)
: name_(name), is_terminal_filter_(is_terminal) {}

private:
virtual bool isTerminalFilterByProtoTyped(const ConfigProto&,
Server::Configuration::FactoryContext&) {
return is_terminal_filter_;
}
virtual Network::FilterFactoryCb
createFilterFactoryFromProtoTyped(const ConfigProto& proto_config,
Server::Configuration::FactoryContext& context) PURE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ class DirectResponseConfigFactory
};
}

bool isTerminalFilter() override { return true; }
bool isTerminalFilterByProtoTyped(
const envoy::extensions::filters::network::direct_response::v3::Config&,
Server::Configuration::FactoryContext&) override {
return true;
}
};

/**
Expand Down
5 changes: 4 additions & 1 deletion source/extensions/filters/network/echo/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ class EchoConfigFactory
};
}

bool isTerminalFilter() override { return true; }
bool isTerminalFilterByProtoTyped(const envoy::extensions::filters::network::echo::v3::Echo&,
Server::Configuration::FactoryContext&) override {
return true;
}
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
void HttpConnectionManagerConfig::processFilter(
const envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter&
proto_config,
int i, absl::string_view prefix, FilterFactoriesList& filter_factories,
const char* filter_chain_type, bool last_filter_in_current_config) {
int i, const std::string& prefix, FilterFactoriesList& filter_factories,
const std::string& filter_chain_type, bool last_filter_in_current_config) {
ENVOY_LOG(debug, " {} filter #{}", prefix, i);
if (proto_config.config_type_case() ==
envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter::ConfigTypeCase::
Expand All @@ -524,7 +524,7 @@ void HttpConnectionManagerConfig::processFilter(
proto_config, context_.messageValidationVisitor(), factory);
Http::FilterFactoryCb callback =
factory.createFilterFactoryFromProto(*message, stats_prefix_, context_);
bool is_terminal = factory.isTerminalFilter();
bool is_terminal = factory.isTerminalFilterByProto(*message, context_);
Copy link
Copy Markdown
Contributor Author

@qinggniq qinggniq Apr 14, 2021

Choose a reason for hiding this comment

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

@snowp Here we should read the message before validating the filter whether it's a terminal filter.

Config::Utility::validateTerminalFilters(proto_config.name(), factory.name(), filter_chain_type,
is_terminal, last_filter_in_current_config);
auto filter_config_provider = filter_config_provider_manager_.createStaticFilterConfigProvider(
Expand All @@ -542,7 +542,7 @@ void HttpConnectionManagerConfig::processFilter(

void HttpConnectionManagerConfig::processDynamicFilterConfig(
const std::string& name, const envoy::config::core::v3::ExtensionConfigSource& config_discovery,
FilterFactoriesList& filter_factories, const char* filter_chain_type,
FilterFactoriesList& filter_factories, const std::string& filter_chain_type,
bool last_filter_in_current_config) {
ENVOY_LOG(debug, " dynamic filter name: {}", name);
if (config_discovery.apply_default_config_without_warming() &&
Expand All @@ -558,13 +558,11 @@ void HttpConnectionManagerConfig::processDynamicFilterConfig(
throw EnvoyException(
fmt::format("Error: no factory found for a required type URL {}.", factory_type_url));
}
Config::Utility::validateTerminalFilters(name, factory->name(), filter_chain_type,
factory->isTerminalFilter(),
last_filter_in_current_config);
}

auto filter_config_provider = filter_config_provider_manager_.createDynamicFilterConfigProvider(
config_discovery, name, context_, stats_prefix_);
config_discovery, name, context_, stats_prefix_, last_filter_in_current_config,
filter_chain_type);
filter_factories.push_back(std::move(filter_config_provider));
}

Expand Down
Loading