cleanup common factory context using (envoyproxy#31072)

* cleanup all common factory context

Signed-off-by: wbpcode <wbphub@live.com>


---------

Signed-off-by: wbpcode <wbphub@live.com>

envoy/formatter/substitution_formatter.h

@@ -36,7 +36,7 @@ template <> class CommandParserFactoryBase<HttpFormatterContext> : public Config
  */
  virtual CommandParserPtr
  createCommandParserFromProto(const Protobuf::Message& config,
- Server::Configuration::CommonFactoryContext& context) PURE;
+ Server::Configuration::GenericFactoryContext& context) PURE;
 
  std::string category() const override { return "envoy.formatter"; }
 };

envoy/formatter/substitution_formatter_base.h

@@ -102,7 +102,7 @@ template <class FormatterContext> class CommandParserFactoryBase : public Config
  */
  virtual CommandParserBasePtr<FormatterContext>
  createCommandParserFromProto(const Protobuf::Message& config,
- Server::Configuration::CommonFactoryContext& context) PURE;
+ Server::Configuration::GenericFactoryContext& context) PURE;
 
  std::string category() const override {
  return fmt::format("envoy.{}.formatters", FormatterContext::category());

envoy/http/stateful_session.h

@@ -67,11 +67,13 @@ class SessionStateFactoryConfig : public Envoy::Config::TypedFactory {
  * Creates a particular session state factory implementation.
  *
  * @param config supplies the configuration for the session state factory extension.
+ * @param context supplies the factory context. Please don't store the reference to
+ * the context as it is only valid during the call.
  * @return SessionStateFactorySharedPtr the session state factory.
  */
  virtual SessionStateFactorySharedPtr
  createSessionStateFactory(const Protobuf::Message& config,
- Server::Configuration::CommonFactoryContext& context) PURE;
+ Server::Configuration::GenericFactoryContext& context) PURE;
 
  std::string category() const override { return "envoy.http.stateful_session"; }
 };

envoy/server/factory_context.h

@@ -194,6 +194,40 @@ class ServerFactoryContext : public virtual CommonFactoryContext {
  virtual bool healthCheckFailed() const PURE;
 };
 
+/**
+ * Generic factory context for multiple scenarios. This context provides a server factory context
+ * reference and other resources. Note that except for server factory context, other resources are
+ * not guaranteed to be available for the entire server lifetime. For example, context powered by a
+ * listener is only available for the lifetime of the listener.
+ */
+class GenericFactoryContext {
+public:
+ virtual ~GenericFactoryContext() = default;
+
+ /**
+ * @return ServerFactoryContext which lifetime is no shorter than the server and provides
+ * access to the server's resources.
+ */
+ virtual ServerFactoryContext& serverFactoryContext() const PURE;
+
+ /**
+ * @return ProtobufMessage::ValidationVisitor& validation visitor for configuration messages.
+ */
+ virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() const PURE;
+
+ /**
+ * @return Init::Manager& the init manager of the server/listener/cluster/etc, depending on the
+ * backend implementation.
+ */
+ virtual Init::Manager& initManager() const PURE;
+
+ /**
+ * @return Stats::Scope& the stats scope of the server/listener/cluster/etc, depending on the
+ * backend implementation.
+ */
+ virtual Stats::Scope& scope() const PURE;
+};
+
 /**
  * Factory context for access loggers that need access to listener properties.
  * This context is supplied to the access log factory when called with the listener context

envoy/server/request_id_extension_config.h

@@ -24,8 +24,8 @@ class RequestIDExtensionFactory : public Envoy::Config::TypedFactory {
  * @param config the custom configuration for this request id extension type.
  * @param context general filter context through which persistent resources can be accessed.
  */
- virtual Http::RequestIDExtensionSharedPtr
- createExtensionInstance(const Protobuf::Message& config, CommonFactoryContext& context) PURE;
+ virtual Http::RequestIDExtensionSharedPtr createExtensionInstance(const Protobuf::Message& config,
+  FactoryContext& context) PURE;
 
  std::string category() const override { return "envoy.request_id"; }
 };

mobile/library/common/engine.cc

@@ -90,8 +90,10 @@ envoy_status_t Engine::main(std::unique_ptr<Envoy::OptionsImplBase>&& options) {
  Envoy::Server::ServerLifecycleNotifier::Stage::PostInit, [this]() -> void {
  ASSERT(Thread::MainThread::isMainOrTestThread());
 
- connectivity_manager_ =
- Network::ConnectivityManagerFactory{server_->serverFactoryContext()}.get();
+ Envoy::Server::GenericFactoryContextImpl generic_context(
+ server_->serverFactoryContext(),
+ server_->serverFactoryContext().messageValidationVisitor());
+ connectivity_manager_ = Network::ConnectivityManagerFactory{generic_context}.get();
  auto v4_interfaces = connectivity_manager_->enumerateV4Interfaces();
  auto v6_interfaces = connectivity_manager_->enumerateV6Interfaces();
  logInterfaces("netconf_get_v4_interfaces", v4_interfaces);

mobile/library/common/network/connectivity_manager.cc

@@ -442,12 +442,12 @@ ConnectivityManagerImpl::enumerateInterfaces([[maybe_unused]] unsigned short fam
 }
 
 ConnectivityManagerSharedPtr ConnectivityManagerFactory::get() {
- return context_.singletonManager().getTyped<ConnectivityManagerImpl>(
- SINGLETON_MANAGER_REGISTERED_NAME(connectivity_manager), [&] {
- Extensions::Common::DynamicForwardProxy::DnsCacheManagerFactoryImpl cache_manager_factory{
- context_};
- return std::make_shared<ConnectivityManagerImpl>(context_.clusterManager(),
-   cache_manager_factory.get());
+ return context_.serverFactoryContext().singletonManager().getTyped<ConnectivityManagerImpl>(
+ SINGLETON_MANAGER_REGISTERED_NAME(connectivity_manager), [this] {
+ Envoy::Extensions::Common::DynamicForwardProxy::DnsCacheManagerFactoryImpl
+ cache_manager_factory{context_};
+ return std::make_shared<ConnectivityManagerImpl>(
+ context_.serverFactoryContext().clusterManager(), cache_manager_factory.get());
  });
 }
 

mobile/library/common/network/connectivity_manager.h

@@ -263,16 +263,15 @@ using ConnectivityManagerSharedPtr = std::shared_ptr<ConnectivityManager>;
  */
 class ConnectivityManagerFactory {
 public:
- ConnectivityManagerFactory(Server::Configuration::CommonFactoryContext& context)
- : context_(context) {}
+ ConnectivityManagerFactory(Server::GenericFactoryContextImpl context) : context_(context) {}
 
  /**
  * @returns singleton ConnectivityManager instance.
  */
  ConnectivityManagerSharedPtr get();
 
 private:
- Server::Configuration::CommonFactoryContext& context_;
+ Server::GenericFactoryContextImpl context_;
 };
 
 /**

mobile/test/common/extensions/retry/options/network_configuration/predicate_test.cc

@@ -25,7 +25,7 @@ TEST(NetworkConfigurationRetryOptionsPredicateTest, PredicateTest) {
  NiceMock<Server::Configuration::MockFactoryContext> mock_factory_context;
  NiceMock<Envoy::StreamInfo::MockStreamInfo> mock_stream_info;
  Upstream::RetryExtensionFactoryContextImpl retry_extension_factory_context{
- *mock_factory_context.singleton_manager_};
+ *mock_factory_context.server_factory_context_.singleton_manager_};
 
  auto connectivity_manager = Network::ConnectivityManagerFactory(mock_factory_context).get();
  ASSERT_NE(nullptr, connectivity_manager);
@@ -45,7 +45,7 @@ TEST(NetworkConfigurationRetryOptionsPredicateTest, PredicateTestWithoutConnecti
  ExtensionRegistry::registerFactories();
  NiceMock<Server::Configuration::MockFactoryContext> mock_factory_context;
  Upstream::RetryExtensionFactoryContextImpl retry_extension_factory_context{
- *mock_factory_context.singleton_manager_};
+ *mock_factory_context.server_factory_context_.singleton_manager_};
 
  auto factory = Registry::FactoryRegistry<Upstream::RetryOptionsPredicateFactory>::getFactory(
  "envoy.retry_options_predicates.network_configuration");

source/common/formatter/BUILD

@@ -49,6 +49,7 @@ envoy_cc_library(
  ":substitution_formatter_lib",
  "//source/common/config:utility_lib",
  "//source/common/protobuf",
+ "//source/server:generic_factory_context_lib",
  "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
  ],
 )

source/common/formatter/substitution_format_string.h

@@ -12,6 +12,7 @@
 #include "source/common/formatter/substitution_formatter.h"
 #include "source/common/protobuf/message_validator_impl.h"
 #include "source/common/protobuf/protobuf.h"
+#include "source/server/generic_factory_context.h"
 
 namespace Envoy {
 namespace Formatter {
@@ -27,7 +28,7 @@ class SubstitutionFormatStringUtils {
  template <class FormatterContext = HttpFormatterContext>
  static FormatterBasePtr<FormatterContext>
  fromProtoConfig(const envoy::config::core::v3::SubstitutionFormatString& config,
- Server::Configuration::CommonFactoryContext& context) {
+ Server::GenericFactoryContextImpl context) {
  // Instantiate formatter extensions.
  std::vector<CommandParserBasePtr<FormatterContext>> commands;
  for (const auto& formatter : config.formatters()) {
@@ -57,7 +58,8 @@ class SubstitutionFormatStringUtils {
  commands);
  case envoy::config::core::v3::SubstitutionFormatString::FormatCase::kTextFormatSource:
  return std::make_unique<FormatterBaseImpl<FormatterContext>>(
- Config::DataSource::read(config.text_format_source(), true, context.api()),
+ Config::DataSource::read(config.text_format_source(), true,
+ context.serverFactoryContext().api()),
  config.omit_empty_values(), commands);
  case envoy::config::core::v3::SubstitutionFormatString::FormatCase::FORMAT_NOT_SET:
  PANIC_DUE_TO_PROTO_UNSET;

source/common/http/request_id_extension_impl.cc

@@ -8,7 +8,7 @@ namespace Http {
 absl::StatusOr<RequestIDExtensionSharedPtr> RequestIDExtensionFactory::fromProto(
  const envoy::extensions::filters::network::http_connection_manager::v3::RequestIDExtension&
  config,
- Server::Configuration::CommonFactoryContext& context) {
+ Server::Configuration::FactoryContext& context) {
  const std::string type{TypeUtil::typeUrlToDescriptorFullName(config.typed_config().type_url())};
  auto* factory =
  Registry::FactoryRegistry<Server::Configuration::RequestIDExtensionFactory>::getFactoryByType(

source/common/http/request_id_extension_impl.h

@@ -18,7 +18,7 @@ class RequestIDExtensionFactory {
  static absl::StatusOr<RequestIDExtensionSharedPtr> fromProto(
  const envoy::extensions::filters::network::http_connection_manager::v3::RequestIDExtension&
  config,
- Server::Configuration::CommonFactoryContext& context);
+ Server::Configuration::FactoryContext& context);
 };
 
 } // namespace Http

source/common/local_reply/local_reply.cc

@@ -23,7 +23,7 @@ class BodyFormatter {
  content_type_(Http::Headers::get().ContentTypeValues.Text) {}
 
  BodyFormatter(const envoy::config::core::v3::SubstitutionFormatString& config,
- Server::Configuration::CommonFactoryContext& context)
+ Server::GenericFactoryContextImpl context)
  : formatter_(Formatter::SubstitutionFormatStringUtils::fromProtoConfig(config, context)),
  content_type_(
  !config.content_type().empty() ? config.content_type()

source/extensions/clusters/dynamic_forward_proxy/cluster.cc

@@ -478,18 +478,9 @@ ClusterFactory::createClusterWithConfig(
  const envoy::extensions::clusters::dynamic_forward_proxy::v3::ClusterConfig& proto_config,
  Upstream::ClusterFactoryContext& context) {
 
- auto& server_context = context.serverFactoryContext();
-
- // The message validation visitor of Upstream::ClusterFactoryContext should be used for
- // validating the cluster config.
- Server::FactoryContextBaseImpl factory_context_base(
- server_context.options(), server_context.mainThreadDispatcher(), server_context.api(),
- server_context.localInfo(), server_context.admin(), server_context.runtime(),
- server_context.singletonManager(), context.messageValidationVisitor(),
- server_context.serverScope().store(), server_context.threadLocal());
-
  Extensions::Common::DynamicForwardProxy::DnsCacheManagerFactoryImpl cache_manager_factory(
- factory_context_base);
+ context.serverFactoryContext(), context.messageValidationVisitor());
+
  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.
@@ -503,7 +494,7 @@ ClusterFactory::createClusterWithConfig(
  new Cluster(cluster_config, proto_config, context, cache_manager_factory));
 
  Extensions::Common::DynamicForwardProxy::DFPClusterStoreFactory cluster_store_factory(
- factory_context_base);
+ context.serverFactoryContext());
  cluster_store_factory.get()->save(new_cluster->info()->name(), new_cluster);
 
  auto& options = new_cluster->info()->upstreamHttpProtocolOptions();

source/extensions/common/dynamic_forward_proxy/BUILD

@@ -50,6 +50,7 @@ envoy_cc_library(
  "//source/common/network:resolver_lib",
  "//source/common/network:utility_lib",
  "//source/common/network/dns_resolver:dns_factory_util_lib",
+ "//source/server:generic_factory_context_lib",
  "@envoy_api//envoy/extensions/common/dynamic_forward_proxy/v3:pkg_cc_proto",
  ],
 )

source/extensions/common/dynamic_forward_proxy/dns_cache_impl.cc

@@ -17,7 +17,7 @@ namespace Common {
 namespace DynamicForwardProxy {
 
 absl::StatusOr<std::shared_ptr<DnsCacheImpl>> DnsCacheImpl::createDnsCacheImpl(
- Server::Configuration::FactoryContextBase& context,
+ const Server::Configuration::GenericFactoryContext& context,
  const envoy::extensions::common::dynamic_forward_proxy::v3::DnsCacheConfig& config) {
  const uint32_t max_hosts = PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_hosts, 1024);
  if (static_cast<size_t>(config.preresolve_hostnames().size()) > max_hosts) {
@@ -30,21 +30,21 @@ absl::StatusOr<std::shared_ptr<DnsCacheImpl>> DnsCacheImpl::createDnsCacheImpl(
 }
 
 DnsCacheImpl::DnsCacheImpl(
- Server::Configuration::FactoryContextBase& context,
+ const Server::Configuration::GenericFactoryContext& context,
  const envoy::extensions::common::dynamic_forward_proxy::v3::DnsCacheConfig& config)
- : main_thread_dispatcher_(context.mainThreadDispatcher()), config_(config),
- random_generator_(context.api().randomGenerator()),
+ : main_thread_dispatcher_(context.serverFactoryContext().mainThreadDispatcher()),
+ config_(config), random_generator_(context.serverFactoryContext().api().randomGenerator()),
  dns_lookup_family_(DnsUtils::getDnsLookupFamilyFromEnum(config.dns_lookup_family())),
- resolver_(selectDnsResolver(config, main_thread_dispatcher_, context)),
- tls_slot_(context.threadLocal()),
+ resolver_(selectDnsResolver(config, main_thread_dispatcher_, context.serverFactoryContext())),
+ tls_slot_(context.serverFactoryContext().threadLocal()),
  scope_(context.scope().createScope(fmt::format("dns_cache.{}.", config.name()))),
  stats_(generateDnsCacheStats(*scope_)),
- resource_manager_(*scope_, context.runtime(), config.name(),
+ resource_manager_(*scope_, context.serverFactoryContext().runtime(), config.name(),
  config.dns_cache_circuit_breaker()),
  refresh_interval_(PROTOBUF_GET_MS_OR_DEFAULT(config, dns_refresh_rate, 60000)),
  min_refresh_interval_(PROTOBUF_GET_MS_OR_DEFAULT(config, dns_min_refresh_rate, 5000)),
  timeout_interval_(PROTOBUF_GET_MS_OR_DEFAULT(config, dns_query_timeout, 5000)),
- file_system_(context.api().fileSystem()),
+ file_system_(context.serverFactoryContext().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)) {

source/extensions/common/dynamic_forward_proxy/dns_cache_impl.h

@@ -11,6 +11,7 @@
 #include "source/common/common/cleanup.h"
 #include "source/extensions/common/dynamic_forward_proxy/dns_cache.h"
 #include "source/extensions/common/dynamic_forward_proxy/dns_cache_resource_manager.h"
+#include "source/server/generic_factory_context.h"
 
 #include "absl/container/flat_hash_map.h"
 
@@ -48,7 +49,7 @@ class DnsCacheImpl : public DnsCache, Logger::Loggable<Logger::Id::forward_proxy
 public:
  // Create a DnsCacheImpl or return a failed status;
  static absl::StatusOr<std::shared_ptr<DnsCacheImpl>> createDnsCacheImpl(
- Server::Configuration::FactoryContextBase& context,
+ const Server::Configuration::GenericFactoryContext& context,
  const envoy::extensions::common::dynamic_forward_proxy::v3::DnsCacheConfig& config);
 
  ~DnsCacheImpl() override;
@@ -69,7 +70,7 @@ class DnsCacheImpl : public DnsCache, Logger::Loggable<Logger::Id::forward_proxy
  void forceRefreshHosts() override;
 
 private:
- DnsCacheImpl(Server::Configuration::FactoryContextBase& context,
+ DnsCacheImpl(const Server::Configuration::GenericFactoryContext& context,
  const envoy::extensions::common::dynamic_forward_proxy::v3::DnsCacheConfig& config);
  struct LoadDnsCacheEntryHandleImpl
  : public LoadDnsCacheEntryHandle,

source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.cc

@@ -34,7 +34,7 @@ DnsCacheSharedPtr DnsCacheManagerImpl::getCache(
 }
 
 DnsCacheSharedPtr DnsCacheManagerImpl::lookUpCacheByName(absl::string_view cache_name) {
- ASSERT(context_.mainThreadDispatcher().isThreadSafe());
+ ASSERT(context_.serverFactoryContext().mainThreadDispatcher().isThreadSafe());
  const auto& existing_cache = caches_.find(cache_name);
  if (existing_cache != caches_.end()) {
  return existing_cache->second.cache_;
@@ -44,7 +44,7 @@ DnsCacheSharedPtr DnsCacheManagerImpl::lookUpCacheByName(absl::string_view cache
 }
 
 DnsCacheManagerSharedPtr DnsCacheManagerFactoryImpl::get() {
- return context_.singletonManager().getTyped<DnsCacheManager>(
+ return context_.serverFactoryContext().singletonManager().getTyped<DnsCacheManager>(
  SINGLETON_MANAGER_REGISTERED_NAME(dns_cache_manager),
  [this] { return std::make_shared<DnsCacheManagerImpl>(context_); });
 }

source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.h

@@ -5,6 +5,7 @@
 
 #include "source/extensions/common/dynamic_forward_proxy/dns_cache.h"
 #include "source/server/factory_context_base_impl.h"
+#include "source/server/generic_factory_context.h"
 
 #include "absl/container/flat_hash_map.h"
 
@@ -15,7 +16,8 @@ namespace DynamicForwardProxy {
 
 class DnsCacheManagerImpl : public DnsCacheManager, public Singleton::Instance {
 public:
- DnsCacheManagerImpl(Server::Configuration::FactoryContextBase& context) : context_(context) {}
+ DnsCacheManagerImpl(const Server::Configuration::GenericFactoryContext& context)
+ : context_(context) {}
 
  // DnsCacheManager
  DnsCacheSharedPtr getCache(
@@ -32,19 +34,21 @@ class DnsCacheManagerImpl : public DnsCacheManager, public Singleton::Instance {
  DnsCacheSharedPtr cache_;
  };
 
- Server::FactoryContextBaseImpl context_;
+ Server::GenericFactoryContextImpl context_;
  absl::flat_hash_map<std::string, ActiveCache> caches_;
 };
 
 class DnsCacheManagerFactoryImpl : public DnsCacheManagerFactory {
 public:
- DnsCacheManagerFactoryImpl(Server::Configuration::FactoryContextBase& context)
- : context_(context) {}
+ DnsCacheManagerFactoryImpl(Server::Configuration::ServerFactoryContext& server_context,
+ ProtobufMessage::ValidationVisitor& validation_visitor)
+ : context_(server_context, validation_visitor) {}
+ DnsCacheManagerFactoryImpl(Server::GenericFactoryContextImpl context) : context_(context) {}
 
  DnsCacheManagerSharedPtr get() override;
 
 private:
- Server::FactoryContextBaseImpl context_;
+ Server::GenericFactoryContextImpl context_;
 };
 
 } // namespace DynamicForwardProxy

source/extensions/filters/common/set_filter_state/filter_config.cc

@@ -10,7 +10,7 @@ namespace SetFilterState {
 
 std::vector<Value>
 Config::parse(const Protobuf::RepeatedPtrField<FilterStateValueProto>& proto_values,
- Server::Configuration::CommonFactoryContext& context) const {
+ Server::Configuration::GenericFactoryContext& context) const {
  std::vector<Value> values;
  values.reserve(proto_values.size());
  for (const auto& proto_value : proto_values) {