downstream: refactoring code to remove listener hard deps (#24394)

Risk Level: low (simply moving code around) Testing: n/a Docs Changes: n/a Release Notes: n/a Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
envoyproxy · Dec 7, 2022 · ac20104 · ac20104
1 parent 50ac04b
commit ac20104
Show file tree

Hide file tree

Showing 17 changed files with 87 additions and 65 deletions.
diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h
@@ -288,6 +288,13 @@ class FilterChainFactoryContext : public virtual FactoryContext {
 };
 
 using FilterChainFactoryContextPtr = std::unique_ptr<FilterChainFactoryContext>;
+using FilterChainsByName = absl::flat_hash_map<std::string, Network::DrainableFilterChainSharedPtr>;
+
+class FilterChainBaseAction : public Matcher::Action {
+public:
+  virtual const Network::FilterChain* get(const FilterChainsByName& filter_chains_by_name,
+                                          const StreamInfo::StreamInfo& info) const PURE;
+};
 
 /**
  * An implementation of FactoryContext. The life time should cover the lifetime of the filter chains

diff --git a/source/common/quic/BUILD b/source/common/quic/BUILD
@@ -108,7 +108,7 @@ envoy_cc_library(
         ":quic_transport_socket_factory_lib",
         "//envoy/ssl:tls_certificate_config_interface",
         "//source/common/stream_info:stream_info_lib",
-        "//source/server:connection_handler_lib",
+        "//source/server:listener_stats",
         "@com_github_google_quiche//:quic_core_crypto_certificate_view_lib",
     ],
 )
@@ -324,7 +324,6 @@ envoy_cc_library(
         ":quic_io_handle_wrapper_lib",
         ":quic_network_connection_lib",
         "//source/common/quic:envoy_quic_utils_lib",
-        "//source/server:connection_handler_lib",
         "@com_github_google_quiche//:quic_core_connection_lib",
     ],
 )
@@ -357,7 +356,6 @@ envoy_cc_library(
         ":envoy_quic_server_session_lib",
         ":quic_stat_names_lib",
         "//envoy/network:listener_interface",
-        "//source/server:connection_handler_lib",
         "@com_github_google_quiche//:quic_core_server_lib",
         "@com_github_google_quiche//:quic_core_utils_lib",
     ],
@@ -387,7 +385,7 @@ envoy_cc_library(
         "//source/common/protobuf:utility_lib",
         "//source/common/runtime:runtime_lib",
         "//source/extensions/quic/connection_id_generator:envoy_deterministic_connection_id_generator_config",
-        "//source/server:connection_handler_lib",
+        "//source/server:active_udp_listener",
         "@envoy_api//envoy/config/listener/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/quic/connection_id_generator/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/quic/crypto_stream/v3:pkg_cc_proto",
@@ -516,7 +514,6 @@ envoy_cc_library(
     tags = ["nofips"],
     deps = [
         "//envoy/config:typed_config_interface",
-        "//source/server:connection_handler_lib",
         "@com_github_google_quiche//:quic_core_crypto_proof_source_lib",
     ],
 )

diff --git a/source/common/quic/envoy_quic_dispatcher.h b/source/common/quic/envoy_quic_dispatcher.h
@@ -7,8 +7,7 @@
 #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"
-#include "source/server/active_listener_base.h"
-#include "source/server/connection_handler_impl.h"
+#include "source/server/listener_stats.h"
 
 #include "quiche/quic/core/quic_dispatcher.h"
 #include "quiche/quic/core/quic_utils.h"

diff --git a/source/common/quic/envoy_quic_proof_source.h b/source/common/quic/envoy_quic_proof_source.h
@@ -2,8 +2,7 @@
 
 #include "source/common/quic/envoy_quic_proof_source_base.h"
 #include "source/common/quic/quic_transport_socket_factory.h"
-#include "source/server/active_listener_base.h"
-#include "source/server/connection_handler_impl.h"
+#include "source/server/listener_stats.h"
 
 namespace Envoy {
 namespace Quic {

diff --git a/source/common/quic/envoy_quic_proof_source_factory_interface.h b/source/common/quic/envoy_quic_proof_source_factory_interface.h
@@ -4,7 +4,7 @@
 #include "envoy/network/filter.h"
 #include "envoy/network/socket.h"
 
-#include "source/server/active_listener_base.h"
+#include "source/server/listener_stats.h"
 
 #include "quiche/quic/core/crypto/proof_source.h"
 

diff --git a/source/common/quic/envoy_quic_server_connection.h b/source/common/quic/envoy_quic_server_connection.h
@@ -4,7 +4,6 @@
 
 #include "source/common/quic/envoy_quic_utils.h"
 #include "source/common/quic/quic_network_connection.h"
-#include "source/server/connection_handler_impl.h"
 
 #include "quiche/quic/core/quic_connection.h"
 

diff --git a/source/extensions/matching/actions/format_string/BUILD b/source/extensions/matching/actions/format_string/BUILD
@@ -20,7 +20,6 @@ envoy_cc_extension(
         "//source/common/formatter:substitution_format_string_lib",
         "//source/common/http:header_map_lib",
         "//source/common/matcher:matcher_lib",
-        "//source/server:filter_chain_manager_lib",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],
 )
diff --git a/source/extensions/matching/actions/format_string/config.cc b/source/extensions/matching/actions/format_string/config.cc
@@ -12,8 +12,9 @@ namespace Matching {
 namespace Actions {
 namespace FormatString {
 
-const Network::FilterChain* ActionImpl::get(const Server::FilterChainsByName& filter_chains_by_name,
-                                            const StreamInfo::StreamInfo& info) const {
+const Network::FilterChain*
+ActionImpl::get(const Server::Configuration::FilterChainsByName& filter_chains_by_name,
+                const StreamInfo::StreamInfo& info) const {
   const std::string name =
       formatter_->format(*Http::StaticEmptyHeaders::get().request_headers,
                          *Http::StaticEmptyHeaders::get().response_headers,
@@ -27,7 +28,7 @@ const Network::FilterChain* ActionImpl::get(const Server::FilterChainsByName& fi
 
 Matcher::ActionFactoryCb
 ActionFactory::createActionFactoryCb(const Protobuf::Message& proto_config,
-                                     Server::FilterChainActionFactoryContext& context,
+                                     FilterChainActionFactoryContext& context,
                                      ProtobufMessage::ValidationVisitor& validator) {
   const auto& config =
       MessageUtil::downcastAndValidate<const envoy::config::core::v3::SubstitutionFormatString&>(
@@ -37,7 +38,7 @@ ActionFactory::createActionFactoryCb(const Protobuf::Message& proto_config,
   return [formatter]() { return std::make_unique<ActionImpl>(formatter); };
 }
 
-REGISTER_FACTORY(ActionFactory, Matcher::ActionFactory<Server::FilterChainActionFactoryContext>);
+REGISTER_FACTORY(ActionFactory, Matcher::ActionFactory<FilterChainActionFactoryContext>);
 
 } // namespace FormatString
 } // namespace Actions

diff --git a/source/extensions/matching/actions/format_string/config.h b/source/extensions/matching/actions/format_string/config.h
@@ -7,7 +7,6 @@
 #include "envoy/server/factory_context.h"
 
 #include "source/common/matcher/matcher.h"
-#include "source/server/filter_chain_manager_impl.h"
 
 namespace Envoy {
 namespace Extensions {
@@ -16,22 +15,24 @@ namespace Actions {
 namespace FormatString {
 
 class ActionImpl : public Matcher::ActionBase<envoy::config::core::v3::SubstitutionFormatString,
-                                              Server::FilterChainBaseAction> {
+                                              Server::Configuration::FilterChainBaseAction> {
 public:
   ActionImpl(const Formatter::FormatterConstSharedPtr& formatter) : formatter_(formatter) {}
-  const Network::FilterChain* get(const Server::FilterChainsByName& filter_chains_by_name,
-                                  const StreamInfo::StreamInfo& info) const override;
+  const Network::FilterChain*
+  get(const Server::Configuration::FilterChainsByName& filter_chains_by_name,
+      const StreamInfo::StreamInfo& info) const override;
 
 private:
   const Formatter::FormatterConstSharedPtr formatter_;
 };
 
-class ActionFactory : public Matcher::ActionFactory<Server::FilterChainActionFactoryContext> {
+using FilterChainActionFactoryContext = Server::Configuration::ServerFactoryContext;
+class ActionFactory : public Matcher::ActionFactory<FilterChainActionFactoryContext> {
 public:
   std::string name() const override { return "envoy.matching.actions.format_string"; }
   Matcher::ActionFactoryCb
   createActionFactoryCb(const Protobuf::Message& proto_config,
-                        Server::FilterChainActionFactoryContext& context,
+                        FilterChainActionFactoryContext& context,
                         ProtobufMessage::ValidationVisitor& validator) override;
   ProtobufTypes::MessagePtr createEmptyConfigProto() override {
     return std::make_unique<envoy::config::core::v3::SubstitutionFormatString>();

diff --git a/source/server/BUILD b/source/server/BUILD
@@ -99,12 +99,23 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "listener_stats",
+    hdrs = [
+        "listener_stats.h",
+    ],
+    deps = [
+        "//source/common/stats:timespan_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "active_listener_base",
     hdrs = [
         "active_listener_base.h",
     ],
     deps = [
+        ":listener_stats",
         "//envoy/network:connection_handler_interface",
         "//envoy/network:listener_interface",
         "//envoy/stats:timespan_interface",

diff --git a/source/server/active_listener_base.h b/source/server/active_listener_base.h
@@ -4,42 +4,11 @@
 #include "envoy/network/listener.h"
 #include "envoy/stats/scope.h"
 
+#include "source/server/listener_stats.h"
+
 namespace Envoy {
 namespace Server {
 
-#define ALL_LISTENER_STATS(COUNTER, GAUGE, HISTOGRAM)                                              \
-  COUNTER(downstream_cx_destroy)                                                                   \
-  COUNTER(downstream_cx_overflow)                                                                  \
-  COUNTER(downstream_cx_total)                                                                     \
-  COUNTER(downstream_cx_transport_socket_connect_timeout)                                          \
-  COUNTER(downstream_cx_overload_reject)                                                           \
-  COUNTER(downstream_global_cx_overflow)                                                           \
-  COUNTER(downstream_pre_cx_timeout)                                                               \
-  COUNTER(downstream_listener_filter_remote_close)                                                 \
-  COUNTER(downstream_listener_filter_error)                                                        \
-  COUNTER(no_filter_chain_match)                                                                   \
-  GAUGE(downstream_cx_active, Accumulate)                                                          \
-  GAUGE(downstream_pre_cx_active, Accumulate)                                                      \
-  HISTOGRAM(downstream_cx_length_ms, Milliseconds)
-
-/**
- * Wrapper struct for listener stats. @see stats_macros.h
- */
-struct ListenerStats {
-  ALL_LISTENER_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT)
-};
-
-#define ALL_PER_HANDLER_LISTENER_STATS(COUNTER, GAUGE)                                             \
-  COUNTER(downstream_cx_total)                                                                     \
-  GAUGE(downstream_cx_active, Accumulate)
-
-/**
- * Wrapper struct for per-handler listener stats. @see stats_macros.h
- */
-struct PerHandlerListenerStats {
-  ALL_PER_HANDLER_LISTENER_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT)
-};
-
 /**
  * Wrapper for an active listener owned by this handler.
  */

diff --git a/source/server/filter_chain_manager_impl.cc b/source/server/filter_chain_manager_impl.cc
@@ -32,7 +32,7 @@ Network::Address::InstanceConstSharedPtr fakeAddress() {
 }
 
 struct FilterChainNameAction
-    : public Matcher::ActionBase<ProtobufWkt::StringValue, FilterChainBaseAction> {
+    : public Matcher::ActionBase<ProtobufWkt::StringValue, Configuration::FilterChainBaseAction> {
   explicit FilterChainNameAction(const std::string& name) : name_(name) {}
   const Network::FilterChain* get(const FilterChainsByName& filter_chains_by_name,
                                   const StreamInfo::StreamInfo&) const override {
@@ -591,7 +591,8 @@ FilterChainManagerImpl::findFilterChainUsingMatcher(const Network::ConnectionSoc
          "Matching must complete for network streams.");
   if (match_result.result_) {
     const auto result = match_result.result_();
-    return result->getTyped<FilterChainBaseAction>().get(filter_chains_by_name_, info);
+    return result->getTyped<Configuration::FilterChainBaseAction>().get(filter_chains_by_name_,
+                                                                        info);
   }
   return default_filter_chain_.get();
 }

diff --git a/source/server/filter_chain_manager_impl.h b/source/server/filter_chain_manager_impl.h
@@ -103,12 +103,6 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor
 using FilterChainActionFactoryContext = Configuration::ServerFactoryContext;
 using FilterChainsByName = absl::flat_hash_map<std::string, Network::DrainableFilterChainSharedPtr>;
 
-class FilterChainBaseAction : public Matcher::Action {
-public:
-  virtual const Network::FilterChain* get(const FilterChainsByName& filter_chains_by_name,
-                                          const StreamInfo::StreamInfo& info) const PURE;
-};
-
 class FilterChainImpl : public Network::DrainableFilterChain {
 public:
   FilterChainImpl(Network::DownstreamTransportSocketFactoryPtr&& transport_socket_factory,

diff --git a/source/server/listener_stats.h b/source/server/listener_stats.h
@@ -0,0 +1,43 @@
+#pragma once
+
+#include "envoy/stats/scope.h"
+#include "envoy/stats/stats_macros.h"
+
+namespace Envoy {
+namespace Server {
+
+#define ALL_LISTENER_STATS(COUNTER, GAUGE, HISTOGRAM)                                              \
+  COUNTER(downstream_cx_destroy)                                                                   \
+  COUNTER(downstream_cx_overflow)                                                                  \
+  COUNTER(downstream_cx_total)                                                                     \
+  COUNTER(downstream_cx_transport_socket_connect_timeout)                                          \
+  COUNTER(downstream_cx_overload_reject)                                                           \
+  COUNTER(downstream_global_cx_overflow)                                                           \
+  COUNTER(downstream_pre_cx_timeout)                                                               \
+  COUNTER(downstream_listener_filter_remote_close)                                                 \
+  COUNTER(downstream_listener_filter_error)                                                        \
+  COUNTER(no_filter_chain_match)                                                                   \
+  GAUGE(downstream_cx_active, Accumulate)                                                          \
+  GAUGE(downstream_pre_cx_active, Accumulate)                                                      \
+  HISTOGRAM(downstream_cx_length_ms, Milliseconds)
+
+/**
+ * Wrapper struct for listener stats. @see stats_macros.h
+ */
+struct ListenerStats {
+  ALL_LISTENER_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT)
+};
+
+#define ALL_PER_HANDLER_LISTENER_STATS(COUNTER, GAUGE)                                             \
+  COUNTER(downstream_cx_total)                                                                     \
+  GAUGE(downstream_cx_active, Accumulate)
+
+/**
+ * Wrapper struct for per-handler listener stats. @see stats_macros.h
+ */
+struct PerHandlerListenerStats {
+  ALL_PER_HANDLER_LISTENER_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT)
+};
+
+} // namespace Server
+} // namespace Envoy
diff --git a/test/common/quic/BUILD b/test/common/quic/BUILD
@@ -242,6 +242,7 @@ envoy_cc_test(
         "//source/common/quic:envoy_quic_server_session_lib",
         "//source/extensions/quic/crypto_stream:envoy_quic_crypto_server_stream_lib",
         "//source/server:configuration_lib",
+        "//source/server:connection_handler_lib",
         "//test/mocks/event:event_mocks",
         "//test/mocks/http:http_mocks",
         "//test/mocks/network:network_mocks",

diff --git a/test/common/quic/envoy_quic_dispatcher_test.cc b/test/common/quic/envoy_quic_dispatcher_test.cc
@@ -12,6 +12,7 @@
 #include "source/common/quic/quic_transport_socket_factory.h"
 #include "source/extensions/quic/crypto_stream/envoy_quic_crypto_server_stream.h"
 #include "source/server/configuration_impl.h"
+#include "source/server/connection_handler_impl.h"
 
 #include "test/common/quic/test_proof_source.h"
 #include "test/common/quic/test_utils.h"

diff --git a/test/extensions/matching/actions/format_string/config_test.cc b/test/extensions/matching/actions/format_string/config_test.cc
@@ -28,9 +28,9 @@ TEST(ConfigTest, TestConfig) {
   ASSERT_NE(nullptr, action_cb);
   auto action = action_cb();
   ASSERT_NE(nullptr, action);
-  const auto& typed_action = action->getTyped<Server::FilterChainBaseAction>();
+  const auto& typed_action = action->getTyped<Server::Configuration::FilterChainBaseAction>();
 
-  Server::FilterChainsByName chains;
+  Server::Configuration::FilterChainsByName chains;
   auto chain = std::make_shared<testing::NiceMock<Network::MockFilterChain>>();
   chains.emplace("foo", chain);