Refactor debug visitor factory so that it receives a

ListenerFactoryContext and can do one-time setup when the listener is
created and it's config

Signed-off-by: Greg Greenway <[email protected]>

source/common/listener_manager/listener_impl.cc

@@ -636,7 +636,7 @@ ListenerImpl::buildUdpListenerFactory(const envoy::config::listener::v3::Listene
     }
     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());
+        validation_visitor_, *listener_factory_context_);
 #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

source/common/quic/BUILD

@@ -65,7 +65,7 @@ envoy_cc_library(
         "//envoy/common:optref_lib",
         "//envoy/common:pure_lib",
         "//envoy/config:typed_config_interface",
-        "//envoy/server:process_context_interface",
+        "//envoy/server:factory_context_interface",
         "//envoy/stream_info:stream_info_interface",
         "@com_github_google_quiche//:quic_core_connection_lib",
         "@com_github_google_quiche//:quic_core_session_lib",

source/common/quic/active_quic_listener.cc

@@ -97,7 +97,7 @@ ActiveQuicListener::ActiveQuicListener(
       crypto_config_.get(), quic_config, &version_manager_, std::move(connection_helper),
       std::move(alarm_factory), quic::kQuicDefaultConnectionIdLength, parent, *config_, stats_,
       per_worker_stats_, dispatcher, listen_socket_, quic_stat_names, crypto_server_stream_factory_,
-      *connection_id_generator_, std::move(debug_visitor_factory));
+      *connection_id_generator_, debug_visitor_factory);
 
   // Create udp_packet_writer
   Network::UdpPacketWriterPtr udp_packet_writer =
@@ -261,7 +261,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,
-    Server::Configuration::ServerFactoryContext& context)
+    Server::Configuration::ListenerFactoryContext& 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,
@@ -313,12 +313,13 @@ ActiveQuicListenerFactory::ActiveQuicListenerFactory(
 
   // Initialize connection debug visitor factory if one is configured.
   if (config.has_connection_debug_visitor_config()) {
-    connection_debug_visitor_factory_ =
-        Config::Utility::getAndCheckFactory<EnvoyQuicConnectionDebugVisitorFactoryInterface>(
+    auto& factory =
+        Config::Utility::getAndCheckFactory<EnvoyQuicConnectionDebugVisitorFactoryFactoryInterface>(
             config.connection_debug_visitor_config());
-    if (connection_debug_visitor_factory_.has_value()) {
-      connection_debug_visitor_factory_->setContext(context_.processContext());
-    }
+    ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig(
+        config.connection_debug_visitor_config().typed_config(),
+        context_.messageValidationVisitor(), factory);
+    connection_debug_visitor_factory_ = factory.createFactory(*message, context_);
   }
 
   // Initialize connection ID generator factory.
@@ -349,7 +350,7 @@ ActiveQuicListenerFactory::ActiveQuicListenerFactory(
             *Config::Utility::translateToFactoryConfig(config.server_preferred_address_config(),
                                                        validation_visitor,
                                                        server_preferred_address_config_factory),
-            validation_visitor, context_);
+            validation_visitor, context_.serverFactoryContext());
   }
 
   worker_selector_ =
@@ -437,7 +438,7 @@ ActiveQuicListenerFactory::createActiveQuicListener(
       listener_config, quic_config, kernel_worker_routing, enabled, quic_stat_names,
       packets_to_read_to_connection_count_ratio, receive_ecn_, crypto_server_stream_factory,
       proof_source_factory, std::move(cid_generator), worker_selector_,
-      connection_debug_visitor_factory_, reject_new_connections_);
+      makeOptRefFromPtr(connection_debug_visitor_factory_.get()), reject_new_connections_);
 }
 
 } // namespace Quic

source/common/quic/active_quic_listener.h

@@ -113,7 +113,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,
-                            Server::Configuration::ServerFactoryContext& context);
+                            Server::Configuration::ListenerFactoryContext& context);
 
   // Network::ActiveUdpListenerFactory.
   Network::ConnectionHandler::ActiveUdpListenerPtr
@@ -147,7 +147,7 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory,
       crypto_server_stream_factory_;
   absl::optional<std::reference_wrapper<EnvoyQuicProofSourceFactoryInterface>>
       proof_source_factory_;
-  EnvoyQuicConnectionDebugVisitorFactoryInterfaceOptRef connection_debug_visitor_factory_;
+  EnvoyQuicConnectionDebugVisitorFactoryInterfacePtr connection_debug_visitor_factory_;
   EnvoyQuicConnectionIdGeneratorFactoryPtr quic_cid_generator_factory_;
   EnvoyQuicServerPreferredAddressConfigPtr server_preferred_address_config_;
   quic::QuicConfig quic_config_;
@@ -159,7 +159,7 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory,
   const Network::Socket::OptionsSharedPtr options_{std::make_shared<Network::Socket::Options>()};
   QuicConnectionIdWorkerSelector worker_selector_;
   bool kernel_worker_routing_{};
-  Server::Configuration::ServerFactoryContext& context_;
+  Server::Configuration::ListenerFactoryContext& context_;
   bool reject_new_connections_{};
 
   static bool disable_kernel_bpf_packet_routing_for_test_;

source/common/quic/envoy_quic_connection_debug_visitor_factory_interface.h

@@ -3,10 +3,9 @@
 #include <memory>
 #include <string>
 
-#include "envoy/common/optref.h"
 #include "envoy/common/pure.h"
 #include "envoy/config/typed_config.h"
-#include "envoy/server/process_context.h"
+#include "envoy/server/factory_context.h"
 #include "envoy/stream_info/stream_info.h"
 
 #include "quiche/quic/core/quic_connection.h"
@@ -15,23 +14,29 @@
 namespace Envoy {
 namespace Quic {
 
-class EnvoyQuicConnectionDebugVisitorFactoryInterface : public Config::TypedFactory {
+class EnvoyQuicConnectionDebugVisitorFactoryInterface {
 public:
-  std::string category() const override { return "envoy.quic.connection_debug_visitor"; }
-
-  void setContext(Envoy::ProcessContextOptRef context) { context_ = context; }
+  virtual ~EnvoyQuicConnectionDebugVisitorFactoryInterface() = default;
 
   // Returns a debug visitor to be attached to a Quic Connection.
   virtual std::unique_ptr<quic::QuicConnectionDebugVisitor>
-  createQuicConnectionDebugVisitor(quic::QuicSession* session,
+  createQuicConnectionDebugVisitor(Event::Dispatcher& dispatcher, quic::QuicSession* session,
                                    const StreamInfo::StreamInfo& stream_info) PURE;
-
-protected:
-  Envoy::ProcessContextOptRef context_;
 };
 
+using EnvoyQuicConnectionDebugVisitorFactoryInterfacePtr =
+    std::unique_ptr<EnvoyQuicConnectionDebugVisitorFactoryInterface>;
 using EnvoyQuicConnectionDebugVisitorFactoryInterfaceOptRef =
     OptRef<EnvoyQuicConnectionDebugVisitorFactoryInterface>;
 
+class EnvoyQuicConnectionDebugVisitorFactoryFactoryInterface : public Config::TypedFactory {
+public:
+  std::string category() const override { return "envoy.quic.connection_debug_visitor"; }
+
+  virtual EnvoyQuicConnectionDebugVisitorFactoryInterfacePtr
+  createFactory(const Protobuf::Message& config,
+                Server::Configuration::ListenerFactoryContext& listener_context) PURE;
+};
+
 } // namespace Quic
 } // namespace Envoy

source/common/quic/envoy_quic_dispatcher.cc

@@ -53,7 +53,7 @@ EnvoyQuicDispatcher::EnvoyQuicDispatcher(
     Network::Socket& listen_socket, QuicStatNames& quic_stat_names,
     EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory,
     quic::ConnectionIdGeneratorInterface& generator,
-    EnvoyQuicConnectionDebugVisitorFactoryInterfaceOptRef&& debug_visitor_factory)
+    EnvoyQuicConnectionDebugVisitorFactoryInterfaceOptRef debug_visitor_factory)
     : quic::QuicDispatcher(&quic_config, crypto_config, version_manager, std::move(helper),
                            std::make_unique<EnvoyQuicCryptoServerStreamHelper>(),
                            std::move(alarm_factory), expected_server_connection_id_length,
@@ -65,7 +65,7 @@ EnvoyQuicDispatcher::EnvoyQuicDispatcher(
       quic_stats_(generateStats(listener_config.listenerScope())),
       connection_stats_({QUIC_CONNECTION_STATS(
           POOL_COUNTER_PREFIX(listener_config.listenerScope(), "quic.connection"))}),
-      debug_visitor_factory_(std::move(debug_visitor_factory)) {}
+      debug_visitor_factory_(debug_visitor_factory) {}
 
 void EnvoyQuicDispatcher::OnConnectionClosed(quic::QuicConnectionId connection_id,
                                              quic::QuicErrorCode error,

source/common/quic/envoy_quic_dispatcher.h

@@ -65,7 +65,7 @@ class EnvoyQuicDispatcher : public quic::QuicDispatcher {
       Network::Socket& listen_socket, QuicStatNames& quic_stat_names,
       EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory,
       quic::ConnectionIdGeneratorInterface& generator,
-      EnvoyQuicConnectionDebugVisitorFactoryInterfaceOptRef&& debug_visitor_factory);
+      EnvoyQuicConnectionDebugVisitorFactoryInterfaceOptRef debug_visitor_factory);
 
   // quic::QuicDispatcher
   void OnConnectionClosed(quic::QuicConnectionId connection_id, quic::QuicErrorCode error,

source/common/quic/envoy_quic_server_session.cc

@@ -55,7 +55,8 @@ EnvoyQuicServerSession::EnvoyQuicServerSession(
 #endif
   // If a factory is available, create a debug visitor and attach it to the connection.
   if (debug_visitor_factory.has_value()) {
-    debug_visitor_ = debug_visitor_factory->createQuicConnectionDebugVisitor(this, streamInfo());
+    debug_visitor_ =
+        debug_visitor_factory->createQuicConnectionDebugVisitor(dispatcher, this, streamInfo());
     quic_connection_->set_debug_visitor(debug_visitor_.get());
   }
   quic_connection_->set_context_listener(

source/extensions/quic/connection_debug_visitor/basic/envoy_quic_connection_debug_visitor_basic.cc

@@ -23,12 +23,12 @@ void EnvoyQuicConnectionDebugVisitorBasic::OnConnectionClosed(
 
 std::unique_ptr<quic::QuicConnectionDebugVisitor>
 EnvoyQuicConnectionDebugVisitorFactoryBasic::createQuicConnectionDebugVisitor(
-    quic::QuicSession* session, const StreamInfo::StreamInfo& stream_info) {
+    Event::Dispatcher&, quic::QuicSession* session, const StreamInfo::StreamInfo& stream_info) {
   return std::make_unique<EnvoyQuicConnectionDebugVisitorBasic>(session, stream_info);
 }
 
-REGISTER_FACTORY(EnvoyQuicConnectionDebugVisitorFactoryBasic,
-                 EnvoyQuicConnectionDebugVisitorFactoryInterface);
+REGISTER_FACTORY(EnvoyQuicConnectionDebugVisitorFactoryFactoryBasic,
+                 EnvoyQuicConnectionDebugVisitorFactoryFactoryInterface);
 
 } // namespace Quic
 } // namespace Envoy

source/extensions/quic/connection_debug_visitor/basic/envoy_quic_connection_debug_visitor_basic.h

@@ -41,16 +41,24 @@ class EnvoyQuicConnectionDebugVisitorBasic : public quic::QuicConnectionDebugVis
 
 class EnvoyQuicConnectionDebugVisitorFactoryBasic
     : public EnvoyQuicConnectionDebugVisitorFactoryInterface {
+  std::unique_ptr<quic::QuicConnectionDebugVisitor>
+  createQuicConnectionDebugVisitor(Event::Dispatcher&, quic::QuicSession* session,
+                                   const StreamInfo::StreamInfo& stream_info) override;
+};
+
+class EnvoyQuicConnectionDebugVisitorFactoryFactoryBasic
+    : public EnvoyQuicConnectionDebugVisitorFactoryFactoryInterface {
 public:
   std::string name() const override { return "envoy.quic.connection_debug_visitor.basic"; }
 
   Envoy::ProtobufTypes::MessagePtr createEmptyConfigProto() override {
     return std::make_unique<envoy::extensions::quic::connection_debug_visitor::v3::BasicConfig>();
   }
 
-  std::unique_ptr<quic::QuicConnectionDebugVisitor>
-  createQuicConnectionDebugVisitor(quic::QuicSession* session,
-                                   const StreamInfo::StreamInfo& stream_info) override;
+  EnvoyQuicConnectionDebugVisitorFactoryInterfacePtr
+  createFactory(const Protobuf::Message&, Server::Configuration::ListenerFactoryContext&) override {
+    return std::make_unique<EnvoyQuicConnectionDebugVisitorFactoryBasic>();
+  }
 };
 
 DECLARE_FACTORY(EnvoyQuicConnectionDebugVisitorFactoryBasic);

test/common/quic/BUILD

@@ -244,6 +244,7 @@ envoy_cc_test(
         "//source/server:process_context_lib",
         "//test/mocks/network:network_mocks",
         "//test/mocks/server:instance_mocks",
+        "//test/mocks/server:listener_factory_context_mocks",
         "//test/test_common:network_utility_lib",
         "//test/test_common:simulated_time_system_lib",
         "//test/test_common:test_runtime_lib",

test/common/quic/active_quic_listener_test.cc

@@ -25,7 +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/server/listener_factory_context.h"
 #include "test/mocks/ssl/mocks.h"
 #include "test/test_common/environment.h"
 #include "test/test_common/network_utility.h"
@@ -118,7 +118,7 @@ class ActiveQuicListenerFactoryPeer {
   }
   static EnvoyQuicConnectionDebugVisitorFactoryInterfaceOptRef
   debugVisitorFactory(ActiveQuicListenerFactory* factory) {
-    return factory->connection_debug_visitor_factory_;
+    return makeOptRefFromPtr(factory->connection_debug_visitor_factory_.get());
   }
 };
 
@@ -365,7 +365,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam<Network::Address::I
                        handshake_timeout_);
   }
 
-  testing::NiceMock<Server::Configuration::MockServerFactoryContext> context_;
+  testing::NiceMock<Server::Configuration::MockListenerFactoryContext> context_;
   TestScopedRuntime scoped_runtime_;
   Network::Address::IpVersion version_;
   Event::SimulatedTimeSystemHelper simulated_time_system_;
@@ -727,13 +727,13 @@ class ActiveQuicListenerFactoryTest : public testing::Test {
   std::unique_ptr<ActiveQuicListenerFactory>
   createQuicListenerFactory(envoy::config::listener::v3::QuicProtocolOptions options) {
     return std::make_unique<ActiveQuicListenerFactory>(options, /*concurrency=*/1, quic_stat_names_,
-                                                       validation_visitor_, server_context_);
+                                                       validation_visitor_, listener_context_);
   }
 
   Stats::SymbolTable symbol_table_;
   QuicStatNames quic_stat_names_ = QuicStatNames(symbol_table_);
   NiceMock<ProtobufMessage::MockValidationVisitor> validation_visitor_;
-  testing::NiceMock<Server::Configuration::MockServerFactoryContext> server_context_;
+  testing::NiceMock<Server::Configuration::MockListenerFactoryContext> listener_context_;
 };
 
 TEST_F(ActiveQuicListenerFactoryTest, NoDebugVisitorConfigured) {
@@ -743,10 +743,6 @@ TEST_F(ActiveQuicListenerFactoryTest, NoDebugVisitorConfigured) {
 }
 
 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");
@@ -756,10 +752,6 @@ TEST_F(ActiveQuicListenerFactoryTest, DebugVisitorConfigured) {
   auto debug_visitor_factory =
       ActiveQuicListenerFactoryPeer::debugVisitorFactory(listener_factory.get());
   EXPECT_TRUE(debug_visitor_factory.has_value());
-  auto test_debug_visitor_factory =
-      dynamic_cast<TestEnvoyQuicConnectionDebugVisitorFactory*>(debug_visitor_factory.ptr());
-  EXPECT_TRUE(test_debug_visitor_factory->processContext().has_value());
-  EXPECT_EQ(&test_debug_visitor_factory->processContext().value().get(), &context);
 }
 
 } // namespace Quic

test/common/quic/test_utils.h

@@ -345,28 +345,36 @@ class MockQuicConnectionDebugVisitor : public quic::QuicConnectionDebugVisitor {
 class TestEnvoyQuicConnectionDebugVisitorFactory
     : public EnvoyQuicConnectionDebugVisitorFactoryInterface {
 public:
-  std::string name() const override { return "envoy.quic.connection_debug_visitor.mock"; }
-
-  Envoy::ProtobufTypes::MessagePtr createEmptyConfigProto() override {
-    return std::make_unique<::test::common::config::DummyConfig>();
-  }
   std::unique_ptr<quic::QuicConnectionDebugVisitor>
-  createQuicConnectionDebugVisitor(quic::QuicSession* session,
+  createQuicConnectionDebugVisitor(Event::Dispatcher&, quic::QuicSession* session,
                                    const StreamInfo::StreamInfo& stream_info) override {
     auto debug_visitor = std::make_unique<MockQuicConnectionDebugVisitor>(session, stream_info);
     mock_debug_visitor_ = debug_visitor.get();
     return debug_visitor;
   }
 
-  Envoy::ProcessContextOptRef processContext() const { return context_; }
-
   MockQuicConnectionDebugVisitor* mock_debug_visitor_;
 };
 
-DECLARE_FACTORY(TestEnvoyQuicConnectionDebugVisitorFactory);
+class TestEnvoyQuicConnectionDebugVisitorFactoryFactory
+    : public EnvoyQuicConnectionDebugVisitorFactoryFactoryInterface {
+public:
+  std::string name() const override { return "envoy.quic.connection_debug_visitor.mock"; }
+
+  Envoy::ProtobufTypes::MessagePtr createEmptyConfigProto() override {
+    return std::make_unique<::test::common::config::DummyConfig>();
+  }
+
+  EnvoyQuicConnectionDebugVisitorFactoryInterfacePtr
+  createFactory(const Protobuf::Message&, Server::Configuration::ListenerFactoryContext&) override {
+    return std::make_unique<TestEnvoyQuicConnectionDebugVisitorFactory>();
+  }
+};
+
+DECLARE_FACTORY(TestEnvoyQuicConnectionDebugVisitorFactoryFactory);
 
-REGISTER_FACTORY(TestEnvoyQuicConnectionDebugVisitorFactory,
-                 Envoy::Quic::EnvoyQuicConnectionDebugVisitorFactoryInterface);
+REGISTER_FACTORY(TestEnvoyQuicConnectionDebugVisitorFactoryFactory,
+                 Envoy::Quic::EnvoyQuicConnectionDebugVisitorFactoryFactoryInterface);
 
 class TestNetworkObserverRegistry : public Quic::EnvoyQuicNetworkObserverRegistry {
 public:

test/integration/BUILD

@@ -1143,6 +1143,7 @@ envoy_cc_test_library(
         "//test/mocks/http:header_validator_mocks",
         "//test/mocks/protobuf:protobuf_mocks",
         "//test/mocks/runtime:runtime_mocks",
+        "//test/mocks/server:listener_factory_context_mocks",
         "//test/mocks/server:overload_manager_mocks",
         "//test/test_common:network_utility_lib",
         "//test/test_common:test_time_system_interface",

test/integration/fake_upstream.h

@@ -39,6 +39,7 @@
 #include "test/mocks/http/header_validator.h"
 #include "test/mocks/protobuf/mocks.h"
 #include "test/mocks/server/instance.h"
+#include "test/mocks/server/listener_factory_context.h"
 
 #if defined(ENVOY_ENABLE_QUIC)
 #include "source/common/quic/active_quic_listener.h"
@@ -866,7 +867,7 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
         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>>();
+              testing::NiceMock<Server::Configuration::MockListenerFactoryContext>>();
         }
         udp_listener_config_.listener_factory_ = std::make_unique<Quic::ActiveQuicListenerFactory>(
             parent_.quic_options_, 1, parent_.quic_stat_names_, parent_.validation_visitor_,
@@ -931,7 +932,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_;
+    std::unique_ptr<Server::Configuration::MockListenerFactoryContext> context_;
   };
 
   void threadRoutine();