Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main' into update_deps_cel
Browse files Browse the repository at this point in the history
Change-Id: I452b4cb002335ac7ef334ab6dd295f0d938997ab
  • Loading branch information
kyessenov committed Jul 31, 2024
2 parents ca36537 + f7e5b59 commit 513d8dd
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 57 deletions.
15 changes: 9 additions & 6 deletions source/common/config/utility.h
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
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
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
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
21 changes: 10 additions & 11 deletions source/common/singleton/threadsafe_singleton.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ template <class T> class InjectableSingleton {
}
static void clear() { loader_ = nullptr; }

// Atomically replace the value, returning the old value.
static T* replaceForTest(T* new_value) { return loader_.exchange(new_value); }

protected:
static std::atomic<T*> loader_;
};
Expand All @@ -86,20 +89,16 @@ template <class T> class ScopedInjectableLoader {
std::unique_ptr<T> instance_;
};

// This class saves the singleton object and restore the original singleton at destroy. This class
// is not thread safe. It can be used in single thread test.
template <class T>
class StackedScopedInjectableLoader :
// To access the protected loader_.
protected InjectableSingleton<T> {
// This class saves the singleton object and restore the original singleton at destroy.
template <class T> class StackedScopedInjectableLoaderForTest {
public:
explicit StackedScopedInjectableLoader(std::unique_ptr<T>&& instance) {
original_loader_ = InjectableSingleton<T>::getExisting();
InjectableSingleton<T>::clear();
explicit StackedScopedInjectableLoaderForTest(std::unique_ptr<T>&& instance) {
instance_ = std::move(instance);
InjectableSingleton<T>::initialize(instance_.get());
original_loader_ = InjectableSingleton<T>::replaceForTest(instance_.get());
}
~StackedScopedInjectableLoaderForTest() {
InjectableSingleton<T>::replaceForTest(original_loader_);
}
~StackedScopedInjectableLoader() { InjectableSingleton<T>::loader_ = original_loader_; }

private:
std::unique_ptr<T> instance_;
Expand Down
6 changes: 4 additions & 2 deletions test/common/listener_manager/listener_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2604,7 +2604,8 @@ TEST_P(ListenerManagerImplTest, BindToPortEqualToFalse) {
InSequence s;
auto mock_interface = std::make_unique<Network::MockSocketInterface>(
std::vector<Network::Address::IpVersion>{Network::Address::IpVersion::v4});
StackedScopedInjectableLoader<Network::SocketInterface> new_interface(std::move(mock_interface));
StackedScopedInjectableLoaderForTest<Network::SocketInterface> new_interface(
std::move(mock_interface));

ProdListenerComponentFactory real_listener_factory(server_);
EXPECT_CALL(*worker_, start(_, _));
Expand Down Expand Up @@ -2643,7 +2644,8 @@ TEST_P(ListenerManagerImplTest, UpdateBindToPortEqualToFalse) {
InSequence s;
auto mock_interface = std::make_unique<Network::MockSocketInterface>(
std::vector<Network::Address::IpVersion>{Network::Address::IpVersion::v4});
StackedScopedInjectableLoader<Network::SocketInterface> new_interface(std::move(mock_interface));
StackedScopedInjectableLoaderForTest<Network::SocketInterface> new_interface(
std::move(mock_interface));

ProdListenerComponentFactory real_listener_factory(server_);
EXPECT_CALL(*worker_, start(_, _));
Expand Down
4 changes: 2 additions & 2 deletions test/common/network/listen_socket_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ TEST_P(ListenSocketImplTestTcp, SupportedIpFamilyVirtualSocketIsCreatedWithNoBsd
auto any_address = version_ == Address::IpVersion::v4 ? Utility::getIpv4AnyAddress()
: Utility::getIpv6AnyAddress();

StackedScopedInjectableLoader<SocketInterface> new_interface(std::move(mock_interface));
StackedScopedInjectableLoaderForTest<SocketInterface> new_interface(std::move(mock_interface));

{
EXPECT_CALL(*mock_interface_ptr, socket(_, _, _)).Times(0);
Expand All @@ -245,7 +245,7 @@ TEST_P(ListenSocketImplTestTcp, DeathAtUnSupportedIpFamilyListenSocket) {
auto* mock_interface_ptr = mock_interface.get();
auto the_other_address = version_ == Address::IpVersion::v4 ? Utility::getIpv6AnyAddress()
: Utility::getIpv4AnyAddress();
StackedScopedInjectableLoader<SocketInterface> new_interface(std::move(mock_interface));
StackedScopedInjectableLoaderForTest<SocketInterface> new_interface(std::move(mock_interface));
{
EXPECT_CALL(*mock_interface_ptr, socket(_, _, _)).Times(0);
EXPECT_CALL(*mock_interface_ptr, socket(_, _, _, _, _)).Times(0);
Expand Down
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
9 changes: 3 additions & 6 deletions test/integration/socket_interface_swap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
namespace Envoy {

SocketInterfaceSwap::SocketInterfaceSwap(Network::Socket::Type socket_type)
: write_matcher_(std::make_shared<IoHandleMatcher>(socket_type)) {
Envoy::Network::SocketInterfaceSingleton::clear();
test_socket_interface_loader_ = std::make_unique<Envoy::Network::SocketInterfaceLoader>(
std::make_unique<Envoy::Network::TestSocketInterface>(
: write_matcher_(std::make_shared<IoHandleMatcher>(socket_type)),
test_socket_interface_loader_(std::make_unique<Envoy::Network::TestSocketInterface>(
[write_matcher = write_matcher_](Envoy::Network::TestIoSocketHandle* io_handle)
-> absl::optional<Api::IoCallUint64Result> {
Api::IoErrorPtr error_override = write_matcher->returnConnectOverride(io_handle);
Expand All @@ -28,8 +26,7 @@ SocketInterfaceSwap::SocketInterfaceSwap(Network::Socket::Type socket_type)
},
[write_matcher = write_matcher_](Network::IoHandle::RecvMsgOutput& output) {
write_matcher->readOverride(output);
}));
}
})) {}

void SocketInterfaceSwap::IoHandleMatcher::setResumeWrites() {
absl::MutexLock lock(&mutex_);
Expand Down
9 changes: 1 addition & 8 deletions test/integration/socket_interface_swap.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,8 @@ class SocketInterfaceSwap {

explicit SocketInterfaceSwap(Network::Socket::Type socket_type);

~SocketInterfaceSwap() {
test_socket_interface_loader_.reset();
Envoy::Network::SocketInterfaceSingleton::initialize(previous_socket_interface_);
}

Envoy::Network::SocketInterface* const previous_socket_interface_{
Envoy::Network::SocketInterfaceSingleton::getExisting()};
std::shared_ptr<IoHandleMatcher> write_matcher_;
std::unique_ptr<Envoy::Network::SocketInterfaceLoader> test_socket_interface_loader_;
StackedScopedInjectableLoaderForTest<Network::SocketInterface> test_socket_interface_loader_;
};

} // namespace Envoy

0 comments on commit 513d8dd

Please sign in to comment.