From 26a28f1b3748b9fce14099e7f9ccdbd24559593d Mon Sep 17 00:00:00 2001 From: Taylor Barrella Date: Sat, 14 Nov 2020 00:58:35 +0000 Subject: [PATCH 1/3] listener: allow setting only a default filter chain Signed-off-by: Taylor Barrella --- source/server/listener_impl.cc | 55 ++++++++++++++--------- test/server/listener_manager_impl_test.cc | 14 ++++++ 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index b62d36dd071b..9d38cde0642c 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -38,15 +38,24 @@ namespace Envoy { namespace Server { namespace { +bool anyFilterChain( + const envoy::config::listener::v3::Listener& config, + std::function predicate) { + if (config.has_default_filter_chain() && predicate(config.default_filter_chain())) { + return true; + } + return std::any_of(config.filter_chains().begin(), config.filter_chains().end(), predicate); +} + bool needTlsInspector(const envoy::config::listener::v3::Listener& config) { - return std::any_of(config.filter_chains().begin(), config.filter_chains().end(), - [](const auto& filter_chain) { - const auto& matcher = filter_chain.filter_chain_match(); - return matcher.transport_protocol() == "tls" || - (matcher.transport_protocol().empty() && - (!matcher.server_names().empty() || - !matcher.application_protocols().empty())); - }) && + return anyFilterChain(config, + [](const auto& filter_chain) { + const auto& matcher = filter_chain.filter_chain_match(); + return matcher.transport_protocol() == "tls" || + (matcher.transport_protocol().empty() && + (!matcher.server_names().empty() || + !matcher.application_protocols().empty())); + }) && !std::any_of( config.listener_filters().begin(), config.listener_filters().end(), [](const auto& filter) { @@ -55,6 +64,12 @@ bool needTlsInspector(const envoy::config::listener::v3::Listener& config) { filter.name() == "envoy.listener.tls_inspector"; }); } + +bool usesProxyProto(const envoy::config::listener::v3::Listener& config) { + return PROTOBUF_GET_WRAPPED_OR_DEFAULT( + config.has_default_filter_chain() ? config.default_filter_chain() : config.filter_chains()[0], + use_proxy_proto, false); +} } // namespace ListenSocketFactoryImpl::ListenSocketFactoryImpl(ListenerComponentFactory& factory, @@ -458,21 +473,22 @@ void ListenerImpl::createListenerFilterFactories(Network::Socket::Type socket_ty } void ListenerImpl::validateFilterChains(Network::Socket::Type socket_type) { - if (config_.filter_chains().empty() && (socket_type == Network::Socket::Type::Stream || - !udp_listener_factory_->isTransportConnectionless())) { + if (config_.filter_chains().empty() && !config_.has_default_filter_chain() && + (socket_type == Network::Socket::Type::Stream || + !udp_listener_factory_->isTransportConnectionless())) { // If we got here, this is a tcp listener or connection-oriented udp listener, so ensure there // is a filter chain specified throw EnvoyException(fmt::format("error adding listener '{}': no filter chains specified", address_->asString())); } else if (udp_listener_factory_ != nullptr && !udp_listener_factory_->isTransportConnectionless()) { - for (auto& filter_chain : config_.filter_chains()) { - // Early fail if any filter chain doesn't have transport socket configured. - if (!filter_chain.has_transport_socket()) { - throw EnvoyException(fmt::format("error adding listener '{}': no transport socket " - "specified for connection oriented UDP listener", - address_->asString())); - } + // Early fail if any filter chain doesn't have transport socket configured. + if (anyFilterChain(config_, [](const auto& filter_chain) { + return !filter_chain.has_transport_socket(); + })) { + throw EnvoyException(fmt::format("error adding listener '{}': no transport socket " + "specified for connection oriented UDP listener", + address_->asString())); } } } @@ -528,7 +544,7 @@ void ListenerImpl::buildProxyProtocolListenerFilter() { // TODO(jrajahalme): This is the last listener filter on purpose. When filter chain matching // is implemented, this needs to be run after the filter chain has been // selected. - if (PROTOBUF_GET_WRAPPED_OR_DEFAULT(config_.filter_chains()[0], use_proxy_proto, false)) { + if (usesProxyProto(config_)) { auto& factory = Config::Utility::getAndCheckFactoryByName( Extensions::ListenerFilters::ListenerFilterNames::get().ProxyProtocol); @@ -714,8 +730,7 @@ bool ListenerImpl::supportUpdateFilterChain(const envoy::config::listener::v3::L // See buildProxyProtocolListenerFilter(). Full listener update guarantees at least 1 filter chain // at tcp listener. - if (PROTOBUF_GET_WRAPPED_OR_DEFAULT(config_.filter_chains()[0], use_proxy_proto, false) ^ - PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.filter_chains()[0], use_proxy_proto, false)) { + if (usesProxyProto(config_) ^ usesProxyProto(config)) { return false; } diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 3d21de26252e..6810d03ff54d 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -371,6 +371,20 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, UdpAddress) { EXPECT_EQ(1u, manager_->listeners().size()); } +TEST_F(ListenerManagerImplWithRealFiltersTest, AllowOnlyDefaultFilterChain) { + const std::string yaml = R"EOF( +address: + socket_address: + address: 127.0.0.1 + port_value: 1234 +default_filter_chain: + filters: [] + )EOF"; + + manager_->addOrUpdateListener(parseListenerFromV3Yaml(yaml), "", true); + EXPECT_EQ(1, manager_->listeners().size()); +} + TEST_F(ListenerManagerImplWithRealFiltersTest, BadListenerConfig) { const std::string yaml = R"EOF( address: From 7d3ed3673db90052ae625d7497884ea77f7c2795 Mon Sep 17 00:00:00 2001 From: Taylor Barrella Date: Mon, 16 Nov 2020 19:30:29 +0000 Subject: [PATCH 2/3] Address comments Signed-off-by: Taylor Barrella --- source/server/listener_impl.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index 9d38cde0642c..d277129dd80f 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -41,10 +41,8 @@ namespace { bool anyFilterChain( const envoy::config::listener::v3::Listener& config, std::function predicate) { - if (config.has_default_filter_chain() && predicate(config.default_filter_chain())) { - return true; - } - return std::any_of(config.filter_chains().begin(), config.filter_chains().end(), predicate); + return (config.has_default_filter_chain() && predicate(config.default_filter_chain())) || + std::any_of(config.filter_chains().begin(), config.filter_chains().end(), predicate); } bool needTlsInspector(const envoy::config::listener::v3::Listener& config) { @@ -728,8 +726,7 @@ bool ListenerImpl::supportUpdateFilterChain(const envoy::config::listener::v3::L return false; } - // See buildProxyProtocolListenerFilter(). Full listener update guarantees at least 1 filter chain - // at tcp listener. + // See buildProxyProtocolListenerFilter(). if (usesProxyProto(config_) ^ usesProxyProto(config)) { return false; } From dda71bb91371511e8178c927f1fba91ffec93ef5 Mon Sep 17 00:00:00 2001 From: Taylor Barrella Date: Thu, 19 Nov 2020 00:52:17 +0000 Subject: [PATCH 3/3] Adjust condition and add comment Signed-off-by: Taylor Barrella --- source/server/listener_impl.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index d277129dd80f..517f6263565e 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -64,8 +64,10 @@ bool needTlsInspector(const envoy::config::listener::v3::Listener& config) { } bool usesProxyProto(const envoy::config::listener::v3::Listener& config) { + // TODO(#14085): `use_proxy_proto` should be deprecated. + // Checking only the first or default filter chain is done for backwards compatibility. return PROTOBUF_GET_WRAPPED_OR_DEFAULT( - config.has_default_filter_chain() ? config.default_filter_chain() : config.filter_chains()[0], + config.filter_chains().empty() ? config.default_filter_chain() : config.filter_chains()[0], use_proxy_proto, false); } } // namespace