factory: cleaning up factory APIs to allow code sharing

envoy/server/factory_context.h

@@ -38,17 +38,15 @@ namespace Envoy {
 namespace Server {
 namespace Configuration {
 
-/**
- * Common interface for downstream and upstream network filters.
- */
-class CommonFactoryContext {
+// Shared factory context between server factories and cluster factories
+class FactoryContextBase {
 public:
-  virtual ~CommonFactoryContext() = default;
+  virtual ~FactoryContextBase() = default;
 
   /**
-   * @return Upstream::ClusterManager& singleton for use by the entire server.
+   * @return Server::Options& the command-line options that Envoy was started with.
    */
-  virtual Upstream::ClusterManager& clusterManager() PURE;
+  virtual const Options& options() PURE;
 
   /**
    * @return Event::Dispatcher& the main thread's dispatcher. This dispatcher should be used
@@ -57,68 +55,73 @@ class CommonFactoryContext {
   virtual Event::Dispatcher& dispatcher() PURE;
 
   /**
-   * @return Server::Options& the command-line options that Envoy was started with.
+   * @return Api::Api& a reference to the api object.
    */
-  virtual const Options& options() PURE;
+  virtual Api::Api& api() PURE;
 
   /**
    * @return information about the local environment the server is running in.
    */
   virtual const LocalInfo::LocalInfo& localInfo() const PURE;
 
   /**
-   * @return ProtobufMessage::ValidationContext& validation visitor for xDS and static configuration
-   *         messages.
+   * @return Server::Admin& the server's global admin HTTP endpoint.
    */
-  virtual ProtobufMessage::ValidationContext& messageValidationContext() PURE;
+  virtual Server::Admin& admin() PURE;
 
   /**
    * @return Runtime::Loader& the singleton runtime loader for the server.
    */
   virtual Envoy::Runtime::Loader& runtime() PURE;
 
   /**
-   * @return Stats::Scope& the filter's stats scope.
+   * @return Singleton::Manager& the server-wide singleton manager.
    */
-  virtual Stats::Scope& scope() PURE;
+  virtual Singleton::Manager& singletonManager() PURE;
 
   /**
-   * @return Singleton::Manager& the server-wide singleton manager.
+   * @return ProtobufMessage::ValidationVisitor& validation visitor for configuration messages.
    */
-  virtual Singleton::Manager& singletonManager() PURE;
+  virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE;
+
+  /**
+   * @return Stats::Scope& the context's stats scope.
+   */
+  virtual Stats::Scope& scope() PURE;
 
   /**
    * @return ThreadLocal::SlotAllocator& the thread local storage engine for the server. This is
    *         used to allow runtime lockless updates to configuration, etc. across multiple threads.
    */
   virtual ThreadLocal::SlotAllocator& threadLocal() PURE;
+};
 
+/**
+ * Common interface for downstream and upstream network filters.
+ */
+class CommonFactoryContext : public FactoryContextBase {
+public:
   /**
-   * @return Server::Admin& the server's global admin HTTP endpoint.
+   * @return Upstream::ClusterManager& singleton for use by the entire server.
    */
-  virtual Server::Admin& admin() PURE;
+  virtual Upstream::ClusterManager& clusterManager() PURE;
 
   /**
-   * @return TimeSource& a reference to the time source.
+   * @return ProtobufMessage::ValidationContext& validation visitor for xDS and static configuration
+   *         messages.
    */
-  virtual TimeSource& timeSource() PURE;
+  virtual ProtobufMessage::ValidationContext& messageValidationContext() PURE;
 
   /**
-   * @return Api::Api& a reference to the api object.
+   * @return TimeSource& a reference to the time source.
    */
-  virtual Api::Api& api() PURE;
+  virtual TimeSource& timeSource() PURE;
 
   /**
    * @return AccessLogManager for use by the entire server.
    */
   virtual AccessLog::AccessLogManager& accessLogManager() PURE;
 
-  /**
-   * @return ProtobufMessage::ValidationVisitor& validation visitor for filter configuration
-   *         messages.
-   */
-  virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE;
-
   /**
    * @return ServerLifecycleNotifier& the lifecycle notifier for the server.
    */

envoy/upstream/cluster_factory.h

@@ -18,6 +18,7 @@
 #include "envoy/network/dns.h"
 #include "envoy/runtime/runtime.h"
 #include "envoy/server/admin.h"
+#include "envoy/server/factory_context.h"
 #include "envoy/server/options.h"
 #include "envoy/singleton/manager.h"
 #include "envoy/ssl/context.h"
@@ -35,66 +36,28 @@ namespace Upstream {
  * Context passed to cluster factory to access envoy resources. Cluster factory should only access
  * the rest of the server through this context object.
  */
-class ClusterFactoryContext {
+class ClusterFactoryContext : public Server::Configuration::FactoryContextBase {
 public:
-  virtual ~ClusterFactoryContext() = default;
-
   /**
    * @return bool flag indicating whether the cluster is added via api.
    */
   virtual bool addedViaApi() PURE;
 
-  /**
-   * @return Server::Admin& the server's admin interface.
-   */
-  virtual Server::Admin& admin() PURE;
-
-  /**
-   * @return Api::Api& a reference to the api object.
-   */
-  virtual Api::Api& api() PURE;
-
   /**
    * @return Upstream::ClusterManager& singleton for use by the entire server.
    */
-  virtual ClusterManager& clusterManager() PURE;
-
-  /**
-   * @return Event::Dispatcher& the main thread's dispatcher. This dispatcher should be used
-   *         for all singleton processing.
-   */
-  virtual Event::Dispatcher& dispatcher() PURE;
+  virtual Upstream::ClusterManager& clusterManager() PURE;
 
   /**
    * @return Network::DnsResolverSharedPtr the dns resolver for the server.
    */
   virtual Network::DnsResolverSharedPtr dnsResolver() PURE;
 
-  /**
-   * @return information about the local environment the server is running in.
-   */
-  virtual const LocalInfo::LocalInfo& localInfo() PURE;
-
-  /**
-   * @return Server::Options& the command-line options that Envoy was started with.
-   */
-  virtual const Server::Options& options() PURE;
-
   /**
    * @return AccessLogManager for use by the entire server.
    */
   virtual AccessLog::AccessLogManager& logManager() PURE;
 
-  /**
-   * @return Runtime::Loader& the singleton runtime loader for the server.
-   */
-  virtual Runtime::Loader& runtime() PURE;
-
-  /**
-   * @return Singleton::Manager& the server-wide singleton manager.
-   */
-  virtual Singleton::Manager& singletonManager() PURE;
-
   /**
    * @return Ssl::ContextManager& the SSL context manager.
    */
@@ -105,21 +68,13 @@ class ClusterFactoryContext {
    */
   virtual Stats::Store& stats() PURE;
 
-  /**
-   * @return the server's TLS slot allocator.
-   */
-  virtual ThreadLocal::SlotAllocator& tls() PURE;
-
   /**
    * @return Outlier::EventLoggerSharedPtr sink for outlier detection event logs.
    */
   virtual Outlier::EventLoggerSharedPtr outlierEventLogger() PURE;
 
-  /**
-   * @return ProtobufMessage::ValidationVisitor& validation visitor for filter configuration
-   *         messages.
-   */
-  virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE;
+  // Server::Configuration::FactoryContextBase
+  Stats::Scope& scope() override { return stats(); }
 };
 
 /**

source/common/upstream/cluster_factory_impl.cc

@@ -128,7 +128,7 @@ ClusterFactoryImplBase::create(const envoy::config::cluster::v3::Cluster& cluste
           std::make_unique<Server::Configuration::TransportSocketFactoryContextImpl>(
               context.admin(), context.sslContextManager(), *stats_scope, context.clusterManager(),
               context.localInfo(), context.dispatcher(), context.stats(),
-              context.singletonManager(), context.tls(), context.messageValidationVisitor(),
+              context.singletonManager(), context.threadLocal(), context.messageValidationVisitor(),
               context.api(), context.options());
 
   std::pair<ClusterImplBaseSharedPtr, ThreadAwareLoadBalancerPtr> new_cluster_pair =

source/common/upstream/cluster_factory_impl.h

@@ -70,13 +70,13 @@ class ClusterFactoryContextImpl : public ClusterFactoryContext {
 
   ClusterManager& clusterManager() override { return cluster_manager_; }
   Stats::Store& stats() override { return stats_; }
-  ThreadLocal::SlotAllocator& tls() override { return tls_; }
+  ThreadLocal::SlotAllocator& threadLocal() override { return tls_; }
   Network::DnsResolverSharedPtr dnsResolver() override { return dns_resolver_; }
   Ssl::ContextManager& sslContextManager() override { return ssl_context_manager_; }
   Runtime::Loader& runtime() override { return runtime_; }
   Event::Dispatcher& dispatcher() override { return dispatcher_; }
   AccessLog::AccessLogManager& logManager() override { return log_manager_; }
-  const LocalInfo::LocalInfo& localInfo() override { return local_info_; }
+  const LocalInfo::LocalInfo& localInfo() const override { return local_info_; }
   const Server::Options& options() override { return options_; }
   Server::Admin& admin() override { return admin_; }
   Singleton::Manager& singletonManager() override { return singleton_manager_; }

source/extensions/clusters/dynamic_forward_proxy/cluster.cc

@@ -177,8 +177,7 @@ ClusterFactory::createClusterWithConfig(
     Server::Configuration::TransportSocketFactoryContextImpl& socket_factory_context,
     Stats::ScopePtr&& stats_scope) {
   Extensions::Common::DynamicForwardProxy::DnsCacheManagerFactoryImpl cache_manager_factory(
-      context.singletonManager(), context.dispatcher(), context.tls(), context.api(),
-      context.runtime(), context.stats(), context.messageValidationVisitor());
+      context);
   envoy::config::cluster::v3::Cluster cluster_config = cluster;
   if (!cluster_config.has_upstream_http_protocol_options()) {
     // This sets defaults which will only apply if using old style http config.

source/extensions/common/dynamic_forward_proxy/dns_cache_impl.cc

@@ -16,23 +16,24 @@ namespace Common {
 namespace DynamicForwardProxy {
 
 DnsCacheImpl::DnsCacheImpl(
-    Event::Dispatcher& main_thread_dispatcher, ThreadLocal::SlotAllocator& tls,
-    Random::RandomGenerator& random, Filesystem::Instance& file_system, Runtime::Loader& loader,
-    Stats::Scope& root_scope, ProtobufMessage::ValidationVisitor& validation_visitor,
+    Server::Configuration::FactoryContextBase& context,
     const envoy::extensions::common::dynamic_forward_proxy::v3::DnsCacheConfig& config)
-    : main_thread_dispatcher_(main_thread_dispatcher),
+    : main_thread_dispatcher_(context.dispatcher()),
       dns_lookup_family_(Upstream::getDnsLookupFamilyFromEnum(config.dns_lookup_family())),
-      resolver_(selectDnsResolver(config, main_thread_dispatcher)), tls_slot_(tls),
-      scope_(root_scope.createScope(fmt::format("dns_cache.{}.", config.name()))),
+      resolver_(selectDnsResolver(config, main_thread_dispatcher_)),
+      tls_slot_(context.threadLocal()),
+      scope_(context.scope().createScope(fmt::format("dns_cache.{}.", config.name()))),
       stats_(generateDnsCacheStats(*scope_)),
-      resource_manager_(*scope_, loader, config.name(), config.dns_cache_circuit_breaker()),
+      resource_manager_(*scope_, context.runtime(), config.name(),
+                        config.dns_cache_circuit_breaker()),
       refresh_interval_(PROTOBUF_GET_MS_OR_DEFAULT(config, dns_refresh_rate, 60000)),
       timeout_interval_(PROTOBUF_GET_MS_OR_DEFAULT(config, dns_query_timeout, 5000)),
       failure_backoff_strategy_(
           Config::Utility::prepareDnsRefreshStrategy<
               envoy::extensions::common::dynamic_forward_proxy::v3::DnsCacheConfig>(
-              config, refresh_interval_.count(), random)),
-      file_system_(file_system), validation_visitor_(validation_visitor),
+              config, refresh_interval_.count(), context.api().randomGenerator())),
+      file_system_(context.api().fileSystem()),
+      validation_visitor_(context.messageValidationVisitor()),
       host_ttl_(PROTOBUF_GET_MS_OR_DEFAULT(config, host_ttl, 300000)),
       max_hosts_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_hosts, 1024)) {
   tls_slot_.set([&](Event::Dispatcher&) { return std::make_shared<ThreadLocalHostInfo>(*this); });

source/extensions/common/dynamic_forward_proxy/dns_cache_impl.h

@@ -5,6 +5,7 @@
 #include "envoy/extensions/common/dynamic_forward_proxy/v3/dns_cache.pb.h"
 #include "envoy/http/filter.h"
 #include "envoy/network/dns.h"
+#include "envoy/server/factory_context.h"
 #include "envoy/thread_local/thread_local.h"
 
 #include "source/common/common/cleanup.h"
@@ -45,10 +46,7 @@ class DnsCacheImplTest;
 
 class DnsCacheImpl : public DnsCache, Logger::Loggable<Logger::Id::forward_proxy> {
 public:
-  DnsCacheImpl(Event::Dispatcher& main_thread_dispatcher, ThreadLocal::SlotAllocator& tls,
-               Random::RandomGenerator& random, Filesystem::Instance& file_system,
-               Runtime::Loader& loader, Stats::Scope& root_scope,
-               ProtobufMessage::ValidationVisitor& validation_visitor,
+  DnsCacheImpl(Server::Configuration::FactoryContextBase& context,
                const envoy::extensions::common::dynamic_forward_proxy::v3::DnsCacheConfig& config);
   ~DnsCacheImpl() override;
   static DnsCacheStats generateDnsCacheStats(Stats::Scope& scope);

source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.cc

@@ -26,19 +26,15 @@ DnsCacheSharedPtr DnsCacheManagerImpl::getCache(
     return existing_cache->second.cache_;
   }
 
-  DnsCacheSharedPtr new_cache =
-      std::make_shared<DnsCacheImpl>(main_thread_dispatcher_, tls_, random_, file_system_, loader_,
-                                     root_scope_, validation_visitor_, config);
+  DnsCacheSharedPtr new_cache = std::make_shared<DnsCacheImpl>(context_, config);
   caches_.emplace(config.name(), ActiveCache{config, new_cache});
   return new_cache;
 }
 
 DnsCacheManagerSharedPtr DnsCacheManagerFactoryImpl::get() {
-  return singleton_manager_.getTyped<DnsCacheManager>(
-      SINGLETON_MANAGER_REGISTERED_NAME(dns_cache_manager), [this] {
-        return std::make_shared<DnsCacheManagerImpl>(dispatcher_, tls_, random_, file_system_,
-                                                     loader_, root_scope_, validation_visitor_);
-      });
+  return context_.singletonManager().getTyped<DnsCacheManager>(
+      SINGLETON_MANAGER_REGISTERED_NAME(dns_cache_manager),
+      [this] { return std::make_shared<DnsCacheManagerImpl>(context_); });
 }
 
 } // namespace DynamicForwardProxy

source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "envoy/extensions/common/dynamic_forward_proxy/v3/dns_cache.pb.h"
+#include "envoy/server/factory_context.h"
 
 #include "source/extensions/common/dynamic_forward_proxy/dns_cache.h"
 
@@ -13,13 +14,7 @@ namespace DynamicForwardProxy {
 
 class DnsCacheManagerImpl : public DnsCacheManager, public Singleton::Instance {
 public:
-  DnsCacheManagerImpl(Event::Dispatcher& main_thread_dispatcher, ThreadLocal::SlotAllocator& tls,
-                      Random::RandomGenerator& random, Filesystem::Instance& file_system,
-                      Runtime::Loader& loader, Stats::Scope& root_scope,
-                      ProtobufMessage::ValidationVisitor& validation_visitor)
-      : main_thread_dispatcher_(main_thread_dispatcher), tls_(tls), random_(random),
-        file_system_(file_system), loader_(loader), root_scope_(root_scope),
-        validation_visitor_(validation_visitor) {}
+  DnsCacheManagerImpl(Server::Configuration::FactoryContextBase& context) : context_(context) {}
 
   // DnsCacheManager
   DnsCacheSharedPtr getCache(
@@ -35,38 +30,20 @@ class DnsCacheManagerImpl : public DnsCacheManager, public Singleton::Instance {
     DnsCacheSharedPtr cache_;
   };
 
-  Event::Dispatcher& main_thread_dispatcher_;
-  ThreadLocal::SlotAllocator& tls_;
-  Random::RandomGenerator& random_;
-  Filesystem::Instance& file_system_;
-  Runtime::Loader& loader_;
-  Stats::Scope& root_scope_;
-  ProtobufMessage::ValidationVisitor& validation_visitor_;
+  Server::Configuration::FactoryContextBase& context_;
 
   absl::flat_hash_map<std::string, ActiveCache> caches_;
 };
 
 class DnsCacheManagerFactoryImpl : public DnsCacheManagerFactory {
 public:
-  DnsCacheManagerFactoryImpl(Singleton::Manager& singleton_manager, Event::Dispatcher& dispatcher,
-                             ThreadLocal::SlotAllocator& tls, Api::Api& api,
-                             Runtime::Loader& loader, Stats::Scope& root_scope,
-                             ProtobufMessage::ValidationVisitor& validation_visitor)
-      : singleton_manager_(singleton_manager), dispatcher_(dispatcher), tls_(tls),
-        random_(api.randomGenerator()), file_system_(api.fileSystem()), loader_(loader),
-        root_scope_(root_scope), validation_visitor_(validation_visitor) {}
+  DnsCacheManagerFactoryImpl(Server::Configuration::FactoryContextBase& context)
+      : context_(context) {}
 
   DnsCacheManagerSharedPtr get() override;
 
 private:
-  Singleton::Manager& singleton_manager_;
-  Event::Dispatcher& dispatcher_;
-  ThreadLocal::SlotAllocator& tls_;
-  Random::RandomGenerator& random_;
-  Filesystem::Instance& file_system_;
-  Runtime::Loader& loader_;
-  Stats::Scope& root_scope_;
-  ProtobufMessage::ValidationVisitor& validation_visitor_;
+  Server::Configuration::FactoryContextBase& context_;
 };
 
 } // namespace DynamicForwardProxy