From e100f231cc446b96a089e3f36d77181da6aa898f Mon Sep 17 00:00:00 2001 From: Greg Greenway Date: Wed, 26 Jun 2024 06:43:51 -0700 Subject: [PATCH] quic: Pass ServerFactoryContext to server_preferred_address extensions (#34879) This allows getting the `Api`, which is needed to read a `DataSource`. Signed-off-by: Greg Greenway --- .../common/listener_manager/listener_impl.cc | 2 +- source/common/quic/active_quic_listener.cc | 4 +-- source/common/quic/active_quic_listener.h | 4 +-- ..._server_preferred_address_config_factory.h | 4 +-- .../fixed_server_preferred_address_config.cc | 2 +- .../fixed_server_preferred_address_config.h | 2 +- test/common/quic/active_quic_listener_test.cc | 16 +++++++---- .../quic/server_preferred_address/BUILD | 1 + .../fixed_server_preferred_address_test.cc | 28 ++++++++++--------- test/integration/fake_upstream.h | 10 +++++-- 10 files changed, 43 insertions(+), 30 deletions(-) diff --git a/source/common/listener_manager/listener_impl.cc b/source/common/listener_manager/listener_impl.cc index a7608ae899dc..c951a931bd08 100644 --- a/source/common/listener_manager/listener_impl.cc +++ b/source/common/listener_manager/listener_impl.cc @@ -583,7 +583,7 @@ void ListenerImpl::buildUdpListenerFactory(const envoy::config::listener::v3::Li } udp_listener_config_->listener_factory_ = std::make_unique( config.udp_listener_config().quic_options(), concurrency, quic_stat_names_, - validation_visitor_, listener_factory_context_->serverFactoryContext().processContext()); + validation_visitor_, listener_factory_context_->serverFactoryContext()); #if UDP_GSO_BATCH_WRITER_COMPILETIME_SUPPORT // TODO(mattklein123): We should be able to use GSO without QUICHE/QUIC. Right now this causes // non-QUIC integration tests to fail, which I haven't investigated yet. Additionally, from diff --git a/source/common/quic/active_quic_listener.cc b/source/common/quic/active_quic_listener.cc index a6a66495ed08..08aca091dd0c 100644 --- a/source/common/quic/active_quic_listener.cc +++ b/source/common/quic/active_quic_listener.cc @@ -238,7 +238,7 @@ void ActiveQuicListener::closeConnectionsWithFilterChain(const Network::FilterCh ActiveQuicListenerFactory::ActiveQuicListenerFactory( const envoy::config::listener::v3::QuicProtocolOptions& config, uint32_t concurrency, QuicStatNames& quic_stat_names, ProtobufMessage::ValidationVisitor& validation_visitor, - ProcessContextOptRef context) + Server::Configuration::ServerFactoryContext& context) : concurrency_(concurrency), enabled_(config.enabled()), quic_stat_names_(quic_stat_names), packets_to_read_to_connection_count_ratio_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, packets_to_read_to_connection_count_ratio, @@ -294,7 +294,7 @@ ActiveQuicListenerFactory::ActiveQuicListenerFactory( Config::Utility::getAndCheckFactory( config.connection_debug_visitor_config()); if (connection_debug_visitor_factory_.has_value()) { - connection_debug_visitor_factory_->setContext(context_); + connection_debug_visitor_factory_->setContext(context_.processContext()); } } diff --git a/source/common/quic/active_quic_listener.h b/source/common/quic/active_quic_listener.h index baa9de43fd00..b17538966578 100644 --- a/source/common/quic/active_quic_listener.h +++ b/source/common/quic/active_quic_listener.h @@ -108,7 +108,7 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory, ActiveQuicListenerFactory(const envoy::config::listener::v3::QuicProtocolOptions& config, uint32_t concurrency, QuicStatNames& quic_stat_names, ProtobufMessage::ValidationVisitor& validation_visitor, - ProcessContextOptRef context); + Server::Configuration::ServerFactoryContext& context); // Network::ActiveUdpListenerFactory. Network::ConnectionHandler::ActiveUdpListenerPtr @@ -154,7 +154,7 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory, const Network::Socket::OptionsSharedPtr options_{std::make_shared()}; QuicConnectionIdWorkerSelector worker_selector_; bool kernel_worker_routing_{}; - ProcessContextOptRef context_; + Server::Configuration::ServerFactoryContext& context_; static bool disable_kernel_bpf_packet_routing_for_test_; }; diff --git a/source/common/quic/envoy_quic_server_preferred_address_config_factory.h b/source/common/quic/envoy_quic_server_preferred_address_config_factory.h index 3ec363968e08..8882b0dd5e24 100644 --- a/source/common/quic/envoy_quic_server_preferred_address_config_factory.h +++ b/source/common/quic/envoy_quic_server_preferred_address_config_factory.h @@ -3,7 +3,7 @@ #include "envoy/config/typed_config.h" #include "envoy/network/address.h" #include "envoy/protobuf/message_validator.h" -#include "envoy/server/process_context.h" +#include "envoy/server/factory_context.h" #include "quiche/quic/platform/api/quic_socket_address.h" @@ -55,7 +55,7 @@ class EnvoyQuicServerPreferredAddressConfigFactory : public Config::TypedFactory virtual EnvoyQuicServerPreferredAddressConfigPtr createServerPreferredAddressConfig(const Protobuf::Message& config, ProtobufMessage::ValidationVisitor& validation_visitor, - ProcessContextOptRef context) PURE; + Server::Configuration::ServerFactoryContext& context) PURE; }; } // namespace Quic diff --git a/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.cc b/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.cc index b801b6e4b222..b4afcb66d704 100644 --- a/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.cc +++ b/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.cc @@ -122,7 +122,7 @@ FixedServerPreferredAddressConfig::getServerPreferredAddresses( Quic::EnvoyQuicServerPreferredAddressConfigPtr FixedServerPreferredAddressConfigFactory::createServerPreferredAddressConfig( const Protobuf::Message& message, ProtobufMessage::ValidationVisitor& validation_visitor, - ProcessContextOptRef /*context*/) { + Server::Configuration::ServerFactoryContext& /*context*/) { auto& config = MessageUtil::downcastAndValidate(message, diff --git a/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.h b/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.h index 0ec0cc2bc7b8..e93b470734c1 100644 --- a/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.h +++ b/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.h @@ -35,7 +35,7 @@ class FixedServerPreferredAddressConfigFactory Quic::EnvoyQuicServerPreferredAddressConfigPtr createServerPreferredAddressConfig(const Protobuf::Message& message, ProtobufMessage::ValidationVisitor& validation_visitor, - ProcessContextOptRef context) override; + Server::Configuration::ServerFactoryContext& context) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override { return ProtobufTypes::MessagePtr{new envoy::extensions::quic::server_preferred_address::v3:: diff --git a/test/common/quic/active_quic_listener_test.cc b/test/common/quic/active_quic_listener_test.cc index 6270ac638b57..ce68869cf538 100644 --- a/test/common/quic/active_quic_listener_test.cc +++ b/test/common/quic/active_quic_listener_test.cc @@ -25,6 +25,7 @@ #include "test/mocks/network/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/server/instance.h" +#include "test/mocks/server/server_factory_context.h" #include "test/mocks/ssl/mocks.h" #include "test/test_common/environment.h" #include "test/test_common/network_utility.h" @@ -204,7 +205,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam( - options, /*concurrency=*/1, quic_stat_names_, validation_visitor_, absl::nullopt); + options, /*concurrency=*/1, quic_stat_names_, validation_visitor_, context_); } void maybeConfigureMocks(int connection_count) { @@ -342,6 +343,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam context_; TestScopedRuntime scoped_runtime_; Network::Address::IpVersion version_; Event::SimulatedTimeSystemHelper simulated_time_system_; @@ -674,32 +676,34 @@ TEST_P(ActiveQuicListenerEmptyFlagConfigTest, ReceiveFullQuicCHLO) { class ActiveQuicListenerFactoryTest : public testing::Test { protected: std::unique_ptr - createQuicListenerFactory(envoy::config::listener::v3::QuicProtocolOptions options, - ProcessContextOptRef context) { + createQuicListenerFactory(envoy::config::listener::v3::QuicProtocolOptions options) { return std::make_unique(options, /*concurrency=*/1, quic_stat_names_, - validation_visitor_, context); + validation_visitor_, server_context_); } Stats::SymbolTable symbol_table_; QuicStatNames quic_stat_names_ = QuicStatNames(symbol_table_); NiceMock validation_visitor_; + testing::NiceMock server_context_; }; TEST_F(ActiveQuicListenerFactoryTest, NoDebugVisitorConfigured) { envoy::config::listener::v3::QuicProtocolOptions options; - auto factory = createQuicListenerFactory(options, std::nullopt); + auto factory = createQuicListenerFactory(options); EXPECT_EQ(ActiveQuicListenerFactoryPeer::debugVisitorFactory(factory.get()), std::nullopt); } TEST_F(ActiveQuicListenerFactoryTest, DebugVisitorConfigured) { TestProcessObject test_process_object; ProcessContextImpl context(test_process_object); + EXPECT_CALL(server_context_, processContext()) + .WillRepeatedly(Return(ProcessContextOptRef(context))); envoy::config::listener::v3::QuicProtocolOptions quic_config; quic_config.mutable_connection_debug_visitor_config()->set_name( "envoy.quic.connection_debug_visitor.mock"); quic_config.mutable_connection_debug_visitor_config()->mutable_typed_config()->PackFrom( test::common::config::DummyConfig()); - auto listener_factory = createQuicListenerFactory(quic_config, context); + auto listener_factory = createQuicListenerFactory(quic_config); auto debug_visitor_factory = ActiveQuicListenerFactoryPeer::debugVisitorFactory(listener_factory.get()); EXPECT_TRUE(debug_visitor_factory.has_value()); diff --git a/test/extensions/quic/server_preferred_address/BUILD b/test/extensions/quic/server_preferred_address/BUILD index 74eff89da95a..5ae655ec0ce0 100644 --- a/test/extensions/quic/server_preferred_address/BUILD +++ b/test/extensions/quic/server_preferred_address/BUILD @@ -19,5 +19,6 @@ envoy_extension_cc_test( deps = [ "//source/extensions/quic/server_preferred_address:fixed_server_preferred_address_config_lib", "//test/mocks/protobuf:protobuf_mocks", + "//test/mocks/server:server_factory_context_mocks", ], ) diff --git a/test/extensions/quic/server_preferred_address/fixed_server_preferred_address_test.cc b/test/extensions/quic/server_preferred_address/fixed_server_preferred_address_test.cc index 081e4b35b2c3..9c86871ff79a 100644 --- a/test/extensions/quic/server_preferred_address/fixed_server_preferred_address_test.cc +++ b/test/extensions/quic/server_preferred_address/fixed_server_preferred_address_test.cc @@ -2,6 +2,7 @@ #include "source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.h" #include "test/mocks/protobuf/mocks.h" +#include "test/mocks/server/server_factory_context.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -14,6 +15,7 @@ class FixedServerPreferredAddressConfigTest : public ::testing::Test { public: FixedServerPreferredAddressConfigFactory factory_; testing::NiceMock visitor_; + testing::NiceMock context_; }; TEST_F(FixedServerPreferredAddressConfigTest, Validation) { @@ -22,14 +24,14 @@ TEST_F(FixedServerPreferredAddressConfigTest, Validation) { envoy::extensions::quic::server_preferred_address::v3::FixedServerPreferredAddressConfig cfg; cfg.mutable_ipv4_config()->mutable_address()->set_address("not an address"); cfg.mutable_ipv4_config()->mutable_address()->set_port_value(1); - EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, {}), + EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, context_), EnvoyException, ".*Invalid address socket_address.*"); } { // Bad address. envoy::extensions::quic::server_preferred_address::v3::FixedServerPreferredAddressConfig cfg; cfg.set_ipv4_address("not an address"); - EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, {}), + EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, context_), EnvoyException, ".*bad v4 server preferred address: not an address.*"); } { @@ -39,7 +41,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, Validation) { cfg.mutable_ipv4_config()->mutable_address()->set_port_value(1); cfg.mutable_ipv4_config()->mutable_dnat_address()->set_address("127.0.0.1"); cfg.mutable_ipv4_config()->mutable_dnat_address()->set_port_value(1); - EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, {}), + EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, context_), EnvoyException, ".*port must be 0 in this version of Envoy in address '127.0.0.1:1'.*"); } @@ -49,7 +51,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, Validation) { cfg.mutable_ipv4_config()->mutable_dnat_address()->set_address("127.0.0.1"); cfg.mutable_ipv4_config()->mutable_dnat_address()->set_port_value(1); EXPECT_THROW_WITH_REGEX( - factory_.createServerPreferredAddressConfig(cfg, visitor_, {}), EnvoyException, + factory_.createServerPreferredAddressConfig(cfg, visitor_, context_), EnvoyException, ".*'dnat_address' but not 'address' is set in server preferred address for v4.*"); } { @@ -57,7 +59,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, Validation) { envoy::extensions::quic::server_preferred_address::v3::FixedServerPreferredAddressConfig cfg; cfg.mutable_ipv4_config()->mutable_address()->set_address("127.0.0.1"); cfg.mutable_ipv4_config()->mutable_address()->set_port_value(1); - EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, {}), + EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, context_), EnvoyException, ".*'address' port must be zero unless 'dnat_address' is set in address " "127.0.0.1:1 for address family v4.*"); @@ -67,7 +69,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, Validation) { envoy::extensions::quic::server_preferred_address::v3::FixedServerPreferredAddressConfig cfg; cfg.mutable_ipv4_config()->mutable_address()->set_address("::1"); cfg.mutable_ipv4_config()->mutable_address()->set_port_value(1); - EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, {}), + EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, context_), EnvoyException, ".*wrong address type for v4 server preferred address.*"); } @@ -76,7 +78,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, Validation) { envoy::extensions::quic::server_preferred_address::v3::FixedServerPreferredAddressConfig cfg; cfg.mutable_ipv6_config()->mutable_address()->set_address("127.0.0.1"); cfg.mutable_ipv6_config()->mutable_address()->set_port_value(1); - EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, {}), + EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, context_), EnvoyException, ".*wrong address type for v6 server preferred address.*"); } @@ -85,7 +87,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, Validation) { TEST_F(FixedServerPreferredAddressConfigTest, AddressGetsCombinedWithPort) { envoy::extensions::quic::server_preferred_address::v3::FixedServerPreferredAddressConfig cfg; cfg.set_ipv4_address("1.2.3.4"); - auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, {}); + auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, context_); auto addresses = obj->getServerPreferredAddresses( Network::Utility::parseInternetAddressNoThrow("127.0.0.1", 1234)); EXPECT_EQ(addresses.ipv4_.ToString(), "1.2.3.4:1234"); @@ -97,7 +99,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, AddressAndPortIgnoresListenerPort) cfg.mutable_ipv4_config()->mutable_address()->set_port_value(5); cfg.mutable_ipv4_config()->mutable_dnat_address()->set_address("127.0.0.1"); cfg.mutable_ipv4_config()->mutable_dnat_address()->set_port_value(0); - auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, {}); + auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, context_); auto addresses = obj->getServerPreferredAddresses( Network::Utility::parseInternetAddressNoThrow("127.0.0.1", 1234)); EXPECT_EQ(addresses.ipv4_.ToString(), "1.2.3.4:5"); @@ -107,7 +109,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, AddressAndZeroPortUsesListenerPort envoy::extensions::quic::server_preferred_address::v3::FixedServerPreferredAddressConfig cfg; cfg.mutable_ipv4_config()->mutable_address()->set_address("1.2.3.4"); cfg.mutable_ipv4_config()->mutable_address()->set_port_value(0); - auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, {}); + auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, context_); auto addresses = obj->getServerPreferredAddresses( Network::Utility::parseInternetAddressNoThrow("127.0.0.1", 1234)); EXPECT_EQ(addresses.ipv4_.ToString(), "1.2.3.4:1234"); @@ -119,7 +121,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, DnatAddressAndZeroPortUsesListener cfg.mutable_ipv4_config()->mutable_address()->set_port_value(0); cfg.mutable_ipv4_config()->mutable_dnat_address()->set_address("127.0.0.1"); cfg.mutable_ipv4_config()->mutable_dnat_address()->set_port_value(0); - auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, {}); + auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, context_); auto addresses = obj->getServerPreferredAddresses( Network::Utility::parseInternetAddressNoThrow("127.0.0.1", 1234)); EXPECT_EQ(addresses.dnat_ipv4_.ToString(), "127.0.0.1:1234"); @@ -131,7 +133,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, FieldPrecedence) { cfg.set_ipv4_address("2.2.2.2"); cfg.mutable_ipv4_config()->mutable_address()->set_address("1.2.3.4"); cfg.mutable_ipv4_config()->mutable_address()->set_port_value(0); - auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, {}); + auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, context_); auto addresses = obj->getServerPreferredAddresses( Network::Utility::parseInternetAddressNoThrow("127.0.0.1", 1234)); EXPECT_EQ(addresses.ipv4_.ToString(), "1.2.3.4:1234"); @@ -141,7 +143,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, FieldPrecedence) { TEST_F(FixedServerPreferredAddressConfigTest, LegacyField) { envoy::extensions::quic::server_preferred_address::v3::FixedServerPreferredAddressConfig cfg; cfg.set_ipv4_address("2.2.2.2"); - auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, {}); + auto obj = factory_.createServerPreferredAddressConfig(cfg, visitor_, context_); auto addresses = obj->getServerPreferredAddresses( Network::Utility::parseInternetAddressNoThrow("127.0.0.1", 1234)); EXPECT_EQ(addresses.ipv4_.ToString(), "2.2.2.2:1234"); diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index 9fdca002d457..e0649ca967d6 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -863,9 +863,14 @@ class FakeUpstream : Logger::Loggable, listener_info_(std::make_shared>()) { if (is_quic) { #if defined(ENVOY_ENABLE_QUIC) + if (context_ == nullptr) { + // Only initialize this when needed to avoid slowing down non-QUIC integration tests. + context_ = std::make_unique< + testing::NiceMock>(); + } udp_listener_config_.listener_factory_ = std::make_unique( parent_.quic_options_, 1, parent_.quic_stat_names_, parent_.validation_visitor_, - absl::nullopt); + *context_); // Initialize QUICHE flags. quiche::FlagRegistry::getInstance(); #else @@ -926,6 +931,7 @@ class FakeUpstream : Logger::Loggable, const std::vector empty_access_logs_; std::unique_ptr init_manager_; const Network::ListenerInfoConstSharedPtr listener_info_; + std::unique_ptr context_; }; void threadRoutine(); @@ -965,6 +971,7 @@ class FakeUpstream : Logger::Loggable, // Setting this true disables all events and does not re-enable as the above does. bool disable_and_do_not_enable_{}; const bool enable_half_close_; + testing::NiceMock validation_visitor_; FakeListener listener_; const Network::FilterChainSharedPtr filter_chain_; std::list received_datagrams_ ABSL_GUARDED_BY(lock_); @@ -972,7 +979,6 @@ class FakeUpstream : Logger::Loggable, Http::Http1::CodecStats::AtomicPtr http1_codec_stats_; Http::Http2::CodecStats::AtomicPtr http2_codec_stats_; Http::Http3::CodecStats::AtomicPtr http3_codec_stats_; - testing::NiceMock validation_visitor_; #ifdef ENVOY_ENABLE_QUIC Quic::QuicStatNames quic_stat_names_ = Quic::QuicStatNames(stats_store_.symbolTable()); #endif