quic: Pass ServerFactoryContext to server_preferred_address extensions (

envoyproxy#34879) This allows getting the `Api`, which is needed to read a `DataSource`. Signed-off-by: Greg Greenway <[email protected]>
datawire · Jun 26, 2024 · e100f23 · e100f23
1 parent 2d9caa0
commit e100f23
Show file tree

Hide file tree

Showing 10 changed files with 43 additions and 30 deletions.
diff --git 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<Quic::ActiveQuicListenerFactory>(
         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
@@ -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<EnvoyQuicConnectionDebugVisitorFactoryInterface>(
             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
@@ -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<Network::Socket::Options>()};
   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
@@ -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
@@ -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<const envoy::extensions::quic::server_preferred_address::v3::
                                            FixedServerPreferredAddressConfig&>(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
@@ -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
@@ -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<Network::Address::I
     envoy::config::listener::v3::QuicProtocolOptions options;
     TestUtility::loadFromYamlAndValidate(yaml, options);
     return std::make_unique<TestActiveQuicListenerFactory>(
-        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<Network::Address::I
                        handshake_timeout_);
   }
 
+  testing::NiceMock<Server::Configuration::MockServerFactoryContext> 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<ActiveQuicListenerFactory>
-  createQuicListenerFactory(envoy::config::listener::v3::QuicProtocolOptions options,
-                            ProcessContextOptRef context) {
+  createQuicListenerFactory(envoy::config::listener::v3::QuicProtocolOptions options) {
     return std::make_unique<ActiveQuicListenerFactory>(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<ProtobufMessage::MockValidationVisitor> validation_visitor_;
+  testing::NiceMock<Server::Configuration::MockServerFactoryContext> 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
@@ -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
@@ -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<ProtobufMessage::MockValidationVisitor> visitor_;
+  testing::NiceMock<Server::Configuration::MockServerFactoryContext> 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,15 +51,15 @@ 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.*");
   }
   {
     // Cannot set port on address if dnat address isn't set.
     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
@@ -863,9 +863,14 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
           listener_info_(std::make_shared<testing::NiceMock<Network::MockListenerInfo>>()) {
       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<Server::Configuration::MockServerFactoryContext>>();
+        }
         udp_listener_config_.listener_factory_ = std::make_unique<Quic::ActiveQuicListenerFactory>(
             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<Logger::Id::testing>,
     const std::vector<AccessLog::InstanceSharedPtr> empty_access_logs_;
     std::unique_ptr<Init::Manager> init_manager_;
     const Network::ListenerInfoConstSharedPtr listener_info_;
+    std::unique_ptr<Server::Configuration::MockServerFactoryContext> context_;
   };
 
   void threadRoutine();
@@ -965,14 +971,14 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
   // 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<ProtobufMessage::MockValidationVisitor> validation_visitor_;
   FakeListener listener_;
   const Network::FilterChainSharedPtr filter_chain_;
   std::list<Network::UdpRecvData> received_datagrams_ ABSL_GUARDED_BY(lock_);
   Stats::ScopeSharedPtr stats_scope_;
   Http::Http1::CodecStats::AtomicPtr http1_codec_stats_;
   Http::Http2::CodecStats::AtomicPtr http2_codec_stats_;
   Http::Http3::CodecStats::AtomicPtr http3_codec_stats_;
-  testing::NiceMock<ProtobufMessage::MockValidationVisitor> validation_visitor_;
 #ifdef ENVOY_ENABLE_QUIC
   Quic::QuicStatNames quic_stat_names_ = Quic::QuicStatNames(stats_store_.symbolTable());
 #endif