diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD index 788180552acfd..abce7d27e5256 100644 --- a/include/envoy/server/BUILD +++ b/include/envoy/server/BUILD @@ -63,7 +63,6 @@ envoy_cc_library( "//include/envoy/init:init_interface", "//include/envoy/local_info:local_info_interface", "//include/envoy/ratelimit:ratelimit_interface", - "//include/envoy/router:route_config_provider_manager_interface", "//include/envoy/runtime:runtime_interface", "//include/envoy/ssl:context_manager_interface", "//include/envoy/thread_local:thread_local_interface", @@ -107,7 +106,6 @@ envoy_cc_library( "//include/envoy/local_info:local_info_interface", "//include/envoy/network:drain_decision_interface", "//include/envoy/ratelimit:ratelimit_interface", - "//include/envoy/router:route_config_provider_manager_interface", "//include/envoy/runtime:runtime_interface", "//include/envoy/singleton:manager_interface", "//include/envoy/thread_local:thread_local_interface", diff --git a/include/envoy/server/filter_config.h b/include/envoy/server/filter_config.h index 3cf0f9ff117ea..5a56629570faa 100644 --- a/include/envoy/server/filter_config.h +++ b/include/envoy/server/filter_config.h @@ -9,7 +9,6 @@ #include "envoy/network/drain_decision.h" #include "envoy/network/filter.h" #include "envoy/ratelimit/ratelimit.h" -#include "envoy/router/route_config_provider_manager.h" #include "envoy/runtime/runtime.h" #include "envoy/singleton/manager.h" #include "envoy/thread_local/thread_local.h" @@ -117,11 +116,6 @@ class FactoryContext { * used to allow runtime lockless updates to configuration, etc. across multiple threads. */ virtual ThreadLocal::SlotAllocator& threadLocal() PURE; - - /** - * @return RouteConfigProviderManager& singleton for use by the entire server. - */ - virtual Router::RouteConfigProviderManager& routeConfigProviderManager() PURE; }; enum class NetworkFilterType { Read, Write, Both }; diff --git a/include/envoy/server/instance.h b/include/envoy/server/instance.h index 872261e24d81f..b6b1299da420a 100644 --- a/include/envoy/server/instance.h +++ b/include/envoy/server/instance.h @@ -10,7 +10,6 @@ #include "envoy/local_info/local_info.h" #include "envoy/network/listen_socket.h" #include "envoy/ratelimit/ratelimit.h" -#include "envoy/router/route_config_provider_manager.h" #include "envoy/runtime/runtime.h" #include "envoy/server/admin.h" #include "envoy/server/drain_manager.h" @@ -180,11 +179,6 @@ class Instance { * @return information about the local environment the server is running in. */ virtual const LocalInfo::LocalInfo& localInfo() PURE; - - /** - * @return the server's http route manager. - */ - virtual Router::ServerRouteConfigProviderManager& routeConfigProviderManager() PURE; }; } // namespace Server diff --git a/include/envoy/singleton/instance.h b/include/envoy/singleton/instance.h index 59d0f7b1ed8e2..8c1c92f2d0b22 100644 --- a/include/envoy/singleton/instance.h +++ b/include/envoy/singleton/instance.h @@ -11,7 +11,7 @@ class Instance { virtual ~Instance() {} }; -typedef std::shared_ptr InstancePtr; +typedef std::shared_ptr InstanceSharedPtr; } // namespace Singleton } // namespace Envoy diff --git a/include/envoy/singleton/manager.h b/include/envoy/singleton/manager.h index 78bfb1e0ebf8b..f9bd23a3f2658 100644 --- a/include/envoy/singleton/manager.h +++ b/include/envoy/singleton/manager.h @@ -36,10 +36,24 @@ template class RegistrationImpl : public Registration { std::string name() override { return name_param; } }; +/** + * Macro used to statically register singletons managed by the singleton manager + * defined in envoy/singleton/manager.h. After the NAME has been registered use the + * SINGLETON_MANAGER_REGISTERED_NAME macro to access the name registered with the + * singleton manager. + */ +#define SINGLETON_MANAGER_REGISTRATION(NAME) \ + static constexpr char NAME##_singleton_name[] = #NAME "_singleton"; \ + static Registry::RegisterFactory, \ + Singleton::Registration> \ + NAME##_singleton_registered_; + +#define SINGLETON_MANAGER_REGISTERED_NAME(NAME) NAME##_singleton_name + /** * Callback function used to create a singleton. */ -typedef std::function SingletonFactoryCb; +typedef std::function SingletonFactoryCb; /** * A manager for all server-side singletons. @@ -66,7 +80,7 @@ class Manager { * singletons must store the shared_ptr for as long as the singleton is needed. * @return InstancePtr the singleton. */ - virtual InstancePtr get(const std::string& name, SingletonFactoryCb) PURE; + virtual InstanceSharedPtr get(const std::string& name, SingletonFactoryCb) PURE; }; typedef std::unique_ptr ManagerPtr; diff --git a/source/common/router/BUILD b/source/common/router/BUILD index e581dba7e0a37..d0d126a11c536 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -64,6 +64,7 @@ envoy_cc_library( "//include/envoy/local_info:local_info_interface", "//include/envoy/router:rds_interface", "//include/envoy/router:route_config_provider_manager_interface", + "//include/envoy/singleton:instance_interface", "//include/envoy/thread_local:thread_local_interface", "//source/common/common:assert_lib", "//source/common/common:logger_lib", diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index ae0d27a199ec6..3396c6f5d2fe8 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -10,6 +10,7 @@ #include "envoy/local_info/local_info.h" #include "envoy/router/rds.h" #include "envoy/router/route_config_provider_manager.h" +#include "envoy/singleton/instance.h" #include "envoy/thread_local/thread_local.h" #include "common/common/logger.h" @@ -126,8 +127,8 @@ class RdsRouteConfigProviderImpl friend class RouteConfigProviderManagerImpl; }; -// TODO(junr03): use then singleton manager in #1410. -class RouteConfigProviderManagerImpl : public ServerRouteConfigProviderManager { +class RouteConfigProviderManagerImpl : public ServerRouteConfigProviderManager, + public Singleton::Instance { public: RouteConfigProviderManagerImpl(Runtime::Loader& runtime, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, diff --git a/source/common/singleton/manager_impl.cc b/source/common/singleton/manager_impl.cc index 97fffb5ef3e83..8a8e87acbb2b1 100644 --- a/source/common/singleton/manager_impl.cc +++ b/source/common/singleton/manager_impl.cc @@ -7,14 +7,14 @@ namespace Envoy { namespace Singleton { -InstancePtr ManagerImpl::get(const std::string& name, SingletonFactoryCb cb) { +InstanceSharedPtr ManagerImpl::get(const std::string& name, SingletonFactoryCb cb) { ASSERT(run_tid_ == Thread::Thread::currentThreadId()); if (nullptr == Registry::FactoryRegistry::getFactory(name)) { PANIC(fmt::format("invalid singleton name '{}'. Make sure it is registered.", name)); } if (nullptr == singletons_[name].lock()) { - InstancePtr singleton = cb(); + InstanceSharedPtr singleton = cb(); singletons_[name] = singleton; return singleton; } else { diff --git a/source/common/singleton/manager_impl.h b/source/common/singleton/manager_impl.h index ff7c272d53989..066b4a3b838c6 100644 --- a/source/common/singleton/manager_impl.h +++ b/source/common/singleton/manager_impl.h @@ -19,7 +19,7 @@ class ManagerImpl : public Manager { ManagerImpl() : run_tid_(Thread::Thread::currentThreadId()) {} // Singleton::Manager - InstancePtr get(const std::string& name, SingletonFactoryCb cb) override; + InstanceSharedPtr get(const std::string& name, SingletonFactoryCb cb) override; private: std::unordered_map> singletons_; diff --git a/source/server/config/network/BUILD b/source/server/config/network/BUILD index d35d7bdd3a107..0e24fdc1b4e92 100644 --- a/source/server/config/network/BUILD +++ b/source/server/config/network/BUILD @@ -37,6 +37,7 @@ envoy_cc_library( "//include/envoy/filesystem:filesystem_interface", "//include/envoy/http:filter_interface", "//include/envoy/registry", + "//include/envoy/router:route_config_provider_manager_interface", "//include/envoy/server:filter_config_interface", "//include/envoy/server:options_interface", "//include/envoy/stats:stats_interface", diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index 7e81b7b8a9a71..a08d4640d23ee 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -29,26 +29,38 @@ namespace Configuration { const std::string HttpConnectionManagerConfig::DEFAULT_SERVER_STRING = "envoy"; -static constexpr char date_provider_singleton_name[] = "date_provider_singleton"; -static Registry::RegisterFactory, - Singleton::Registration> - date_provider_singleton_registered_; +// Singleton registration via macro defined in envoy/singleton/manager.h +SINGLETON_MANAGER_REGISTRATION(date_provider); +SINGLETON_MANAGER_REGISTRATION(route_config_provider_manager); NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactory( const Json::Object& json_http_connection_manager, FactoryContext& context) { std::shared_ptr date_provider = context.singletonManager().getTyped( - date_provider_singleton_name, [&context] { + SINGLETON_MANAGER_REGISTERED_NAME(date_provider), [&context] { return std::make_shared(context.dispatcher(), context.threadLocal()); }); + std::shared_ptr route_config_provider_manager = + context.singletonManager().getTyped( + SINGLETON_MANAGER_REGISTERED_NAME(route_config_provider_manager), [&context] { + return std::make_shared( + context.runtime(), context.dispatcher(), context.random(), context.localInfo(), + context.threadLocal()); + }); + envoy::api::v2::filter::HttpConnectionManager http_connection_manager; Config::FilterJson::translateHttpConnectionManager(json_http_connection_manager, http_connection_manager); - std::shared_ptr http_config( - new HttpConnectionManagerConfig(http_connection_manager, context, *date_provider)); - return [http_config, &context, date_provider](Network::FilterManager& filter_manager) -> void { + std::shared_ptr http_config(new HttpConnectionManagerConfig( + http_connection_manager, context, *date_provider, *route_config_provider_manager)); + + // This lambda captures the shared_ptrs created above, thus preserving the + // reference count. Moreover, keep in mind the capture list determines + // destruction order. + return [route_config_provider_manager, http_config, &context, + date_provider](Network::FilterManager& filter_manager) -> void { filter_manager.addReadFilter(Network::ReadFilterSharedPtr{new Http::ConnectionManagerImpl( *http_config, context.drainDecision(), context.random(), context.httpTracer(), context.runtime(), context.localInfo(), context.clusterManager())}); @@ -82,21 +94,24 @@ HttpConnectionManagerConfigUtility::determineNextProtocol(Network::Connection& c HttpConnectionManagerConfig::HttpConnectionManagerConfig( const envoy::api::v2::filter::HttpConnectionManager& config, FactoryContext& context, - Http::DateProvider& date_provider) + Http::DateProvider& date_provider, + Router::RouteConfigProviderManager& route_config_provider_manager) : context_(context), stats_prefix_(fmt::format("http.{}.", config.stat_prefix())), stats_(Http::ConnectionManagerImpl::generateStats(stats_prefix_, context_.scope())), tracing_stats_( Http::ConnectionManagerImpl::generateTracingStats(stats_prefix_, context_.scope())), use_remote_address_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, use_remote_address, false)), + route_config_provider_manager_(route_config_provider_manager), http2_settings_(Http::Utility::parseHttp2Settings(config.http2_protocol_options())), http1_settings_(Http::Utility::parseHttp1Settings(config.http_protocol_options())), drain_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, drain_timeout, 5000)), generate_request_id_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, generate_request_id, true)), + date_provider_(date_provider) { route_config_provider_ = Router::RouteConfigProviderUtil::create( config, context_.runtime(), context_.clusterManager(), context_.scope(), stats_prefix_, - context_.initManager(), context_.routeConfigProviderManager()); + context_.initManager(), route_config_provider_manager_); switch (config.forward_client_cert_details()) { case envoy::api::v2::filter::HttpConnectionManager::SANITIZE: diff --git a/source/server/config/network/http_connection_manager.h b/source/server/config/network/http_connection_manager.h index 56845e9a3a5ab..9463f566e060f 100644 --- a/source/server/config/network/http_connection_manager.h +++ b/source/server/config/network/http_connection_manager.h @@ -7,6 +7,7 @@ #include #include "envoy/http/filter.h" +#include "envoy/router/route_config_provider_manager.h" #include "envoy/server/filter_config.h" #include "common/common/logger.h" @@ -67,7 +68,8 @@ class HttpConnectionManagerConfig : Logger::Loggable, public Http::ConnectionManagerConfig { public: HttpConnectionManagerConfig(const envoy::api::v2::filter::HttpConnectionManager& config, - FactoryContext& context, Http::DateProvider& date_provider); + FactoryContext& context, Http::DateProvider& date_provider, + Router::RouteConfigProviderManager& route_config_provider_manager); // Http::FilterChainFactory void createFilterChain(Http::FilterChainFactoryCallbacks& callbacks) override; @@ -134,6 +136,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, const bool use_remote_address_{}; Http::ForwardClientCertType forward_client_cert_; std::vector set_current_client_cert_details_; + Router::RouteConfigProviderManager& route_config_provider_manager_; CodecType codec_type_; const Http::Http2Settings http2_settings_; const Http::Http1Settings http1_settings_; diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index ea28a70b0e474..bb1c3086ebffa 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -73,8 +73,6 @@ void ValidationInstance::initialize(Options& options, thread_local_.registerThread(*dispatcher_, true); runtime_loader_ = component_factory.createRuntime(*this, initial_config); ssl_context_manager_.reset(new Ssl::ContextManagerImpl(*runtime_loader_)); - route_config_provider_manager_.reset(new Router::RouteConfigProviderManagerImpl( - runtime(), dispatcher(), random(), localInfo(), threadLocal())); cluster_manager_factory_.reset(new Upstream::ValidationClusterManagerFactory( runtime(), stats(), threadLocal(), random(), dnsResolver(), sslContextManager(), dispatcher(), localInfo())); diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index 1140d69ec47cf..bc36cf548781a 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -86,9 +86,6 @@ class ValidationInstance : Logger::Loggable, Tracing::HttpTracer& httpTracer() override { return config_->httpTracer(); } ThreadLocal::Instance& threadLocal() override { return thread_local_; } const LocalInfo::LocalInfo& localInfo() override { return *local_info_; } - Router::ServerRouteConfigProviderManager& routeConfigProviderManager() override { - return *route_config_provider_manager_; - } // Server::ListenerComponentFactory std::vector @@ -131,7 +128,6 @@ class ValidationInstance : Logger::Loggable, AccessLog::AccessLogManagerImpl access_log_manager_; std::unique_ptr cluster_manager_factory_; InitManagerImpl init_manager_; - std::unique_ptr route_config_provider_manager_; ListenerManagerImpl listener_manager_; }; diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index 448e010509b12..c70fc252892fe 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -222,9 +222,6 @@ class ListenerImpl : public Listener, Stats::Scope& scope() override { return *global_scope_; } Singleton::Manager& singletonManager() override { return parent_.server_.singletonManager(); } ThreadLocal::Instance& threadLocal() override { return parent_.server_.threadLocal(); } - Router::RouteConfigProviderManager& routeConfigProviderManager() override { - return parent_.server_.routeConfigProviderManager(); - } // Network::DrainDecision bool drainClose() const override; diff --git a/source/server/server.cc b/source/server/server.cc index 9886dcc6287f9..3fde57ea4046d 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -201,9 +201,6 @@ void InstanceImpl::initialize(Options& options, runtime(), stats(), threadLocal(), random(), dnsResolver(), sslContextManager(), dispatcher(), localInfo())); - route_config_provider_manager_.reset(new Router::RouteConfigProviderManagerImpl( - runtime(), dispatcher(), random(), localInfo(), threadLocal())); - // Now the configuration gets parsed. The configuration may start setting thread local data // per above. See MainImpl::initialize() for why we do this pointer dance. Configuration::MainImpl* main_config = new Configuration::MainImpl(); diff --git a/source/server/server.h b/source/server/server.h index e027fa3c084d5..f15e3536765e8 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -133,9 +133,6 @@ class InstanceImpl : Logger::Loggable, public Instance { Tracing::HttpTracer& httpTracer() override; ThreadLocal::Instance& threadLocal() override { return thread_local_; } const LocalInfo::LocalInfo& localInfo() override { return *local_info_; } - Router::ServerRouteConfigProviderManager& routeConfigProviderManager() override { - return *route_config_provider_manager_; - } private: void flushStats(); diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index af3b0bd4061d2..ad0016a9262c2 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -102,8 +102,6 @@ MockInstance::MockInstance() ON_CALL(*this, drainManager()).WillByDefault(ReturnRef(drain_manager_)); ON_CALL(*this, initManager()).WillByDefault(ReturnRef(init_manager_)); ON_CALL(*this, listenerManager()).WillByDefault(ReturnRef(listener_manager_)); - ON_CALL(*this, routeConfigProviderManager()) - .WillByDefault(ReturnRef(route_config_provider_manager_)); ON_CALL(*this, singletonManager()).WillByDefault(ReturnRef(*singleton_manager_)); } @@ -133,8 +131,6 @@ MockFactoryContext::MockFactoryContext() : singleton_manager_(new Singleton::Man ON_CALL(*this, server()).WillByDefault(ReturnRef(server_)); ON_CALL(*this, singletonManager()).WillByDefault(ReturnRef(*singleton_manager_)); ON_CALL(*this, threadLocal()).WillByDefault(ReturnRef(thread_local_)); - ON_CALL(*this, routeConfigProviderManager()) - .WillByDefault(ReturnRef(route_config_provider_manager_)); } MockFactoryContext::~MockFactoryContext() {} diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index f28a6be516a2e..7f005da3e34e6 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -267,7 +267,6 @@ class MockInstance : public Instance { MOCK_METHOD0(httpTracer, Tracing::HttpTracer&()); MOCK_METHOD0(threadLocal, ThreadLocal::Instance&()); MOCK_METHOD0(localInfo, const LocalInfo::LocalInfo&()); - MOCK_METHOD0(routeConfigProviderManager, Router::ServerRouteConfigProviderManager&()); testing::NiceMock thread_local_; Stats::IsolatedStoreImpl stats_store_; @@ -288,7 +287,6 @@ class MockInstance : public Instance { testing::NiceMock random_; testing::NiceMock local_info_; testing::NiceMock init_manager_; - testing::NiceMock route_config_provider_manager_; testing::NiceMock listener_manager_; Singleton::ManagerPtr singleton_manager_; }; @@ -342,11 +340,9 @@ class MockFactoryContext : public FactoryContext { MOCK_METHOD0(server, Instance&()); MOCK_METHOD0(singletonManager, Singleton::Manager&()); MOCK_METHOD0(threadLocal, ThreadLocal::Instance&()); - MOCK_METHOD0(routeConfigProviderManager, Router::RouteConfigProviderManager&()); testing::NiceMock access_log_manager_; testing::NiceMock cluster_manager_; - testing::NiceMock route_config_provider_manager_; testing::NiceMock dispatcher_; testing::NiceMock drain_manager_; testing::NiceMock http_tracer_; diff --git a/test/server/config/network/BUILD b/test/server/config/network/BUILD index cfbb5dd5b83e5..3d37140f7a772 100644 --- a/test/server/config/network/BUILD +++ b/test/server/config/network/BUILD @@ -31,6 +31,7 @@ envoy_cc_test( "//source/common/buffer:buffer_lib", "//source/common/config:filter_json_lib", "//source/common/event:dispatcher_lib", + "//source/common/router:rds_lib", "//source/server/config/http:dynamo_lib", "//source/server/config/network:http_connection_manager_lib", "//test/mocks/network:network_mocks", diff --git a/test/server/config/network/http_connection_manager_test.cc b/test/server/config/network/http_connection_manager_test.cc index c9a1e922d6879..6482aa60f3590 100644 --- a/test/server/config/network/http_connection_manager_test.cc +++ b/test/server/config/network/http_connection_manager_test.cc @@ -1,6 +1,7 @@ #include "common/buffer/buffer_impl.h" #include "common/config/filter_json.h" #include "common/http/date_provider_impl.h" +#include "common/router/rds_impl.h" #include "server/config/network/http_connection_manager.h" @@ -32,6 +33,9 @@ class HttpConnectionManagerConfigTest : public testing::Test { public: NiceMock context_; Http::SlowDateProviderImpl date_provider_; + Router::RouteConfigProviderManagerImpl route_config_provider_manager_{ + context_.runtime(), context_.dispatcher(), context_.random(), context_.localInfo(), + context_.threadLocal()}; }; TEST_F(HttpConnectionManagerConfigTest, InvalidFilterName) { @@ -63,7 +67,7 @@ TEST_F(HttpConnectionManagerConfigTest, InvalidFilterName) { EXPECT_THROW_WITH_MESSAGE( HttpConnectionManagerConfig(parseHttpConnectionManagerFromJson(json_string), context_, - date_provider_), + date_provider_, route_config_provider_manager_), EnvoyException, "unable to create http filter factory for 'foo'"); } @@ -98,7 +102,8 @@ TEST_F(HttpConnectionManagerConfigTest, MiscConfig) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromJson(json_string), context_, - date_provider_); + date_provider_, route_config_provider_manager_); + EXPECT_THAT(std::vector({Http::LowerCaseString("foo")}), ContainerEq(config.tracingConfig()->request_headers_for_tags_)); EXPECT_EQ(*context_.local_info_.address_, config.localAddress());