Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 11 additions & 8 deletions source/common/config/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -324,20 +324,23 @@ class Utility {
/**
* Verify that any filter designed to be terminal is configured to be terminal, and vice versa.
* @param name the name of the filter.
* @param name the type of filter.
* @param filter_type the type of filter.
* @param filter_chain_type the type of filter chain.
* @param is_terminal_filter true if the filter is designed to be terminal.
* @param last_filter_in_current_config true if the filter is last in the configuration.
* @throws EnvoyException if there is a mismatch between design and configuration.
*/
static void validateTerminalFilters(const std::string& name, const char* filter_type,
bool is_terminal_filter, bool last_filter_in_current_config) {
static void validateTerminalFilters(const std::string& name, const std::string& filter_type,
const char* filter_chain_type, bool is_terminal_filter,
bool last_filter_in_current_config) {
if (is_terminal_filter && !last_filter_in_current_config) {
throw EnvoyException(
fmt::format("Error: {} must be the terminal {} filter.", name, filter_type));
throw EnvoyException(fmt::format("Error: terminal filter named {} of type {} must be the "
"last filter in a {} filter chain.",
name, filter_type, filter_chain_type));
} else if (!is_terminal_filter && last_filter_in_current_config) {
throw EnvoyException(
fmt::format("Error: non-terminal filter {} is the last filter in a {} filter chain.",
name, filter_type));
throw EnvoyException(fmt::format(
"Error: non-terminal filter named {} of type {} is the last filter in a {} filter chain.",
name, filter_type, filter_chain_type));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(

const auto& filters = config.http_filters();
for (int32_t i = 0; i < filters.size(); i++) {
bool is_terminal = false;
processFilter(filters[i], i, "http", filter_factories_, is_terminal);
Config::Utility::validateTerminalFilters(filters[i].name(), "http", is_terminal,
i == filters.size() - 1);
processFilter(filters[i], i, "http", filter_factories_, "http", i == filters.size() - 1);
}

for (const auto& upgrade_config : config.upgrade_configs()) {
Expand All @@ -412,11 +409,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
if (!upgrade_config.filters().empty()) {
std::unique_ptr<FilterFactoriesList> factories = std::make_unique<FilterFactoriesList>();
for (int32_t j = 0; j < upgrade_config.filters().size(); j++) {
bool is_terminal = false;
processFilter(upgrade_config.filters(j), j, name, *factories, is_terminal);
Config::Utility::validateTerminalFilters(upgrade_config.filters(j).name(), "http upgrade",
is_terminal,
j == upgrade_config.filters().size() - 1);
processFilter(upgrade_config.filters(j), j, name, *factories, "http upgrade",
j == upgrade_config.filters().size() - 1);
}
upgrade_filter_factories_.emplace(
std::make_pair(name, FilterConfig{std::move(factories), enabled}));
Expand All @@ -432,7 +426,7 @@ void HttpConnectionManagerConfig::processFilter(
const envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter&
proto_config,
int i, absl::string_view prefix, std::list<Http::FilterFactoryCb>& filter_factories,
bool& is_terminal) {
const char* filter_chain_type, bool last_filter_in_current_config) {
ENVOY_LOG(debug, " {} filter #{}", prefix, i);
ENVOY_LOG(debug, " name: {}", proto_config.name());
ENVOY_LOG(debug, " config: {}",
Expand All @@ -451,7 +445,9 @@ void HttpConnectionManagerConfig::processFilter(
proto_config, context_.messageValidationVisitor(), factory);
Http::FilterFactoryCb callback =
factory.createFilterFactoryFromProto(*message, stats_prefix_, context_);
is_terminal = factory.isTerminalFilter();
bool is_terminal = factory.isTerminalFilter();
Config::Utility::validateTerminalFilters(proto_config.name(), factory.name(), filter_chain_type,
is_terminal, last_filter_in_current_config);
filter_factories.push_back(callback);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
processFilter(const envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter&
proto_config,
int i, absl::string_view prefix, FilterFactoriesList& filter_factories,
bool& is_terminal);
const char* filter_chain_type, bool last_filter_in_current_config);

/**
* Determines what tracing provider to use for a given
Expand Down
2 changes: 1 addition & 1 deletion source/server/listener_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ std::vector<Network::FilterFactoryCb> ProdListenerComponentFactory::createNetwor
Config::Utility::getAndCheckFactory<Configuration::NamedNetworkFilterConfigFactory>(
proto_config);

Config::Utility::validateTerminalFilters(filters[i].name(), "network",
Config::Utility::validateTerminalFilters(filters[i].name(), factory.name(), "network",
factory.isTerminalFilter(), i == filters.size() - 1);

auto message = Config::Utility::translateToFactoryConfig(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ stat_prefix: router
HttpConnectionManagerConfig(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_,
date_provider_, route_config_provider_manager_,
scoped_routes_config_provider_manager_, http_tracer_manager_),
EnvoyException, "Error: envoy.filters.http.router must be the terminal http filter.");
EnvoyException,
"Error: terminal filter named envoy.filters.http.router of type envoy.filters.http.router "
"must be the last filter in a http filter chain.");
}

TEST_F(HttpConnectionManagerConfigTest, NonTerminalFilter) {
Expand Down Expand Up @@ -141,7 +143,9 @@ stat_prefix: router
date_provider_, route_config_provider_manager_,
scoped_routes_config_provider_manager_, http_tracer_manager_),
EnvoyException,
"Error: non-terminal filter health_check is the last filter in a http filter chain.");
"Error: non-terminal filter named health_check of type "
"envoy.filters.http.health_check is the last filter in a http filter "
"chain.");
}

TEST_F(HttpConnectionManagerConfigTest, MiscConfig) {
Expand Down Expand Up @@ -1335,7 +1339,9 @@ TEST_F(FilterChainTest, CreateCustomUpgradeFilterChainWithRouterNotLast) {
HttpConnectionManagerConfig(hcm_config, context_, date_provider_,
route_config_provider_manager_,
scoped_routes_config_provider_manager_, http_tracer_manager_),
EnvoyException, "Error: envoy.filters.http.router must be the terminal http upgrade filter.");
EnvoyException,
"Error: terminal filter named envoy.filters.http.router of type envoy.filters.http.router "
"must be the last filter in a http upgrade filter chain.");
}

TEST_F(FilterChainTest, InvalidConfig) {
Expand Down
11 changes: 6 additions & 5 deletions test/server/listener_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,10 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TerminalNotLast) {
config: {}
)EOF";

EXPECT_THROW_WITH_REGEX(manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true),
EnvoyException,
"Error: non-terminal filter non_terminal is the last "
"filter in a network filter chain.");
EXPECT_THROW_WITH_REGEX(
manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true), EnvoyException,
"Error: non-terminal filter named non_terminal of type non_terminal is the last "
"filter in a network filter chain.");
}

TEST_F(ListenerManagerImplWithRealFiltersTest, NotTerminalLast) {
Expand All @@ -399,7 +399,8 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, NotTerminalLast) {

EXPECT_THROW_WITH_REGEX(
manager_->addOrUpdateListener(parseListenerFromV2Yaml(yaml), "", true), EnvoyException,
"Error: envoy.filters.network.tcp_proxy must be the terminal network filter.");
"Error: terminal filter named envoy.filters.network.tcp_proxy of type "
"envoy.filters.network.tcp_proxy must be the last filter in a network filter chain.");
}

TEST_F(ListenerManagerImplWithRealFiltersTest, BadFilterName) {
Expand Down