listener: add listener info to the contexts (envoyproxy#31067)

Commit Message: Refactor to consolidate listener info and expose it in wider contexts. The concrete issue was that I wanted to use listener properties (direction) in the access logger, but only the filter context has it. By keeping the information in one place, I think we can also trim the large listener proto in the future. Risk Level: low, internal refactor Testing: regression Docs Changes: none Release Notes: none Signed-off-by: Kuat Yessenov <[email protected]>
fredyw · Dec 6, 2023 · 148fd48 · 148fd48
1 parent 93c26eb
commit 148fd48
Show file tree

Hide file tree

Showing 49 changed files with 394 additions and 339 deletions.
diff --git a/contrib/generic_proxy/filters/network/source/config.cc b/contrib/generic_proxy/filters/network/source/config.cc
@@ -107,7 +107,7 @@ Factory::createFilterFactoryFromProtoTyped(const ProxyConfig& proto_config,
       tracer = tracer_manager->getOrCreateTracer(&proto_config.tracing().provider());
     }
     tracing_config = std::make_unique<Tracing::ConnectionManagerTracingConfigImpl>(
-        context.direction(), proto_config.tracing());
+        context.listenerInfo().direction(), proto_config.tracing());
   }
 
   // Access log configuration.

diff --git a/contrib/generic_proxy/filters/network/test/config_test.cc b/contrib/generic_proxy/filters/network/test/config_test.cc
@@ -363,7 +363,9 @@ TEST(BasicFilterConfigTest, TestConfigurationWithTracing) {
   NiceMock<MockStreamCodecFactoryConfig> codec_factory_config;
   Registry::InjectFactory<CodecFactoryConfig> registration(codec_factory_config);
 
+  NiceMock<Network::MockListenerInfo> listener_info;
   NiceMock<Server::Configuration::MockFactoryContext> factory_context;
+  ON_CALL(factory_context, listenerInfo()).WillByDefault(testing::ReturnRef(listener_info));
   factory_context.server_factory_context_.cluster_manager_.initializeClusters({"zipkin"}, {});
   factory_context.server_factory_context_.cluster_manager_.initializeThreadLocalClusters(
       {"zipkin"});

diff --git a/envoy/network/listener.h b/envoy/network/listener.h
@@ -133,6 +133,37 @@ class InternalListenerConfig {
 
 using InternalListenerConfigOptRef = OptRef<InternalListenerConfig>;
 
+/**
+ * Description of the listener.
+ */
+class ListenerInfo {
+public:
+  virtual ~ListenerInfo() = default;
+
+  /**
+   * @return const envoy::config::core::v3::Metadata& the config metadata associated with this
+   * listener.
+   */
+  virtual const envoy::config::core::v3::Metadata& metadata() const PURE;
+
+  /**
+   * @return const Envoy::Config::TypedMetadata& return the typed metadata provided in the config
+   * for this listener.
+   */
+  virtual const Envoy::Config::TypedMetadata& typedMetadata() const PURE;
+
+  /**
+   * @return envoy::config::core::v3::TrafficDirection the direction of the traffic relative to
+   * the local proxy.
+   */
+  virtual envoy::config::core::v3::TrafficDirection direction() const PURE;
+
+  /**
+   * @return whether the listener is a Quic listener.
+   */
+  virtual bool isQuic() const PURE;
+};
+
 /**
  * A configuration for an individual listener.
  */
@@ -206,6 +237,11 @@ class ListenerConfig {
    */
   virtual const std::string& name() const PURE;
 
+  /**
+   * @return ListenerInfo& description of the listener.
+   */
+  virtual const ListenerInfo& listenerInfo() const PURE;
+
   /**
    * @return the UDP configuration for the listener IFF it is a UDP listener.
    */
@@ -216,11 +252,6 @@ class ListenerConfig {
    */
   virtual InternalListenerConfigOptRef internalListenerConfig() PURE;
 
-  /**
-   * @return traffic direction of the listener.
-   */
-  virtual envoy::config::core::v3::TrafficDirection direction() const PURE;
-
   /**
    * @param address is used for query the address specific connection balancer.
    * @return the connection balancer for this listener. All listeners have a connection balancer,

diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h
@@ -235,34 +235,25 @@ class GenericFactoryContext {
  * NOTE: this interface is used in proprietary access loggers, please do not delete
  * without reaching to Envoy maintainers first.
  */
-class ListenerAccessLogFactoryContext : public virtual CommonFactoryContext {
+class FactoryContext : public virtual CommonFactoryContext {
 public:
+  ~FactoryContext() override = default;
+
   /**
    * @return Stats::Scope& the listener's stats scope.
    */
   virtual Stats::Scope& listenerScope() PURE;
 
-  /**
-   * @return const envoy::config::core::v3::Metadata& the config metadata associated with this
-   * listener.
-   */
-  virtual const envoy::config::core::v3::Metadata& listenerMetadata() const PURE;
-
   /**
    * @return ProcessContextOptRef an optional reference to the
    * process context. Will be unset when running in validation mode.
    */
   virtual ProcessContextOptRef processContext() PURE;
-};
 
-/**
- * Context passed to network and HTTP filters to access server resources.
- * TODO(mattklein123): When we lock down visibility of the rest of the code, filters should only
- * access the rest of the server via interfaces exposed here.
- */
-class FactoryContext : public virtual ListenerAccessLogFactoryContext {
-public:
-  ~FactoryContext() override = default;
+  /**
+   * @return ListenerInfo description of the listener.
+   */
+  virtual const Network::ListenerInfo& listenerInfo() const PURE;
 
   /**
    * @return ServerFactoryContext which lifetime is no shorter than the server.
@@ -274,12 +265,6 @@ class FactoryContext : public virtual ListenerAccessLogFactoryContext {
    */
   virtual TransportSocketFactoryContext& getTransportSocketFactoryContext() const PURE;
 
-  /**
-   * @return envoy::config::core::v3::TrafficDirection the direction of the traffic relative to
-   * the local proxy.
-   */
-  virtual envoy::config::core::v3::TrafficDirection direction() const PURE;
-
   /**
    * @return const Network::DrainDecision& a drain decision that filters can use to determine if
    *         they should be doing graceful closes on connections when possible.
@@ -291,17 +276,6 @@ class FactoryContext : public virtual ListenerAccessLogFactoryContext {
    */
   virtual bool healthCheckFailed() PURE;
 
-  /**
-   * @return bool if these filters are created under the scope of a Quic listener.
-   */
-  virtual bool isQuicListener() const PURE;
-
-  /**
-   * @return const Envoy::Config::TypedMetadata& return the typed metadata provided in the config
-   * for this listener.
-   */
-  virtual const Envoy::Config::TypedMetadata& listenerTypedMetadata() const PURE;
-
   /**
    * @return OverloadManager& the overload manager for the server.
    */
@@ -356,13 +330,7 @@ class FilterChainBaseAction : public Matcher::Action {
  * An implementation of FactoryContext. The life time should cover the lifetime of the filter chains
  * and connections. It can be used to create ListenerFilterChain.
  */
-class ListenerFactoryContext : public virtual FactoryContext {
-public:
-  /**
-   * Give access to the listener configuration
-   */
-  virtual const Network::ListenerConfig& listenerConfig() const PURE;
-};
+class ListenerFactoryContext : public virtual FactoryContext {};
 
 /**
  * FactoryContext for ProtocolOptionsFactory.

diff --git a/source/extensions/filters/http/wasm/wasm_filter.cc b/source/extensions/filters/http/wasm/wasm_filter.cc
@@ -12,7 +12,8 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::http::wasm::v3::Was
       server.threadLocal());
 
   const auto plugin = std::make_shared<Common::Wasm::Plugin>(
-      config.config(), context.direction(), server.localInfo(), &context.listenerMetadata());
+      config.config(), context.listenerInfo().direction(), server.localInfo(),
+      &context.listenerInfo().metadata());
 
   auto callback = [plugin, this](const Common::Wasm::WasmHandleSharedPtr& base_wasm) {
     // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call.

diff --git a/source/extensions/filters/listener/original_dst/config.cc b/source/extensions/filters/listener/original_dst/config.cc
@@ -29,7 +29,7 @@ class OriginalDstConfigFactory : public Server::Configuration::NamedListenerFilt
     // 1. The platform supports the original destination feature
     // 2. The `traffic_direction` property is set on the listener. This is required to redirect the
     // traffic.
-    if (context.listenerConfig().direction() == envoy::config::core::v3::UNSPECIFIED) {
+    if (context.listenerInfo().direction() == envoy::config::core::v3::UNSPECIFIED) {
       throw EnvoyException("[Windows] Setting original destination filter on a listener without "
                            "specifying the traffic_direction."
                            "Configure the traffic_direction listener option");
@@ -40,7 +40,7 @@ class OriginalDstConfigFactory : public Server::Configuration::NamedListenerFilt
     }
 #endif
 
-    return [listener_filter_matcher, traffic_direction = context.listenerConfig().direction()](
+    return [listener_filter_matcher, traffic_direction = context.listenerInfo().direction()](
                Network::ListenerFilterManager& filter_manager) -> void {
       filter_manager.addAcceptFilter(listener_filter_matcher,
                                      std::make_unique<OriginalDstFilter>(traffic_direction));

diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc
@@ -559,8 +559,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
 
   if (config.has_tracing()) {
     tracer_ = tracer_manager.getOrCreateTracer(getPerFilterTracerConfig(config));
-    tracing_config_ = std::make_unique<Http::TracingConnectionManagerConfig>(context.direction(),
-                                                                             config.tracing());
+    tracing_config_ = std::make_unique<Http::TracingConnectionManagerConfig>(
+        context.listenerInfo().direction(), config.tracing());
   }
 
   for (const auto& access_log : config.access_log()) {
@@ -628,15 +628,15 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
       HTTP3:
 #ifdef ENVOY_ENABLE_QUIC
     codec_type_ = CodecType::HTTP3;
-    if (!context_.isQuicListener()) {
+    if (!context_.listenerInfo().isQuic()) {
       throwEnvoyExceptionOrPanic("HTTP/3 codec configured on non-QUIC listener.");
     }
 #else
     throwEnvoyExceptionOrPanic("HTTP3 configured but not enabled in the build.");
 #endif
     break;
   }
-  if (codec_type_ != CodecType::HTTP3 && context_.isQuicListener()) {
+  if (codec_type_ != CodecType::HTTP3 && context_.listenerInfo().isQuic()) {
     throwEnvoyExceptionOrPanic("Non-HTTP/3 codec configured on QUIC listener.");
   }
 

diff --git a/source/extensions/filters/network/wasm/wasm_filter.cc b/source/extensions/filters/network/wasm/wasm_filter.cc
@@ -10,8 +10,8 @@ FilterConfig::FilterConfig(const envoy::extensions::filters::network::wasm::v3::
     : tls_slot_(ThreadLocal::TypedSlot<Common::Wasm::PluginHandleSharedPtrThreadLocal>::makeUnique(
           context.serverFactoryContext().threadLocal())) {
   const auto plugin = std::make_shared<Common::Wasm::Plugin>(
-      config.config(), context.direction(), context.serverFactoryContext().localInfo(),
-      &context.listenerMetadata());
+      config.config(), context.listenerInfo().direction(),
+      context.serverFactoryContext().localInfo(), &context.listenerInfo().metadata());
 
   auto callback = [plugin, this](Common::Wasm::WasmHandleSharedPtr base_wasm) {
     // NB: the Slot set() call doesn't complete inline, so all arguments must outlive this call.

diff --git a/source/extensions/listener_managers/listener_manager/BUILD b/source/extensions/listener_managers/listener_manager/BUILD
@@ -32,6 +32,7 @@ envoy_cc_extension(
         ":connection_handler_lib",
         ":filter_chain_manager_lib",
         ":lds_api_lib",
+        ":listener_info_lib",
         "//envoy/access_log:access_log_interface",
         "//envoy/config:typed_metadata_interface",
         "//envoy/network:connection_interface",
@@ -44,7 +45,6 @@ envoy_cc_extension(
         "//source/common/access_log:access_log_lib",
         "//source/common/common:basic_resource_lib",
         "//source/common/common:empty_string",
-        "//source/common/config:metadata_lib",
         "//source/common/config:utility_lib",
         "//source/common/http:conn_manager_lib",
         "//source/common/init:manager_lib",
@@ -244,6 +244,22 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_extension(
+    name = "listener_info_lib",
+    srcs = ["listener_info_impl.cc"],
+    hdrs = [
+        "listener_info_impl.h",
+    ],
+    extra_visibility = [
+        "//source/server/admin:__subpackages__",
+    ],
+    deps = [
+        "//envoy/network:listener_interface",
+        "//source/common/config:metadata_lib",
+        "@envoy_api//envoy/config/listener/v3:pkg_cc_proto",
+    ],
+)
+
 envoy_cc_library(
     name = "active_tcp_socket",
     srcs = ["active_tcp_socket.cc"],

diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc
@@ -92,18 +92,8 @@ ThreadLocal::SlotAllocator& PerFilterChainFactoryContextImpl::threadLocal() {
   return parent_context_.threadLocal();
 }
 
-const envoy::config::core::v3::Metadata&
-PerFilterChainFactoryContextImpl::listenerMetadata() const {
-  return parent_context_.listenerMetadata();
-}
-
-const Envoy::Config::TypedMetadata&
-PerFilterChainFactoryContextImpl::listenerTypedMetadata() const {
-  return parent_context_.listenerTypedMetadata();
-}
-
-envoy::config::core::v3::TrafficDirection PerFilterChainFactoryContextImpl::direction() const {
-  return parent_context_.direction();
+const Network::ListenerInfo& PerFilterChainFactoryContextImpl::listenerInfo() const {
+  return parent_context_.listenerInfo();
 }
 
 ProtobufMessage::ValidationContext& PerFilterChainFactoryContextImpl::messageValidationContext() {
@@ -189,10 +179,6 @@ PerFilterChainFactoryContextImpl::getTransportSocketFactoryContext() const {
 
 Stats::Scope& PerFilterChainFactoryContextImpl::listenerScope() { return *filter_chain_scope_; }
 
-bool PerFilterChainFactoryContextImpl::isQuicListener() const {
-  return parent_context_.isQuicListener();
-}
-
 FilterChainManagerImpl::FilterChainManagerImpl(
     const std::vector<Network::Address::InstanceConstSharedPtr>& addresses,
     Configuration::FactoryContext& factory_context, Init::Manager& init_manager,

diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h
@@ -76,9 +76,7 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor
   OverloadManager& overloadManager() override;
   ThreadLocal::SlotAllocator& threadLocal() override;
   OptRef<Admin> admin() override;
-  const envoy::config::core::v3::Metadata& listenerMetadata() const override;
-  const Envoy::Config::TypedMetadata& listenerTypedMetadata() const override;
-  envoy::config::core::v3::TrafficDirection direction() const override;
+  const Network::ListenerInfo& listenerInfo() const override;
   TimeSource& timeSource() override;
   ProtobufMessage::ValidationVisitor& messageValidationVisitor() override;
   ProtobufMessage::ValidationContext& messageValidationContext() override;
@@ -88,7 +86,6 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor
   Configuration::ServerFactoryContext& serverFactoryContext() const override;
   Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override;
   Stats::Scope& listenerScope() override;
-  bool isQuicListener() const override;
 
   void startDraining() override { is_draining_.store(true); }