Skip to content

Commit

Permalink
utility: removing some exceptions (#35414)
Browse files Browse the repository at this point in the history 
Risk Level: low
Testing: updated tests
Docs Changes: n/a
Release Notes: n/a
envoyproxy/envoy-mobile#176

Signed-off-by: Alyssa Wilk <[email protected]>
  • Loading branch information
@alyssawilk
alyssawilk committed Jul 31, 2024
1 parent 7b8132a commit 34bdb94
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 28 deletions.
15 changes: 9 additions & 6 deletions source/common/config/utility.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,21 +418,24 @@ class Utility {
* @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.
* @return a status indicating if there is a mismatch between design and configuration.
*/
static void validateTerminalFilters(const std::string& name, const std::string& filter_type,
const std::string& filter_chain_type, bool is_terminal_filter,
bool last_filter_in_current_config) {
static absl::Status validateTerminalFilters(const std::string& name,
const std::string& filter_type,
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(
return absl::InvalidArgumentError(
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) {
ExceptionUtil::throwEnvoyException(fmt::format(
return absl::InvalidArgumentError(fmt::format(
"Error: non-terminal filter named {} of type {} is the last filter in a {} filter chain.",
name, filter_type, filter_chain_type));
}
return absl::OkStatus();
}

/**
Expand Down
26 changes: 15 additions & 11 deletions source/common/filter/config_discovery_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,9 @@ class HttpDynamicFilterConfigProviderImpl
auto* factory =
Registry::FactoryRegistry<NeutralHttpFilterConfigFactory>::getFactory(factory_name);
const bool is_terminal_filter = factory->isTerminalFilterByProto(message, server_context_);
Config::Utility::validateTerminalFilters(config_name, factory_name, filter_chain_type_,
is_terminal_filter, last_filter_in_filter_chain_);
THROW_IF_NOT_OK(Config::Utility::validateTerminalFilters(config_name, factory_name,
filter_chain_type_, is_terminal_filter,
last_filter_in_filter_chain_));
}

private:
Expand Down Expand Up @@ -279,9 +280,9 @@ class DownstreamNetworkDynamicFilterConfigProviderImpl
Registry::FactoryRegistry<NeutralNetworkFilterConfigFactory>::getFactory(factory_name);
const bool is_terminal_filter =
factory->isTerminalFilterByProto(message, this->server_context_);
Config::Utility::validateTerminalFilters(config_name, factory_name, this->filter_chain_type_,
is_terminal_filter,
this->last_filter_in_filter_chain_);
THROW_IF_NOT_OK(Config::Utility::validateTerminalFilters(
config_name, factory_name, this->filter_chain_type_, is_terminal_filter,
this->last_filter_in_filter_chain_));
}
};

Expand Down Expand Up @@ -668,8 +669,9 @@ class HttpFilterConfigProviderManagerImpl
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);
THROW_IF_NOT_OK(Config::Utility::validateTerminalFilters(filter_config_name, filter_type,
filter_chain_type, is_terminal_filter,
last_filter_in_filter_chain));
}
const std::string getConfigDumpType() const override { return "ecds_filter_http"; }
};
Expand All @@ -695,8 +697,9 @@ class UpstreamHttpFilterConfigProviderManagerImpl
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);
THROW_IF_NOT_OK(Config::Utility::validateTerminalFilters(filter_config_name, filter_type,
filter_chain_type, is_terminal_filter,
last_filter_in_filter_chain));
}
const std::string getConfigDumpType() const override { return "ecds_filter_upstream_http"; }
};
Expand All @@ -722,8 +725,9 @@ class NetworkFilterConfigProviderManagerImpl
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);
THROW_IF_NOT_OK(Config::Utility::validateTerminalFilters(filter_config_name, filter_type,
filter_chain_type, is_terminal_filter,
last_filter_in_filter_chain));
}
const std::string getConfigDumpType() const override { return "ecds_filter_network"; }
};
Expand Down
6 changes: 3 additions & 3 deletions source/common/http/filter_chain_helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,9 @@ class FilterChainHelper : Logger::Loggable<Logger::Id::config> {
Http::FilterFactoryCb callback = callback_or_error.value();
dependency_manager.registerFilter(factory->name(), *factory->dependencies());
const bool is_terminal = factory->isTerminalFilterByProto(*message, server_context_);
Config::Utility::validateTerminalFilters(proto_config.name(), factory->name(),
filter_chain_type, is_terminal,
last_filter_in_current_config);
RETURN_IF_NOT_OK(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(
{factory->name(), callback}, proto_config.name());
#ifdef ENVOY_ENABLE_YAML
Expand Down
4 changes: 2 additions & 2 deletions source/common/listener_manager/listener_manager_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,11 @@ ProdListenerComponentFactory::createNetworkFilterFactoryListImpl(

auto message = Config::Utility::translateToFactoryConfig(
proto_config, filter_chain_factory_context.messageValidationVisitor(), factory);
Config::Utility::validateTerminalFilters(
RETURN_IF_NOT_OK(Config::Utility::validateTerminalFilters(
filters[i].name(), factory.name(), "network",
factory.isTerminalFilterByProto(*message,
filter_chain_factory_context.serverFactoryContext()),
is_terminal);
is_terminal));
auto callback_or_error =
factory.createFilterFactoryFromProto(*message, filter_chain_factory_context);
RETURN_IF_NOT_OK(callback_or_error.status());
Expand Down
12 changes: 6 additions & 6 deletions test/extensions/filters/network/http_connection_manager/config_filter_chain_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,12 +335,12 @@ TEST_F(FilterChainTest, CreateCustomUpgradeFilterChainWithRouterNotLast) {
"\x1D"
"encoder-decoder-buffer-filter");

EXPECT_THROW_WITH_MESSAGE(
HttpConnectionManagerConfig(hcm_config, context_, date_provider_,
route_config_provider_manager_,
&scoped_routes_config_provider_manager_, tracer_manager_,
filter_config_provider_manager_, creation_status_),
EnvoyException,
HttpConnectionManagerConfig config(hcm_config, context_, date_provider_,
route_config_provider_manager_,
&scoped_routes_config_provider_manager_, tracer_manager_,
filter_config_provider_manager_, creation_status_);
EXPECT_EQ(
creation_status_.message(),
"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.");
}
Expand Down

0 comments on commit 34bdb94

Please sign in to comment.