diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index c8e227d34b58d..7a480fdd35017 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -114,6 +114,8 @@ New Features * route config: added :ref:`dynamic_metadata ` for routing based on dynamic metadata. * sxg_filter: added filter to transform response to SXG package to :ref:`contrib images `. This can be enabled by setting :ref:`SXG ` configuration. * thrift_proxy: added support for :ref:`mirroring requests `. +* udp: allows updating filter chain in-place through LDS, which is supported by Quic listener. Such listener config will be rejected in other connection-less UDP listener implementations. It can be reverted by ``envoy.reloadable_features.udp_listener_updates_filter_chain_in_place``. +* udp: disallow L4 filter chain in config which configures connection-less UDP listener. It can be reverted by ``envoy.reloadable_features.udp_listener_updates_filter_chain_in_place``. Deprecated ---------- diff --git a/envoy/network/connection_handler.h b/envoy/network/connection_handler.h index beabccddf8cf2..5cab5acdc26bc 100644 --- a/envoy/network/connection_handler.h +++ b/envoy/network/connection_handler.h @@ -132,6 +132,17 @@ class ConnectionHandler { * Stop listening according to implementation's own definition. */ virtual void shutdownListener() PURE; + + /** + * Update the listener config. + */ + virtual void updateListenerConfig(Network::ListenerConfig& config) PURE; + + /** + * Called when the given filter chains are about to be removed. + */ + virtual void onFilterChainDraining( + const std::list& draining_filter_chains) PURE; }; using ActiveListenerPtr = std::unique_ptr; diff --git a/source/common/quic/BUILD b/source/common/quic/BUILD index fed592a8ddb11..f1fd86dd5c2b5 100644 --- a/source/common/quic/BUILD +++ b/source/common/quic/BUILD @@ -265,6 +265,7 @@ envoy_cc_library( ":envoy_quic_utils_lib", ":quic_filter_manager_connection_lib", ":quic_stat_names_lib", + ":quic_transport_socket_factory_lib", "//source/common/buffer:buffer_lib", "//source/common/common:assert_lib", "//source/common/http:codes_lib", diff --git a/source/common/quic/active_quic_listener.cc b/source/common/quic/active_quic_listener.cc index 5bd7651758ada..304ff8bc7d34f 100644 --- a/source/common/quic/active_quic_listener.cc +++ b/source/common/quic/active_quic_listener.cc @@ -221,6 +221,24 @@ size_t ActiveQuicListener::numPacketsExpectedPerEventLoop() const { return quic_dispatcher_->NumSessions() * packets_to_read_to_connection_count_ratio_; } +void ActiveQuicListener::updateListenerConfig(Network::ListenerConfig& config) { + config_ = &config; + dynamic_cast(crypto_config_->proof_source()) + ->updateFilterChainManager(config.filterChainManager()); + quic_dispatcher_->updateListenerConfig(config); +} + +void ActiveQuicListener::onFilterChainDraining( + const std::list& draining_filter_chains) { + for (auto* filter_chain : draining_filter_chains) { + closeConnectionsWithFilterChain(filter_chain); + } +} + +void ActiveQuicListener::closeConnectionsWithFilterChain(const Network::FilterChain* filter_chain) { + quic_dispatcher_->closeConnectionsWithFilterChain(filter_chain); +} + ActiveQuicListenerFactory::ActiveQuicListenerFactory( const envoy::config::listener::v3::QuicProtocolOptions& config, uint32_t concurrency, QuicStatNames& quic_stat_names) diff --git a/source/common/quic/active_quic_listener.h b/source/common/quic/active_quic_listener.h index be72d17334770..eca1f28c0caa8 100644 --- a/source/common/quic/active_quic_listener.h +++ b/source/common/quic/active_quic_listener.h @@ -70,10 +70,15 @@ class ActiveQuicListener : public Envoy::Server::ActiveUdpListenerBase, void pauseListening() override; void resumeListening() override; void shutdownListener() override; + void updateListenerConfig(Network::ListenerConfig& config) override; + void onFilterChainDraining( + const std::list& draining_filter_chains) override; private: friend class ActiveQuicListenerPeer; + void closeConnectionsWithFilterChain(const Network::FilterChain* filter_chain); + uint8_t random_seed_[16]; std::unique_ptr crypto_config_; Event::Dispatcher& dispatcher_; diff --git a/source/common/quic/envoy_quic_client_session.h b/source/common/quic/envoy_quic_client_session.h index def847ea0eb23..a4d650166393f 100644 --- a/source/common/quic/envoy_quic_client_session.h +++ b/source/common/quic/envoy_quic_client_session.h @@ -99,7 +99,8 @@ class EnvoyQuicClientSession : public QuicFilterManagerConnectionImpl, // These callbacks are owned by network filters and quic session should outlive // them. Http::ConnectionCallbacks* http_connection_callbacks_{nullptr}; - const absl::string_view host_name_; + // TODO(danzh) deprecate this field once server_id() is made const. + const std::string host_name_; std::shared_ptr crypto_config_; EnvoyQuicCryptoClientStreamFactoryInterface& crypto_stream_factory_; QuicStatNames& quic_stat_names_; diff --git a/source/common/quic/envoy_quic_dispatcher.cc b/source/common/quic/envoy_quic_dispatcher.cc index 217e561e2b65d..37fd7472c9a6a 100644 --- a/source/common/quic/envoy_quic_dispatcher.cc +++ b/source/common/quic/envoy_quic_dispatcher.cc @@ -2,12 +2,14 @@ #include +#include +#include + #include "envoy/common/optref.h" #include "source/common/common/safe_memcpy.h" #include "source/common/http/utility.h" #include "source/common/quic/envoy_quic_server_connection.h" -#include "source/common/quic/envoy_quic_server_session.h" #include "source/common/quic/envoy_quic_utils.h" namespace Envoy { @@ -26,7 +28,7 @@ EnvoyQuicDispatcher::EnvoyQuicDispatcher( : quic::QuicDispatcher(&quic_config, crypto_config, version_manager, std::move(helper), std::make_unique(), std::move(alarm_factory), expected_server_connection_id_length), - connection_handler_(connection_handler), listener_config_(listener_config), + connection_handler_(connection_handler), listener_config_(&listener_config), listener_stats_(listener_stats), per_worker_stats_(per_worker_stats), dispatcher_(dispatcher), listen_socket_(listen_socket), quic_stat_names_(quic_stat_names), crypto_server_stream_factory_(crypto_server_stream_factory) { @@ -52,19 +54,21 @@ void EnvoyQuicDispatcher::OnConnectionClosed(quic::QuicConnectionId connection_i listener_stats_.downstream_cx_active_.dec(); per_worker_stats_.downstream_cx_active_.dec(); connection_handler_.decNumConnections(); - quic_stat_names_.chargeQuicConnectionCloseStats(listener_config_.listenerScope(), error, source, + quic_stat_names_.chargeQuicConnectionCloseStats(listener_config_->listenerScope(), error, source, /*is_upstream*/ false); } std::unique_ptr EnvoyQuicDispatcher::CreateQuicSession( quic::QuicConnectionId server_connection_id, const quic::QuicSocketAddress& self_address, - const quic::QuicSocketAddress& peer_address, absl::string_view alpn, + const quic::QuicSocketAddress& peer_address, absl::string_view /*alpn*/, const quic::ParsedQuicVersion& version, absl::string_view sni) { quic::QuicConfig quic_config = config(); + // TODO(danzh) use passed-in ALPN instead of hard-coded h3 after proof source interfaces takes in + // ALPN. Network::ConnectionSocketPtr connection_socket = createServerConnectionSocket( - listen_socket_.ioHandle(), self_address, peer_address, std::string(sni), alpn); + listen_socket_.ioHandle(), self_address, peer_address, std::string(sni), "h3"); const Network::FilterChain* filter_chain = - listener_config_.filterChainManager().findFilterChain(*connection_socket); + listener_config_->filterChainManager().findFilterChain(*connection_socket); auto quic_connection = std::make_unique( server_connection_id, self_address, peer_address, *helper(), *alarm_factory(), writer(), @@ -72,24 +76,21 @@ std::unique_ptr EnvoyQuicDispatcher::CreateQuicSession( auto quic_session = std::make_unique( quic_config, quic::ParsedQuicVersionVector{version}, std::move(quic_connection), this, session_helper(), crypto_config(), compressed_certs_cache(), dispatcher_, - listener_config_.perConnectionBufferLimitBytes(), quic_stat_names_, - listener_config_.listenerScope(), crypto_server_stream_factory_, - makeOptRefFromPtr(filter_chain == nullptr ? nullptr - : &filter_chain->transportSocketFactory())); + listener_config_->perConnectionBufferLimitBytes(), quic_stat_names_, + listener_config_->listenerScope(), crypto_server_stream_factory_); if (filter_chain != nullptr) { + // Setup filter chain before Initialize(). const bool has_filter_initialized = - listener_config_.filterChainFactory().createNetworkFilterChain( + listener_config_->filterChainFactory().createNetworkFilterChain( *quic_session, filter_chain->networkFilterFactories()); // QUIC listener must have HCM filter configured. Otherwise, stream creation later will fail. ASSERT(has_filter_initialized); + connections_by_filter_chain_[filter_chain].push_front( + std::reference_wrapper(*quic_session)); + quic_session->storeConnectionMapPosition(connections_by_filter_chain_, *filter_chain, + connections_by_filter_chain_[filter_chain].begin()); } quic_session->Initialize(); - // Filter chain can't be retrieved here as self address is unknown at this - // point. - // TODO(danzh): change QUIC interface to pass in self address as it is already - // known. In this way, filter chain can be retrieved at this point. But one - // thing to pay attention is that if the retrieval fails, connection needs to - // be closed, and it should be added to time wait list instead of session map. connection_handler_.incNumConnections(); listener_stats_.downstream_cx_active_.inc(); listener_stats_.downstream_cx_total_.inc(); @@ -107,5 +108,27 @@ quic::QuicConnectionId EnvoyQuicDispatcher::ReplaceLongServerConnectionId( return new_connection_id; } +void EnvoyQuicDispatcher::closeConnectionsWithFilterChain( + const Network::FilterChain* filter_chain) { + auto iter = connections_by_filter_chain_.find(filter_chain); + if (iter != connections_by_filter_chain_.end()) { + std::list>& connections = iter->second; + // Retain the number of connections in the list early because closing the connection will change + // the size. + const size_t num_connections = connections.size(); + for (size_t i = 0; i < num_connections; ++i) { + Network::Connection& connection = connections.front().get(); + // This will remove the connection from the list. And the last removal will remove connections + // from the map as well. + connection.close(Network::ConnectionCloseType::NoFlush); + } + ASSERT(connections_by_filter_chain_.find(filter_chain) == connections_by_filter_chain_.end()); + } +} + +void EnvoyQuicDispatcher::updateListenerConfig(Network::ListenerConfig& new_listener_config) { + listener_config_ = &new_listener_config; +} + } // namespace Quic } // namespace Envoy diff --git a/source/common/quic/envoy_quic_dispatcher.h b/source/common/quic/envoy_quic_dispatcher.h index b429e908d11be..77ed2ffcb361c 100644 --- a/source/common/quic/envoy_quic_dispatcher.h +++ b/source/common/quic/envoy_quic_dispatcher.h @@ -20,6 +20,7 @@ #include "source/server/connection_handler_impl.h" #include "source/server/active_listener_base.h" #include "source/common/quic/envoy_quic_crypto_stream_factory.h" +#include "source/common/quic/envoy_quic_server_session.h" #include "source/common/quic/quic_stat_names.h" namespace Envoy { @@ -54,6 +55,9 @@ class EnvoyQuicDispatcher : public quic::QuicDispatcher { void OnConnectionClosed(quic::QuicConnectionId connection_id, quic::QuicErrorCode error, const std::string& error_details, quic::ConnectionCloseSource source) override; + void closeConnectionsWithFilterChain(const Network::FilterChain* filter_chain); + + void updateListenerConfig(Network::ListenerConfig& new_listener_config); protected: // quic::QuicDispatcher @@ -72,13 +76,14 @@ class EnvoyQuicDispatcher : public quic::QuicDispatcher { private: Network::ConnectionHandler& connection_handler_; - Network::ListenerConfig& listener_config_; + Network::ListenerConfig* listener_config_{nullptr}; Server::ListenerStats& listener_stats_; Server::PerHandlerListenerStats& per_worker_stats_; Event::Dispatcher& dispatcher_; Network::Socket& listen_socket_; QuicStatNames& quic_stat_names_; EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory_; + FilterChainToConnectionMap connections_by_filter_chain_; }; } // namespace Quic diff --git a/source/common/quic/envoy_quic_proof_source.cc b/source/common/quic/envoy_quic_proof_source.cc index 67d9e0ce3ce55..84019c951fb6b 100644 --- a/source/common/quic/envoy_quic_proof_source.cc +++ b/source/common/quic/envoy_quic_proof_source.cc @@ -100,13 +100,15 @@ EnvoyQuicProofSource::getTlsCertConfigAndFilterChain(const quic::QuicSocketAddre Network::ConnectionSocketPtr connection_socket = createServerConnectionSocket( listen_socket_.ioHandle(), server_address, client_address, hostname, "h3"); const Network::FilterChain* filter_chain = - filter_chain_manager_.findFilterChain(*connection_socket); + filter_chain_manager_->findFilterChain(*connection_socket); if (filter_chain == nullptr) { listener_stats_.no_filter_chain_match_.inc(); ENVOY_LOG(warn, "No matching filter chain found for handshake."); return {absl::nullopt, absl::nullopt}; } + ENVOY_LOG(trace, "Got a matching cert chain {}", filter_chain->name()); + auto& transport_socket_factory = dynamic_cast(filter_chain->transportSocketFactory()); @@ -122,5 +124,10 @@ EnvoyQuicProofSource::getTlsCertConfigAndFilterChain(const quic::QuicSocketAddre return {tls_cert_configs[0].get(), *filter_chain}; } +void EnvoyQuicProofSource::updateFilterChainManager( + Network::FilterChainManager& filter_chain_manager) { + filter_chain_manager_ = &filter_chain_manager; +} + } // namespace Quic } // namespace Envoy diff --git a/source/common/quic/envoy_quic_proof_source.h b/source/common/quic/envoy_quic_proof_source.h index fcf388c609140..8d32ad7176ee1 100644 --- a/source/common/quic/envoy_quic_proof_source.h +++ b/source/common/quic/envoy_quic_proof_source.h @@ -14,7 +14,7 @@ class EnvoyQuicProofSource : public EnvoyQuicProofSourceBase { EnvoyQuicProofSource(Network::Socket& listen_socket, Network::FilterChainManager& filter_chain_manager, Server::ListenerStats& listener_stats) - : listen_socket_(listen_socket), filter_chain_manager_(filter_chain_manager), + : listen_socket_(listen_socket), filter_chain_manager_(&filter_chain_manager), listener_stats_(listener_stats) {} ~EnvoyQuicProofSource() override = default; @@ -24,6 +24,8 @@ class EnvoyQuicProofSource : public EnvoyQuicProofSourceBase { GetCertChain(const quic::QuicSocketAddress& server_address, const quic::QuicSocketAddress& client_address, const std::string& hostname) override; + void updateFilterChainManager(Network::FilterChainManager& filter_chain_manager); + protected: // quic::ProofSource void signPayload(const quic::QuicSocketAddress& server_address, @@ -43,7 +45,7 @@ class EnvoyQuicProofSource : public EnvoyQuicProofSourceBase { const std::string& hostname); Network::Socket& listen_socket_; - Network::FilterChainManager& filter_chain_manager_; + Network::FilterChainManager* filter_chain_manager_{nullptr}; Server::ListenerStats& listener_stats_; }; diff --git a/source/common/quic/envoy_quic_server_session.cc b/source/common/quic/envoy_quic_server_session.cc index 15d1d28745dd3..fc3fe9b2b938f 100644 --- a/source/common/quic/envoy_quic_server_session.cc +++ b/source/common/quic/envoy_quic_server_session.cc @@ -1,5 +1,6 @@ #include "source/common/quic/envoy_quic_server_session.h" +#include #include #include "source/common/common/assert.h" @@ -15,15 +16,14 @@ EnvoyQuicServerSession::EnvoyQuicServerSession( quic::QuicCryptoServerStream::Helper* helper, const quic::QuicCryptoServerConfig* crypto_config, quic::QuicCompressedCertsCache* compressed_certs_cache, Event::Dispatcher& dispatcher, uint32_t send_buffer_limit, QuicStatNames& quic_stat_names, Stats::Scope& listener_scope, - EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory, - OptRef transport_socket_factory) + EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory) : quic::QuicServerSessionBase(config, supported_versions, connection.get(), visitor, helper, crypto_config, compressed_certs_cache), QuicFilterManagerConnectionImpl(*connection, connection->connection_id(), dispatcher, send_buffer_limit), quic_connection_(std::move(connection)), quic_stat_names_(quic_stat_names), - listener_scope_(listener_scope), crypto_server_stream_factory_(crypto_server_stream_factory), - transport_socket_factory_(transport_socket_factory) {} + listener_scope_(listener_scope), crypto_server_stream_factory_(crypto_server_stream_factory) { +} EnvoyQuicServerSession::~EnvoyQuicServerSession() { ASSERT(!quic_connection_->connected()); @@ -39,7 +39,9 @@ EnvoyQuicServerSession::CreateQuicCryptoServerStream( const quic::QuicCryptoServerConfig* crypto_config, quic::QuicCompressedCertsCache* compressed_certs_cache) { return crypto_server_stream_factory_.createEnvoyQuicCryptoServerStream( - crypto_config, compressed_certs_cache, this, stream_helper(), transport_socket_factory_, + crypto_config, compressed_certs_cache, this, stream_helper(), + makeOptRefFromPtr(position_.has_value() ? &position_->filter_chain_.transportSocketFactory() + : nullptr), dispatcher()); } @@ -89,6 +91,17 @@ void EnvoyQuicServerSession::OnConnectionClosed(const quic::QuicConnectionCloseF quic::ConnectionCloseSource source) { quic::QuicServerSessionBase::OnConnectionClosed(frame, source); onConnectionCloseEvent(frame, source, version()); + if (position_.has_value()) { + // Remove this connection from the map. + std::list>& connections = + position_->connection_map_[&position_->filter_chain_]; + connections.erase(position_->iterator_); + if (connections.empty()) { + // Remove the whole entry if this is the last connection using this filter chain. + position_->connection_map_.erase(&position_->filter_chain_); + } + position_.reset(); + } } void EnvoyQuicServerSession::Initialize() { @@ -133,5 +146,11 @@ void EnvoyQuicServerSession::OnRstStream(const quic::QuicRstStreamFrame& frame) /*from_self*/ false, /*is_upstream*/ false); } +void EnvoyQuicServerSession::storeConnectionMapPosition(FilterChainToConnectionMap& connection_map, + const Network::FilterChain& filter_chain, + ConnectionMapIter position) { + position_.emplace(connection_map, filter_chain, position); +} + } // namespace Quic } // namespace Envoy diff --git a/source/common/quic/envoy_quic_server_session.h b/source/common/quic/envoy_quic_server_session.h index a3a804023aef9..313c447101bb4 100644 --- a/source/common/quic/envoy_quic_server_session.h +++ b/source/common/quic/envoy_quic_server_session.h @@ -29,6 +29,25 @@ namespace Envoy { namespace Quic { +using FilterChainToConnectionMap = + absl::flat_hash_map>>; +using ConnectionMapIter = std::list>::iterator; + +// Used to track the matching filter chain and its position in the filter chain to connection map. +struct ConnectionMapPosition { + ConnectionMapPosition(FilterChainToConnectionMap& connection_map, + const Network::FilterChain& filter_chain, ConnectionMapIter iterator) + : connection_map_(connection_map), filter_chain_(filter_chain), iterator_(iterator) {} + + // Stores the map from filter chain of connections. + FilterChainToConnectionMap& connection_map_; + // The matching filter chain of a connection. + const Network::FilterChain& filter_chain_; + // The position of the connection in the map. + ConnectionMapIter iterator_; +}; + // Act as a Network::Connection to HCM and a FilterManager to FilterFactoryCb. // TODO(danzh) Lifetime of quic connection and filter manager connection can be // simplified by changing the inheritance to a member variable instantiated @@ -45,8 +64,7 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase, quic::QuicCompressedCertsCache* compressed_certs_cache, Event::Dispatcher& dispatcher, uint32_t send_buffer_limit, QuicStatNames& quic_stat_names, Stats::Scope& listener_scope, - EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory, - OptRef transport_socket_factory); + EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory); ~EnvoyQuicServerSession() override; @@ -77,6 +95,10 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase, headers_with_underscores_action_ = headers_with_underscores_action; } + void storeConnectionMapPosition(FilterChainToConnectionMap& connection_map, + const Network::FilterChain& filter_chain, + ConnectionMapIter position); + using quic::QuicSession::PerformActionOnActiveStreams; protected: @@ -113,7 +135,7 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase, Stats::Scope& listener_scope_; EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory_; - OptRef transport_socket_factory_; + absl::optional position_; }; } // namespace Quic diff --git a/source/common/quic/quic_filter_manager_connection_impl.cc b/source/common/quic/quic_filter_manager_connection_impl.cc index a2049252c60db..b69a63f2681d1 100644 --- a/source/common/quic/quic_filter_manager_connection_impl.cc +++ b/source/common/quic/quic_filter_manager_connection_impl.cc @@ -12,7 +12,9 @@ QuicFilterManagerConnectionImpl::QuicFilterManagerConnectionImpl( // Using this for purpose other than logging is not safe. Because QUIC connection id can be // 18 bytes, so there might be collision when it's hashed to 8 bytes. : Network::ConnectionImplBase(dispatcher, /*id=*/connection_id.Hash()), - network_connection_(&connection), filter_manager_(*this, *connection.connectionSocket()), + network_connection_(&connection), + filter_manager_( + std::make_unique(*this, *connection.connectionSocket())), stream_info_(dispatcher.timeSource(), connection.connectionSocket()->connectionInfoProviderSharedPtr()), write_buffer_watermark_simulation_( @@ -22,23 +24,23 @@ QuicFilterManagerConnectionImpl::QuicFilterManagerConnectionImpl( } void QuicFilterManagerConnectionImpl::addWriteFilter(Network::WriteFilterSharedPtr filter) { - filter_manager_.addWriteFilter(filter); + filter_manager_->addWriteFilter(filter); } void QuicFilterManagerConnectionImpl::addFilter(Network::FilterSharedPtr filter) { - filter_manager_.addFilter(filter); + filter_manager_->addFilter(filter); } void QuicFilterManagerConnectionImpl::addReadFilter(Network::ReadFilterSharedPtr filter) { - filter_manager_.addReadFilter(filter); + filter_manager_->addReadFilter(filter); } void QuicFilterManagerConnectionImpl::removeReadFilter(Network::ReadFilterSharedPtr filter) { - filter_manager_.removeReadFilter(filter); + filter_manager_->removeReadFilter(filter); } bool QuicFilterManagerConnectionImpl::initializeReadFilters() { - return filter_manager_.initializeReadFilters(); + return filter_manager_->initializeReadFilters(); } void QuicFilterManagerConnectionImpl::enableHalfClose(bool enabled) { @@ -171,6 +173,7 @@ void QuicFilterManagerConnectionImpl::onConnectionCloseEvent( network_connection_ = nullptr; } + filter_manager_ = nullptr; if (!codec_stats_.has_value()) { // The connection was closed before it could be used. Stats are not recorded. return; diff --git a/source/common/quic/quic_filter_manager_connection_impl.h b/source/common/quic/quic_filter_manager_connection_impl.h index 6fabaa5ded3ce..5597ec905f3c6 100644 --- a/source/common/quic/quic_filter_manager_connection_impl.h +++ b/source/common/quic/quic_filter_manager_connection_impl.h @@ -179,7 +179,7 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, // filters are added, ConnectionManagerImpl should always be the last one. // Its onRead() is only called once to trigger ReadFilter::onNewConnection() // and the rest incoming data bypasses these filters. - Network::FilterManagerImpl filter_manager_; + std::unique_ptr filter_manager_; StreamInfo::StreamInfoImpl stream_info_; std::string transport_failure_reason_; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index cf0aeb7ae4807..18fafb77f4781 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -90,6 +90,7 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.strip_port_from_connect", "envoy.reloadable_features.treat_host_like_authority", "envoy.reloadable_features.treat_upstream_connect_timeout_as_connect_failure", + "envoy.reloadable_features.udp_listener_updates_filter_chain_in_place", "envoy.reloadable_features.udp_per_event_loop_read_limit", "envoy.reloadable_features.unquote_log_string_values", "envoy.reloadable_features.upstream_host_weight_change_causes_rebuild", diff --git a/source/server/active_stream_listener_base.h b/source/server/active_stream_listener_base.h index 89ebd2877c659..38e50a1a4af58 100644 --- a/source/server/active_stream_listener_base.h +++ b/source/server/active_stream_listener_base.h @@ -38,8 +38,8 @@ class ActiveStreamListenerBase : public ActiveListenerImplBase, * Schedule to remove and destroy the active connections which are not tracked by listener * config. Caution: The connection are not destroyed yet when function returns. */ - void - deferredRemoveFilterChains(const std::list& draining_filter_chains) { + void onFilterChainDraining( + const std::list& draining_filter_chains) override { // Need to recover the original deleting state. const bool was_deleting = is_deleting_; is_deleting_ = true; diff --git a/source/server/active_tcp_listener.h b/source/server/active_tcp_listener.h index 00d93e744a8d7..0d0299a44914f 100644 --- a/source/server/active_tcp_listener.h +++ b/source/server/active_tcp_listener.h @@ -74,7 +74,7 @@ class ActiveTcpListener final : public Network::TcpListenerCallbacks, * Update the listener config. The follow up connections will see the new config. The existing * connections are not impacted. */ - void updateListenerConfig(Network::ListenerConfig& config); + void updateListenerConfig(Network::ListenerConfig& config) override; Network::TcpConnectionHandler& tcp_conn_handler_; // The number of connections currently active on this listener. This is typically used for diff --git a/source/server/active_udp_listener.h b/source/server/active_udp_listener.h index eef7ca228e738..68918ffc39301 100644 --- a/source/server/active_udp_listener.h +++ b/source/server/active_udp_listener.h @@ -103,6 +103,12 @@ class ActiveRawUdpListener : public ActiveUdpListenerBase, read_filter_.reset(); udp_listener_.reset(); } + // These two are unreachable because a config will be rejected if it configures both this listener + // and any L4 filter chain. + void updateListenerConfig(Network::ListenerConfig&) override { NOT_REACHED_GCOVR_EXCL_LINE; } + void onFilterChainDraining(const std::list&) override { + NOT_REACHED_GCOVR_EXCL_LINE; + } // Network::UdpListenerFilterManager void addReadFilter(Network::UdpListenerReadFilterPtr&& filter) override; diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 4f64afc227b06..4bcdcd9cc16aa 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -27,9 +27,19 @@ void ConnectionHandlerImpl::decNumConnections() { void ConnectionHandlerImpl::addListener(absl::optional overridden_listener, Network::ListenerConfig& config) { + const bool support_udp_in_place_filter_chain_update = Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.udp_listener_updates_filter_chain_in_place"); + if (support_udp_in_place_filter_chain_update && overridden_listener.has_value()) { + ActiveListenerDetailsOptRef listener_detail = + findActiveListenerByTag(overridden_listener.value()); + ASSERT(listener_detail.has_value()); + listener_detail->get().listener_->updateListenerConfig(config); + return; + } + ActiveListenerDetails details; if (config.listenSocketFactory().socketType() == Network::Socket::Type::Stream) { - if (overridden_listener.has_value()) { + if (!support_udp_in_place_filter_chain_update && overridden_listener.has_value()) { for (auto& listener : listeners_) { if (listener.second.listener_->listenerTag() == overridden_listener) { listener.second.tcpListener()->get().updateListenerConfig(config); @@ -89,7 +99,7 @@ void ConnectionHandlerImpl::removeFilterChains( std::function completion) { for (auto& listener : listeners_) { if (listener.second.listener_->listenerTag() == listener_tag) { - listener.second.tcpListener()->get().deferredRemoveFilterChains(filter_chains); + listener.second.listener_->onFilterChainDraining(filter_chains); break; } } diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index c6a666477ce35..bb051e5c78438 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -371,7 +371,7 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin, const envoy::config::listener::v3::Listener& config, const std::string& version_info, ListenerManagerImpl& parent, const std::string& name, bool added_via_api, bool workers_started, - uint64_t hash, uint32_t concurrency) + uint64_t hash) : parent_(parent), address_(origin.address_), bind_to_port_(shouldBindToPort(config)), hand_off_restored_destination_connections_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, use_original_dst, false)), @@ -392,6 +392,7 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin, listener_filters_timeout_( PROTOBUF_GET_MS_OR_DEFAULT(config, listener_filters_timeout, 15000)), continue_on_listener_filters_timeout_(config.continue_on_listener_filters_timeout()), + udp_listener_config_(origin.udp_listener_config_), connection_balancer_(origin.connection_balancer_), listener_factory_context_(std::make_shared( origin.listener_factory_context_->listener_factory_context_base_, this, *this)), @@ -407,18 +408,18 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin, quic_stat_names_(parent_.quicStatNames()) { buildAccessLog(); auto socket_type = Network::Utility::protobufAddressSocketType(config.address()); - // buildUdpListenerFactory() must come before buildListenSocketOptions() because the UDP - // listener factory can provide additional options. - buildUdpListenerFactory(socket_type, concurrency); buildListenSocketOptions(socket_type); createListenerFilterFactories(socket_type); validateFilterChains(socket_type); buildFilterChains(); - // In place update is tcp only so it's safe to apply below tcp only initialization. - buildSocketOptions(); - buildOriginalDstListenerFilter(); - buildProxyProtocolListenerFilter(); - open_connections_ = origin.open_connections_; + + if (socket_type == Network::Socket::Type::Stream) { + // Apply the options below only for TCP. + buildSocketOptions(); + buildOriginalDstListenerFilter(); + buildProxyProtocolListenerFilter(); + open_connections_ = origin.open_connections_; + } } void ListenerImpl::buildAccessLog() { @@ -441,7 +442,7 @@ void ListenerImpl::buildUdpListenerFactory(Network::Socket::Type socket_type, "set concurrency = 1."); } - udp_listener_config_ = std::make_unique(config_.udp_listener_config()); + udp_listener_config_ = std::make_shared(config_.udp_listener_config()); if (config_.udp_listener_config().has_quic_options()) { #ifdef ENVOY_ENABLE_QUIC if (config_.has_connection_balance_config()) { @@ -552,6 +553,15 @@ void ListenerImpl::validateFilterChains(Network::Socket::Type socket_type) { "specified for connection oriented UDP listener", address_->asString())); } + } else if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.udp_listener_updates_filter_chain_in_place") && + (!config_.filter_chains().empty() || config_.has_default_filter_chain()) && + udp_listener_config_ != nullptr && + udp_listener_config_->listener_factory_->isTransportConnectionless()) { + + throw EnvoyException(fmt::format("error adding listener '{}': {} filter chain(s) specified for " + "connection-less UDP listener.", + address_->asString(), config_.filter_chains_size())); } } @@ -767,11 +777,12 @@ bool ListenerImpl::supportUpdateFilterChain(const envoy::config::listener::v3::L return false; } - // Currently we only support TCP filter chain update. - if (Network::Utility::protobufAddressSocketType(config_.address()) != - Network::Socket::Type::Stream || - Network::Utility::protobufAddressSocketType(config.address()) != - Network::Socket::Type::Stream) { + if (!Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.udp_listener_updates_filter_chain_in_place") && + (Network::Utility::protobufAddressSocketType(config_.address()) != + Network::Socket::Type::Stream || + Network::Utility::protobufAddressSocketType(config.address()) != + Network::Socket::Type::Stream)) { return false; } @@ -793,10 +804,10 @@ ListenerImplPtr ListenerImpl::newListenerWithFilterChain(const envoy::config::listener::v3::Listener& config, bool workers_started, uint64_t hash) { // Use WrapUnique since the constructor is private. - return absl::WrapUnique( - new ListenerImpl(*this, config, version_info_, parent_, name_, added_via_api_, - /* new new workers started state */ workers_started, - /* use new hash */ hash, parent_.server_.options().concurrency())); + return absl::WrapUnique(new ListenerImpl(*this, config, version_info_, parent_, name_, + added_via_api_, + /* new new workers started state */ workers_started, + /* use new hash */ hash)); } void ListenerImpl::diffFilterChain(const ListenerImpl& another_listener, diff --git a/source/server/listener_impl.h b/source/server/listener_impl.h index 0114ff9c9e33e..83ed5d562f2a6 100644 --- a/source/server/listener_impl.h +++ b/source/server/listener_impl.h @@ -365,8 +365,7 @@ class ListenerImpl final : public Network::ListenerConfig, */ ListenerImpl(ListenerImpl& origin, const envoy::config::listener::v3::Listener& config, const std::string& version_info, ListenerManagerImpl& parent, - const std::string& name, bool added_via_api, bool workers_started, uint64_t hash, - uint32_t concurrency); + const std::string& name, bool added_via_api, bool workers_started, uint64_t hash); // Helpers for constructor. void buildAccessLog(); void buildUdpListenerFactory(Network::Socket::Type socket_type, uint32_t concurrency); @@ -413,7 +412,7 @@ class ListenerImpl final : public Network::ListenerConfig, Network::Socket::OptionsSharedPtr listen_socket_options_; const std::chrono::milliseconds listener_filters_timeout_; const bool continue_on_listener_filters_timeout_; - std::unique_ptr udp_listener_config_; + std::shared_ptr udp_listener_config_; Network::ConnectionBalancerSharedPtr connection_balancer_; std::shared_ptr listener_factory_context_; FilterChainManagerImpl filter_chain_manager_; diff --git a/test/common/quic/active_quic_listener_test.cc b/test/common/quic/active_quic_listener_test.cc index 06b49bd5cac6a..9f92852df8769 100644 --- a/test/common/quic/active_quic_listener_test.cc +++ b/test/common/quic/active_quic_listener_test.cc @@ -190,12 +190,12 @@ class ActiveQuicListenerTest : public testing::TestWithParam read_filter(new Network::MockReadFilter()); + Network::MockConnectionCallbacks network_connection_callbacks; + testing::StrictMock read_total; + testing::StrictMock read_current; + testing::StrictMock write_total; + testing::StrictMock write_current; + + std::vector filter_factory( + {[&](Network::FilterManager& filter_manager) { + filter_manager.addReadFilter(read_filter); + read_filter->callbacks_->connection().addConnectionCallbacks(network_connection_callbacks); + read_filter->callbacks_->connection().setConnectionStats( + {read_total, read_current, write_total, write_current, nullptr, nullptr}); + }}); + EXPECT_CALL(listener_config_, filterChainManager()).WillOnce(ReturnRef(filter_chain_manager)); + EXPECT_CALL(filter_chain_manager, findFilterChain(_)) + .WillOnce(Return(&proof_source_->filterChain())); + Network::MockTransportSocketFactory transport_socket_factory; + EXPECT_CALL(proof_source_->filterChain(), transportSocketFactory()) + .WillOnce(ReturnRef(transport_socket_factory)); + EXPECT_CALL(proof_source_->filterChain(), networkFilterFactories()) + .WillOnce(ReturnRef(filter_factory)); + EXPECT_CALL(listener_config_, filterChainFactory()); + EXPECT_CALL(listener_config_.filter_chain_factory_, createNetworkFilterChain(_, _)) + .WillOnce(Invoke([](Network::Connection& connection, + const std::vector& filter_factories) { + EXPECT_EQ(1u, filter_factories.size()); + Server::Configuration::FilterChainUtility::buildFilterChain(connection, filter_factories); + return true; + })); + EXPECT_CALL(*read_filter, onNewConnection()) + // Stop iteration to avoid calling getRead/WriteBuffer(). + .WillOnce(Return(Network::FilterStatus::StopIteration)); + + quic::QuicSocketAddress peer_addr(version_ == Network::Address::IpVersion::v4 + ? quic::QuicIpAddress::Loopback4() + : quic::QuicIpAddress::Loopback6(), + 54321); + // Set QuicDispatcher::new_sessions_allowed_per_event_loop_ to + // |kNumSessionsToCreatePerLoopForTests| so that received CHLOs can be + // processed immediately. + envoy_quic_dispatcher_.ProcessBufferedChlos(kNumSessionsToCreatePerLoopForTests); + + processValidChloPacket(peer_addr); + + EXPECT_CALL(network_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); + envoy_quic_dispatcher_.closeConnectionsWithFilterChain(&proof_source_->filterChain()); +} + } // namespace Quic } // namespace Envoy diff --git a/test/common/quic/envoy_quic_proof_source_test.cc b/test/common/quic/envoy_quic_proof_source_test.cc index 75230db0f504f..db54a3f07deaa 100644 --- a/test/common/quic/envoy_quic_proof_source_test.cc +++ b/test/common/quic/envoy_quic_proof_source_test.cc @@ -144,6 +144,7 @@ class EnvoyQuicProofSourceTest : public ::testing::Test { listener_config_.listenerScope(), std::unique_ptr(mock_context_config_)); transport_socket_factory_->initialize(); + EXPECT_CALL(filter_chain_, name()).WillRepeatedly(Return("")); } void expectCertChainAndPrivateKey(const std::string& cert, bool expect_private_key) { diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc index f44b359cc2b78..6ffc8b2ff8038 100644 --- a/test/common/quic/envoy_quic_server_session_test.cc +++ b/test/common/quic/envoy_quic_server_session_test.cc @@ -166,8 +166,7 @@ class EnvoyQuicServerSessionTest : public testing::Test { &compressed_certs_cache_, *dispatcher_, /*send_buffer_limit*/ quic::kDefaultFlowControlSendWindow * 1.5, quic_stat_names_, listener_config_.listenerScope(), - crypto_stream_factory_, - makeOptRefFromPtr(nullptr)), + crypto_stream_factory_), stats_({ALL_HTTP3_CODEC_STATS( POOL_COUNTER_PREFIX(listener_config_.listenerScope(), "http3."), POOL_GAUGE_PREFIX(listener_config_.listenerScope(), "http3."))}) { diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index 5fe18fe47eadb..13d2741bdb4f7 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/test/integration/quic_http_integration_test.cc @@ -87,6 +87,12 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, uint32_t port, const Network::ConnectionSocket::OptionsSharedPtr& options) override { // Setting socket options is not supported. ASSERT(!options); + return makeClientConnectionWithHost(port, ""); + } + + Network::ClientConnectionPtr makeClientConnectionWithHost(uint32_t port, + const std::string& host) { + // Setting socket options is not supported. server_addr_ = Network::Utility::resolveUrl( fmt::format("udp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), port)); Network::Address::InstanceConstSharedPtr local_addr = @@ -103,8 +109,9 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, auto& persistent_info = static_cast(*quic_connection_persistent_info_); auto session = std::make_unique( persistent_info.quic_config_, supported_versions_, std::move(connection), - persistent_info.server_id_, persistent_info.cryptoConfig(), &push_promise_index_, - *dispatcher_, + (host.empty() ? persistent_info.server_id_ + : quic::QuicServerId{host, static_cast(port), false}), + persistent_info.cryptoConfig(), &push_promise_index_, *dispatcher_, // Use smaller window than the default one to have test coverage of client codec buffer // exceeding high watermark. /*send_buffer_limit=*/2 * Http2::Utility::OptionsLimits::MIN_INITIAL_STREAM_WINDOW_SIZE, @@ -119,8 +126,6 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, HttpIntegrationTest::makeRawHttpConnection(std::move(conn), http2_options); if (!codec->disconnected()) { codec->setCodecClientCallbacks(client_codec_callback_); - EXPECT_EQ(transport_socket_factory_->clientContextConfig().serverNameIndication(), - codec->connection()->requestedServerName()); } return codec; } @@ -240,6 +245,8 @@ TEST_P(QuicHttpIntegrationTest, ZeroRtt) { initialize(); // Start the first connection. codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); + EXPECT_EQ(transport_socket_factory_->clientContextConfig().serverNameIndication(), + codec_client_->connection()->requestedServerName()); // Send a complete request on the first connection. auto response1 = codec_client_->makeHeaderOnlyRequest(default_request_headers_); waitForNextUpstreamRequest(0); @@ -436,5 +443,187 @@ TEST_P(QuicHttpIntegrationTest, ResetRequestWithInvalidCharacter) { ASSERT_TRUE(response->waitForReset()); } +class QuicInplaceLdsIntegrationTest : public QuicHttpIntegrationTest { +public: + void inplaceInitialize(bool add_default_filter_chain = false) { + autonomous_upstream_ = true; + setUpstreamCount(2); + + config_helper_.addConfigModifier([add_default_filter_chain]( + envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* filter_chain_0 = + bootstrap.mutable_static_resources()->mutable_listeners(0)->mutable_filter_chains(0); + *filter_chain_0->mutable_filter_chain_match()->mutable_server_names()->Add() = "www.lyft.com"; + auto* filter_chain_1 = bootstrap.mutable_static_resources() + ->mutable_listeners(0) + ->mutable_filter_chains() + ->Add(); + filter_chain_1->MergeFrom(*filter_chain_0); + + // filter chain 1 route to cluster_1 + *filter_chain_1->mutable_filter_chain_match()->mutable_server_names(0) = "lyft.com"; + + filter_chain_0->set_name("filter_chain_0"); + filter_chain_1->set_name("filter_chain_1"); + + auto* config_blob = filter_chain_1->mutable_filters(0)->mutable_typed_config(); + + ASSERT_TRUE(config_blob->Is()); + auto hcm_config = MessageUtil::anyConvert< + envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager>( + *config_blob); + hcm_config.mutable_route_config() + ->mutable_virtual_hosts(0) + ->mutable_routes(0) + ->mutable_route() + ->set_cluster("cluster_1"); + config_blob->PackFrom(hcm_config); + bootstrap.mutable_static_resources()->mutable_clusters()->Add()->MergeFrom( + *bootstrap.mutable_static_resources()->mutable_clusters(0)); + bootstrap.mutable_static_resources()->mutable_clusters(1)->set_name("cluster_1"); + + if (add_default_filter_chain) { + auto default_filter_chain = bootstrap.mutable_static_resources() + ->mutable_listeners(0) + ->mutable_default_filter_chain(); + default_filter_chain->MergeFrom(*filter_chain_0); + default_filter_chain->set_name("filter_chain_default"); + } + }); + + QuicHttpIntegrationTest::initialize(); + } + + void makeRequestAndWaitForResponse(IntegrationCodecClient& codec_client) { + IntegrationStreamDecoderPtr response = + codec_client.makeHeaderOnlyRequest(default_request_headers_); + + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); + EXPECT_FALSE(codec_client.sawGoAway()); + } +}; + +INSTANTIATE_TEST_SUITE_P(QuicHttpIntegrationTests, QuicInplaceLdsIntegrationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +TEST_P(QuicInplaceLdsIntegrationTest, ReloadConfigUpdateNonDefaultFilterChain) { + inplaceInitialize(/*add_default_filter_chain=*/false); + + auto codec_client_0 = + makeHttpConnection(makeClientConnectionWithHost(lookupPort("http"), "www.lyft.com")); + auto codec_client_1 = + makeHttpConnection(makeClientConnectionWithHost(lookupPort("http"), "lyft.com")); + + // Remove filter_chain_1. + ConfigHelper new_config_helper( + version_, *api_, MessageUtil::getJsonStringFromMessageOrDie(config_helper_.bootstrap())); + new_config_helper.addConfigModifier( + [&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { + auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); + listener->mutable_filter_chains()->RemoveLast(); + }); + + new_config_helper.setLds("1"); + test_server_->waitForCounterGe("listener_manager.listener_in_place_updated", 1); + test_server_->waitForGaugeGe("listener_manager.total_filter_chains_draining", 1); + test_server_->waitForGaugeEq("listener_manager.total_filter_chains_draining", 0); + makeRequestAndWaitForResponse(*codec_client_0); + EXPECT_TRUE(codec_client_1->sawGoAway()); + codec_client_1->close(); + + auto codec_client_2 = + makeHttpConnection(makeClientConnectionWithHost(lookupPort("http"), "www.lyft.com")); + makeRequestAndWaitForResponse(*codec_client_2); + codec_client_2->close(); + + // Update filter chain again to add back filter_chain_1. + config_helper_.setLds("1"); + test_server_->waitForCounterGe("listener_manager.listener_in_place_updated", 2); + test_server_->waitForCounterGe("listener_manager.listener_create_success", 3); + + auto codec_client_3 = + makeHttpConnection(makeClientConnectionWithHost(lookupPort("http"), "lyft.com")); + makeRequestAndWaitForResponse(*codec_client_3); + makeRequestAndWaitForResponse(*codec_client_0); + codec_client_0->close(); + codec_client_3->close(); +} + +// Verify that the connection received GO_AWAY after its filter chain gets deleted during the +// listener update. +TEST_P(QuicInplaceLdsIntegrationTest, ReloadConfigUpdateDefaultFilterChain) { + inplaceInitialize(/*add_default_filter_chain=*/true); + + auto codec_client_0 = + makeHttpConnection(makeClientConnectionWithHost(lookupPort("http"), "www.lyft.com")); + + // Remove filter_chain_1. + ConfigHelper new_config_helper( + version_, *api_, MessageUtil::getJsonStringFromMessageOrDie(config_helper_.bootstrap())); + new_config_helper.addConfigModifier( + [&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { + auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); + listener->mutable_filter_chains()->RemoveLast(); + }); + + new_config_helper.setLds("1"); + test_server_->waitForCounterGe("listener_manager.listener_in_place_updated", 1); + test_server_->waitForGaugeGe("listener_manager.total_filter_chains_draining", 1); + + test_server_->waitForGaugeEq("listener_manager.total_filter_chains_draining", 0); + // This connection should pick up the default filter chain. + auto codec_client_default = + makeHttpConnection(makeClientConnectionWithHost(lookupPort("http"), "lyft.com")); + makeRequestAndWaitForResponse(*codec_client_default); + makeRequestAndWaitForResponse(*codec_client_0); + + // Modify the default filter chain. + ConfigHelper new_config_helper1( + version_, *api_, MessageUtil::getJsonStringFromMessageOrDie(new_config_helper.bootstrap())); + new_config_helper1.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) + -> void { + auto default_filter_chain = + bootstrap.mutable_static_resources()->mutable_listeners(0)->mutable_default_filter_chain(); + default_filter_chain->set_name("default_filter_chain_v3"); + }); + + new_config_helper1.setLds("1"); + test_server_->waitForCounterGe("listener_manager.listener_in_place_updated", 2); + test_server_->waitForGaugeGe("listener_manager.total_filter_chains_draining", 1); + test_server_->waitForGaugeEq("listener_manager.total_filter_chains_draining", 0); + + makeRequestAndWaitForResponse(*codec_client_0); + EXPECT_TRUE(codec_client_default->sawGoAway()); + codec_client_default->close(); + + // This connection should pick up the new default filter chain. + auto codec_client_1 = + makeHttpConnection(makeClientConnectionWithHost(lookupPort("http"), "lyft.com")); + makeRequestAndWaitForResponse(*codec_client_1); + + // Remove the default filter chain. + ConfigHelper new_config_helper2( + version_, *api_, MessageUtil::getJsonStringFromMessageOrDie(new_config_helper1.bootstrap())); + new_config_helper2.addConfigModifier( + [&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { + auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); + listener->clear_default_filter_chain(); + }); + + new_config_helper2.setLds("1"); + test_server_->waitForCounterGe("listener_manager.listener_in_place_updated", 3); + test_server_->waitForGaugeGe("listener_manager.total_filter_chains_draining", 1); + test_server_->waitForGaugeEq("listener_manager.total_filter_chains_draining", 0); + + makeRequestAndWaitForResponse(*codec_client_0); + codec_client_0->close(); + EXPECT_TRUE(codec_client_1->sawGoAway()); + codec_client_1->close(); +} + } // namespace Quic } // namespace Envoy diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 442c38adc9999..5de495694bfc1 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -216,13 +216,13 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggablesocket_factory_, socketType()).WillOnce(Return(socket_type)); if (listener == nullptr) { // Expecting listener config in place update. // If so, dispatcher would not create new network listener. return listeners_.back().get(); } + EXPECT_CALL(listeners_.back()->socket_factory_, socketType()).WillOnce(Return(socket_type)); EXPECT_CALL(listeners_.back()->socket_factory_, getListenSocket(_)) .WillOnce(Return(listeners_.back()->socket_)); if (socket_type == Network::Socket::Type::Stream) { diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 43f0e9b0269e7..8bf8e13880543 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -297,7 +297,6 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, UdpAddress) { port_value: 1234 } } - filter_chains: {} )EOF"; envoy::config::listener::v3::Listener listener_proto; EXPECT_TRUE(Protobuf::TextFormat::ParseFromString(proto_text, &listener_proto)); @@ -389,6 +388,22 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, BadFilterConfig) { EXPECT_THROW_WITH_REGEX(manager_->addOrUpdateListener(parseListenerFromV3Yaml(yaml), "", true), EnvoyException, "foo: Cannot find field"); } + +TEST_F(ListenerManagerImplWithRealFiltersTest, BadConnectionLessUdpConfigWithFilterChain) { + const std::string yaml = R"EOF( +address: + socket_address: + protocol: UDP + address: 127.0.0.1 + port_value: 1234 +filter_chains: {} + )EOF"; + + EXPECT_THROW_WITH_REGEX(manager_->addOrUpdateListener(parseListenerFromV3Yaml(yaml), "", true), + EnvoyException, + "1 filter chain\\(s\\) specified for connection-less UDP listener"); +} + class NonTerminalFilterFactory : public Configuration::NamedNetworkFilterConfigFactory { public: // Configuration::NamedNetworkFilterConfigFactory @@ -5102,9 +5117,9 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfWo EXPECT_EQ(0, server_.stats_store_.counter("listener_manager.listener_in_place_updated").value()); } -// This case also verifies that listeners that share port but do not share socket type (TCP vs. UDP) +// This case verifies that listeners that share port but do not share socket type (TCP vs. UDP) // do not share a listener. -TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfAnyListenerIsNotTcp) { +TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfDifferentSocketType) { EXPECT_CALL(*worker_, start(_, _)); manager_->startWorkers(guard_dog_, callback_.AsStdFunction()); @@ -5117,12 +5132,15 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfAn auto new_listener_proto = listener_proto; new_listener_proto.mutable_address()->mutable_socket_address()->set_protocol( envoy::config::core::v3::SocketAddress_Protocol::SocketAddress_Protocol_UDP); + EXPECT_CALL(server_.validation_context_, staticValidationVisitor()).Times(0); + EXPECT_CALL(server_.validation_context_, dynamicValidationVisitor()); + EXPECT_CALL(listener_factory_, createDrainManager_(_)); + EXPECT_THROW_WITH_MESSAGE(manager_->addOrUpdateListener(new_listener_proto, "", true), + EnvoyException, + "error adding listener '127.0.0.1:1234': 1 filter chain(s) specified " + "for connection-less UDP listener."); - ListenerHandle* listener_foo_update1 = expectListenerCreate(false, true); - expectUpdateToThenDrain(new_listener_proto, listener_foo, OptRef(), - ListenerComponentFactory::BindType::ReusePort); - expectRemove(new_listener_proto, listener_foo_update1, *listener_factory_.socket_); - + expectRemove(new_listener_proto, listener_foo, *listener_factory_.socket_); EXPECT_EQ(0UL, manager_->listeners().size()); EXPECT_EQ(0, server_.stats_store_.counter("listener_manager.listener_in_place_updated").value()); } @@ -5204,8 +5222,6 @@ TEST_F(ListenerManagerImplTest, UdpDefaultWriterConfig) { address: 127.0.0.1 protocol: UDP port_value: 1234 -filter_chains: - filters: [] )EOF"); manager_->addOrUpdateListener(listener, "", true); EXPECT_EQ(1U, manager_->listeners().size());