diff --git a/include/envoy/config/BUILD b/include/envoy/config/BUILD index 777cf7a32fb2..ab8c3a0899f6 100644 --- a/include/envoy/config/BUILD +++ b/include/envoy/config/BUILD @@ -38,6 +38,15 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "subscription_factory_interface", + hdrs = ["subscription_factory.h"], + deps = [ + ":subscription_interface", + "@envoy_api//envoy/api/v2/core:config_source_cc", + ], +) + envoy_cc_library( name = "subscription_interface", hdrs = ["subscription.h"], diff --git a/include/envoy/config/subscription.h b/include/envoy/config/subscription.h index 707fab98ea3b..822a3eec68a6 100644 --- a/include/envoy/config/subscription.h +++ b/include/envoy/config/subscription.h @@ -79,6 +79,8 @@ class Subscription { virtual void updateResources(const std::set& update_to_these_names) PURE; }; +using SubscriptionPtr = std::unique_ptr; + /** * Per subscription stats. @see stats_macros.h */ diff --git a/include/envoy/config/subscription_factory.h b/include/envoy/config/subscription_factory.h new file mode 100644 index 000000000000..d5d6b5394b01 --- /dev/null +++ b/include/envoy/config/subscription_factory.h @@ -0,0 +1,33 @@ +#pragma once + +#include "envoy/api/api.h" +#include "envoy/api/v2/core/base.pb.h" +#include "envoy/config/subscription.h" +#include "envoy/stats/scope.h" +#include "envoy/upstream/cluster_manager.h" + +namespace Envoy { +namespace Config { + +class SubscriptionFactory { +public: + virtual ~SubscriptionFactory() {} + + /** + * Subscription factory interface. + * + * @param config envoy::api::v2::core::ConfigSource to construct from. + * @param type_url type URL for the resource being subscribed to. + * @param scope stats scope for any stats tracked by the subscription. + * @param callbacks the callbacks needed by all Subscription objects, to deliver config updates. + * The callbacks must not result in the deletion of the Subscription object. + * @return SubscriptionPtr subscription object corresponding for config and type_url. + */ + virtual SubscriptionPtr + subscriptionFromConfigSource(const envoy::api::v2::core::ConfigSource& config, + absl::string_view type_url, Stats::Scope& scope, + SubscriptionCallbacks& callbacks) PURE; +}; + +} // namespace Config +} // namespace Envoy diff --git a/include/envoy/upstream/BUILD b/include/envoy/upstream/BUILD index e20c85e1d24e..b30d86753289 100644 --- a/include/envoy/upstream/BUILD +++ b/include/envoy/upstream/BUILD @@ -18,6 +18,7 @@ envoy_cc_library( ":upstream_interface", "//include/envoy/access_log:access_log_interface", "//include/envoy/config:grpc_mux_interface", + "//include/envoy/config:subscription_factory_interface", "//include/envoy/grpc:async_client_manager_interface", "//include/envoy/http:async_client_interface", "//include/envoy/http:conn_pool_interface", diff --git a/include/envoy/upstream/cluster_manager.h b/include/envoy/upstream/cluster_manager.h index 1928db4a7e9c..b376b3838bde 100644 --- a/include/envoy/upstream/cluster_manager.h +++ b/include/envoy/upstream/cluster_manager.h @@ -11,6 +11,7 @@ #include "envoy/api/v2/cds.pb.h" #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "envoy/config/grpc_mux.h" +#include "envoy/config/subscription_factory.h" #include "envoy/grpc/async_client_manager.h" #include "envoy/http/async_client.h" #include "envoy/http/conn_pool.h" @@ -29,6 +30,11 @@ #include "envoy/upstream/upstream.h" namespace Envoy { + +namespace Config { +class SubscriptionFactory; +} + namespace Upstream { /** @@ -131,7 +137,7 @@ class ClusterManager { * If information about the cluster needs to be kept, use the ThreadLocalCluster::info() method to * obtain cluster information that is safe to store. */ - virtual ThreadLocalCluster* get(const std::string& cluster) PURE; + virtual ThreadLocalCluster* get(absl::string_view cluster) PURE; /** * Allocate a load balanced HTTP connection pool for a cluster. This is *per-thread* so that @@ -235,6 +241,14 @@ class ClusterManager { virtual ClusterManagerFactory& clusterManagerFactory() PURE; + /** + * Obtain the subscription factory for the cluster manager. Since subscriptions may have an + * upstream component, the factory is a facet of the cluster manager. + * + * @return Config::SubscriptionFactory& the subscription factory. + */ + virtual Config::SubscriptionFactory& subscriptionFactory() PURE; + virtual std::size_t warmingClusterCount() const PURE; }; diff --git a/source/common/config/BUILD b/source/common/config/BUILD index f1080638fb34..6dee36decdbb 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -311,8 +311,8 @@ envoy_cc_library( envoy_cc_library( name = "subscription_factory_lib", - srcs = ["subscription_factory.cc"], - hdrs = ["subscription_factory.h"], + srcs = ["subscription_factory_impl.cc"], + hdrs = ["subscription_factory_impl.h"], deps = [ ":delta_subscription_lib", ":filesystem_subscription_lib", @@ -321,6 +321,7 @@ envoy_cc_library( ":http_subscription_lib", ":type_to_endpoint_lib", ":utility_lib", + "//include/envoy/config:subscription_factory_interface", "//include/envoy/config:subscription_interface", "//include/envoy/upstream:cluster_manager_interface", "//source/common/protobuf", diff --git a/source/common/config/subscription_factory.h b/source/common/config/subscription_factory.h deleted file mode 100644 index 8b557e46fc2f..000000000000 --- a/source/common/config/subscription_factory.h +++ /dev/null @@ -1,42 +0,0 @@ -#pragma once - -#include "envoy/api/api.h" -#include "envoy/api/v2/core/base.pb.h" -#include "envoy/config/subscription.h" -#include "envoy/stats/scope.h" -#include "envoy/upstream/cluster_manager.h" - -namespace Envoy { -namespace Config { - -class SubscriptionFactory { -public: - /** - * Subscription factory. - * @param config envoy::api::v2::core::ConfigSource to construct from. - * @param local_info LocalInfo::LocalInfo local info. - * @param dispatcher event dispatcher. - * @param cm cluster manager for async clients (when REST/gRPC). - * @param random random generator for jittering polling delays (when REST). - * @param scope stats scope. - * @param rest_legacy_constructor constructor function for Subscription adapters (when legacy v1 - * REST). - * @param rest_method fully qualified name of v2 REST API method (as per protobuf service - * description). - * @param grpc_method fully qualified name of v2 gRPC API bidi streaming method (as per protobuf - * service description). - * @param validation_visitor message validation visitor instance. - * @param api reference to the Api object - * @param callbacks the callbacks needed by all Subscription objects, to deliver config updates. - * The callbacks must not result in the deletion of the Subscription object. - */ - static std::unique_ptr subscriptionFromConfigSource( - const envoy::api::v2::core::ConfigSource& config, const LocalInfo::LocalInfo& local_info, - Event::Dispatcher& dispatcher, Upstream::ClusterManager& cm, Runtime::RandomGenerator& random, - Stats::Scope& scope, absl::string_view type_url, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, - SubscriptionCallbacks& callbacks); -}; - -} // namespace Config -} // namespace Envoy diff --git a/source/common/config/subscription_factory.cc b/source/common/config/subscription_factory_impl.cc similarity index 66% rename from source/common/config/subscription_factory.cc rename to source/common/config/subscription_factory_impl.cc index 677634e822c3..16647dbc3154 100644 --- a/source/common/config/subscription_factory.cc +++ b/source/common/config/subscription_factory_impl.cc @@ -1,4 +1,4 @@ -#include "common/config/subscription_factory.h" +#include "common/config/subscription_factory_impl.h" #include "common/config/delta_subscription_impl.h" #include "common/config/filesystem_subscription_impl.h" @@ -12,24 +12,29 @@ namespace Envoy { namespace Config { -std::unique_ptr SubscriptionFactory::subscriptionFromConfigSource( - const envoy::api::v2::core::ConfigSource& config, const LocalInfo::LocalInfo& local_info, - Event::Dispatcher& dispatcher, Upstream::ClusterManager& cm, Runtime::RandomGenerator& random, - Stats::Scope& scope, absl::string_view type_url, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, - SubscriptionCallbacks& callbacks) { +SubscriptionFactoryImpl::SubscriptionFactoryImpl( + const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, + Upstream::ClusterManager& cm, Runtime::RandomGenerator& random, + ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) + : local_info_(local_info), dispatcher_(dispatcher), cm_(cm), random_(random), + validation_visitor_(validation_visitor), api_(api) {} + +SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( + const envoy::api::v2::core::ConfigSource& config, absl::string_view type_url, + Stats::Scope& scope, SubscriptionCallbacks& callbacks) { + Config::Utility::checkLocalInfo(type_url, local_info_); std::unique_ptr result; SubscriptionStats stats = Utility::generateStats(scope); switch (config.config_source_specifier_case()) { case envoy::api::v2::core::ConfigSource::kPath: { - Utility::checkFilesystemSubscriptionBackingPath(config.path(), api); + Utility::checkFilesystemSubscriptionBackingPath(config.path(), api_); result = std::make_unique( - dispatcher, config.path(), callbacks, stats, validation_visitor, api); + dispatcher_, config.path(), callbacks, stats, validation_visitor_, api_); break; } case envoy::api::v2::core::ConfigSource::kApiConfigSource: { const envoy::api::v2::core::ApiConfigSource& api_config_source = config.api_config_source(); - Utility::checkApiConfigSourceSubscriptionBackingCluster(cm.clusters(), api_config_source); + Utility::checkApiConfigSourceSubscriptionBackingCluster(cm_.clusters(), api_config_source); switch (api_config_source.api_type()) { case envoy::api::v2::core::ApiConfigSource::UNSUPPORTED_REST_LEGACY: throw EnvoyException( @@ -38,29 +43,29 @@ std::unique_ptr SubscriptionFactory::subscriptionFromConfigSource( config.DebugString()); case envoy::api::v2::core::ApiConfigSource::REST: result = std::make_unique( - local_info, cm, api_config_source.cluster_names()[0], dispatcher, random, + local_info_, cm_, api_config_source.cluster_names()[0], dispatcher_, random_, Utility::apiConfigSourceRefreshDelay(api_config_source), Utility::apiConfigSourceRequestTimeout(api_config_source), restMethod(type_url), - callbacks, stats, Utility::configSourceInitialFetchTimeout(config), validation_visitor); + callbacks, stats, Utility::configSourceInitialFetchTimeout(config), validation_visitor_); break; case envoy::api::v2::core::ApiConfigSource::GRPC: result = std::make_unique( - local_info, - Config::Utility::factoryForGrpcApiConfigSource(cm.grpcAsyncClientManager(), + local_info_, + Config::Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), api_config_source, scope) ->create(), - dispatcher, random, sotwGrpcMethod(type_url), type_url, callbacks, stats, scope, + dispatcher_, random_, sotwGrpcMethod(type_url), type_url, callbacks, stats, scope, Utility::parseRateLimitSettings(api_config_source), Utility::configSourceInitialFetchTimeout(config)); break; case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { - Utility::checkApiConfigSourceSubscriptionBackingCluster(cm.clusters(), api_config_source); + Utility::checkApiConfigSourceSubscriptionBackingCluster(cm_.clusters(), api_config_source); result = std::make_unique( - local_info, - Config::Utility::factoryForGrpcApiConfigSource(cm.grpcAsyncClientManager(), + local_info_, + Config::Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), api_config_source, scope) ->create(), - dispatcher, deltaGrpcMethod(type_url), type_url, random, scope, + dispatcher_, deltaGrpcMethod(type_url), type_url, random_, scope, Utility::parseRateLimitSettings(api_config_source), callbacks, stats, Utility::configSourceInitialFetchTimeout(config)); break; @@ -72,7 +77,7 @@ std::unique_ptr SubscriptionFactory::subscriptionFromConfigSource( } case envoy::api::v2::core::ConfigSource::kAds: { result = std::make_unique( - cm.adsMux(), callbacks, stats, type_url, dispatcher, + cm_.adsMux(), callbacks, stats, type_url, dispatcher_, Utility::configSourceInitialFetchTimeout(config)); break; } diff --git a/source/common/config/subscription_factory_impl.h b/source/common/config/subscription_factory_impl.h new file mode 100644 index 000000000000..3a7f0ba1e31c --- /dev/null +++ b/source/common/config/subscription_factory_impl.h @@ -0,0 +1,34 @@ +#pragma once + +#include "envoy/api/api.h" +#include "envoy/api/v2/core/base.pb.h" +#include "envoy/config/subscription.h" +#include "envoy/config/subscription_factory.h" +#include "envoy/stats/scope.h" +#include "envoy/upstream/cluster_manager.h" + +namespace Envoy { +namespace Config { + +class SubscriptionFactoryImpl : public SubscriptionFactory { +public: + SubscriptionFactoryImpl(const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, + Upstream::ClusterManager& cm, Runtime::RandomGenerator& random, + ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); + + // Config::SubscriptionFactory + SubscriptionPtr subscriptionFromConfigSource(const envoy::api::v2::core::ConfigSource& config, + absl::string_view type_url, Stats::Scope& scope, + SubscriptionCallbacks& callbacks) override; + +private: + const LocalInfo::LocalInfo& local_info_; + Event::Dispatcher& dispatcher_; + Upstream::ClusterManager& cm_; + Runtime::RandomGenerator& random_; + ProtobufMessage::ValidationVisitor& validation_visitor_; + Api::Api& api_; +}; + +} // namespace Config +} // namespace Envoy diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index 68aa486bde8a..d22cc0a4cca6 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -44,7 +44,7 @@ void Utility::translateApiConfigSource(const std::string& cluster, uint32_t refr Protobuf::util::TimeUtil::MillisecondsToDuration(refresh_delay_ms)); } -void Utility::checkCluster(const std::string& error_prefix, const std::string& cluster_name, +void Utility::checkCluster(absl::string_view error_prefix, absl::string_view cluster_name, Upstream::ClusterManager& cm) { Upstream::ThreadLocalCluster* cluster = cm.get(cluster_name); if (cluster == nullptr) { @@ -58,15 +58,14 @@ void Utility::checkCluster(const std::string& error_prefix, const std::string& c } } -void Utility::checkClusterAndLocalInfo(const std::string& error_prefix, - const std::string& cluster_name, - Upstream::ClusterManager& cm, +void Utility::checkClusterAndLocalInfo(absl::string_view error_prefix, + absl::string_view cluster_name, Upstream::ClusterManager& cm, const LocalInfo::LocalInfo& local_info) { checkCluster(error_prefix, cluster_name, cm); checkLocalInfo(error_prefix, local_info); } -void Utility::checkLocalInfo(const std::string& error_prefix, +void Utility::checkLocalInfo(absl::string_view error_prefix, const LocalInfo::LocalInfo& local_info) { if (local_info.clusterName().empty() || local_info.nodeName().empty()) { throw EnvoyException( diff --git a/source/common/config/utility.h b/source/common/config/utility.h index b51e8a61c243..8181e2022773 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -110,7 +110,7 @@ class Utility { * @param cluster_name supplies the cluster name to check. * @param cm supplies the cluster manager. */ - static void checkCluster(const std::string& error_prefix, const std::string& cluster_name, + static void checkCluster(absl::string_view error_prefix, absl::string_view cluster_name, Upstream::ClusterManager& cm); /** @@ -120,9 +120,8 @@ class Utility { * @param cm supplies the cluster manager. * @param local_info supplies the local info. */ - static void checkClusterAndLocalInfo(const std::string& error_prefix, - const std::string& cluster_name, - Upstream::ClusterManager& cm, + static void checkClusterAndLocalInfo(absl::string_view error_prefix, + absl::string_view cluster_name, Upstream::ClusterManager& cm, const LocalInfo::LocalInfo& local_info); /** @@ -130,7 +129,7 @@ class Utility { * @param error_prefix supplies the prefix to use in error messages. * @param local_info supplies the local info. */ - static void checkLocalInfo(const std::string& error_prefix, + static void checkLocalInfo(absl::string_view error_prefix, const LocalInfo::LocalInfo& local_info); /** diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 4bc5d19057f7..445cc2232673 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -108,7 +108,6 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", "//source/common/config:rds_json_lib", - "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", "//source/common/init:target_lib", "//source/common/protobuf:utility_lib", @@ -181,7 +180,6 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", "//source/common/config:config_provider_lib", - "//source/common/config:subscription_factory_lib", "@envoy_api//envoy/admin/v2alpha:config_dump_cc", "@envoy_api//envoy/api/v2:srds_cc", ], diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 5ea463ec37ce..12b5a7565802 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -65,13 +65,11 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( route_config_provider_manager_(route_config_provider_manager), manager_identifier_(manager_identifier), validation_visitor_(factory_context_.messageValidationVisitor()) { - Envoy::Config::Utility::checkLocalInfo("rds", factory_context.localInfo()); - - subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource( - rds.config_source(), factory_context.localInfo(), factory_context.dispatcher(), - factory_context.clusterManager(), factory_context.random(), *scope_, - Grpc::Common::typeUrl(envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name()), - factory_context.messageValidationVisitor(), factory_context.api(), *this); + subscription_ = + factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( + rds.config_source(), + Grpc::Common::typeUrl(envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name()), + *scope_, *this); config_update_info_ = std::make_unique( factory_context.timeSource(), factory_context.messageValidationVisitor()); diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index c404b8da1e81..ee311b63a163 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -23,7 +23,6 @@ #include "envoy/thread_local/thread_local.h" #include "common/common/logger.h" -#include "common/config/subscription_factory.h" #include "common/init/target_impl.h" #include "common/protobuf/utility.h" #include "common/router/route_config_update_receiver_impl.h" @@ -108,6 +107,7 @@ class RdsRouteConfigSubscription : Envoy::Config::SubscriptionCallbacks, } RouteConfigUpdatePtr& routeConfigUpdate() { return config_update_info_; } +private: // Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const std::string& version_info) override; @@ -121,7 +121,6 @@ class RdsRouteConfigSubscription : Envoy::Config::SubscriptionCallbacks, .name(); } -private: RdsRouteConfigSubscription( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, const uint64_t manager_identifier, Server::Configuration::FactoryContext& factory_context, diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index b2de09f8a8e9..9694c38f78f9 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -7,7 +7,6 @@ #include "common/common/assert.h" #include "common/common/logger.h" -#include "common/config/subscription_factory.h" // Types are deeply nested under Envoy::Config::ConfigProvider; use 'using-directives' across all // ConfigProvider related types for consistency. @@ -85,13 +84,12 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription( scope_(factory_context.scope().createScope(stat_prefix + "scoped_rds." + name + ".")), stats_({ALL_SCOPED_RDS_STATS(POOL_COUNTER(*scope_))}), validation_visitor_(factory_context.messageValidationVisitor()) { - subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource( - scoped_rds.scoped_rds_config_source(), factory_context.localInfo(), - factory_context.dispatcher(), factory_context.clusterManager(), factory_context.random(), - *scope_, - Grpc::Common::typeUrl( - envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name()), - validation_visitor_, factory_context.api(), *this); + subscription_ = + factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( + scoped_rds.scoped_rds_config_source(), + Grpc::Common::typeUrl( + envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name()), + *scope_, *this); } void ScopedRdsConfigSubscription::onConfigUpdate( diff --git a/source/common/router/scoped_rds.h b/source/common/router/scoped_rds.h index 19dab8f4482b..f0cc72f71c15 100644 --- a/source/common/router/scoped_rds.h +++ b/source/common/router/scoped_rds.h @@ -4,7 +4,6 @@ #include "envoy/api/v2/srds.pb.h" #include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.h" -#include "envoy/config/subscription.h" #include "envoy/stats/scope.h" #include "common/config/config_provider_impl.h" @@ -94,6 +93,11 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio const std::string& name() const { return name_; } + const ScopedConfigManager::ScopedRouteMap& scopedRouteMap() const { + return scoped_config_manager_.scopedRouteMap(); + } + +private: // Envoy::Config::ConfigSubscriptionCommonBase void start() override { subscription_->start({}); } @@ -112,11 +116,7 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio validation_visitor_) .name(); } - const ScopedConfigManager::ScopedRouteMap& scopedRouteMap() const { - return scoped_config_manager_.scopedRouteMap(); - } -private: const std::string name_; std::unique_ptr subscription_; Stats::ScopePtr scope_; diff --git a/source/common/router/vhds.cc b/source/common/router/vhds.cc index 3bd07c05fee9..100c9ef6034e 100644 --- a/source/common/router/vhds.cc +++ b/source/common/router/vhds.cc @@ -21,8 +21,7 @@ namespace Router { VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, - std::unordered_set& route_config_providers, - SubscriptionFactoryFunction factory_function) + std::unordered_set& route_config_providers) : config_update_info_(config_update_info), init_target_(fmt::format("VhdsConfigSubscription {}", config_update_info_->routeConfigName()), [this]() { subscription_->start({}); }), @@ -31,7 +30,6 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, stats_({ALL_VHDS_STATS(POOL_COUNTER(*scope_))}), route_config_providers_(route_config_providers), validation_visitor_(factory_context.messageValidationVisitor()) { - Envoy::Config::Utility::checkLocalInfo("vhds", factory_context.localInfo()); const auto& config_source = config_update_info_->routeConfiguration() .vhds() .config_source() @@ -41,12 +39,11 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, throw EnvoyException("vhds: only 'DELTA_GRPC' is supported as an api_type."); } - subscription_ = factory_function( - config_update_info_->routeConfiguration().vhds().config_source(), factory_context.localInfo(), - factory_context.dispatcher(), factory_context.clusterManager(), factory_context.random(), - *scope_, - Grpc::Common::typeUrl(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name()), - factory_context.messageValidationVisitor(), factory_context.api(), *this); + subscription_ = + factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( + config_update_info_->routeConfiguration().vhds().config_source(), + Grpc::Common::typeUrl(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name()), + *scope_, *this); } void VhdsSubscription::onConfigUpdateFailed(const EnvoyException*) { diff --git a/source/common/router/vhds.h b/source/common/router/vhds.h index 2463b177c6bb..7c5b370babe1 100644 --- a/source/common/router/vhds.h +++ b/source/common/router/vhds.h @@ -20,7 +20,6 @@ #include "envoy/thread_local/thread_local.h" #include "common/common/logger.h" -#include "common/config/subscription_factory.h" #include "common/init/target_impl.h" #include "common/protobuf/utility.h" @@ -38,23 +37,18 @@ struct VhdsStats { ALL_VHDS_STATS(GENERATE_COUNTER_STRUCT) }; -typedef std::unique_ptr (*SubscriptionFactoryFunction)( - const envoy::api::v2::core::ConfigSource&, const LocalInfo::LocalInfo&, Event::Dispatcher&, - Upstream::ClusterManager&, Envoy::Runtime::RandomGenerator&, Stats::Scope&, absl::string_view, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api&, - Envoy::Config::SubscriptionCallbacks&); - class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, Logger::Loggable { public: VhdsSubscription(RouteConfigUpdatePtr& config_update_info, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, - std::unordered_set& route_config_providers, - SubscriptionFactoryFunction factory_function = - Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource); + std::unordered_set& route_config_providers); ~VhdsSubscription() override { init_target_.ready(); } + void registerInitTargetWithInitManager(Init::Manager& m) { m.add(init_target_); } + +private: // Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField&, const std::string&) override { @@ -68,7 +62,6 @@ class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, validation_visitor_) .name(); } - void registerInitTargetWithInitManager(Init::Manager& m) { m.add(init_target_); } RouteConfigUpdatePtr& config_update_info_; std::unique_ptr subscription_; diff --git a/source/common/secret/BUILD b/source/common/secret/BUILD index a876b3f69788..ca30d94ddfb3 100644 --- a/source/common/secret/BUILD +++ b/source/common/secret/BUILD @@ -40,6 +40,7 @@ envoy_cc_library( srcs = ["sds_api.cc"], hdrs = ["sds_api.h"], deps = [ + "//include/envoy/config:subscription_factory_interface", "//include/envoy/config:subscription_interface", "//include/envoy/event:dispatcher_interface", "//include/envoy/init:manager_interface", @@ -51,7 +52,6 @@ envoy_cc_library( "//source/common/common:callback_impl_lib", "//source/common/common:cleanup_lib", "//source/common/config:resources_lib", - "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", "//source/common/init:target_lib", "//source/common/protobuf:utility_lib", diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index 657aa9d3d54e..5b5ea85395fc 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -5,25 +5,20 @@ #include "envoy/api/v2/auth/cert.pb.validate.h" #include "common/config/resources.h" -#include "common/config/subscription_factory.h" -#include "common/config/utility.h" #include "common/protobuf/utility.h" namespace Envoy { namespace Secret { -SdsApi::SdsApi(const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random, Stats::Store& stats, - Upstream::ClusterManager& cluster_manager, Init::Manager& init_manager, - const envoy::api::v2::core::ConfigSource& sds_config, - const std::string& sds_config_name, std::function destructor_cb, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) +SdsApi::SdsApi(const envoy::api::v2::core::ConfigSource& sds_config, + const std::string& sds_config_name, + Config::SubscriptionFactory& subscription_factory, + ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats, + Init::Manager& init_manager, std::function destructor_cb) : init_target_(fmt::format("SdsApi {}", sds_config_name), [this] { initialize(); }), - local_info_(local_info), dispatcher_(dispatcher), random_(random), stats_(stats), - cluster_manager_(cluster_manager), sds_config_(sds_config), sds_config_name_(sds_config_name), - secret_hash_(0), clean_up_(destructor_cb), validation_visitor_(validation_visitor), - api_(api) { - Config::Utility::checkLocalInfo("sds", local_info_); + stats_(stats), sds_config_(sds_config), sds_config_name_(sds_config_name), secret_hash_(0), + clean_up_(destructor_cb), validation_visitor_(validation_visitor), + subscription_factory_(subscription_factory) { // TODO(JimmyCYJ): Implement chained_init_manager, so that multiple init_manager // can be chained together to behave as one init_manager. In that way, we let // two listeners which share same SdsApi to register at separate init managers, and @@ -78,10 +73,10 @@ void SdsApi::validateUpdateSize(int num_resources) { } void SdsApi::initialize() { - subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource( - sds_config_, local_info_, dispatcher_, cluster_manager_, random_, stats_, - Grpc::Common::typeUrl(envoy::api::v2::auth::Secret().GetDescriptor()->full_name()), - validation_visitor_, api_, *this); + subscription_ = subscription_factory_.subscriptionFromConfigSource( + sds_config_, + Grpc::Common::typeUrl(envoy::api::v2::auth::Secret().GetDescriptor()->full_name()), stats_, + *this); subscription_->start({sds_config_name_}); } diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index 7bff87d90e22..77336cecbd59 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -6,6 +6,7 @@ #include "envoy/api/v2/auth/cert.pb.h" #include "envoy/api/v2/core/config_source.pb.h" #include "envoy/config/subscription.h" +#include "envoy/config/subscription_factory.h" #include "envoy/event/dispatcher.h" #include "envoy/init/manager.h" #include "envoy/local_info/local_info.h" @@ -18,6 +19,7 @@ #include "common/common/callback_impl.h" #include "common/common/cleanup.h" +#include "common/config/utility.h" #include "common/init/target_impl.h" #include "common/ssl/certificate_validation_context_config_impl.h" #include "common/ssl/tls_certificate_config_impl.h" @@ -30,12 +32,16 @@ namespace Secret { */ class SdsApi : public Config::SubscriptionCallbacks { public: - SdsApi(const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random, Stats::Store& stats, - Upstream::ClusterManager& cluster_manager, Init::Manager& init_manager, - const envoy::api::v2::core::ConfigSource& sds_config, const std::string& sds_config_name, - std::function destructor_cb, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); + SdsApi(const envoy::api::v2::core::ConfigSource& sds_config, const std::string& sds_config_name, + Config::SubscriptionFactory& subscription_factory, + ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats, + Init::Manager& init_manager, std::function destructor_cb); + +protected: + // Creates new secrets. + virtual void setSecret(const envoy::api::v2::auth::Secret&) PURE; + virtual void validateConfig(const envoy::api::v2::auth::Secret&) PURE; + Common::CallbackManager<> update_callback_manager_; // Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, @@ -48,21 +54,11 @@ class SdsApi : public Config::SubscriptionCallbacks { .name(); } -protected: - // Creates new secrets. - virtual void setSecret(const envoy::api::v2::auth::Secret&) PURE; - virtual void validateConfig(const envoy::api::v2::auth::Secret&) PURE; - Common::CallbackManager<> update_callback_manager_; - private: void validateUpdateSize(int num_resources); void initialize(); Init::TargetImpl init_target_; - const LocalInfo::LocalInfo& local_info_; - Event::Dispatcher& dispatcher_; - Runtime::RandomGenerator& random_; Stats::Store& stats_; - Upstream::ClusterManager& cluster_manager_; const envoy::api::v2::core::ConfigSource sds_config_; std::unique_ptr subscription_; @@ -71,7 +67,7 @@ class SdsApi : public Config::SubscriptionCallbacks { uint64_t secret_hash_; Cleanup clean_up_; ProtobufMessage::ValidationVisitor& validation_visitor_; - Api::Api& api_; + Config::SubscriptionFactory& subscription_factory_; }; class TlsCertificateSdsApi; @@ -89,22 +85,22 @@ class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider create(Server::Configuration::TransportSocketFactoryContext& secret_provider_context, const envoy::api::v2::core::ConfigSource& sds_config, const std::string& sds_config_name, std::function destructor_cb) { + // We need to do this early as we invoke the subscription factory during initialization, which + // is too late to throw. + Config::Utility::checkLocalInfo("TlsCertificateSdsApi", secret_provider_context.localInfo()); return std::make_shared( - secret_provider_context.localInfo(), secret_provider_context.dispatcher(), - secret_provider_context.random(), secret_provider_context.stats(), - secret_provider_context.clusterManager(), *secret_provider_context.initManager(), - sds_config, sds_config_name, destructor_cb, - secret_provider_context.messageValidationVisitor(), secret_provider_context.api()); + sds_config, sds_config_name, secret_provider_context.clusterManager().subscriptionFactory(), + secret_provider_context.messageValidationVisitor(), secret_provider_context.stats(), + *secret_provider_context.initManager(), destructor_cb); } - TlsCertificateSdsApi(const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random, Stats::Store& stats, - Upstream::ClusterManager& cluster_manager, Init::Manager& init_manager, - const envoy::api::v2::core::ConfigSource& sds_config, - const std::string& sds_config_name, std::function destructor_cb, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) - : SdsApi(local_info, dispatcher, random, stats, cluster_manager, init_manager, sds_config, - sds_config_name, destructor_cb, validation_visitor, api) {} + TlsCertificateSdsApi(const envoy::api::v2::core::ConfigSource& sds_config, + const std::string& sds_config_name, + Config::SubscriptionFactory& subscription_factory, + ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats, + Init::Manager& init_manager, std::function destructor_cb) + : SdsApi(sds_config, sds_config_name, subscription_factory, validation_visitor, stats, + init_manager, destructor_cb) {} // SecretProvider const envoy::api::v2::auth::TlsCertificate* secret() const override { @@ -136,22 +132,23 @@ class CertificateValidationContextSdsApi : public SdsApi, create(Server::Configuration::TransportSocketFactoryContext& secret_provider_context, const envoy::api::v2::core::ConfigSource& sds_config, const std::string& sds_config_name, std::function destructor_cb) { + // We need to do this early as we invoke the subscription factory during initialization, which + // is too late to throw. + Config::Utility::checkLocalInfo("CertificateValidationContextSdsApi", + secret_provider_context.localInfo()); return std::make_shared( - secret_provider_context.localInfo(), secret_provider_context.dispatcher(), - secret_provider_context.random(), secret_provider_context.stats(), - secret_provider_context.clusterManager(), *secret_provider_context.initManager(), - sds_config, sds_config_name, destructor_cb, - secret_provider_context.messageValidationVisitor(), secret_provider_context.api()); + sds_config, sds_config_name, secret_provider_context.clusterManager().subscriptionFactory(), + secret_provider_context.messageValidationVisitor(), secret_provider_context.stats(), + *secret_provider_context.initManager(), destructor_cb); } - CertificateValidationContextSdsApi( - const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random, Stats::Store& stats, - Upstream::ClusterManager& cluster_manager, Init::Manager& init_manager, - const envoy::api::v2::core::ConfigSource& sds_config, std::string sds_config_name, - std::function destructor_cb, ProtobufMessage::ValidationVisitor& validation_visitor, - Api::Api& api) - : SdsApi(local_info, dispatcher, random, stats, cluster_manager, init_manager, sds_config, - sds_config_name, destructor_cb, validation_visitor, api) {} + CertificateValidationContextSdsApi(const envoy::api::v2::core::ConfigSource& sds_config, + const std::string& sds_config_name, + Config::SubscriptionFactory& subscription_factory, + ProtobufMessage::ValidationVisitor& validation_visitor, + Stats::Store& stats, Init::Manager& init_manager, + std::function destructor_cb) + : SdsApi(sds_config, sds_config_name, subscription_factory, validation_visitor, stats, + init_manager, destructor_cb) {} // SecretProvider const envoy::api::v2::auth::CertificateValidationContext* secret() const override { diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 66d7dfce6b4b..f1a69acf8942 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -14,13 +14,13 @@ envoy_cc_library( srcs = ["cds_api_impl.cc"], hdrs = ["cds_api_impl.h"], deps = [ + "//include/envoy/config:subscription_factory_interface", "//include/envoy/config:subscription_interface", "//include/envoy/event:dispatcher_interface", "//include/envoy/local_info:local_info_interface", "//source/common/common:cleanup_lib", "//source/common/common:minimal_logger_lib", "//source/common/config:resources_lib", - "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", "//source/common/protobuf:utility_lib", "@envoy_api//envoy/api/v2:cds_cc", @@ -51,6 +51,7 @@ envoy_cc_library( "//source/common/common:utility_lib", "//source/common/config:cds_json_lib", "//source/common/config:grpc_mux_lib", + "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", "//source/common/grpc:async_client_manager_lib", "//source/common/http:async_client_lib", @@ -312,6 +313,7 @@ envoy_cc_library( ":cluster_factory_lib", ":upstream_includes", "//include/envoy/config:grpc_mux_interface", + "//include/envoy/config:subscription_factory_interface", "//include/envoy/config:subscription_interface", "//include/envoy/local_info:local_info_interface", "//include/envoy/secret:secret_manager_interface", @@ -495,6 +497,7 @@ envoy_cc_library( "//source/common/common:enum_to_int", "//source/common/common:minimal_logger_lib", "//source/common/config:metadata_lib", + "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", "//source/common/config:well_known_names", "//source/common/stats:isolated_store_lib", diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index a2840fdefe59..ff375bc5ff50 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -9,7 +9,6 @@ #include "common/common/cleanup.h" #include "common/common/utility.h" #include "common/config/resources.h" -#include "common/config/subscription_factory.h" #include "common/config/utility.h" #include "common/protobuf/utility.h" @@ -17,27 +16,18 @@ namespace Envoy { namespace Upstream { CdsApiPtr CdsApiImpl::create(const envoy::api::v2::core::ConfigSource& cds_config, - ClusterManager& cm, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random, - const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, - ProtobufMessage::ValidationVisitor& validation_visitor, - Api::Api& api) { - return CdsApiPtr{new CdsApiImpl(cds_config, cm, dispatcher, random, local_info, scope, - validation_visitor, api)}; + ClusterManager& cm, Stats::Scope& scope, + ProtobufMessage::ValidationVisitor& validation_visitor) { + return CdsApiPtr{new CdsApiImpl(cds_config, cm, scope, validation_visitor)}; } CdsApiImpl::CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm, - Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, - const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) + Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor) : cm_(cm), scope_(scope.createScope("cluster_manager.cds.")), validation_visitor_(validation_visitor) { - Config::Utility::checkLocalInfo("cds", local_info); - - subscription_ = Config::SubscriptionFactory::subscriptionFromConfigSource( - cds_config, local_info, dispatcher, cm, random, *scope_, - Grpc::Common::typeUrl(envoy::api::v2::Cluster().GetDescriptor()->full_name()), - validation_visitor, api, *this); + subscription_ = cm_.subscriptionFactory().subscriptionFromConfigSource( + cds_config, Grpc::Common::typeUrl(envoy::api::v2::Cluster().GetDescriptor()->full_name()), + *scope_, *this); } void CdsApiImpl::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, @@ -80,7 +70,8 @@ void CdsApiImpl::onConfigUpdate( validation_visitor_); MessageUtil::validate(cluster); if (!cluster_names.insert(cluster.name()).second) { - // NOTE: at this point, the first of these duplicates has already been successfully applied. + // NOTE: at this point, the first of these duplicates has already been successfully + // applied. throw EnvoyException(fmt::format("duplicate cluster {} found", cluster.name())); } if (cm_.addOrUpdateCluster( @@ -88,21 +79,22 @@ void CdsApiImpl::onConfigUpdate( [this](const std::string&, ClusterManager::ClusterWarmingState state) { // Following if/else block implements a control flow mechanism that can be used // by an ADS implementation to properly sequence CDS and RDS update. It is not - // enforcing on ADS. ADS can use it to detect when a previously sent cluster becomes - // warm before sending routes that depend on it. This can improve incidence of HTTP - // 503 responses from Envoy when a route is used before it's supporting cluster is - // ready. + // enforcing on ADS. ADS can use it to detect when a previously sent cluster + // becomes warm before sending routes that depend on it. This can improve + // incidence of HTTP 503 responses from Envoy when a route is used before it's + // supporting cluster is ready. // - // We achieve that by leaving CDS in the paused state as long as there is at least - // one cluster in the warming state. This prevents CDS ACK from being sent to ADS. - // Once cluster is warmed up, CDS is resumed, and ACK is sent to ADS, providing a - // signal to ADS to proceed with RDS updates. + // We achieve that by leaving CDS in the paused state as long as there is at + // least one cluster in the warming state. This prevents CDS ACK from being sent + // to ADS. Once cluster is warmed up, CDS is resumed, and ACK is sent to ADS, + // providing a signal to ADS to proceed with RDS updates. // - // Major concern with this approach is CDS being left in the paused state forever. - // As long as ClusterManager::removeCluster() is not called on a warming cluster - // this is not an issue. CdsApiImpl takes care of doing this properly, and there - // is no other component removing clusters from the ClusterManagerImpl. If this - // ever changes, we would need to correct the following logic. + // Major concern with this approach is CDS being left in the paused state + // forever. As long as ClusterManager::removeCluster() is not called on a + // warming cluster this is not an issue. CdsApiImpl takes care of doing this + // properly, and there is no other component removing clusters from the + // ClusterManagerImpl. If this ever changes, we would need to correct the + // following logic. if (state == ClusterManager::ClusterWarmingState::Starting && cm_.warmingClusterCount() == 1) { cm_.adsMux().pause(Config::TypeUrl::get().Cluster); diff --git a/source/common/upstream/cds_api_impl.h b/source/common/upstream/cds_api_impl.h index 2dbed586e459..50de489b934f 100644 --- a/source/common/upstream/cds_api_impl.h +++ b/source/common/upstream/cds_api_impl.h @@ -5,6 +5,7 @@ #include "envoy/api/api.h" #include "envoy/api/v2/cds.pb.h" #include "envoy/config/subscription.h" +#include "envoy/config/subscription_factory.h" #include "envoy/event/dispatcher.h" #include "envoy/local_info/local_info.h" #include "envoy/stats/scope.h" @@ -23,9 +24,8 @@ class CdsApiImpl : public CdsApi, Logger::Loggable { public: static CdsApiPtr create(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm, - Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, - const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); + Stats::Scope& scope, + ProtobufMessage::ValidationVisitor& validation_visitor); // Upstream::CdsApi void initialize() override { subscription_->start({}); } @@ -34,6 +34,7 @@ class CdsApiImpl : public CdsApi, } const std::string versionInfo() const override { return system_version_info_; } +private: // Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const std::string& version_info) override; @@ -45,11 +46,8 @@ class CdsApiImpl : public CdsApi, return MessageUtil::anyConvert(resource, validation_visitor_).name(); } -private: CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm, - Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, - const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); + Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor); void runInitializeCallbackIfAny(); ClusterManager& cm_; diff --git a/source/common/upstream/cluster_factory_impl.cc b/source/common/upstream/cluster_factory_impl.cc index 19ccb9fad71b..43c6d5581977 100644 --- a/source/common/upstream/cluster_factory_impl.cc +++ b/source/common/upstream/cluster_factory_impl.cc @@ -29,7 +29,6 @@ std::pair ClusterFactoryImplBase:: Server::Admin& admin, Singleton::Manager& singleton_manager, Outlier::EventLoggerSharedPtr outlier_event_logger, bool added_via_api, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) { - std::string cluster_type; if (!cluster.has_cluster_type()) { @@ -94,7 +93,6 @@ ClusterFactoryImplBase::selectDnsResolver(const envoy::api::v2::Cluster& cluster std::pair ClusterFactoryImplBase::create(const envoy::api::v2::Cluster& cluster, ClusterFactoryContext& context) { - auto stats_scope = generateStatsScope(cluster, context.stats()); Server::Configuration::TransportSocketFactoryContextImpl factory_context( context.admin(), context.sslContextManager(), *stats_scope, context.clusterManager(), diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index d73bb6e932f8..c80b95d28f2d 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -183,7 +183,8 @@ ClusterManagerImpl::ClusterManagerImpl( Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime, Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, - Server::Admin& admin, Api::Api& api, Http::Context& http_context) + Server::Admin& admin, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, + Http::Context& http_context) : factory_(factory), runtime_(runtime), stats_(stats), tls_(tls.allocateSlot()), random_(random), bind_config_(bootstrap.cluster_manager().upstream_bind_config()), local_info_(local_info), cm_stats_(generateStats(stats)), @@ -191,7 +192,8 @@ ClusterManagerImpl::ClusterManagerImpl( config_tracker_entry_( admin.getConfigTracker().add("clusters", [this] { return dumpClusterConfigs(); })), time_source_(main_thread_dispatcher.timeSource()), dispatcher_(main_thread_dispatcher), - http_context_(http_context) { + http_context_(http_context), subscription_factory_(local_info, main_thread_dispatcher, *this, + random, validation_visitor, api) { async_client_manager_ = std::make_unique(*this, tls, time_source_, api); const auto& cm_config = bootstrap.cluster_manager(); @@ -653,7 +655,7 @@ void ClusterManagerImpl::updateGauges() { cm_stats_.warming_clusters_.set(warming_clusters_.size()); } -ThreadLocalCluster* ClusterManagerImpl::get(const std::string& cluster) { +ThreadLocalCluster* ClusterManagerImpl::get(absl::string_view cluster) { ThreadLocalClusterManagerImpl& cluster_manager = tls_->getTyped(); auto entry = cluster_manager.thread_local_clusters_.find(cluster); @@ -1216,9 +1218,9 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::tcpConnPool( ClusterManagerPtr ProdClusterManagerFactory::clusterManagerFromProto( const envoy::config::bootstrap::v2::Bootstrap& bootstrap) { - return ClusterManagerPtr{ - new ClusterManagerImpl(bootstrap, *this, stats_, tls_, runtime_, random_, local_info_, - log_manager_, main_thread_dispatcher_, admin_, api_, http_context_)}; + return ClusterManagerPtr{new ClusterManagerImpl( + bootstrap, *this, stats_, tls_, runtime_, random_, local_info_, log_manager_, + main_thread_dispatcher_, admin_, validation_visitor_, api_, http_context_)}; } Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( @@ -1254,8 +1256,7 @@ std::pair ProdClusterManagerFactor CdsApiPtr ProdClusterManagerFactory::createCds(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm) { // TODO(htuch): Differentiate static vs. dynamic validation visitors. - return CdsApiImpl::create(cds_config, cm, main_thread_dispatcher_, random_, local_info_, stats_, - validation_visitor_, api_); + return CdsApiImpl::create(cds_config, cm, stats_, validation_visitor_); } } // namespace Upstream diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index 9898e09cde17..b4545acbe59c 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -22,6 +22,7 @@ #include "common/common/cleanup.h" #include "common/config/grpc_mux_impl.h" +#include "common/config/subscription_factory_impl.h" #include "common/http/async_client_impl.h" #include "common/upstream/load_stats_reporter.h" #include "common/upstream/priority_conn_pool_map.h" @@ -171,7 +172,8 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable thread_local_clusters_; + absl::flat_hash_map thread_local_clusters_; // These maps are owned by the ThreadLocalClusterManagerImpl instead of the ClusterEntry // to prevent lifetime/ownership issues when a cluster is dynamically removed. @@ -464,6 +468,7 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable void { onAssignmentTimeout(); }); const auto& eds_config = cluster.eds_cluster_config().eds_config(); - subscription_ = Config::SubscriptionFactory::subscriptionFromConfigSource( - eds_config, local_info_, dispatcher, cm, random, info_->statsScope(), - Grpc::Common::typeUrl(envoy::api::v2::ClusterLoadAssignment().GetDescriptor()->full_name()), - factory_context.messageValidationVisitor(), factory_context.api(), *this); + subscription_ = + factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( + eds_config, + Grpc::Common::typeUrl( + envoy::api::v2::ClusterLoadAssignment().GetDescriptor()->full_name()), + info_->statsScope(), *this); } void EdsClusterImpl::startPreInit() { subscription_->start({cluster_name_}); } diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 7a6104a467fa..9953555db0e1 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -3,6 +3,7 @@ #include "envoy/api/v2/core/base.pb.h" #include "envoy/api/v2/eds.pb.h" #include "envoy/config/subscription.h" +#include "envoy/config/subscription_factory.h" #include "envoy/local_info/local_info.h" #include "envoy/secret/secret_manager.h" #include "envoy/stats/scope.h" @@ -20,7 +21,6 @@ namespace Upstream { * Cluster implementation that reads host information from the Endpoint Discovery Service. */ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallbacks { - public: EdsClusterImpl(const envoy::api::v2::Cluster& cluster, Runtime::Loader& runtime, Server::Configuration::TransportSocketFactoryContext& factory_context, @@ -29,6 +29,7 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallba // Upstream::Cluster InitializePhase initializePhase() const override { return InitializePhase::Secondary; } +private: // Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const std::string& version_info) override; @@ -41,7 +42,6 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallba .cluster_name(); } -private: using LocalityWeightsMap = std::unordered_map; bool updateHostsPerLocality(const uint32_t priority, const uint32_t overprovisioning_factor, diff --git a/source/server/BUILD b/source/server/BUILD index 66be09dc6881..5be009fb26fd 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -233,12 +233,12 @@ envoy_cc_library( srcs = ["lds_api.cc"], hdrs = ["lds_api.h"], deps = [ + "//include/envoy/config:subscription_factory_interface", "//include/envoy/config:subscription_interface", "//include/envoy/init:manager_interface", "//include/envoy/server:listener_manager_interface", "//source/common/common:cleanup_lib", "//source/common/config:resources_lib", - "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", "//source/common/init:target_lib", "//source/common/protobuf:utility_lib", diff --git a/source/server/config_validation/cluster_manager.cc b/source/server/config_validation/cluster_manager.cc index 56817278b2b7..8aee20745977 100644 --- a/source/server/config_validation/cluster_manager.cc +++ b/source/server/config_validation/cluster_manager.cc @@ -9,7 +9,7 @@ ClusterManagerPtr ValidationClusterManagerFactory::clusterManagerFromProto( const envoy::config::bootstrap::v2::Bootstrap& bootstrap) { return std::make_unique( bootstrap, *this, stats_, tls_, runtime_, random_, local_info_, log_manager_, - main_thread_dispatcher_, admin_, api_, http_context_, time_system_); + main_thread_dispatcher_, admin_, validation_visitor_, api_, http_context_, time_system_); } CdsApiPtr @@ -26,10 +26,10 @@ ValidationClusterManager::ValidationClusterManager( Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime, Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, - Server::Admin& admin, Api::Api& api, Http::Context& http_context, - Event::TimeSystem& time_system) + Server::Admin& admin, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, + Http::Context& http_context, Event::TimeSystem& time_system) : ClusterManagerImpl(bootstrap, factory, stats, tls, runtime, random, local_info, log_manager, - main_thread_dispatcher, admin, api, http_context), + main_thread_dispatcher, admin, validation_visitor, api, http_context), async_client_(api, time_system) {} Http::ConnectionPool::Instance* diff --git a/source/server/config_validation/cluster_manager.h b/source/server/config_validation/cluster_manager.h index c20399523654..f4b4ce40bd9b 100644 --- a/source/server/config_validation/cluster_manager.h +++ b/source/server/config_validation/cluster_manager.h @@ -55,8 +55,9 @@ class ValidationClusterManager : public ClusterManagerImpl { ThreadLocal::Instance& tls, Runtime::Loader& runtime, Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager, Event::Dispatcher& dispatcher, - Server::Admin& admin, Api::Api& api, Http::Context& http_context, - Event::TimeSystem& time_system); + Server::Admin& admin, + ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, + Http::Context& http_context, Event::TimeSystem& time_system); Http::ConnectionPool::Instance* httpConnPoolForCluster(const std::string&, ResourcePriority, Http::Protocol, diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index b5a0365b068c..9e7784e5bcbe 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -111,9 +111,8 @@ class ValidationInstance : Logger::Loggable, // Server::ListenerComponentFactory LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config) override { - return std::make_unique(lds_config, clusterManager(), dispatcher(), random(), - initManager(), localInfo(), stats(), listenerManager(), - messageValidationVisitor(), api()); + return std::make_unique(lds_config, clusterManager(), initManager(), stats(), + listenerManager(), messageValidationVisitor()); } std::vector createNetworkFilterFactoryList( const Protobuf::RepeatedPtrField& filters, diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index df2bc694cade..6f9c5c8c0ef2 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -8,7 +8,6 @@ #include "common/common/cleanup.h" #include "common/config/resources.h" -#include "common/config/subscription_factory.h" #include "common/config/utility.h" #include "common/protobuf/utility.h" @@ -16,19 +15,15 @@ namespace Envoy { namespace Server { LdsApiImpl::LdsApiImpl(const envoy::api::v2::core::ConfigSource& lds_config, - Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random, Init::Manager& init_manager, - const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, - ListenerManager& lm, ProtobufMessage::ValidationVisitor& validation_visitor, - Api::Api& api) + Upstream::ClusterManager& cm, Init::Manager& init_manager, + Stats::Scope& scope, ListenerManager& lm, + ProtobufMessage::ValidationVisitor& validation_visitor) : listener_manager_(lm), scope_(scope.createScope("listener_manager.lds.")), cm_(cm), init_target_("LDS", [this]() { subscription_->start({}); }), validation_visitor_(validation_visitor) { - subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource( - lds_config, local_info, dispatcher, cm, random, *scope_, - Grpc::Common::typeUrl(envoy::api::v2::Listener().GetDescriptor()->full_name()), - validation_visitor_, api, *this); - Config::Utility::checkLocalInfo("lds", local_info); + subscription_ = cm.subscriptionFactory().subscriptionFromConfigSource( + lds_config, Grpc::Common::typeUrl(envoy::api::v2::Listener().GetDescriptor()->full_name()), + *scope_, *this); init_manager.add(init_target_); } diff --git a/source/server/lds_api.h b/source/server/lds_api.h index 3bbc0e3e7076..394b452cf797 100644 --- a/source/server/lds_api.h +++ b/source/server/lds_api.h @@ -2,9 +2,9 @@ #include -#include "envoy/api/api.h" #include "envoy/api/v2/lds.pb.h" #include "envoy/config/subscription.h" +#include "envoy/config/subscription_factory.h" #include "envoy/init/manager.h" #include "envoy/server/listener_manager.h" #include "envoy/stats/scope.h" @@ -23,14 +23,13 @@ class LdsApiImpl : public LdsApi, Logger::Loggable { public: LdsApiImpl(const envoy::api::v2::core::ConfigSource& lds_config, Upstream::ClusterManager& cm, - Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, - Init::Manager& init_manager, const LocalInfo::LocalInfo& local_info, - Stats::Scope& scope, ListenerManager& lm, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); + Init::Manager& init_manager, Stats::Scope& scope, ListenerManager& lm, + ProtobufMessage::ValidationVisitor& validation_visitor); // Server::LdsApi std::string versionInfo() const override { return system_version_info_; } +private: // Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const std::string& version_info) override; @@ -42,7 +41,6 @@ class LdsApiImpl : public LdsApi, return MessageUtil::anyConvert(resource, validation_visitor_).name(); } -private: std::unique_ptr subscription_; std::string system_version_info_; ListenerManager& listener_manager_; diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index 64466ff84ec4..6a4703e21375 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -53,10 +53,9 @@ class ProdListenerComponentFactory : public ListenerComponentFactory, // Server::ListenerComponentFactory LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config) override { - return std::make_unique( - lds_config, server_.clusterManager(), server_.dispatcher(), server_.random(), - server_.initManager(), server_.localInfo(), server_.stats(), server_.listenerManager(), - server_.messageValidationVisitor(), server_.api()); + return std::make_unique(lds_config, server_.clusterManager(), server_.initManager(), + server_.stats(), server_.listenerManager(), + server_.messageValidationVisitor()); } std::vector createNetworkFilterFactoryList( const Protobuf::RepeatedPtrField& filters, diff --git a/test/common/config/BUILD b/test/common/config/BUILD index 0b3b5a9367b6..7e254c5ee8c7 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -179,8 +179,8 @@ envoy_cc_test_library( ) envoy_cc_test( - name = "subscription_factory_test", - srcs = ["subscription_factory_test.cc"], + name = "subscription_factory_impl_test", + srcs = ["subscription_factory_impl_test.cc"], deps = [ "//source/common/config:subscription_factory_lib", "//test/mocks/config:config_mocks", diff --git a/test/common/config/subscription_factory_test.cc b/test/common/config/subscription_factory_impl_test.cc similarity index 98% rename from test/common/config/subscription_factory_test.cc rename to test/common/config/subscription_factory_impl_test.cc index b0ae6366b7ae..9c10b7dad282 100644 --- a/test/common/config/subscription_factory_test.cc +++ b/test/common/config/subscription_factory_impl_test.cc @@ -4,7 +4,7 @@ #include "envoy/common/exception.h" #include "envoy/stats/scope.h" -#include "common/config/subscription_factory.h" +#include "common/config/subscription_factory_impl.h" #include "test/mocks/config/mocks.h" #include "test/mocks/event/mocks.h" @@ -35,9 +35,10 @@ class SubscriptionFactoryTest : public testing::Test { std::unique_ptr subscriptionFromConfigSource(const envoy::api::v2::core::ConfigSource& config) { - return SubscriptionFactory::subscriptionFromConfigSource( - config, local_info_, dispatcher_, cm_, random_, stats_store_, - Config::TypeUrl::get().ClusterLoadAssignment, validation_visitor_, *api_, callbacks_); + return SubscriptionFactoryImpl(local_info_, dispatcher_, cm_, random_, validation_visitor_, + *api_) + .subscriptionFromConfigSource(config, Config::TypeUrl::get().ClusterLoadAssignment, + stats_store_, callbacks_); } Upstream::MockClusterManager cm_; diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index 089a766e5fa8..12351a8d8b8f 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -290,7 +290,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { std::move(shadow_writer_ptr_), http_context_); EXPECT_CALL(cm_, httpAsyncClientForCluster(fake_cluster_name_)) .WillRepeatedly(ReturnRef(*http_async_client_)); - EXPECT_CALL(cm_, get(fake_cluster_name_)).WillRepeatedly(Return(&thread_local_cluster_)); + EXPECT_CALL(cm_, get(Eq(fake_cluster_name_))).WillRepeatedly(Return(&thread_local_cluster_)); envoy::api::v2::core::GrpcService config; config.mutable_envoy_grpc()->set_cluster_name(fake_cluster_name_); fillServiceWideInitialMetadata(config); diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 9e5a061f7370..26b96a6431a2 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -65,7 +65,6 @@ envoy_cc_test( deps = [ "//source/common/config:filter_json_lib", "//source/common/config:utility_lib", - "//source/common/http:message_lib", "//source/common/json:json_loader_lib", "//source/common/router:rds_lib", "//source/server/http:admin_lib", @@ -101,14 +100,12 @@ envoy_cc_test( name = "vhds_test", srcs = ["vhds_test.cc"], deps = [ - "//source/common/config:filter_json_lib", "//source/common/config:utility_lib", - "//source/common/http:message_lib", - "//source/common/json:json_loader_lib", "//source/common/protobuf", "//source/common/router:rds_lib", "//source/common/router:vhds_lib", "//source/server/http:admin_lib", + "//test/mocks/config:config_mocks", "//test/mocks/local_info:local_info_mocks", "//test/mocks/server:server_mocks", "//test/mocks/thread_local:thread_local_mocks", diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 0bf195945fbf..eb9adf152501 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -1979,9 +1979,9 @@ TEST_F(RouteMatcherTest, ShadowClusterNotFound) { cluster: www2 )EOF"; - EXPECT_CALL(factory_context_.cluster_manager_, get("www2")) + EXPECT_CALL(factory_context_.cluster_manager_, get(Eq("www2"))) .WillRepeatedly(Return(&factory_context_.cluster_manager_.thread_local_cluster_)); - EXPECT_CALL(factory_context_.cluster_manager_, get("some_cluster")) + EXPECT_CALL(factory_context_.cluster_manager_, get(Eq("some_cluster"))) .WillRepeatedly(Return(nullptr)); EXPECT_THROW(TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true), @@ -2001,7 +2001,7 @@ TEST_F(RouteMatcherTest, ClusterNotFound) { cluster: www2 )EOF"; - EXPECT_CALL(factory_context_.cluster_manager_, get("www2")).WillRepeatedly(Return(nullptr)); + EXPECT_CALL(factory_context_.cluster_manager_, get(Eq("www2"))).WillRepeatedly(Return(nullptr)); EXPECT_THROW(TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true), EnvoyException); @@ -2020,7 +2020,7 @@ TEST_F(RouteMatcherTest, ClusterNotFoundNotChecking) { cluster: www2 )EOF"; - EXPECT_CALL(factory_context_.cluster_manager_, get("www2")).WillRepeatedly(Return(nullptr)); + EXPECT_CALL(factory_context_.cluster_manager_, get(Eq("www2"))).WillRepeatedly(Return(nullptr)); TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, false); } @@ -2039,7 +2039,7 @@ validate_clusters: false cluster: www )EOF"; - EXPECT_CALL(factory_context_.cluster_manager_, get("www2")).WillRepeatedly(Return(nullptr)); + EXPECT_CALL(factory_context_.cluster_manager_, get(Eq("www2"))).WillRepeatedly(Return(nullptr)); TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true); } @@ -3794,11 +3794,11 @@ TEST_F(RouteMatcherTest, TestWeightedClusterInvalidClusterName) { weight: 34 )EOF"; - EXPECT_CALL(factory_context_.cluster_manager_, get("cluster1")) + EXPECT_CALL(factory_context_.cluster_manager_, get(Eq("cluster1"))) .WillRepeatedly(Return(&factory_context_.cluster_manager_.thread_local_cluster_)); - EXPECT_CALL(factory_context_.cluster_manager_, get("cluster2")) + EXPECT_CALL(factory_context_.cluster_manager_, get(Eq("cluster2"))) .WillRepeatedly(Return(&factory_context_.cluster_manager_.thread_local_cluster_)); - EXPECT_CALL(factory_context_.cluster_manager_, get("cluster3-invalid")) + EXPECT_CALL(factory_context_.cluster_manager_, get(Eq("cluster3-invalid"))) .WillRepeatedly(Return(nullptr)); EXPECT_THROW(TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true), @@ -5711,4 +5711,4 @@ name: foo } // namespace } // namespace Router -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index 5975de0eeeaf..daa93436b370 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -8,7 +8,6 @@ #include "common/config/filter_json.h" #include "common/config/utility.h" -#include "common/http/message_impl.h" #include "common/json/json_loader.h" #include "common/router/rds_impl.h" @@ -27,6 +26,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::InSequence; using testing::Invoke; using testing::Return; @@ -49,10 +49,7 @@ parseHttpConnectionManagerFromJson(const std::string& json_string) { class RdsTestBase : public testing::Test { public: - RdsTestBase() - : request_(&factory_context_.cluster_manager_.async_client_), - rds_version_(factory_context_.scope_.gauge("foo.rds.foo_route_config.version", - Stats::Gauge::ImportMode::NeverImport)) { + RdsTestBase() { ON_CALL(factory_context_.init_manager_, add(_)) .WillByDefault(Invoke([this](const Init::Target& target) { init_target_handle_ = target.createHandle("test"); @@ -62,33 +59,13 @@ class RdsTestBase : public testing::Test { [this](const Init::Watcher& watcher) { init_target_handle_->initialize(watcher); })); } - void expectRequest() { - EXPECT_CALL(factory_context_.cluster_manager_, httpAsyncClientForCluster("foo_cluster")); - EXPECT_CALL(factory_context_.cluster_manager_.async_client_, send_(_, _, _)) - .WillOnce( - Invoke([&](Http::MessagePtr& request, Http::AsyncClient::Callbacks& callbacks, - const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { - request->headers().removeContentLength(); - EXPECT_EQ((Http::TestHeaderMapImpl{{":method", "POST"}, - {":path", "/v2/discovery:routes"}, - {":authority", "foo_cluster"}, - {"content-type", "application/json"}}), - request->headers()); - callbacks_ = &callbacks; - return &request_; - })); - } - Event::SimulatedTimeSystem& timeSystem() { return time_system_; } Event::SimulatedTimeSystem time_system_; NiceMock factory_context_; Init::ExpectableWatcherImpl init_watcher_; Init::TargetHandlePtr init_target_handle_; - Http::MockAsyncClientRequest request_; - Http::AsyncClient::Callbacks* callbacks_{}; - Event::MockTimer* interval_timer_{}; - Stats::Gauge& rds_version_; + Envoy::Config::SubscriptionCallbacks* rds_callbacks_{}; }; class RdsImplTest : public RdsTestBase { @@ -117,20 +94,12 @@ class RdsImplTest : public RdsTestBase { } )EOF"; - Upstream::ClusterManager::ClusterInfoMap cluster_map; - Upstream::MockClusterMockPrioritySet cluster; - cluster_map.emplace("foo_cluster", cluster); - EXPECT_CALL(factory_context_.cluster_manager_, clusters()).WillOnce(Return(cluster_map)); - EXPECT_CALL(cluster, info()); - EXPECT_CALL(*cluster.info_, addedViaApi()); - EXPECT_CALL(cluster, info()); - EXPECT_CALL(*cluster.info_, type()); - interval_timer_ = new Event::MockTimer(&factory_context_.dispatcher_); EXPECT_CALL(factory_context_.init_manager_, add(_)); rds_ = RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json), factory_context_, "foo.", *route_config_provider_manager_); - expectRequest(); + rds_callbacks_ = factory_context_.cluster_manager_.subscription_factory_.callbacks_; + EXPECT_CALL(*factory_context_.cluster_manager_.subscription_factory_.subscription_, start(_)); factory_context_.init_manager_.initialize(init_watcher_); } @@ -164,61 +133,11 @@ TEST_F(RdsImplTest, RdsAndStatic) { EnvoyException); } -TEST_F(RdsImplTest, LocalInfoNotDefined) { - const std::string config_json = R"EOF( - { - "rds": { - "cluster": "foo_cluster", - "route_config_name": "foo_route_config" - }, - "codec_type": "auto", - "stat_prefix": "foo", - "filters": [ - { "name": "http_dynamo_filter", "config": {} } - ] - } - )EOF"; - - factory_context_.local_info_.node_.set_cluster(""); - factory_context_.local_info_.node_.set_id(""); - EXPECT_THROW(RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json), - factory_context_, "foo.", - *route_config_provider_manager_), - EnvoyException); -} - -TEST_F(RdsImplTest, UnknownCluster) { - const std::string config_json = R"EOF( - { - "rds": { - "cluster": "foo_cluster", - "route_config_name": "foo_route_config" - }, - "codec_type": "auto", - "stat_prefix": "foo", - "filters": [ - { "name": "http_dynamo_filter", "config": {} } - ] - } - )EOF"; - - Upstream::ClusterManager::ClusterInfoMap cluster_map; - EXPECT_CALL(factory_context_.cluster_manager_, clusters()).WillOnce(Return(cluster_map)); - EXPECT_THROW_WITH_MESSAGE( - RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json), - factory_context_, "foo.", *route_config_provider_manager_), - EnvoyException, - "envoy::api::v2::core::ConfigSource must have a statically defined non-EDS " - "cluster: 'foo_cluster' does not exist, was added via api, or is an " - "EDS cluster"); -} - TEST_F(RdsImplTest, DestroyDuringInitialize) { InSequence s; setup(); EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(request_, cancel()); rds_.reset(); } @@ -231,7 +150,6 @@ TEST_F(RdsImplTest, Basic) { // Make sure the initial empty route table works. EXPECT_EQ(nullptr, route(Http::TestHeaderMapImpl{{":authority", "foo"}})); - EXPECT_EQ(0UL, rds_version_.value()); // Initial request. const std::string response1_json = R"EOF( @@ -246,34 +164,16 @@ TEST_F(RdsImplTest, Basic) { ] } )EOF"; - - Http::MessagePtr message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - message->body() = std::make_unique(response1_json); + auto response1 = TestUtility::parseYaml(response1_json); EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + rds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); EXPECT_EQ(nullptr, route(Http::TestHeaderMapImpl{{":authority", "foo"}})); - EXPECT_EQ(13237225503670494420U, rds_version_.value()); - - expectRequest(); - interval_timer_->callback_(); // 2nd request with same response. Based on hash should not reload config. - message = std::make_unique( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}); - message->body() = std::make_unique(response1_json); - - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + rds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); EXPECT_EQ(nullptr, route(Http::TestHeaderMapImpl{{":authority", "foo"}})); - EXPECT_EQ(13237225503670494420U, rds_version_.value()); - - expectRequest(); - interval_timer_->callback_(); - // Load the config and verified shared count. ConfigConstSharedPtr config = rds_->config(); EXPECT_EQ(2, config.use_count()); @@ -308,38 +208,27 @@ TEST_F(RdsImplTest, Basic) { ] } )EOF"; - - message = std::make_unique( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}); - message->body() = std::make_unique(response2_json); + auto response2 = TestUtility::parseYaml(response2_json); // Make sure we don't lookup/verify clusters. - EXPECT_CALL(factory_context_.cluster_manager_, get("bar")).Times(0); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + EXPECT_CALL(factory_context_.cluster_manager_, get(Eq("bar"))).Times(0); + rds_callbacks_->onConfigUpdate(response2.resources(), response2.version_info()); EXPECT_EQ("foo", route(Http::TestHeaderMapImpl{{":authority", "foo"}, {":path", "/foo"}}) ->routeEntry() ->clusterName()); - EXPECT_EQ(6927017134761466251U, rds_version_.value()); - // Old config use count should be 1 now. EXPECT_EQ(1, config.use_count()); - EXPECT_EQ(2UL, factory_context_.scope_.counter("foo.rds.foo_route_config.config_reload").value()); - EXPECT_EQ(3UL, - factory_context_.scope_.counter("foo.rds.foo_route_config.update_attempt").value()); - EXPECT_EQ(3UL, - factory_context_.scope_.counter("foo.rds.foo_route_config.update_success").value()); - EXPECT_EQ(6927017134761466251U, rds_version_.value()); } -TEST_F(RdsImplTest, Failure) { +// Validate behavior when the config is delivered but it fails PGV validation. +TEST_F(RdsImplTest, FailureInvalidConfig) { InSequence s; setup(); - std::string response_json = R"EOF( + std::string response1_json = R"EOF( { "version_info": "1", "resources": [ @@ -351,56 +240,34 @@ TEST_F(RdsImplTest, Failure) { ] } )EOF"; - - Http::MessagePtr message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - message->body() = std::make_unique(response_json); + auto response1 = TestUtility::parseYaml(response1_json); EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + EXPECT_THROW_WITH_MESSAGE( + rds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()), + EnvoyException, + "Unexpected RDS configuration (expecting foo_route_config): INVALID_NAME_FOR_route_config"); +} - expectRequest(); - interval_timer_->callback_(); +// Validate behavior when the config fails delivery at the subscription level. +TEST_F(RdsImplTest, FailureSubscription) { + InSequence s; - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onFailure(Http::AsyncClient::FailureReason::Reset); + setup(); - EXPECT_EQ(2UL, - factory_context_.scope_.counter("foo.rds.foo_route_config.update_attempt").value()); - EXPECT_EQ(1UL, - factory_context_.scope_.counter("foo.rds.foo_route_config.update_failure").value()); - // Validate that the schema error increments update_rejected stat. - EXPECT_EQ(1UL, - factory_context_.scope_.counter("foo.rds.foo_route_config.update_rejected").value()); + EXPECT_CALL(init_watcher_, ready()); + rds_callbacks_->onConfigUpdateFailed({}); } class RouteConfigProviderManagerImplTest : public RdsTestBase { public: void setup() { - std::string config_json = R"EOF( - { - "api_type": "REST", - "cluster": "foo_cluster", - "route_config_name": "foo_route_config", - "refresh_delay_ms": 1000 - } - )EOF"; - - Json::ObjectSharedPtr config = Json::Factory::loadFromString(config_json); - Envoy::Config::Utility::translateRdsConfig(*config, rds_); - // Get a RouteConfigProvider. This one should create an entry in the RouteConfigProviderManager. - Upstream::ClusterManager::ClusterInfoMap cluster_map; - Upstream::MockClusterMockPrioritySet cluster; - cluster_map.emplace("foo_cluster", cluster); - EXPECT_CALL(factory_context_.cluster_manager_, clusters()).WillOnce(Return(cluster_map)); - EXPECT_CALL(cluster, info()).Times(2); - EXPECT_CALL(*cluster.info_, addedViaApi()); - EXPECT_CALL(*cluster.info_, type()); - interval_timer_ = new Event::MockTimer(&factory_context_.dispatcher_); + rds_.set_route_config_name("foo_route_config"); + rds_.mutable_config_source()->set_path("foo_path"); provider_ = route_config_provider_manager_->createRdsRouteConfigProvider(rds_, factory_context_, "foo_prefix."); + rds_callbacks_ = factory_context_.cluster_manager_.subscription_factory_.callbacks_; } RouteConfigProviderManagerImplTest() { @@ -477,7 +344,7 @@ name: foo // Static + dynamic. setup(); - expectRequest(); + EXPECT_CALL(*factory_context_.cluster_manager_.subscription_factory_.subscription_, start(_)); factory_context_.init_manager_.initialize(init_watcher_); const std::string response1_json = R"EOF( @@ -492,13 +359,10 @@ name: foo ] } )EOF"; + auto response1 = TestUtility::parseYaml(response1_json); - Http::MessagePtr message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - message->body() = std::make_unique(response1_json); EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + rds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); message_ptr = factory_context_.admin_.config_tracker_.config_tracker_callbacks_["routes"](); const auto& route_config_dump3 = MessageUtil::downcastAndValidate( @@ -548,10 +412,8 @@ name: foo_route_config route: { cluster: baz } )EOF")); - RdsRouteConfigSubscription& subscription = - dynamic_cast(*provider_).subscription(); - - subscription.onConfigUpdate(route_configs, "1"); + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate(route_configs, + "1"); RouteConfigProviderPtr provider2 = route_config_provider_manager_->createRdsRouteConfigProvider( rds_, factory_context_, "foo_prefix"); @@ -564,34 +426,14 @@ name: foo_route_config &dynamic_cast(*provider2).subscription()); EXPECT_EQ(&provider_->configInfo().value().config_, &provider2->configInfo().value().config_); - std::string config_json2 = R"EOF( - { - "api_type": "REST", - "cluster": "bar_cluster", - "route_config_name": "foo_route_config", - "refresh_delay_ms": 1000 - } - )EOF"; - - Json::ObjectSharedPtr config2 = Json::Factory::loadFromString(config_json2); envoy::config::filter::network::http_connection_manager::v2::Rds rds2; - Envoy::Config::Utility::translateRdsConfig(*config2, rds2); - - Upstream::ClusterManager::ClusterInfoMap cluster_map; - Upstream::MockClusterMockPrioritySet cluster; - cluster_map.emplace("bar_cluster", cluster); - EXPECT_CALL(factory_context_.cluster_manager_, clusters()).WillOnce(Return(cluster_map)); - EXPECT_CALL(cluster, info()).Times(2); - EXPECT_CALL(*cluster.info_, addedViaApi()); - EXPECT_CALL(*cluster.info_, type()); - new Event::MockTimer(&factory_context_.dispatcher_); + rds2.set_route_config_name("foo_route_config"); + rds2.mutable_config_source()->set_path("bar_path"); RouteConfigProviderPtr provider3 = route_config_provider_manager_->createRdsRouteConfigProvider( rds2, factory_context_, "foo_prefix"); EXPECT_NE(provider3, provider_); - dynamic_cast(*provider3) - .subscription() - .onConfigUpdate(route_configs, "provider3"); - + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate(route_configs, + "provider3"); EXPECT_EQ(2UL, route_config_provider_manager_->dumpRouteConfigs()->dynamic_route_configs().size()); @@ -616,36 +458,36 @@ name: foo_route_config // Negative test for protoc-gen-validate constraints. TEST_F(RouteConfigProviderManagerImplTest, ValidateFail) { setup(); - auto& provider_impl = dynamic_cast(*provider_.get()); Protobuf::RepeatedPtrField route_configs; envoy::api::v2::RouteConfiguration route_config; route_config.set_name("foo_route_config"); route_config.mutable_virtual_hosts()->Add(); route_configs.Add()->PackFrom(route_config); - EXPECT_THROW(provider_impl.subscription().onConfigUpdate(route_configs, ""), + EXPECT_THROW(factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( + route_configs, ""), ProtoValidationException); } TEST_F(RouteConfigProviderManagerImplTest, onConfigUpdateEmpty) { setup(); + EXPECT_CALL(*factory_context_.cluster_manager_.subscription_factory_.subscription_, start(_)); factory_context_.init_manager_.initialize(init_watcher_); - auto& provider_impl = dynamic_cast(*provider_.get()); EXPECT_CALL(init_watcher_, ready()); - provider_impl.subscription().onConfigUpdate({}, ""); - EXPECT_EQ( - 1UL, factory_context_.scope_.counter("foo_prefix.rds.foo_route_config.update_empty").value()); + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate({}, ""); } TEST_F(RouteConfigProviderManagerImplTest, onConfigUpdateWrongSize) { setup(); + EXPECT_CALL(*factory_context_.cluster_manager_.subscription_factory_.subscription_, start(_)); factory_context_.init_manager_.initialize(init_watcher_); - auto& provider_impl = dynamic_cast(*provider_.get()); Protobuf::RepeatedPtrField route_configs; route_configs.Add(); route_configs.Add(); EXPECT_CALL(init_watcher_, ready()); - EXPECT_THROW_WITH_MESSAGE(provider_impl.subscription().onConfigUpdate(route_configs, ""), - EnvoyException, "Unexpected RDS resource length: 2"); + EXPECT_THROW_WITH_MESSAGE( + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( + route_configs, ""), + EnvoyException, "Unexpected RDS resource length: 2"); } } // namespace diff --git a/test/common/router/scoped_rds_test.cc b/test/common/router/scoped_rds_test.cc index 15b282bd77b2..1476dac9dce0 100644 --- a/test/common/router/scoped_rds_test.cc +++ b/test/common/router/scoped_rds_test.cc @@ -47,29 +47,10 @@ class ScopedRoutesTestBase : public testing::Test { EXPECT_CALL(factory_context_.admin_.config_tracker_, add_("route_scopes", _)); config_provider_manager_ = std::make_unique(factory_context_.admin_); - - const std::string rds_config_yaml = R"EOF( -api_config_source: - api_type: REST - cluster_names: - - foo_rds_cluster - refresh_delay: { seconds: 1, nanos: 0 } -)EOF"; - TestUtility::loadFromYaml(rds_config_yaml, rds_config_source_); } ~ScopedRoutesTestBase() override { factory_context_.thread_local_.shutdownThread(); } - void setupMockClusterMap() { - InSequence s; - cluster_map_.emplace("foo_cluster", cluster_); - EXPECT_CALL(factory_context_.cluster_manager_, clusters()).WillOnce(Return(cluster_map_)); - EXPECT_CALL(cluster_, info()); - EXPECT_CALL(*cluster_.info_, addedViaApi()); - EXPECT_CALL(cluster_, info()); - EXPECT_CALL(*cluster_.info_, type()); - } - Event::SimulatedTimeSystem& timeSystem() { return time_system_; } NiceMock factory_context_; @@ -85,25 +66,11 @@ class ScopedRdsTest : public ScopedRoutesTestBase { void setup() { InSequence s; - setupMockClusterMap(); const std::string config_yaml = R"EOF( name: foo_scoped_routes scope_key_builder: fragments: - header_value_extractor: { name: X-Google-VIP } -rds_config_source: - api_config_source: - api_type: REST - cluster_names: - - foo_cluster - refresh_delay: { seconds: 1, nanos: 0 } -scoped_rds: - scoped_rds_config_source: - api_config_source: - api_type: REST - cluster_names: - - foo_cluster - refresh_delay: { seconds: 1, nanos: 0 } )EOF"; envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes scoped_routes_config; TestUtility::loadFromYaml(config_yaml, scoped_routes_config); @@ -112,21 +79,16 @@ name: foo_scoped_routes ScopedRoutesConfigProviderManagerOptArg(scoped_routes_config.name(), scoped_routes_config.rds_config_source(), scoped_routes_config.scope_key_builder())); - subscription_ = &dynamic_cast(*provider_).subscription(); + subscription_callbacks_ = factory_context_.cluster_manager_.subscription_factory_.callbacks_; } - ScopedRdsConfigSubscription& subscription() const { return *subscription_; } - - ScopedRdsConfigSubscription* subscription_; + Envoy::Config::SubscriptionCallbacks* subscription_callbacks_{}; Envoy::Config::ConfigProviderPtr provider_; }; TEST_F(ScopedRdsTest, ValidateFail) { setup(); - ScopedRdsConfigSubscription& subscription = - dynamic_cast(*provider_).subscription(); - // 'name' validation: value must be > 1 byte. const std::string config_yaml = R"EOF( name: @@ -137,7 +99,7 @@ route_configuration_name: foo_routes )EOF"; Protobuf::RepeatedPtrField resources; parseScopedRouteConfigurationFromYaml(*resources.Add(), config_yaml); - EXPECT_THROW(subscription.onConfigUpdate(resources, "1"), ProtoValidationException); + EXPECT_THROW(subscription_callbacks_->onConfigUpdate(resources, "1"), ProtoValidationException); // 'route_configuration_name' validation: value must be > 1 byte. const std::string config_yaml2 = R"EOF( @@ -149,7 +111,7 @@ name: foo_scope )EOF"; Protobuf::RepeatedPtrField resources2; parseScopedRouteConfigurationFromYaml(*resources2.Add(), config_yaml2); - EXPECT_THROW(subscription.onConfigUpdate(resources2, "1"), ProtoValidationException); + EXPECT_THROW(subscription_callbacks_->onConfigUpdate(resources2, "1"), ProtoValidationException); // 'key' validation: must define at least 1 fragment. const std::string config_yaml3 = R"EOF( @@ -159,7 +121,7 @@ route_configuration_name: foo_routes )EOF"; Protobuf::RepeatedPtrField resources3; parseScopedRouteConfigurationFromYaml(*resources3.Add(), config_yaml3); - EXPECT_THROW(subscription.onConfigUpdate(resources3, "1"), ProtoValidationException); + EXPECT_THROW(subscription_callbacks_->onConfigUpdate(resources3, "1"), ProtoValidationException); } // Tests that multiple uniquely named resources are allowed in config updates. @@ -183,7 +145,7 @@ route_configuration_name: foo_routes - string_key: x-foo-key )EOF"; parseScopedRouteConfigurationFromYaml(*resources.Add(), config_yaml2); - EXPECT_NO_THROW(subscription().onConfigUpdate(resources, "1")); + EXPECT_NO_THROW(subscription_callbacks_->onConfigUpdate(resources, "1")); EXPECT_EQ( 1UL, factory_context_.scope_.counter("foo.scoped_rds.foo_scoped_routes.config_reload").value()); @@ -203,48 +165,10 @@ route_configuration_name: foo_routes Protobuf::RepeatedPtrField resources; parseScopedRouteConfigurationFromYaml(*resources.Add(), config_yaml); parseScopedRouteConfigurationFromYaml(*resources.Add(), config_yaml); - EXPECT_THROW_WITH_MESSAGE(subscription().onConfigUpdate(resources, "1"), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(subscription_callbacks_->onConfigUpdate(resources, "1"), EnvoyException, "duplicate scoped route configuration foo_scope found"); } -// Tests that defining an invalid cluster in the SRDS config results in an error. -TEST_F(ScopedRdsTest, UnknownCluster) { - const std::string config_yaml = R"EOF( -name: foo_scoped_routes -scope_key_builder: - fragments: - - header_value_extractor: { name: X-Google-VIP } -rds_config_source: - api_config_source: - api_type: REST - cluster_names: - - foo_cluster - refresh_delay: { seconds: 1, nanos: 0 } -scoped_rds: - scoped_rds_config_source: - api_config_source: - api_type: REST - cluster_names: - - foo_cluster - refresh_delay: { seconds: 1, nanos: 0 } -)EOF"; - envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes scoped_routes_config; - TestUtility::loadFromYaml(config_yaml, scoped_routes_config); - - Upstream::ClusterManager::ClusterInfoMap cluster_map; - EXPECT_CALL(factory_context_.cluster_manager_, clusters()).WillOnce(Return(cluster_map)); - EXPECT_THROW_WITH_MESSAGE( - config_provider_manager_->createXdsConfigProvider( - scoped_routes_config.scoped_rds(), factory_context_, "foo.", - ScopedRoutesConfigProviderManagerOptArg(scoped_routes_config.name(), - scoped_routes_config.rds_config_source(), - scoped_routes_config.scope_key_builder())), - EnvoyException, - "envoy::api::v2::core::ConfigSource must have a statically defined non-EDS " - "cluster: 'foo_cluster' does not exist, was added via api, or is an " - "EDS cluster"); -} - // Tests a config update failure. TEST_F(ScopedRdsTest, ConfigUpdateFailure) { setup(); @@ -253,7 +177,7 @@ TEST_F(ScopedRdsTest, ConfigUpdateFailure) { timeSystem().setSystemTime(time); const EnvoyException ex(fmt::format("config failure")); // Verify the failure updates the lastUpdated() timestamp. - subscription().onConfigUpdateFailed(&ex); + subscription_callbacks_->onConfigUpdateFailed(&ex); EXPECT_EQ(std::chrono::time_point_cast(provider_->lastUpdated()) .time_since_epoch(), time); @@ -291,12 +215,6 @@ stat_prefix: foo scope_key_builder: fragments: - header_value_extractor: { name: X-Google-VIP } - rds_config_source: - api_config_source: - api_type: REST - cluster_names: - - foo_rds_cluster - refresh_delay: { seconds: 1, nanos: 0 } $1 )EOF"; const std::string inline_scoped_route_configs_yaml = R"EOF( @@ -340,15 +258,9 @@ stat_prefix: foo expected_config_dump); EXPECT_EQ(expected_config_dump.DebugString(), scoped_routes_config_dump2.DebugString()); - setupMockClusterMap(); const std::string scoped_rds_config_yaml = R"EOF( scoped_rds: scoped_rds_config_source: - api_config_source: - api_type: REST - cluster_names: - - foo_cluster - refresh_delay: { seconds: 1, nanos: 0 } )EOF"; Envoy::Config::ConfigProviderPtr dynamic_provider = ScopedRoutesConfigProviderUtil::create( parseHttpConnectionManagerFromYaml(absl::Substitute( @@ -364,9 +276,8 @@ route_configuration_name: dynamic-foo-route-config )EOF")); timeSystem().setSystemTime(std::chrono::milliseconds(1234567891567)); - ScopedRdsConfigSubscription& subscription = - dynamic_cast(*dynamic_provider).subscription(); - subscription.onConfigUpdate(resources, "1"); + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate(resources, + "1"); TestUtility::loadFromYaml(R"EOF( inline_scoped_route_configs: @@ -403,7 +314,8 @@ route_configuration_name: dynamic-foo-route-config EXPECT_EQ(expected_config_dump.DebugString(), scoped_routes_config_dump3.DebugString()); resources.Clear(); - subscription.onConfigUpdate(resources, "2"); + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate(resources, + "2"); TestUtility::loadFromYaml(R"EOF( inline_scoped_route_configs: - name: foo-scoped-routes diff --git a/test/common/router/shadow_writer_impl_test.cc b/test/common/router/shadow_writer_impl_test.cc index 245c67be85b3..e329b91d2926 100644 --- a/test/common/router/shadow_writer_impl_test.cc +++ b/test/common/router/shadow_writer_impl_test.cc @@ -11,6 +11,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::InSequence; using testing::Invoke; using testing::Return; @@ -24,7 +25,7 @@ class ShadowWriterImplTest : public testing::Test { void expectShadowWriter(absl::string_view host, absl::string_view shadowed_host) { Http::MessagePtr message(new Http::RequestMessageImpl()); message->headers().insertHost().value(std::string(host)); - EXPECT_CALL(cm_, get("foo")); + EXPECT_CALL(cm_, get(Eq("foo"))); EXPECT_CALL(cm_, httpAsyncClientForCluster("foo")).WillOnce(ReturnRef(cm_.async_client_)); Http::MockAsyncClientRequest request(&cm_.async_client_); EXPECT_CALL( @@ -65,7 +66,7 @@ TEST_F(ShadowWriterImplTest, NoCluster) { InSequence s; Http::MessagePtr message(new Http::RequestMessageImpl()); - EXPECT_CALL(cm_, get("foo")).WillOnce(Return(nullptr)); + EXPECT_CALL(cm_, get(Eq("foo"))).WillOnce(Return(nullptr)); EXPECT_CALL(cm_, httpAsyncClientForCluster("foo")).Times(0); writer_.shadow("foo", std::move(message), std::chrono::milliseconds(5)); } diff --git a/test/common/router/vhds_test.cc b/test/common/router/vhds_test.cc index cb35c34cb3ee..01d3287421e0 100644 --- a/test/common/router/vhds_test.cc +++ b/test/common/router/vhds_test.cc @@ -6,17 +6,14 @@ #include "envoy/admin/v2alpha/config_dump.pb.validate.h" #include "envoy/stats/scope.h" -#include "common/config/filter_json.h" #include "common/config/utility.h" -#include "common/http/message_impl.h" -#include "common/json/json_loader.h" #include "common/protobuf/protobuf.h" #include "common/router/rds_impl.h" #include "server/http/admin.h" +#include "test/mocks/config/mocks.h" #include "test/mocks/init/mocks.h" -#include "test/mocks/local_info/mocks.h" #include "test/mocks/server/mocks.h" #include "test/mocks/thread_local/mocks.h" #include "test/mocks/upstream/mocks.h" @@ -41,14 +38,6 @@ namespace { class VhdsTest : public testing::Test { public: void SetUp() override { - factory_function_ = { - [](const envoy::api::v2::core::ConfigSource&, const LocalInfo::LocalInfo&, - Event::Dispatcher&, Upstream::ClusterManager&, Envoy::Runtime::RandomGenerator&, - Stats::Scope&, absl::string_view, ProtobufMessage::ValidationVisitor&, Api::Api&, - Envoy::Config::SubscriptionCallbacks&) -> std::unique_ptr { - return std::unique_ptr(); - }}; - default_vhds_config_ = R"EOF( name: my_route vhds: @@ -101,11 +90,11 @@ name: my_route NiceMock factory_context_; Init::ExpectableWatcherImpl init_watcher_; Init::TargetHandlePtr init_target_handle_; - Envoy::Router::SubscriptionFactoryFunction factory_function_; const std::string context_ = "vhds_test"; std::unordered_set providers_; Protobuf::util::MessageDifferencer messageDifferencer_; std::string default_vhds_config_; + NiceMock subscription_factory_; }; // verify that api_type: DELTA_GRPC passes validation @@ -114,8 +103,7 @@ TEST_F(VhdsTest, VhdsInstantiationShouldSucceedWithDELTA_GRPC) { TestUtility::parseYaml(default_vhds_config_); RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config); - EXPECT_NO_THROW(VhdsSubscription(config_update_info, factory_context_, context_, providers_, - factory_function_)); + EXPECT_NO_THROW(VhdsSubscription(config_update_info, factory_context_, context_, providers_)); } // verify that api_type: GRPC fails validation @@ -132,8 +120,7 @@ name: my_route )EOF"); RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config); - EXPECT_THROW(VhdsSubscription(config_update_info, factory_context_, context_, providers_, - factory_function_), + EXPECT_THROW(VhdsSubscription(config_update_info, factory_context_, context_, providers_), EnvoyException); } @@ -143,14 +130,14 @@ TEST_F(VhdsTest, VhdsAddsVirtualHosts) { TestUtility::parseYaml(default_vhds_config_); RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config); - VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_, - factory_function_); + VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_); EXPECT_EQ(0UL, config_update_info->routeConfiguration().virtual_hosts_size()); auto vhost = buildVirtualHost("vhost1", "vhost.first"); const auto& added_resources = buildAddedResources({vhost}); const Protobuf::RepeatedPtrField removed_resources; - subscription.onConfigUpdate(added_resources, removed_resources, "1"); + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( + added_resources, removed_resources, "1"); EXPECT_EQ(1UL, config_update_info->routeConfiguration().virtual_hosts_size()); EXPECT_TRUE( @@ -177,15 +164,15 @@ name: my_route )EOF"); RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config); - VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_, - factory_function_); + VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_); EXPECT_EQ(1UL, config_update_info->routeConfiguration().virtual_hosts_size()); EXPECT_EQ("vhost_rds1", config_update_info->routeConfiguration().virtual_hosts(0).name()); auto vhost = buildVirtualHost("vhost1", "vhost.first"); const auto& added_resources = buildAddedResources({vhost}); const Protobuf::RepeatedPtrField removed_resources; - subscription.onConfigUpdate(added_resources, removed_resources, "1"); + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( + added_resources, removed_resources, "1"); EXPECT_EQ(2UL, config_update_info->routeConfiguration().virtual_hosts_size()); auto actual_vhost_0 = config_update_info->routeConfiguration().virtual_hosts(0); @@ -214,14 +201,14 @@ name: my_route cluster_name: xds_cluster )EOF"); RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config); - VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_, - factory_function_); + VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_); EXPECT_EQ(1UL, config_update_info->routeConfiguration().virtual_hosts_size()); EXPECT_EQ("vhost_rds1", config_update_info->routeConfiguration().virtual_hosts(0).name()); const Protobuf::RepeatedPtrField added_resources; const auto removed_resources = buildRemovedResources({"vhost_rds1"}); - subscription.onConfigUpdate(added_resources, removed_resources, "1"); + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( + added_resources, removed_resources, "1"); EXPECT_EQ(0UL, config_update_info->routeConfiguration().virtual_hosts_size()); } @@ -246,15 +233,15 @@ name: my_route )EOF"); RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config); - VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_, - factory_function_); + VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_); EXPECT_EQ(1UL, config_update_info->routeConfiguration().virtual_hosts_size()); EXPECT_EQ("vhost_rds1", config_update_info->routeConfiguration().virtual_hosts(0).name()); auto vhost = buildVirtualHost("vhost_rds1", "vhost.rds.first.mk2"); const auto& added_resources = buildAddedResources({vhost}); const Protobuf::RepeatedPtrField removed_resources; - subscription.onConfigUpdate(added_resources, removed_resources, "1"); + factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( + added_resources, removed_resources, "1"); EXPECT_EQ(1UL, config_update_info->routeConfiguration().virtual_hosts_size()); EXPECT_TRUE( @@ -267,8 +254,7 @@ TEST_F(VhdsTest, VhdsValidatesAddedVirtualHosts) { TestUtility::parseYaml(default_vhds_config_); RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config); - VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_, - factory_function_); + VhdsSubscription subscription(config_update_info, factory_context_, context_, providers_); auto vhost = TestUtility::parseYaml(R"EOF( name: invalid_vhost @@ -281,7 +267,8 @@ TEST_F(VhdsTest, VhdsValidatesAddedVirtualHosts) { const auto& added_resources = buildAddedResources({vhost}); const Protobuf::RepeatedPtrField removed_resources; - EXPECT_THROW(subscription.onConfigUpdate(added_resources, removed_resources, "1"), + EXPECT_THROW(factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( + added_resources, removed_resources, "1"), ProtoValidationException); } diff --git a/test/common/secret/BUILD b/test/common/secret/BUILD index f0b53f5f43a7..2a354658e4d3 100644 --- a/test/common/secret/BUILD +++ b/test/common/secret/BUILD @@ -38,6 +38,7 @@ envoy_cc_test( "//source/common/ssl:tls_certificate_config_impl_lib", "//test/mocks/grpc:grpc_mocks", "//test/mocks/init:init_mocks", + "//test/mocks/protobuf:protobuf_mocks", "//test/mocks/secret:secret_mocks", "//test/mocks/server:server_mocks", "//test/test_common:environment_lib", diff --git a/test/common/secret/sds_api_test.cc b/test/common/secret/sds_api_test.cc index feee6201d634..f85edd205787 100644 --- a/test/common/secret/sds_api_test.cc +++ b/test/common/secret/sds_api_test.cc @@ -10,6 +10,7 @@ #include "test/mocks/grpc/mocks.h" #include "test/mocks/init/mocks.h" +#include "test/mocks/protobuf/mocks.h" #include "test/mocks/secret/mocks.h" #include "test/mocks/server/mocks.h" #include "test/test_common/environment.h" @@ -28,9 +29,20 @@ namespace { class SdsApiTest : public testing::Test { protected: - SdsApiTest() : api_(Api::createApiForTest()) {} + SdsApiTest() : api_(Api::createApiForTest()) { + EXPECT_CALL(init_manager_, add(_)).WillOnce(Invoke([this](const Init::Target& target) { + init_target_handle_ = target.createHandle("test"); + })); + } + + void initialize() { init_target_handle_->initialize(init_watcher_); } Api::ApiPtr api_; + NiceMock validation_visitor_; + NiceMock subscription_factory_; + NiceMock init_manager_; + NiceMock init_watcher_; + Init::TargetHandlePtr init_target_handle_; }; // Validate that SdsApi object is created and initialized successfully. @@ -38,50 +50,21 @@ TEST_F(SdsApiTest, BasicTest) { ::testing::InSequence s; const envoy::service::discovery::v2::SdsDummy dummy; NiceMock server; - NiceMock init_manager; - NiceMock init_watcher; - Init::TargetHandlePtr init_target_handle; - EXPECT_CALL(init_manager, add(_)) - .WillOnce(Invoke([&init_target_handle](const Init::Target& target) { - init_target_handle = target.createHandle("test"); - })); envoy::api::v2::core::ConfigSource config_source; - config_source.mutable_api_config_source()->set_api_type( - envoy::api::v2::core::ApiConfigSource::GRPC); - auto grpc_service = config_source.mutable_api_config_source()->add_grpc_services(); - auto google_grpc = grpc_service->mutable_google_grpc(); - google_grpc->set_target_uri("fake_address"); - google_grpc->set_stat_prefix("test"); - TlsCertificateSdsApi sds_api( - server.localInfo(), server.dispatcher(), server.random(), server.stats(), - server.clusterManager(), init_manager, config_source, "abc.com", []() {}, - server.messageValidationVisitor(), *api_); - - Grpc::MockAsyncClient* grpc_client{new NiceMock()}; - NiceMock* factory{new NiceMock()}; - EXPECT_CALL(server.cluster_manager_.async_client_manager_, factoryForGrpcService(_, _, _)) - .WillOnce(Invoke([factory](const envoy::api::v2::core::GrpcService&, Stats::Scope&, bool) { - return Grpc::AsyncClientFactoryPtr{factory}; - })); - EXPECT_CALL(*factory, create()).WillOnce(Invoke([grpc_client] { - return Grpc::RawAsyncClientPtr{grpc_client}; - })); - EXPECT_CALL(init_watcher, ready()); - init_target_handle->initialize(init_watcher); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, + server.stats(), init_manager_, []() {}); + initialize(); } // Validate that TlsCertificateSdsApi updates secrets successfully if a good secret // is passed to onConfigUpdate(). TEST_F(SdsApiTest, DynamicTlsCertificateUpdateSuccess) { NiceMock server; - NiceMock init_manager; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api( - server.localInfo(), server.dispatcher(), server.random(), server.stats(), - server.clusterManager(), init_manager, config_source, "abc.com", []() {}, - server.messageValidationVisitor(), *api_); - + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, + server.stats(), init_manager_, []() {}); + initialize(); NiceMock secret_callback; auto handle = sds_api.addUpdateCallback([&secret_callback]() { secret_callback.onAddOrUpdateSecret(); }); @@ -101,7 +84,7 @@ TEST_F(SdsApiTest, DynamicTlsCertificateUpdateSuccess) { secret_resources.Add()->PackFrom(typed_secret); EXPECT_CALL(secret_callback, onAddOrUpdateSecret()); - sds_api.onConfigUpdate(secret_resources, ""); + subscription_factory_.callbacks_->onConfigUpdate(secret_resources, ""); Ssl::TlsCertificateConfigImpl tls_config(*sds_api.secret(), *api_); const std::string cert_pem = @@ -119,13 +102,11 @@ TEST_F(SdsApiTest, DynamicTlsCertificateUpdateSuccess) { class PartialMockSds : public SdsApi { public: - PartialMockSds(NiceMock& server, Api::Api& api, - NiceMock& init_manager, - envoy::api::v2::core::ConfigSource& config_source) - : SdsApi( - server.localInfo(), server.dispatcher(), server.random(), server.stats(), - server.clusterManager(), init_manager, config_source, "abc.com", []() {}, - server.messageValidationVisitor(), api) {} + PartialMockSds(NiceMock& server, NiceMock& init_manager, + envoy::api::v2::core::ConfigSource& config_source, + Config::SubscriptionFactory& subscription_factory) + : SdsApi(config_source, "abc.com", subscription_factory, validation_visitor_, server.stats(), + init_manager, []() {}) {} MOCK_METHOD2(onConfigUpdate, void(const Protobuf::RepeatedPtrField&, const std::string&)); @@ -136,6 +117,8 @@ class PartialMockSds : public SdsApi { } void setSecret(const envoy::api::v2::auth::Secret&) override {} void validateConfig(const envoy::api::v2::auth::Secret&) override {} + + NiceMock validation_visitor_; }; // Basic test of delta's passthrough call to the state-of-the-world variant, to @@ -153,11 +136,11 @@ TEST_F(SdsApiTest, Delta) { for_matching.Add()->PackFrom(secret); NiceMock server; - NiceMock init_manager; envoy::api::v2::core::ConfigSource config_source; - PartialMockSds sds(server, *api_, init_manager, config_source); + PartialMockSds sds(server, init_manager_, config_source, subscription_factory_); + initialize(); EXPECT_CALL(sds, onConfigUpdate(RepeatedProtoEq(for_matching), "version1")); - sds.SdsApi::onConfigUpdate(resources, {}, "ignored"); + subscription_factory_.callbacks_->onConfigUpdate(resources, {}, "ignored"); // An attempt to remove a resource logs an error, but otherwise just carries on (ignoring the // removal attempt). @@ -165,18 +148,15 @@ TEST_F(SdsApiTest, Delta) { EXPECT_CALL(sds, onConfigUpdate(RepeatedProtoEq(for_matching), "version2")); Protobuf::RepeatedPtrField removals; *removals.Add() = "route_0"; - sds.SdsApi::onConfigUpdate(resources, removals, "ignored"); + subscription_factory_.callbacks_->onConfigUpdate(resources, removals, "ignored"); } // Tests SDS's use of the delta variant of onConfigUpdate(). TEST_F(SdsApiTest, DeltaUpdateSuccess) { NiceMock server; - NiceMock init_manager; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api( - server.localInfo(), server.dispatcher(), server.random(), server.stats(), - server.clusterManager(), init_manager, config_source, "abc.com", []() {}, - server.messageValidationVisitor(), *api_); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, + server.stats(), init_manager_, []() {}); NiceMock secret_callback; auto handle = @@ -197,7 +177,8 @@ TEST_F(SdsApiTest, DeltaUpdateSuccess) { secret_resources.Add()->mutable_resource()->PackFrom(typed_secret); EXPECT_CALL(secret_callback, onAddOrUpdateSecret()); - sds_api.onConfigUpdate(secret_resources, {}, ""); + initialize(); + subscription_factory_.callbacks_->onConfigUpdate(secret_resources, {}, ""); Ssl::TlsCertificateConfigImpl tls_config(*sds_api.secret(), *api_); const std::string cert_pem = @@ -217,12 +198,10 @@ TEST_F(SdsApiTest, DeltaUpdateSuccess) { // a good secret is passed to onConfigUpdate(). TEST_F(SdsApiTest, DynamicCertificateValidationContextUpdateSuccess) { NiceMock server; - NiceMock init_manager; envoy::api::v2::core::ConfigSource config_source; - CertificateValidationContextSdsApi sds_api( - server.localInfo(), server.dispatcher(), server.random(), server.stats(), - server.clusterManager(), init_manager, config_source, "abc.com", []() {}, - server.messageValidationVisitor(), *api_); + CertificateValidationContextSdsApi sds_api(config_source, "abc.com", subscription_factory_, + validation_visitor_, server.stats(), init_manager_, + []() {}); NiceMock secret_callback; auto handle = @@ -241,7 +220,8 @@ TEST_F(SdsApiTest, DynamicCertificateValidationContextUpdateSuccess) { Protobuf::RepeatedPtrField secret_resources; secret_resources.Add()->PackFrom(typed_secret); EXPECT_CALL(secret_callback, onAddOrUpdateSecret()); - sds_api.onConfigUpdate(secret_resources, ""); + initialize(); + subscription_factory_.callbacks_->onConfigUpdate(secret_resources, ""); Ssl::CertificateValidationContextConfigImpl cvc_config(*sds_api.secret(), *api_); const std::string ca_cert = @@ -270,12 +250,10 @@ class MockCvcValidationCallback : public CvcValidationCallback { // provides correct information. TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { NiceMock server; - NiceMock init_manager; envoy::api::v2::core::ConfigSource config_source; - CertificateValidationContextSdsApi sds_api( - server.localInfo(), server.dispatcher(), server.random(), server.stats(), - server.clusterManager(), init_manager, config_source, "abc.com", []() {}, - server.messageValidationVisitor(), *api_); + CertificateValidationContextSdsApi sds_api(config_source, "abc.com", subscription_factory_, + validation_visitor_, server.stats(), init_manager_, + []() {}); NiceMock secret_callback; auto handle = @@ -301,7 +279,8 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { Protobuf::RepeatedPtrField secret_resources; secret_resources.Add()->PackFrom(typed_secret); - sds_api.onConfigUpdate(secret_resources, ""); + initialize(); + subscription_factory_.callbacks_->onConfigUpdate(secret_resources, ""); const std::string default_verify_certificate_hash = "0000000000000000000000000000000000000000000000000000000000000000"; @@ -341,28 +320,24 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { // Validate that SdsApi throws exception if an empty secret is passed to onConfigUpdate(). TEST_F(SdsApiTest, EmptyResource) { NiceMock server; - NiceMock init_manager; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api( - server.localInfo(), server.dispatcher(), server.random(), server.stats(), - server.clusterManager(), init_manager, config_source, "abc.com", []() {}, - server.messageValidationVisitor(), *api_); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, + server.stats(), init_manager_, []() {}); Protobuf::RepeatedPtrField secret_resources; - EXPECT_THROW_WITH_MESSAGE(sds_api.onConfigUpdate(secret_resources, ""), EnvoyException, + initialize(); + EXPECT_THROW_WITH_MESSAGE(subscription_factory_.callbacks_->onConfigUpdate(secret_resources, ""), + EnvoyException, "Missing SDS resources for abc.com in onConfigUpdate()"); } // Validate that SdsApi throws exception if multiple secrets are passed to onConfigUpdate(). TEST_F(SdsApiTest, SecretUpdateWrongSize) { NiceMock server; - NiceMock init_manager; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api( - server.localInfo(), server.dispatcher(), server.random(), server.stats(), - server.clusterManager(), init_manager, config_source, "abc.com", []() {}, - server.messageValidationVisitor(), *api_); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, + server.stats(), init_manager_, []() {}); std::string yaml = R"EOF( @@ -380,20 +355,18 @@ TEST_F(SdsApiTest, SecretUpdateWrongSize) { secret_resources.Add()->PackFrom(typed_secret); secret_resources.Add()->PackFrom(typed_secret); - EXPECT_THROW_WITH_MESSAGE(sds_api.onConfigUpdate(secret_resources, ""), EnvoyException, - "Unexpected SDS secrets length: 2"); + initialize(); + EXPECT_THROW_WITH_MESSAGE(subscription_factory_.callbacks_->onConfigUpdate(secret_resources, ""), + EnvoyException, "Unexpected SDS secrets length: 2"); } // Validate that SdsApi throws exception if secret name passed to onConfigUpdate() // does not match configured name. TEST_F(SdsApiTest, SecretUpdateWrongSecretName) { NiceMock server; - NiceMock init_manager; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api( - server.localInfo(), server.dispatcher(), server.random(), server.stats(), - server.clusterManager(), init_manager, config_source, "abc.com", []() {}, - server.messageValidationVisitor(), *api_); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, + server.stats(), init_manager_, []() {}); std::string yaml = R"EOF( @@ -410,7 +383,9 @@ TEST_F(SdsApiTest, SecretUpdateWrongSecretName) { Protobuf::RepeatedPtrField secret_resources; secret_resources.Add()->PackFrom(typed_secret); - EXPECT_THROW_WITH_MESSAGE(sds_api.onConfigUpdate(secret_resources, ""), EnvoyException, + initialize(); + EXPECT_THROW_WITH_MESSAGE(subscription_factory_.callbacks_->onConfigUpdate(secret_resources, ""), + EnvoyException, "Unexpected SDS secret (expecting abc.com): wrong.name.com"); } diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index 5ebcd901fad9..89950982d7ca 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -159,13 +159,14 @@ TEST_F(SecretManagerImplTest, SdsDynamicSecretUpdateSuccess) { NiceMock dispatcher; NiceMock random; Stats::IsolatedStoreImpl stats; - NiceMock cluster_manager; NiceMock init_manager; - EXPECT_CALL(secret_context, localInfo()).WillOnce(ReturnRef(local_info)); - EXPECT_CALL(secret_context, dispatcher()).WillOnce(ReturnRef(dispatcher)); - EXPECT_CALL(secret_context, random()).WillOnce(ReturnRef(random)); + NiceMock init_watcher; + Init::TargetHandlePtr init_target_handle; + EXPECT_CALL(init_manager, add(_)) + .WillOnce(Invoke([&init_target_handle](const Init::Target& target) { + init_target_handle = target.createHandle("test"); + })); EXPECT_CALL(secret_context, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(secret_context, clusterManager()).WillOnce(ReturnRef(cluster_manager)); EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); auto secret_provider = @@ -183,7 +184,9 @@ name: "abc.com" TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), typed_secret); Protobuf::RepeatedPtrField secret_resources; secret_resources.Add()->PackFrom(typed_secret); - dynamic_cast(*secret_provider).onConfigUpdate(secret_resources, ""); + init_target_handle->initialize(init_watcher); + secret_context.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate(secret_resources, + ""); Ssl::TlsCertificateConfigImpl tls_config(*secret_provider->secret(), *api_); const std::string cert_pem = "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/selfsigned_cert.pem"; diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 13985c09deb4..cfc06fe84930 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -17,11 +17,8 @@ envoy_cc_test( deps = [ ":utility_lib", "//source/common/config:utility_lib", - "//source/common/http:message_lib", - "//source/common/json:json_loader_lib", "//source/common/protobuf:utility_lib", "//source/common/upstream:cds_api_lib", - "//test/mocks/local_info:local_info_mocks", "//test/mocks/protobuf:protobuf_mocks", "//test/mocks/upstream:upstream_mocks", "//test/test_common:utility_lib", diff --git a/test/common/upstream/cds_api_impl_test.cc b/test/common/upstream/cds_api_impl_test.cc index 5a790c80cf23..a69bdbb786ea 100644 --- a/test/common/upstream/cds_api_impl_test.cc +++ b/test/common/upstream/cds_api_impl_test.cc @@ -6,13 +6,10 @@ #include "envoy/api/v2/core/config_source.pb.validate.h" #include "common/config/utility.h" -#include "common/http/message_impl.h" -#include "common/json/json_loader.h" #include "common/protobuf/utility.h" #include "common/upstream/cds_api_impl.h" #include "test/common/upstream/utility.h" -#include "test/mocks/local_info/mocks.h" #include "test/mocks/protobuf/mocks.h" #include "test/mocks/upstream/mocks.h" #include "test/test_common/printers.h" @@ -38,34 +35,14 @@ MATCHER_P(WithName, expectedName, "") { return arg.name() == expectedName; } class CdsApiImplTest : public testing::Test { protected: - CdsApiImplTest() - : request_(&cm_.async_client_), api_(Api::createApiForTest(store_)), - cds_version_( - store_.gauge("cluster_manager.cds.version", Stats::Gauge::ImportMode::NeverImport)) {} - void setup() { - const std::string config_yaml = R"EOF( -api_config_source: - cluster_names: - - foo_cluster - refresh_delay: 1s - api_type: REST - )EOF"; - envoy::api::v2::core::ConfigSource cds_config; - TestUtility::loadFromYamlAndValidate(config_yaml, cds_config); - cluster_map_.emplace("foo_cluster", mock_cluster_); - EXPECT_CALL(cm_, clusters()).WillRepeatedly(Return(cluster_map_)); - EXPECT_CALL(mock_cluster_, info()).Times(AnyNumber()); - EXPECT_CALL(*mock_cluster_.info_, addedViaApi()); - EXPECT_CALL(mock_cluster_, info()).Times(AnyNumber()); - EXPECT_CALL(*mock_cluster_.info_, type()); - cds_ = CdsApiImpl::create(cds_config, cm_, dispatcher_, random_, local_info_, store_, - validation_visitor_, *api_); + cds_ = CdsApiImpl::create(cds_config, cm_, store_, validation_visitor_); resetCdsInitializedCb(); - expectRequest(); + EXPECT_CALL(*cm_.subscription_factory_.subscription_, start(_)); cds_->initialize(); + cds_callbacks_ = cm_.subscription_factory_.callbacks_; } void resetCdsInitializedCb() { @@ -75,26 +52,6 @@ class CdsApiImplTest : public testing::Test { }); } - void expectRequest() { - EXPECT_CALL(cm_, httpAsyncClientForCluster("foo_cluster")); - EXPECT_CALL(cm_.async_client_, send_(_, _, _)) - .WillOnce( - Invoke([&](Http::MessagePtr& request, Http::AsyncClient::Callbacks& callbacks, - const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { - EXPECT_EQ( - (Http::TestHeaderMapImpl{ - {":method", "POST"}, - {":path", "/v2/discovery:clusters"}, - {":authority", "foo_cluster"}, - {"content-type", "application/json"}, - {"content-length", - request->body() ? fmt::format_int(request->body()->length()).str() : "0"}}), - request->headers()); - callbacks_ = &callbacks; - return &request_; - })); - } - ClusterManager::ClusterInfoMap makeClusterMap(std::vector clusters) { ClusterManager::ClusterInfoMap map; for (auto cluster : clusters) { @@ -103,15 +60,6 @@ class CdsApiImplTest : public testing::Test { return map; } - static Http::MessagePtr parseResponseMessageFromYaml(const std::string& yaml) { - Http::MessagePtr message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - const auto json = Json::Factory::loadFromYamlString(yaml); - ASSERT(json->isObject()); - message->body() = std::make_unique(json->asJsonString()); - return message; - } - class MockWarmingClusterManager : public MockClusterManager { public: explicit MockWarmingClusterManager(TimeSource& time_source) : MockClusterManager(time_source) {} @@ -174,18 +122,11 @@ class CdsApiImplTest : public testing::Test { NiceMock cm_; Upstream::ClusterManager::ClusterInfoMap cluster_map_; Upstream::MockClusterMockPrioritySet mock_cluster_; - NiceMock dispatcher_; - NiceMock random_; - NiceMock local_info_; Stats::IsolatedStoreImpl store_; - Http::MockAsyncClientRequest request_; CdsApiPtr cds_; - Event::MockTimer* interval_timer_; - Http::AsyncClient::Callbacks* callbacks_{}; + Config::SubscriptionCallbacks* cds_callbacks_{}; ReadyWatcher initialized_; NiceMock validation_visitor_; - Api::ApiPtr api_; - Stats::Gauge& cds_version_; }; // Negative test for protoc-gen-validate constraints. @@ -200,14 +141,12 @@ TEST_F(CdsApiImplTest, ValidateFail) { EXPECT_CALL(cm_, clusters()).WillRepeatedly(Return(cluster_map_)); EXPECT_CALL(initialized_, ready()); - EXPECT_THROW(dynamic_cast(cds_.get())->onConfigUpdate(clusters, ""), EnvoyException); - EXPECT_CALL(request_, cancel()); + EXPECT_THROW(cds_callbacks_->onConfigUpdate(clusters, ""), EnvoyException); } // Regression test against only updating versionInfo() if at least one cluster // is are added/updated even if one or more are removed. TEST_F(CdsApiImplTest, UpdateVersionOnClusterRemove) { - interval_timer_ = new Event::MockTimer(&dispatcher_); InSequence s; setup(); @@ -222,34 +161,24 @@ version_info: '0' eds_config: path: eds path )EOF"; + auto response1 = TestUtility::parseYaml(response1_yaml); EXPECT_CALL(cm_, clusters()).WillOnce(Return(ClusterManager::ClusterInfoMap{})); cm_.expectAdd("cluster1", "0"); EXPECT_CALL(initialized_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); EXPECT_EQ("", cds_->versionInfo()); - EXPECT_EQ( - 0UL, - store_.gauge("cluster_manager.cds.version", Stats::Gauge::ImportMode::NeverImport).value()); - callbacks_->onSuccess(parseResponseMessageFromYaml(response1_yaml)); + cds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); EXPECT_EQ("0", cds_->versionInfo()); - expectRequest(); - interval_timer_->callback_(); - const std::string response2_yaml = R"EOF( version_info: '1' resources: )EOF"; + auto response2 = TestUtility::parseYaml(response2_yaml); EXPECT_CALL(cm_, clusters()).WillOnce(Return(makeClusterMap({"cluster1"}))); - EXPECT_CALL(cm_, removeCluster("cluster1")).WillOnce(Return(true)); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(parseResponseMessageFromYaml(response2_yaml)); - - EXPECT_EQ(2UL, store_.counter("cluster_manager.cds.update_attempt").value()); - EXPECT_EQ(2UL, store_.counter("cluster_manager.cds.update_success").value()); + cds_callbacks_->onConfigUpdate(response2.resources(), response2.version_info()); EXPECT_EQ("1", cds_->versionInfo()); } @@ -267,11 +196,9 @@ TEST_F(CdsApiImplTest, ValidateDuplicateClusters) { EXPECT_CALL(cm_, clusters()).WillRepeatedly(Return(cluster_map_)); EXPECT_CALL(initialized_, ready()); - EXPECT_THROW_WITH_MESSAGE(dynamic_cast(cds_.get())->onConfigUpdate(clusters, ""), - EnvoyException, + EXPECT_THROW_WITH_MESSAGE(cds_callbacks_->onConfigUpdate(clusters, ""), EnvoyException, "Error adding/updating cluster(s) duplicate_cluster: duplicate cluster " "duplicate_cluster found"); - EXPECT_CALL(request_, cancel()); } TEST_F(CdsApiImplTest, EmptyConfigUpdate) { @@ -281,10 +208,9 @@ TEST_F(CdsApiImplTest, EmptyConfigUpdate) { EXPECT_CALL(cm_, clusters()).WillOnce(Return(ClusterManager::ClusterInfoMap{})); EXPECT_CALL(initialized_, ready()); - EXPECT_CALL(request_, cancel()); Protobuf::RepeatedPtrField clusters; - dynamic_cast(cds_.get())->onConfigUpdate(clusters, ""); + cds_callbacks_->onConfigUpdate(clusters, ""); } TEST_F(CdsApiImplTest, ConfigUpdateWith2ValidClusters) { @@ -295,7 +221,6 @@ TEST_F(CdsApiImplTest, ConfigUpdateWith2ValidClusters) { EXPECT_CALL(cm_, clusters()).WillOnce(Return(ClusterManager::ClusterInfoMap{})); EXPECT_CALL(initialized_, ready()); - EXPECT_CALL(request_, cancel()); Protobuf::RepeatedPtrField clusters; @@ -309,7 +234,7 @@ TEST_F(CdsApiImplTest, ConfigUpdateWith2ValidClusters) { clusters.Add()->PackFrom(cluster_2); cm_.expectAdd("cluster_2"); - dynamic_cast(cds_.get())->onConfigUpdate(clusters, ""); + cds_callbacks_->onConfigUpdate(clusters, ""); } TEST_F(CdsApiImplTest, DeltaConfigUpdate) { @@ -318,7 +243,6 @@ TEST_F(CdsApiImplTest, DeltaConfigUpdate) { setup(); } EXPECT_CALL(initialized_, ready()); - EXPECT_CALL(request_, cancel()); { Protobuf::RepeatedPtrField resources; @@ -340,7 +264,7 @@ TEST_F(CdsApiImplTest, DeltaConfigUpdate) { resource->set_name("cluster_2"); resource->set_version("v1"); } - dynamic_cast(cds_.get())->onConfigUpdate(resources, {}, "v1"); + cds_callbacks_->onConfigUpdate(resources, {}, "v1"); } { @@ -357,7 +281,7 @@ TEST_F(CdsApiImplTest, DeltaConfigUpdate) { Protobuf::RepeatedPtrField removed; *removed.Add() = "cluster_1"; EXPECT_CALL(cm_, removeCluster(StrEq("cluster_1"))).WillOnce(Return(true)); - dynamic_cast(cds_.get())->onConfigUpdate(resources, removed, "v2"); + cds_callbacks_->onConfigUpdate(resources, removed, "v2"); } } @@ -369,7 +293,6 @@ TEST_F(CdsApiImplTest, ConfigUpdateAddsSecondClusterEvenIfFirstThrows) { EXPECT_CALL(cm_, clusters()).WillOnce(Return(ClusterManager::ClusterInfoMap{})); EXPECT_CALL(initialized_, ready()); - EXPECT_CALL(request_, cancel()); Protobuf::RepeatedPtrField clusters; @@ -389,29 +312,11 @@ TEST_F(CdsApiImplTest, ConfigUpdateAddsSecondClusterEvenIfFirstThrows) { cm_.expectAddToThrow("cluster_3", "Another exception"); EXPECT_THROW_WITH_MESSAGE( - dynamic_cast(cds_.get())->onConfigUpdate(clusters, ""), EnvoyException, + cds_callbacks_->onConfigUpdate(clusters, ""), EnvoyException, "Error adding/updating cluster(s) cluster_1: An exception, cluster_3: Another exception"); } -TEST_F(CdsApiImplTest, InvalidOptions) { - const std::string config_yaml = R"EOF( -api_config_source: - cluster_names: - - foo_cluster - refresh_delay: 1s - )EOF"; - - local_info_.node_.set_cluster(""); - local_info_.node_.set_id(""); - envoy::api::v2::core::ConfigSource cds_config; - TestUtility::loadFromYamlAndValidate(config_yaml, cds_config); - EXPECT_THROW(CdsApiImpl::create(cds_config, cm_, dispatcher_, random_, local_info_, store_, - validation_visitor_, *api_), - EnvoyException); -} - TEST_F(CdsApiImplTest, Basic) { - interval_timer_ = new Event::MockTimer(&dispatcher_); InSequence s; setup(); @@ -432,20 +337,15 @@ version_info: '0' eds_config: path: eds path )EOF"; + auto response1 = TestUtility::parseYaml(response1_yaml); EXPECT_CALL(cm_, clusters()).WillOnce(Return(ClusterManager::ClusterInfoMap{})); cm_.expectAdd("cluster1", "0"); cm_.expectAdd("cluster2", "0"); EXPECT_CALL(initialized_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); EXPECT_EQ("", cds_->versionInfo()); - EXPECT_EQ(0UL, cds_version_.value()); - callbacks_->onSuccess(parseResponseMessageFromYaml(response1_yaml)); + cds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); EXPECT_EQ("0", cds_->versionInfo()); - EXPECT_EQ(7148434200721666028U, cds_version_.value()); - - expectRequest(); - interval_timer_->callback_(); const std::string response2_yaml = R"EOF( version_info: '1' @@ -463,22 +363,18 @@ version_info: '1' eds_config: path: eds path )EOF"; + auto response2 = TestUtility::parseYaml(response2_yaml); EXPECT_CALL(cm_, clusters()).WillOnce(Return(makeClusterMap({"cluster1", "cluster2"}))); cm_.expectAdd("cluster1", "1"); cm_.expectAdd("cluster3", "1"); EXPECT_CALL(cm_, removeCluster("cluster2")); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(parseResponseMessageFromYaml(response2_yaml)); + cds_callbacks_->onConfigUpdate(response2.resources(), response2.version_info()); - EXPECT_EQ(2UL, store_.counter("cluster_manager.cds.update_attempt").value()); - EXPECT_EQ(2UL, store_.counter("cluster_manager.cds.update_success").value()); EXPECT_EQ("1", cds_->versionInfo()); - EXPECT_EQ(13237225503670494420U, cds_version_.value()); } TEST_F(CdsApiImplTest, CdsPauseOnWarming) { - interval_timer_ = new Event::MockTimer(&dispatcher_); EXPECT_CALL(cm_, clusters()).WillRepeatedly(Return(ClusterManager::ClusterInfoMap{})); InSequence s; @@ -500,24 +396,21 @@ version_info: '0' eds_config: path: eds path )EOF"; + auto response1 = TestUtility::parseYaml(response1_yaml); // Two clusters updated, both warmed up. - EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().ClusterLoadAssignment)).Times(1); + EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().ClusterLoadAssignment)); cm_.expectAddWithWarming("cluster1", "0"); cm_.expectWarmingClusterCount(); - EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().Cluster)).Times(1); + EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().Cluster)); cm_.expectAddWithWarming("cluster2", "0"); cm_.expectWarmingClusterCount(); EXPECT_CALL(initialized_, ready()); cm_.expectWarmingClusterCount(2); - EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().Cluster)).Times(1); - EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().ClusterLoadAssignment)).Times(1); - EXPECT_CALL(*interval_timer_, enableTimer(_)); + EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().Cluster)); + EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().ClusterLoadAssignment)); cm_.clustersToWarmUp({"cluster1", "cluster2"}); - callbacks_->onSuccess(parseResponseMessageFromYaml(response1_yaml)); - - expectRequest(); - interval_timer_->callback_(); + cds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); // Two clusters updated, only one warmed up. const std::string response2_yaml = R"EOF( @@ -536,23 +429,20 @@ version_info: '1' eds_config: path: eds path )EOF"; + auto response2 = TestUtility::parseYaml(response2_yaml); - EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().ClusterLoadAssignment)).Times(1); + EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().ClusterLoadAssignment)); cm_.expectAddWithWarming("cluster1", "1"); cm_.expectWarmingClusterCount(); - EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().Cluster)).Times(1); + EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().Cluster)); cm_.expectAddWithWarming("cluster3", "1"); cm_.expectWarmingClusterCount(); EXPECT_CALL(initialized_, ready()); cm_.expectWarmingClusterCount(); - EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().ClusterLoadAssignment)).Times(1); - EXPECT_CALL(*interval_timer_, enableTimer(_)); + EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().ClusterLoadAssignment)); resetCdsInitializedCb(); cm_.clustersToWarmUp({"cluster1"}); - callbacks_->onSuccess(parseResponseMessageFromYaml(response2_yaml)); - - expectRequest(); - interval_timer_->callback_(); + cds_callbacks_->onConfigUpdate(response2.resources(), response2.version_info()); // One cluster updated and warmed up. Also finish warming up of the previously added cluster3. const std::string response3_yaml = R"EOF( @@ -565,21 +455,18 @@ version_info: '2' eds_config: path: eds path )EOF"; + auto response3 = TestUtility::parseYaml(response3_yaml); - EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().ClusterLoadAssignment)).Times(1); + EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().ClusterLoadAssignment)); cm_.expectAddWithWarming("cluster4", "2"); cm_.expectWarmingClusterCount(); EXPECT_CALL(initialized_, ready()); cm_.expectWarmingClusterCount(2); - EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().Cluster)).Times(1); - EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().ClusterLoadAssignment)).Times(1); - EXPECT_CALL(*interval_timer_, enableTimer(_)); + EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().Cluster)); + EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().ClusterLoadAssignment)); resetCdsInitializedCb(); cm_.clustersToWarmUp({"cluster4", "cluster3"}); - callbacks_->onSuccess(parseResponseMessageFromYaml(response3_yaml)); - - expectRequest(); - interval_timer_->callback_(); + cds_callbacks_->onConfigUpdate(response3.resources(), response3.version_info()); const std::string response4_yaml = R"EOF( version_info: '3' @@ -597,34 +484,34 @@ version_info: '3' eds_config: path: eds path )EOF"; + auto response4 = TestUtility::parseYaml(response4_yaml); // Two clusters updated, first one warmed up before processing of the second one starts. - EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().ClusterLoadAssignment)).Times(1); + EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().ClusterLoadAssignment)); cm_.expectAddWithWarming("cluster5", "3", true); cm_.expectWarmingClusterCount(); - EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().Cluster)).Times(1); + EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().Cluster)); cm_.expectWarmingClusterCount(); - EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().Cluster)).Times(1); + EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().Cluster)); cm_.expectAddWithWarming("cluster6", "3"); cm_.expectWarmingClusterCount(); - EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().Cluster)).Times(1); + EXPECT_CALL(cm_.ads_mux_, pause(Config::TypeUrl::get().Cluster)); EXPECT_CALL(initialized_, ready()); cm_.expectWarmingClusterCount(); - EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().Cluster)).Times(1); - EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().ClusterLoadAssignment)).Times(1); - EXPECT_CALL(*interval_timer_, enableTimer(_)); + EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().Cluster)); + EXPECT_CALL(cm_.ads_mux_, resume(Config::TypeUrl::get().ClusterLoadAssignment)); resetCdsInitializedCb(); cm_.clustersToWarmUp({"cluster6"}); - callbacks_->onSuccess(parseResponseMessageFromYaml(response4_yaml)); + cds_callbacks_->onConfigUpdate(response4.resources(), response4.version_info()); } -TEST_F(CdsApiImplTest, Failure) { - interval_timer_ = new Event::MockTimer(&dispatcher_); +// Validate behavior when the config is delivered but it fails PGV validation. +TEST_F(CdsApiImplTest, FailureInvalidConfig) { InSequence s; setup(); - const std::string response_yaml = R"EOF( + const std::string response1_yaml = R"EOF( version_info: '0' resources: - "@type": type.googleapis.com/envoy.api.v2.Cluster @@ -640,25 +527,24 @@ version_info: '0' eds_config: path: eds path )EOF"; + auto response1 = TestUtility::parseYaml(response1_yaml); EXPECT_CALL(cm_, clusters()).WillRepeatedly(Return(cluster_map_)); EXPECT_CALL(initialized_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(parseResponseMessageFromYaml(response_yaml)); - - expectRequest(); - interval_timer_->callback_(); + EXPECT_THROW(cds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()), + EnvoyException); + EXPECT_EQ("", cds_->versionInfo()); +} - EXPECT_CALL(*interval_timer_, enableTimer(_)); +// Validate behavior when the config fails delivery at the subscription level. +TEST_F(CdsApiImplTest, FailureSubscription) { + InSequence s; - callbacks_->onFailure(Http::AsyncClient::FailureReason::Reset); + setup(); + EXPECT_CALL(initialized_, ready()); + cds_callbacks_->onConfigUpdateFailed({}); EXPECT_EQ("", cds_->versionInfo()); - EXPECT_EQ(2UL, store_.counter("cluster_manager.cds.update_attempt").value()); - EXPECT_EQ(1UL, store_.counter("cluster_manager.cds.update_failure").value()); - // Validate that the schema error increments update_rejected stat. - EXPECT_EQ(1UL, store_.counter("cluster_manager.cds.update_rejected").value()); - EXPECT_EQ(0UL, cds_version_.value()); } } // namespace diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 37f7f587e88e..002a15e3a2d4 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -155,6 +155,16 @@ class TestClusterManagerImpl : public ClusterManagerImpl { public: using ClusterManagerImpl::ClusterManagerImpl; + TestClusterManagerImpl(const envoy::config::bootstrap::v2::Bootstrap& bootstrap, + ClusterManagerFactory& factory, Stats::Store& stats, + ThreadLocal::Instance& tls, Runtime::Loader& runtime, + Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, + AccessLog::AccessLogManager& log_manager, + Event::Dispatcher& main_thread_dispatcher, Server::Admin& admin, + Api::Api& api, Http::Context& http_context) + : ClusterManagerImpl(bootstrap, factory, stats, tls, runtime, random, local_info, log_manager, + main_thread_dispatcher, admin, validation_visitor_, api, http_context) {} + std::map> activeClusters() { std::map> clusters; for (auto& cluster : active_clusters_) { @@ -162,6 +172,8 @@ class TestClusterManagerImpl : public ClusterManagerImpl { } return clusters; } + + NiceMock validation_visitor_; }; // Override postThreadLocalClusterUpdate so we can test that merged updates calls diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc index 4da7882a21f8..9a92063f073f 100644 --- a/test/common/upstream/eds_test.cc +++ b/test/common/upstream/eds_test.cc @@ -72,12 +72,6 @@ class EdsTest : public testing::Test { void resetCluster(const std::string& yaml_config) { local_info_.node_.mutable_locality()->set_zone("us-east-1a"); eds_cluster_ = parseClusterFromV2Yaml(yaml_config); - Upstream::ClusterManager::ClusterInfoMap cluster_map; - Upstream::MockClusterMockPrioritySet cluster; - cluster_map.emplace("eds", cluster); - EXPECT_CALL(cm_, clusters()).WillOnce(Return(cluster_map)); - EXPECT_CALL(cluster, info()).Times(2); - EXPECT_CALL(*cluster.info_, addedViaApi()); Envoy::Stats::ScopePtr scope = stats_.createScope(fmt::format( "cluster.{}.", eds_cluster_.alt_stat_name().empty() ? eds_cluster_.name() : eds_cluster_.alt_stat_name())); @@ -87,21 +81,29 @@ class EdsTest : public testing::Test { cluster_.reset( new EdsClusterImpl(eds_cluster_, runtime_, factory_context, std::move(scope), false)); EXPECT_EQ(Cluster::InitializePhase::Secondary, cluster_->initializePhase()); + eds_callbacks_ = cm_.subscription_factory_.callbacks_; + } + + void initialize() { + EXPECT_CALL(*cm_.subscription_factory_.subscription_, start(_)); + cluster_->initialize([this] { initialized_ = true; }); } void doOnConfigUpdateVerifyNoThrow( const envoy::api::v2::ClusterLoadAssignment& cluster_load_assignment) { Protobuf::RepeatedPtrField resources; resources.Add()->PackFrom(cluster_load_assignment); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); + VERBOSE_EXPECT_NO_THROW(eds_callbacks_->onConfigUpdate(resources, "")); } + bool initialized_{}; Stats::IsolatedStoreImpl stats_; Ssl::MockContextManager ssl_context_manager_; envoy::api::v2::Cluster eds_cluster_; NiceMock cm_; NiceMock dispatcher_; std::shared_ptr cluster_; + Config::SubscriptionCallbacks* eds_callbacks_{}; NiceMock random_; NiceMock runtime_; NiceMock local_info_; @@ -200,10 +202,12 @@ class EdsWithHealthCheckUpdateTest : public EdsTest { // Negative test for protoc-gen-validate constraints. TEST_F(EdsTest, ValidateFail) { + initialize(); envoy::api::v2::ClusterLoadAssignment resource; Protobuf::RepeatedPtrField resources; resources.Add()->PackFrom(resource); - EXPECT_THROW(cluster_->onConfigUpdate(resources, ""), ProtoValidationException); + EXPECT_THROW(eds_callbacks_->onConfigUpdate(resources, ""), ProtoValidationException); + EXPECT_FALSE(initialized_); } // Validate that onConfigUpdate() with unexpected cluster names rejects config. @@ -212,47 +216,43 @@ TEST_F(EdsTest, OnConfigUpdateWrongName) { cluster_load_assignment.set_cluster_name("wrong name"); Protobuf::RepeatedPtrField resources; resources.Add()->PackFrom(cluster_load_assignment); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); - EXPECT_THROW(cluster_->onConfigUpdate(resources, ""), EnvoyException); - cluster_->onConfigUpdateFailed(nullptr); - EXPECT_TRUE(initialized); + initialize(); + EXPECT_THROW(eds_callbacks_->onConfigUpdate(resources, ""), EnvoyException); + eds_callbacks_->onConfigUpdateFailed(nullptr); + EXPECT_TRUE(initialized_); } // Validate that onConfigUpdate() with empty cluster vector size ignores config. TEST_F(EdsTest, OnConfigUpdateEmpty) { - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); - cluster_->onConfigUpdate({}, ""); + initialize(); + eds_callbacks_->onConfigUpdate({}, ""); Protobuf::RepeatedPtrField resources; Protobuf::RepeatedPtrField removed_resources; - cluster_->onConfigUpdate(resources, removed_resources, ""); + eds_callbacks_->onConfigUpdate(resources, removed_resources, ""); EXPECT_EQ(2UL, stats_.counter("cluster.name.update_empty").value()); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); } // Validate that onConfigUpdate() with unexpected cluster vector size rejects config. TEST_F(EdsTest, OnConfigUpdateWrongSize) { - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); envoy::api::v2::ClusterLoadAssignment cluster_load_assignment; cluster_load_assignment.set_cluster_name("fare"); Protobuf::RepeatedPtrField resources; resources.Add()->PackFrom(cluster_load_assignment); resources.Add()->PackFrom(cluster_load_assignment); - EXPECT_THROW(cluster_->onConfigUpdate(resources, ""), EnvoyException); - cluster_->onConfigUpdateFailed(nullptr); - EXPECT_TRUE(initialized); + EXPECT_THROW(eds_callbacks_->onConfigUpdate(resources, ""), EnvoyException); + eds_callbacks_->onConfigUpdateFailed(nullptr); + EXPECT_TRUE(initialized_); } // Validate that onConfigUpdate() with the expected cluster accepts config. TEST_F(EdsTest, OnConfigUpdateSuccess) { envoy::api::v2::ClusterLoadAssignment cluster_load_assignment; cluster_load_assignment.set_cluster_name("fare"); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); EXPECT_EQ(1UL, stats_.counter("cluster.name.update_no_rebuild").value()); } @@ -260,16 +260,15 @@ TEST_F(EdsTest, OnConfigUpdateSuccess) { TEST_F(EdsTest, DeltaOnConfigUpdateSuccess) { envoy::api::v2::ClusterLoadAssignment cluster_load_assignment; cluster_load_assignment.set_cluster_name("fare"); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); Protobuf::RepeatedPtrField resources; auto* resource = resources.Add(); resource->mutable_resource()->PackFrom(cluster_load_assignment); resource->set_version("v1"); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, {}, "v1")); + VERBOSE_EXPECT_NO_THROW(eds_callbacks_->onConfigUpdate(resources, {}, "v1")); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); EXPECT_EQ(1UL, stats_.counter("cluster.name.update_no_rebuild").value()); } @@ -290,10 +289,9 @@ TEST_F(EdsTest, NoServiceNameOnSuccessConfigUpdate) { )EOF"); envoy::api::v2::ClusterLoadAssignment cluster_load_assignment; cluster_load_assignment.set_cluster_name("name"); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); } // Validate that onConfigUpdate() updates the endpoint metadata. @@ -323,10 +321,9 @@ TEST_F(EdsTest, EndpointMetadata) { Config::MetadataFilters::get().ENVOY_LB, "version") .set_string_value("v1"); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); EXPECT_EQ(0UL, stats_.counter("cluster.name.update_no_rebuild").value()); auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); @@ -401,10 +398,9 @@ TEST_F(EdsTest, EndpointHealthStatus) { endpoint->set_health_status(hs.first); } - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); { auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); EXPECT_EQ(hosts.size(), health_status_expected.size()); @@ -913,10 +909,9 @@ TEST_F(EdsTest, EndpointAddedWithNewOverprovisioningFactor) { endpoint_address->set_port_value(80); } - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); EXPECT_EQ(1, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ("1.2.3.4:80", @@ -950,10 +945,9 @@ TEST_F(EdsTest, EndpointLocality) { endpoint_address->set_port_value(80); } - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); EXPECT_EQ(hosts.size(), 2); @@ -989,10 +983,9 @@ TEST_F(EdsTest, EndpointLocalityWeightsIgnored) { endpoint_address->set_port_value(80); } - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); EXPECT_EQ(nullptr, cluster_->prioritySet().hostSetsPerPriority()[0]->localityWeights()); } @@ -1066,10 +1059,9 @@ TEST_F(EdsTest, EndpointLocalityWeights) { endpoint_address->set_port_value(80); } - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); const auto& locality_weights = *cluster_->prioritySet().hostSetsPerPriority()[0]->localityWeights(); @@ -1112,10 +1104,9 @@ TEST_F(EdsTest, RemoveUnreferencedLocalities) { add_hosts_to_locality("oceania", "bear", "best", 4, 1); add_hosts_to_locality("", "us-west-1a", "", 2, 1); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); { auto& hosts_per_locality = @@ -1192,10 +1183,9 @@ TEST_F(EdsTest, EndpointHostsPerLocality) { add_hosts_to_locality("oceania", "koala", "ingsoc", 2); add_hosts_to_locality("", "us-east-1a", "", 1); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); { auto& hosts_per_locality = cluster_->prioritySet().hostSetsPerPriority()[0]->hostsPerLocality(); @@ -1260,10 +1250,9 @@ TEST_F(EdsTest, EndpointHostPerPriority) { add_hosts_to_locality("oceania", "koala", "ingsoc", 2, 0); add_hosts_to_locality("", "us-east-1a", "", 1, 1); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); { auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); @@ -1314,10 +1303,9 @@ TEST_F(EdsTest, EndpointHostsPerPriority) { add_hosts_to_priority(0, 2); add_hosts_to_priority(1, 1); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); ASSERT_EQ(2, cluster_->prioritySet().hostSetsPerPriority().size()); EXPECT_EQ(2, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); @@ -1372,11 +1360,10 @@ TEST_F(EdsTest, NoPriorityForLocalCluster) { // should fail. add_hosts_to_priority(0, 2); add_hosts_to_priority(1, 1); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); Protobuf::RepeatedPtrField resources; resources.Add()->PackFrom(cluster_load_assignment); - EXPECT_THROW_WITH_MESSAGE(cluster_->onConfigUpdate(resources, ""), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(eds_callbacks_->onConfigUpdate(resources, ""), EnvoyException, "Unexpected non-zero priority for local cluster 'fare'."); // Try an update which only has endpoints with P=0. This should go through. @@ -1418,10 +1405,9 @@ TEST_F(EdsTest, PriorityAndLocality) { add_hosts_to_locality_and_priority("", "us-east-1a", "", 1, 8); add_hosts_to_locality_and_priority("foo", "bar", "eep", 1, 2); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); { auto& first_hosts_per_locality = @@ -1528,10 +1514,9 @@ TEST_F(EdsTest, PriorityAndLocalityWeighted) { add_hosts_to_locality_and_priority("", "us-east-1a", "", 1, 8, 60); add_hosts_to_locality_and_priority("foo", "bar", "eep", 1, 2, 40); - bool initialized = false; - cluster_->initialize([&initialized] { initialized = true; }); + initialize(); doOnConfigUpdateVerifyNoThrow(cluster_load_assignment); - EXPECT_TRUE(initialized); + EXPECT_TRUE(initialized_); EXPECT_EQ(0UL, stats_.counter("cluster.name.update_no_rebuild").value()); { @@ -1663,10 +1648,10 @@ TEST_F(EdsTest, MalformedIP) { "foo.bar.com"); endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); - cluster_->initialize([] {}); + initialize(); Protobuf::RepeatedPtrField resources; resources.Add()->PackFrom(cluster_load_assignment); - EXPECT_THROW_WITH_MESSAGE(cluster_->onConfigUpdate(resources, ""), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(eds_callbacks_->onConfigUpdate(resources, ""), EnvoyException, "malformed IP address: foo.bar.com. Consider setting resolver_name or " "setting cluster type to 'STRICT_DNS' or 'LOGICAL_DNS'"); } diff --git a/test/extensions/filters/http/health_check/health_check_test.cc b/test/extensions/filters/http/health_check/health_check_test.cc index 69ee52ac31fe..03ead8af99cd 100644 --- a/test/extensions/filters/http/health_check/health_check_test.cc +++ b/test/extensions/filters/http/health_check/health_check_test.cc @@ -17,6 +17,7 @@ using testing::_; using testing::DoAll; +using testing::Eq; using testing::Invoke; using testing::NiceMock; using testing::Return; @@ -145,8 +146,8 @@ TEST_F(HealthCheckFilterNoPassThroughTest, ComputedHealth) { MockHealthCheckCluster cluster_www2(1000, 800); EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false)); EXPECT_CALL(context_, clusterManager()); - EXPECT_CALL(context_.cluster_manager_, get("www1")).WillRepeatedly(Return(&cluster_www1)); - EXPECT_CALL(context_.cluster_manager_, get("www2")).WillRepeatedly(Return(&cluster_www2)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www1"))).WillRepeatedly(Return(&cluster_www1)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www2"))).WillRepeatedly(Return(&cluster_www2)); EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true)); @@ -159,8 +160,8 @@ TEST_F(HealthCheckFilterNoPassThroughTest, ComputedHealth) { MockHealthCheckCluster cluster_www2(1000, 800); EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false)); EXPECT_CALL(context_, clusterManager()); - EXPECT_CALL(context_.cluster_manager_, get("www1")).WillRepeatedly(Return(&cluster_www1)); - EXPECT_CALL(context_.cluster_manager_, get("www2")).WillRepeatedly(Return(&cluster_www2)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www1"))).WillRepeatedly(Return(&cluster_www1)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www2"))).WillRepeatedly(Return(&cluster_www2)); EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true)); @@ -173,8 +174,8 @@ TEST_F(HealthCheckFilterNoPassThroughTest, ComputedHealth) { MockHealthCheckCluster cluster_www2(1000, 800); EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false)); EXPECT_CALL(context_, clusterManager()); - EXPECT_CALL(context_.cluster_manager_, get("www1")).WillRepeatedly(Return(&cluster_www1)); - EXPECT_CALL(context_.cluster_manager_, get("www2")).WillRepeatedly(Return(&cluster_www2)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www1"))).WillRepeatedly(Return(&cluster_www1)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www2"))).WillRepeatedly(Return(&cluster_www2)); EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true)); @@ -190,8 +191,8 @@ TEST_F(HealthCheckFilterNoPassThroughTest, ComputedHealth) { MockHealthCheckCluster cluster_www2(1000, 0); EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false)); EXPECT_CALL(context_, clusterManager()); - EXPECT_CALL(context_.cluster_manager_, get("www1")).WillRepeatedly(Return(&cluster_www1)); - EXPECT_CALL(context_.cluster_manager_, get("www2")).WillRepeatedly(Return(&cluster_www2)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www1"))).WillRepeatedly(Return(&cluster_www1)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www2"))).WillRepeatedly(Return(&cluster_www2)); EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true)); @@ -205,8 +206,8 @@ TEST_F(HealthCheckFilterNoPassThroughTest, ComputedHealth) { MockHealthCheckCluster cluster_www2(1000, 0, 800); EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false)); EXPECT_CALL(context_, clusterManager()); - EXPECT_CALL(context_.cluster_manager_, get("www1")).WillRepeatedly(Return(&cluster_www1)); - EXPECT_CALL(context_.cluster_manager_, get("www2")).WillRepeatedly(Return(&cluster_www2)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www1"))).WillRepeatedly(Return(&cluster_www1)); + EXPECT_CALL(context_.cluster_manager_, get(Eq("www2"))).WillRepeatedly(Return(&cluster_www2)); EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true)); diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index f133c1471e4b..e24b267b7823 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -18,6 +18,7 @@ using testing::_; using testing::AtLeast; +using testing::Eq; using testing::InSequence; using testing::Invoke; using testing::Return; @@ -758,7 +759,7 @@ TEST_F(LuaHttpFilterTest, HttpCall) { Http::TestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(cluster_manager_, get("cluster")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster")); EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) .WillOnce( @@ -833,7 +834,7 @@ TEST_F(LuaHttpFilterTest, DoubleHttpCall) { Http::TestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(cluster_manager_, get("cluster")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster")); EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) .WillOnce( @@ -856,7 +857,7 @@ TEST_F(LuaHttpFilterTest, DoubleHttpCall) { response_message->body() = std::make_unique("response"); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq(":status 200"))); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("response"))); - EXPECT_CALL(cluster_manager_, get("cluster2")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster2"))); EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster2")); EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) .WillOnce( @@ -912,7 +913,7 @@ TEST_F(LuaHttpFilterTest, HttpCallNoBody) { Http::TestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(cluster_manager_, get("cluster")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster")); EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) .WillOnce( @@ -967,7 +968,7 @@ TEST_F(LuaHttpFilterTest, HttpCallImmediateResponse) { Http::TestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(cluster_manager_, get("cluster")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster")); EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) .WillOnce( @@ -1015,7 +1016,7 @@ TEST_F(LuaHttpFilterTest, HttpCallErrorAfterResumeSuccess) { Http::TestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(cluster_manager_, get("cluster")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster")); EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) .WillOnce( @@ -1065,7 +1066,7 @@ TEST_F(LuaHttpFilterTest, HttpCallFailure) { Http::TestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(cluster_manager_, get("cluster")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster")); EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) .WillOnce( @@ -1107,7 +1108,7 @@ TEST_F(LuaHttpFilterTest, HttpCallReset) { Http::TestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); Http::AsyncClient::Callbacks* callbacks; - EXPECT_CALL(cluster_manager_, get("cluster")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster")); EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) .WillOnce( @@ -1150,7 +1151,7 @@ TEST_F(LuaHttpFilterTest, HttpCallImmediateFailure) { Http::TestHeaderMapImpl request_headers{{":path", "/"}}; Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); - EXPECT_CALL(cluster_manager_, get("cluster")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster")); EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) .WillOnce( @@ -1202,7 +1203,7 @@ TEST_F(LuaHttpFilterTest, HttpCallInvalidCluster) { setup(SCRIPT); Http::TestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_CALL(cluster_manager_, get("cluster")).WillOnce(Return(nullptr)); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))).WillOnce(Return(nullptr)); EXPECT_CALL( *filter_, scriptLog(spdlog::level::err, @@ -1226,7 +1227,7 @@ TEST_F(LuaHttpFilterTest, HttpCallInvalidHeaders) { setup(SCRIPT); Http::TestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_CALL(cluster_manager_, get("cluster")); + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); EXPECT_CALL(*filter_, scriptLog(spdlog::level::err, StrEq("[string \"...\"]:3: http call headers must include " "':path', ':method', and ':authority'"))); diff --git a/test/extensions/filters/http/squash/squash_filter_test.cc b/test/extensions/filters/http/squash/squash_filter_test.cc index c10f6c654e83..ac0c30f1516b 100644 --- a/test/extensions/filters/http/squash/squash_filter_test.cc +++ b/test/extensions/filters/http/squash/squash_filter_test.cc @@ -18,6 +18,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::Invoke; using testing::NiceMock; using testing::Return; @@ -63,7 +64,7 @@ TEST(SoloFilterConfigTest, V1ApiConversion) { Envoy::Json::ObjectSharedPtr json_config = Envoy::Json::Factory::loadFromString(json); NiceMock factory_context; - EXPECT_CALL(factory_context.cluster_manager_, get("fake_cluster")).Times(1); + EXPECT_CALL(factory_context.cluster_manager_, get(Eq("fake_cluster"))).Times(1); auto config = constructSquashFilterConfigFromJson(*json_config, factory_context); EXPECT_EQ("fake_cluster", config.clusterName()); @@ -84,7 +85,7 @@ TEST(SoloFilterConfigTest, NoCluster) { Envoy::Json::ObjectSharedPtr config = Envoy::Json::Factory::loadFromString(json); NiceMock factory_context; - EXPECT_CALL(factory_context.cluster_manager_, get("fake_cluster")).WillOnce(Return(nullptr)); + EXPECT_CALL(factory_context.cluster_manager_, get(Eq("fake_cluster"))).WillOnce(Return(nullptr)); EXPECT_THROW_WITH_MESSAGE(constructSquashFilterConfigFromJson(*config, factory_context), Envoy::EnvoyException, @@ -102,7 +103,7 @@ TEST(SoloFilterConfigTest, ParsesEnvironment) { Envoy::Json::ObjectSharedPtr json_config = Envoy::Json::Factory::loadFromString(json); NiceMock factory_context; - EXPECT_CALL(factory_context.cluster_manager_, get("squash")).Times(1); + EXPECT_CALL(factory_context.cluster_manager_, get(Eq("squash"))).Times(1); auto config = constructSquashFilterConfigFromJson(*json_config, factory_context); EXPECT_JSON_EQ(expected_json, config.attachmentJson()); @@ -122,7 +123,7 @@ TEST(SoloFilterConfigTest, ParsesAndEscapesEnvironment) { Envoy::Json::ObjectSharedPtr json_config = Envoy::Json::Factory::loadFromString(json); NiceMock factory_context; - EXPECT_CALL(factory_context.cluster_manager_, get("squash")).Times(1); + EXPECT_CALL(factory_context.cluster_manager_, get(Eq("squash"))).Times(1); auto config = constructSquashFilterConfigFromJson(*json_config, factory_context); EXPECT_JSON_EQ(expected_json, config.attachmentJson()); } @@ -159,7 +160,7 @@ TEST(SoloFilterConfigTest, ParsesEnvironmentInComplexTemplate) { Envoy::Json::ObjectSharedPtr json_config = Envoy::Json::Factory::loadFromString(json); NiceMock factory_context; - EXPECT_CALL(factory_context.cluster_manager_, get("squash")).Times(1); + EXPECT_CALL(factory_context.cluster_manager_, get(Eq("squash"))).Times(1); auto config = constructSquashFilterConfigFromJson(*json_config, factory_context); EXPECT_JSON_EQ(expected_json, config.attachmentJson()); } diff --git a/test/extensions/filters/network/client_ssl_auth/client_ssl_auth_test.cc b/test/extensions/filters/network/client_ssl_auth/client_ssl_auth_test.cc index 8a7764ec607f..7c5e09642488 100644 --- a/test/extensions/filters/network/client_ssl_auth/client_ssl_auth_test.cc +++ b/test/extensions/filters/network/client_ssl_auth/client_ssl_auth_test.cc @@ -21,6 +21,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::InSequence; using testing::Invoke; using testing::Return; @@ -73,7 +74,7 @@ stat_prefix: vpn envoy::config::filter::network::client_ssl_auth::v2::ClientSSLAuth proto_config{}; TestUtility::loadFromYaml(yaml, proto_config); - EXPECT_CALL(cm_, get("vpn")); + EXPECT_CALL(cm_, get(Eq("vpn"))); setupRequest(); config_ = ClientSslAuthConfig::create(proto_config, tls_, cm_, dispatcher_, stats_store_, random_); @@ -125,7 +126,7 @@ stat_prefix: bad_cluster envoy::config::filter::network::client_ssl_auth::v2::ClientSSLAuth proto_config{}; TestUtility::loadFromYaml(yaml, proto_config); - EXPECT_CALL(cm_, get("bad_cluster")).WillOnce(Return(nullptr)); + EXPECT_CALL(cm_, get(Eq("bad_cluster"))).WillOnce(Return(nullptr)); EXPECT_THROW( ClientSslAuthConfig::create(proto_config, tls_, cm_, dispatcher_, stats_store_, random_), EnvoyException); diff --git a/test/extensions/filters/network/dubbo_proxy/router_test.cc b/test/extensions/filters/network/dubbo_proxy/router_test.cc index a5fec366c810..6376a8a96166 100644 --- a/test/extensions/filters/network/dubbo_proxy/router_test.cc +++ b/test/extensions/filters/network/dubbo_proxy/router_test.cc @@ -13,6 +13,7 @@ using testing::_; using testing::ContainsRegex; +using testing::Eq; using testing::InSequence; using testing::Invoke; using testing::NiceMock; @@ -354,7 +355,7 @@ TEST_F(DubboRouterTest, NoCluster) { EXPECT_CALL(callbacks_, route()).WillOnce(Return(route_ptr_)); EXPECT_CALL(*route_, routeEntry()).WillOnce(Return(&route_entry_)); EXPECT_CALL(route_entry_, clusterName()).WillRepeatedly(ReturnRef(cluster_name_)); - EXPECT_CALL(context_.cluster_manager_, get(cluster_name_)).WillOnce(Return(nullptr)); + EXPECT_CALL(context_.cluster_manager_, get(Eq(cluster_name_))).WillOnce(Return(nullptr)); EXPECT_CALL(callbacks_, sendLocalReply(_, _)) .WillOnce(Invoke([&](const DubboFilters::DirectResponse& response, bool end_stream) -> void { auto& app_ex = dynamic_cast(response); diff --git a/test/extensions/filters/network/redis_proxy/conn_pool_impl_test.cc b/test/extensions/filters/network/redis_proxy/conn_pool_impl_test.cc index ab8fc0a16a3e..5ecb9e9aed53 100644 --- a/test/extensions/filters/network/redis_proxy/conn_pool_impl_test.cc +++ b/test/extensions/filters/network/redis_proxy/conn_pool_impl_test.cc @@ -45,7 +45,7 @@ class RedisConnPoolImplTest : public testing::Test, public Common::Redis::Client .WillOnce(DoAll(SaveArgAddress(&update_callbacks_), ReturnNew())); if (!cluster_exists) { - EXPECT_CALL(cm_, get("fake_cluster")).WillOnce(Return(nullptr)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillOnce(Return(nullptr)); } std::unique_ptr conn_pool_impl = std::make_unique( diff --git a/test/extensions/filters/network/thrift_proxy/router_test.cc b/test/extensions/filters/network/thrift_proxy/router_test.cc index dcc72d52780d..ad4686746b4b 100644 --- a/test/extensions/filters/network/thrift_proxy/router_test.cc +++ b/test/extensions/filters/network/thrift_proxy/router_test.cc @@ -23,6 +23,7 @@ using testing::_; using testing::ContainsRegex; +using testing::Eq; using testing::InSequence; using testing::Invoke; using testing::NiceMock; @@ -441,7 +442,7 @@ TEST_F(ThriftRouterTest, NoCluster) { EXPECT_CALL(callbacks_, route()).WillOnce(Return(route_ptr_)); EXPECT_CALL(*route_, routeEntry()).WillOnce(Return(&route_entry_)); EXPECT_CALL(route_entry_, clusterName()).WillRepeatedly(ReturnRef(cluster_name_)); - EXPECT_CALL(context_.cluster_manager_, get(cluster_name_)).WillOnce(Return(nullptr)); + EXPECT_CALL(context_.cluster_manager_, get(Eq(cluster_name_))).WillOnce(Return(nullptr)); EXPECT_CALL(callbacks_, sendLocalReply(_, _)) .WillOnce(Invoke([&](const DirectResponse& response, bool end_stream) -> void { auto& app_ex = dynamic_cast(response); diff --git a/test/extensions/tracers/datadog/config_test.cc b/test/extensions/tracers/datadog/config_test.cc index 82a551181fdb..363be39bba76 100644 --- a/test/extensions/tracers/datadog/config_test.cc +++ b/test/extensions/tracers/datadog/config_test.cc @@ -6,6 +6,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::NiceMock; using testing::Return; @@ -17,7 +18,7 @@ namespace { TEST(DatadogTracerConfigTest, DatadogHttpTracer) { NiceMock server; - EXPECT_CALL(server.cluster_manager_, get("fake_cluster")) + EXPECT_CALL(server.cluster_manager_, get(Eq("fake_cluster"))) .WillRepeatedly(Return(&server.cluster_manager_.thread_local_cluster_)); ON_CALL(*server.cluster_manager_.thread_local_cluster_.cluster_.info_, features()) .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2)); diff --git a/test/extensions/tracers/datadog/datadog_tracer_impl_test.cc b/test/extensions/tracers/datadog/datadog_tracer_impl_test.cc index 76229e3f1f7e..1870d31c4a12 100644 --- a/test/extensions/tracers/datadog/datadog_tracer_impl_test.cc +++ b/test/extensions/tracers/datadog/datadog_tracer_impl_test.cc @@ -28,6 +28,7 @@ using testing::_; using testing::AtLeast; +using testing::Eq; using testing::Invoke; using testing::NiceMock; using testing::Return; @@ -54,7 +55,7 @@ class DatadogDriverTest : public testing::Test { } void setupValidDriver() { - EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillRepeatedly(Return(&cm_.thread_local_cluster_)); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features()) .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2)); @@ -94,7 +95,7 @@ TEST_F(DatadogDriverTest, InitializeDriver) { { // Valid config but not valid cluster. - EXPECT_CALL(cm_, get("fake_cluster")).WillOnce(Return(nullptr)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillOnce(Return(nullptr)); const std::string yaml_string = R"EOF( collector_cluster: fake_cluster @@ -106,7 +107,7 @@ TEST_F(DatadogDriverTest, InitializeDriver) { } { - EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillRepeatedly(Return(&cm_.thread_local_cluster_)); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features()) .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2)); diff --git a/test/extensions/tracers/dynamic_ot/config_test.cc b/test/extensions/tracers/dynamic_ot/config_test.cc index 00e685a736e6..825f8b0aae70 100644 --- a/test/extensions/tracers/dynamic_ot/config_test.cc +++ b/test/extensions/tracers/dynamic_ot/config_test.cc @@ -8,6 +8,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::NiceMock; using testing::Return; @@ -19,7 +20,7 @@ namespace { TEST(DynamicOtTracerConfigTest, DynamicOpentracingHttpTracer) { NiceMock server; - EXPECT_CALL(server.cluster_manager_, get("fake_cluster")) + EXPECT_CALL(server.cluster_manager_, get(Eq("fake_cluster"))) .WillRepeatedly(Return(&server.cluster_manager_.thread_local_cluster_)); ON_CALL(*server.cluster_manager_.thread_local_cluster_.cluster_.info_, features()) .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2)); diff --git a/test/extensions/tracers/lightstep/config_test.cc b/test/extensions/tracers/lightstep/config_test.cc index 55345a1aaf90..c0a681183cd1 100644 --- a/test/extensions/tracers/lightstep/config_test.cc +++ b/test/extensions/tracers/lightstep/config_test.cc @@ -6,6 +6,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::NiceMock; using testing::Return; @@ -17,7 +18,7 @@ namespace { TEST(LightstepTracerConfigTest, LightstepHttpTracer) { NiceMock server; - EXPECT_CALL(server.cluster_manager_, get("fake_cluster")) + EXPECT_CALL(server.cluster_manager_, get(Eq("fake_cluster"))) .WillRepeatedly(Return(&server.cluster_manager_.thread_local_cluster_)); ON_CALL(*server.cluster_manager_.thread_local_cluster_.cluster_.info_, features()) .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2)); diff --git a/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc b/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc index 2f42fcb203b7..a43f9da6be4c 100644 --- a/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc +++ b/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc @@ -29,6 +29,7 @@ using testing::_; using testing::AtLeast; +using testing::Eq; using testing::Invoke; using testing::NiceMock; using testing::Return; @@ -64,7 +65,7 @@ class LightStepDriverTest : public testing::Test { void setupValidDriver(Common::Ot::OpenTracingDriver::PropagationMode propagation_mode = Common::Ot::OpenTracingDriver::PropagationMode::TracerNative) { - EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillRepeatedly(Return(&cm_.thread_local_cluster_)); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features()) .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2)); @@ -114,7 +115,7 @@ TEST_F(LightStepDriverTest, InitializeDriver) { { // Valid config but not valid cluster. - EXPECT_CALL(cm_, get("fake_cluster")).WillOnce(Return(nullptr)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillOnce(Return(nullptr)); const std::string yaml_string = R"EOF( collector_cluster: fake_cluster @@ -127,7 +128,7 @@ TEST_F(LightStepDriverTest, InitializeDriver) { { // Valid config, but upstream cluster does not support http2. - EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillRepeatedly(Return(&cm_.thread_local_cluster_)); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features()).WillByDefault(Return(0)); const std::string yaml_string = R"EOF( @@ -140,7 +141,7 @@ TEST_F(LightStepDriverTest, InitializeDriver) { } { - EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillRepeatedly(Return(&cm_.thread_local_cluster_)); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features()) .WillByDefault(Return(Upstream::ClusterInfo::Features::HTTP2)); diff --git a/test/extensions/tracers/zipkin/config_test.cc b/test/extensions/tracers/zipkin/config_test.cc index b710dc7e609b..b62865dd2601 100644 --- a/test/extensions/tracers/zipkin/config_test.cc +++ b/test/extensions/tracers/zipkin/config_test.cc @@ -7,6 +7,8 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +using ::testing::Eq; + namespace Envoy { namespace Extensions { namespace Tracers { @@ -15,7 +17,7 @@ namespace { TEST(ZipkinTracerConfigTest, ZipkinHttpTracer) { NiceMock server; - EXPECT_CALL(server.cluster_manager_, get("fake_cluster")) + EXPECT_CALL(server.cluster_manager_, get(Eq("fake_cluster"))) .WillRepeatedly(Return(&server.cluster_manager_.thread_local_cluster_)); const std::string yaml_string = R"EOF( @@ -38,7 +40,7 @@ TEST(ZipkinTracerConfigTest, ZipkinHttpTracer) { TEST(ZipkinTracerConfigTest, ZipkinHttpTracerWithTypedConfig) { NiceMock server; - EXPECT_CALL(server.cluster_manager_, get("fake_cluster")) + EXPECT_CALL(server.cluster_manager_, get(Eq("fake_cluster"))) .WillRepeatedly(Return(&server.cluster_manager_.thread_local_cluster_)); const std::string yaml_string = R"EOF( diff --git a/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc b/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc index 8ea8fd1e370b..5810778974f0 100644 --- a/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc +++ b/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc @@ -27,6 +27,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::Invoke; using testing::NiceMock; using testing::Return; @@ -56,7 +57,7 @@ class ZipkinDriverTest : public testing::Test { } void setupValidDriver() { - EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillRepeatedly(Return(&cm_.thread_local_cluster_)); const std::string yaml_string = R"EOF( collector_cluster: fake_cluster @@ -104,7 +105,7 @@ TEST_F(ZipkinDriverTest, InitializeDriver) { { // Valid config but not valid cluster. - EXPECT_CALL(cm_, get("fake_cluster")).WillOnce(Return(nullptr)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillOnce(Return(nullptr)); const std::string yaml_string = R"EOF( collector_cluster: fake_cluster collector_endpoint: /api/v1/spans @@ -117,7 +118,7 @@ TEST_F(ZipkinDriverTest, InitializeDriver) { { // valid config - EXPECT_CALL(cm_, get("fake_cluster")).WillRepeatedly(Return(&cm_.thread_local_cluster_)); + EXPECT_CALL(cm_, get(Eq("fake_cluster"))).WillRepeatedly(Return(&cm_.thread_local_cluster_)); ON_CALL(*cm_.thread_local_cluster_.cluster_.info_, features()).WillByDefault(Return(0)); const std::string yaml_string = R"EOF( diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index 8d44835fa8ab..a8e7ebdfc96b 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -745,17 +745,9 @@ TEST_F(ClientContextConfigImplTest, TlsCertificatesAndSdsConfig) { // downloaded. TEST_F(ClientContextConfigImplTest, SecretNotReady) { envoy::api::v2::auth::UpstreamTlsContext tls_context; - NiceMock local_info; - NiceMock dispatcher; - NiceMock random; Stats::IsolatedStoreImpl stats; - NiceMock cluster_manager; NiceMock init_manager; - EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); - EXPECT_CALL(factory_context_, dispatcher()).WillOnce(ReturnRef(dispatcher)); - EXPECT_CALL(factory_context_, random()).WillOnce(ReturnRef(random)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(factory_context_, clusterManager()).WillOnce(ReturnRef(cluster_manager)); EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_tls_certificate_sds_secret_configs()->Add(); @@ -781,17 +773,9 @@ TEST_F(ClientContextConfigImplTest, ValidationContextNotReady) { "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/selfsigned_cert.pem")); client_cert->mutable_private_key()->set_filename(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/selfsigned_key.pem")); - NiceMock local_info; - NiceMock dispatcher; - NiceMock random; Stats::IsolatedStoreImpl stats; - NiceMock cluster_manager; NiceMock init_manager; - EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); - EXPECT_CALL(factory_context_, dispatcher()).WillOnce(ReturnRef(dispatcher)); - EXPECT_CALL(factory_context_, random()).WillOnce(ReturnRef(random)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(factory_context_, clusterManager()).WillOnce(ReturnRef(cluster_manager)); EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_validation_context_sds_secret_config(); @@ -1088,17 +1072,9 @@ TEST_F(ServerContextConfigImplTest, MultiSdsConfig) { TEST_F(ServerContextConfigImplTest, SecretNotReady) { envoy::api::v2::auth::DownstreamTlsContext tls_context; - NiceMock local_info; - NiceMock dispatcher; - NiceMock random; Stats::IsolatedStoreImpl stats; - NiceMock cluster_manager; NiceMock init_manager; - EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); - EXPECT_CALL(factory_context_, dispatcher()).WillOnce(ReturnRef(dispatcher)); - EXPECT_CALL(factory_context_, random()).WillOnce(ReturnRef(random)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(factory_context_, clusterManager()).WillOnce(ReturnRef(cluster_manager)); EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_tls_certificate_sds_secret_configs()->Add(); @@ -1124,17 +1100,9 @@ TEST_F(ServerContextConfigImplTest, ValidationContextNotReady) { "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/selfsigned_cert.pem")); server_cert->mutable_private_key()->set_filename(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/selfsigned_key.pem")); - NiceMock local_info; - NiceMock dispatcher; - NiceMock random; Stats::IsolatedStoreImpl stats; - NiceMock cluster_manager; NiceMock init_manager; - EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); - EXPECT_CALL(factory_context_, dispatcher()).WillOnce(ReturnRef(dispatcher)); - EXPECT_CALL(factory_context_, random()).WillOnce(ReturnRef(random)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(factory_context_, clusterManager()).WillOnce(ReturnRef(cluster_manager)); EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_validation_context_sds_secret_config(); diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 36e259259b7b..fb858a80af41 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -3708,16 +3708,8 @@ TEST_P(SslSocketTest, OverrideRequestedServerNameWithoutSniInUpstreamTlsContext) TEST_P(SslSocketTest, DownstreamNotReadySslSocket) { Stats::IsolatedStoreImpl stats_store; testing::NiceMock factory_context; - NiceMock local_info; - NiceMock dispatcher; - NiceMock random; - NiceMock cluster_manager; NiceMock init_manager; - EXPECT_CALL(factory_context, localInfo()).WillOnce(ReturnRef(local_info)); - EXPECT_CALL(factory_context, dispatcher()).WillOnce(ReturnRef(dispatcher)); - EXPECT_CALL(factory_context, random()).WillOnce(ReturnRef(random)); EXPECT_CALL(factory_context, stats()).WillOnce(ReturnRef(stats_store)); - EXPECT_CALL(factory_context, clusterManager()).WillOnce(ReturnRef(cluster_manager)); EXPECT_CALL(factory_context, initManager()).WillRepeatedly(Return(&init_manager)); envoy::api::v2::auth::DownstreamTlsContext tls_context; @@ -3748,16 +3740,8 @@ TEST_P(SslSocketTest, DownstreamNotReadySslSocket) { TEST_P(SslSocketTest, UpstreamNotReadySslSocket) { Stats::IsolatedStoreImpl stats_store; testing::NiceMock factory_context; - NiceMock local_info; - NiceMock dispatcher; - NiceMock random; - NiceMock cluster_manager; NiceMock init_manager; - EXPECT_CALL(factory_context, localInfo()).WillOnce(ReturnRef(local_info)); - EXPECT_CALL(factory_context, dispatcher()).WillOnce(ReturnRef(dispatcher)); - EXPECT_CALL(factory_context, random()).WillOnce(ReturnRef(random)); EXPECT_CALL(factory_context, stats()).WillOnce(ReturnRef(stats_store)); - EXPECT_CALL(factory_context, clusterManager()).WillOnce(ReturnRef(cluster_manager)); EXPECT_CALL(factory_context, initManager()).WillRepeatedly(Return(&init_manager)); envoy::api::v2::auth::UpstreamTlsContext tls_context; diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index ca639b8dfb07..ab92cbcb73ec 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -213,8 +213,9 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithStats) { // 2019/05/07 6794 49957 Stats for excluded hosts in cluster // 2019/04/27 6733 50213 Use SymbolTable API for HTTP codes // 2019/05/31 6866 50157 libstdc++ upgrade in CI + // 2019/06/07 50240 Subscription factory - EXPECT_EQ(m_per_cluster, 50157); + EXPECT_EQ(m_per_cluster, 50240); } } // namespace diff --git a/test/mocks/config/mocks.cc b/test/mocks/config/mocks.cc index 1c6e4b7d32ff..f1e4fb6947b3 100644 --- a/test/mocks/config/mocks.cc +++ b/test/mocks/config/mocks.cc @@ -9,6 +9,22 @@ namespace Envoy { namespace Config { +MockSubscriptionFactory::MockSubscriptionFactory() { + ON_CALL(*this, subscriptionFromConfigSource(_, _, _, _)) + .WillByDefault(testing::Invoke([this](const envoy::api::v2::core::ConfigSource&, + absl::string_view, Stats::Scope&, + SubscriptionCallbacks& callbacks) -> SubscriptionPtr { + auto ret = std::make_unique>(); + subscription_ = ret.get(); + callbacks_ = &callbacks; + return ret; + })); + ON_CALL(*this, messageValidationVisitor()) + .WillByDefault(testing::ReturnRef(ProtobufMessage::getStrictValidationVisitor())); +} + +MockSubscriptionFactory::~MockSubscriptionFactory() {} + MockGrpcMuxWatch::MockGrpcMuxWatch() {} MockGrpcMuxWatch::~MockGrpcMuxWatch() { cancel(); } diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index 7c37573a764f..4143aaa490c7 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -47,6 +47,21 @@ class MockSubscription : public Subscription { MOCK_METHOD1(updateResources, void(const std::set& update_to_these_names)); }; +class MockSubscriptionFactory : public SubscriptionFactory { +public: + MockSubscriptionFactory(); + ~MockSubscriptionFactory(); + + MOCK_METHOD4(subscriptionFromConfigSource, + SubscriptionPtr(const envoy::api::v2::core::ConfigSource& config, + absl::string_view type_url, Stats::Scope& scope, + SubscriptionCallbacks& callbacks)); + MOCK_METHOD0(messageValidationVisitor, ProtobufMessage::ValidationVisitor&()); + + MockSubscription* subscription_{}; + SubscriptionCallbacks* callbacks_{}; +}; + class MockGrpcMuxWatch : public GrpcMuxWatch { public: MockGrpcMuxWatch(); diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 01789c5f1719..59f342352be2 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -198,6 +198,7 @@ MockFactoryContext::~MockFactoryContext() = default; MockTransportSocketFactoryContext::MockTransportSocketFactoryContext() : secret_manager_(new Secret::SecretManagerImpl()) { + ON_CALL(*this, clusterManager()).WillByDefault(ReturnRef(cluster_manager_)); ON_CALL(*this, api()).WillByDefault(ReturnRef(api_)); ON_CALL(*this, messageValidationVisitor()) .WillByDefault(ReturnRef(ProtobufMessage::getStrictValidationVisitor())); diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 4d9cd6e966a3..93d6573fcb89 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -501,6 +501,7 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext { MOCK_METHOD0(messageValidationVisitor, ProtobufMessage::ValidationVisitor&()); MOCK_METHOD0(api, Api::Api&()); + testing::NiceMock cluster_manager_; std::unique_ptr secret_manager_; testing::NiceMock api_; }; diff --git a/test/mocks/upstream/mocks.cc b/test/mocks/upstream/mocks.cc index 35f96382ac94..05785eead37b 100644 --- a/test/mocks/upstream/mocks.cc +++ b/test/mocks/upstream/mocks.cc @@ -9,6 +9,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::Invoke; using testing::Return; using testing::ReturnPointee; @@ -145,7 +146,8 @@ MockClusterManager::MockClusterManager() { // Matches are LIFO so "" will match first. ON_CALL(*this, get(_)).WillByDefault(Return(&thread_local_cluster_)); - ON_CALL(*this, get("")).WillByDefault(Return(nullptr)); + ON_CALL(*this, get(Eq(""))).WillByDefault(Return(nullptr)); + ON_CALL(*this, subscriptionFactory()).WillByDefault(ReturnRef(subscription_factory_)); } MockClusterManager::~MockClusterManager() = default; diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index a5f152b4d3f7..00ae7bd017b1 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -304,7 +304,7 @@ class MockClusterManager : public ClusterManager { ClusterWarmingCallback cluster_warming_cb)); MOCK_METHOD1(setInitializedCb, void(std::function)); MOCK_METHOD0(clusters, ClusterInfoMap()); - MOCK_METHOD1(get, ThreadLocalCluster*(const std::string& cluster)); + MOCK_METHOD1(get, ThreadLocalCluster*(absl::string_view cluster)); MOCK_METHOD4(httpConnPoolForCluster, Http::ConnectionPool::Instance*(const std::string& cluster, ResourcePriority priority, Http::Protocol protocol, @@ -328,6 +328,7 @@ class MockClusterManager : public ClusterManager { MOCK_METHOD1(addThreadLocalClusterUpdateCallbacks_, ClusterUpdateCallbacksHandle*(ClusterUpdateCallbacks& callbacks)); MOCK_CONST_METHOD0(warmingClusterCount, std::size_t()); + MOCK_METHOD0(subscriptionFactory, Config::SubscriptionFactory&()); NiceMock conn_pool_; NiceMock async_client_; @@ -338,6 +339,7 @@ class MockClusterManager : public ClusterManager { NiceMock async_client_manager_; std::string local_cluster_name_; NiceMock cluster_manager_factory_; + NiceMock subscription_factory_; }; class MockHealthChecker : public HealthChecker { diff --git a/test/server/BUILD b/test/server/BUILD index f3e2eee2d743..4cfa5be816ee 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -149,6 +149,7 @@ envoy_cc_test( deps = [ "//source/common/protobuf:utility_lib", "//source/server:lds_api_lib", + "//test/mocks/config:config_mocks", "//test/mocks/protobuf:protobuf_mocks", "//test/mocks/server:server_mocks", "//test/test_common:environment_lib", diff --git a/test/server/lds_api_test.cc b/test/server/lds_api_test.cc index 99e0046aa418..efbd1c5f8f24 100644 --- a/test/server/lds_api_test.cc +++ b/test/server/lds_api_test.cc @@ -2,11 +2,11 @@ #include "envoy/api/v2/lds.pb.h" -#include "common/http/message_impl.h" #include "common/protobuf/utility.h" #include "server/lds_api.h" +#include "test/mocks/config/mocks.h" #include "test/mocks/protobuf/mocks.h" #include "test/mocks/server/mocks.h" #include "test/test_common/environment.h" @@ -27,44 +27,20 @@ namespace { class LdsApiTest : public testing::Test { public: - LdsApiTest() - : request_(&cluster_manager_.async_client_), api_(Api::createApiForTest(store_)), - lds_version_( - store_.gauge("listener_manager.lds.version", Stats::Gauge::ImportMode::NeverImport)) { + LdsApiTest() { ON_CALL(init_manager_, add(_)).WillByDefault(Invoke([this](const Init::Target& target) { init_target_handle_ = target.createHandle("test"); })); } void setup() { - const std::string config_yaml = R"EOF( -api_config_source: - api_type: REST - cluster_names: - - foo_cluster - refresh_delay: 1s - )EOF"; - envoy::api::v2::core::ConfigSource lds_config; - TestUtility::loadFromYaml(config_yaml, lds_config); - lds_config.mutable_api_config_source()->set_api_type( - envoy::api::v2::core::ApiConfigSource::REST); - Upstream::ClusterManager::ClusterInfoMap cluster_map; - Upstream::MockClusterMockPrioritySet cluster; - cluster_map.emplace("foo_cluster", cluster); - EXPECT_CALL(cluster_manager_, clusters()).WillOnce(Return(cluster_map)); - EXPECT_CALL(cluster, info()); - EXPECT_CALL(*cluster.info_, addedViaApi()); - EXPECT_CALL(cluster, info()); - EXPECT_CALL(*cluster.info_, type()); - interval_timer_ = new Event::MockTimer(&dispatcher_); EXPECT_CALL(init_manager_, add(_)); - lds_ = std::make_unique(lds_config, cluster_manager_, dispatcher_, random_, - init_manager_, local_info_, store_, listener_manager_, - validation_visitor_, *api_); - - expectRequest(); + lds_ = std::make_unique(lds_config, cluster_manager_, init_manager_, store_, + listener_manager_, validation_visitor_); + EXPECT_CALL(*cluster_manager_.subscription_factory_.subscription_, start(_)); init_target_handle_->initialize(init_watcher_); + lds_callbacks_ = cluster_manager_.subscription_factory_.callbacks_; } void expectAdd(const std::string& listener_name, absl::optional version, @@ -86,26 +62,6 @@ class LdsApiTest : public testing::Test { } } - void expectRequest() { - EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("foo_cluster")); - EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) - .WillOnce( - Invoke([&](Http::MessagePtr& request, Http::AsyncClient::Callbacks& callbacks, - const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { - EXPECT_EQ( - (Http::TestHeaderMapImpl{ - {":method", "POST"}, - {":path", "/v2/discovery:listeners"}, - {":authority", "foo_cluster"}, - {"content-type", "application/json"}, - {"content-length", - request->body() ? fmt::format_int(request->body()->length()).str() : "0"}}), - request->headers()); - callbacks_ = &callbacks; - return &request_; - })); - } - void makeListenersAndExpectCall(const std::vector& listener_names) { std::vector> refs; listeners_.clear(); @@ -129,21 +85,14 @@ class LdsApiTest : public testing::Test { } NiceMock cluster_manager_; - Event::MockDispatcher dispatcher_; - NiceMock random_; Init::MockManager init_manager_; Init::ExpectableWatcherImpl init_watcher_; Init::TargetHandlePtr init_target_handle_; - NiceMock local_info_; Stats::IsolatedStoreImpl store_; MockListenerManager listener_manager_; - Http::MockAsyncClientRequest request_; + Config::SubscriptionCallbacks* lds_callbacks_{}; std::unique_ptr lds_; - Event::MockTimer* interval_timer_{}; - Http::AsyncClient::Callbacks* callbacks_{}; NiceMock validation_visitor_; - Api::ApiPtr api_; - Stats::Gauge& lds_version_; private: std::list> listeners_; @@ -162,30 +111,7 @@ TEST_F(LdsApiTest, ValidateFail) { EXPECT_CALL(listener_manager_, listeners()).WillOnce(Return(existing_listeners)); EXPECT_CALL(init_watcher_, ready()); - EXPECT_THROW(lds_->onConfigUpdate(listeners, ""), EnvoyException); - EXPECT_CALL(request_, cancel()); -} - -TEST_F(LdsApiTest, UnknownCluster) { - const std::string config_yaml = R"EOF( -api_config_source: - api_type: REST - cluster_names: - - foo_cluster - refresh_delay: 1s - )EOF"; - - envoy::api::v2::core::ConfigSource lds_config; - TestUtility::loadFromYaml(config_yaml, lds_config); - Upstream::ClusterManager::ClusterInfoMap cluster_map; - EXPECT_CALL(cluster_manager_, clusters()).WillOnce(Return(cluster_map)); - EXPECT_THROW_WITH_MESSAGE( - LdsApiImpl(lds_config, cluster_manager_, dispatcher_, random_, init_manager_, local_info_, - store_, listener_manager_, validation_visitor_, *api_), - EnvoyException, - "envoy::api::v2::core::ConfigSource must have a statically defined non-EDS " - "cluster: 'foo_cluster' does not exist, was added via api, or is an " - "EDS cluster"); + EXPECT_THROW(lds_callbacks_->onConfigUpdate(listeners, ""), EnvoyException); } TEST_F(LdsApiTest, MisconfiguredListenerNameIsPresentInException) { @@ -212,9 +138,8 @@ TEST_F(LdsApiTest, MisconfiguredListenerNameIsPresentInException) { listeners.Add()->PackFrom(listener); EXPECT_THROW_WITH_MESSAGE( - lds_->onConfigUpdate(listeners, ""), EnvoyException, + lds_callbacks_->onConfigUpdate(listeners, ""), EnvoyException, "Error adding/updating listener(s) invalid-listener: something is wrong"); - EXPECT_CALL(request_, cancel()); } TEST_F(LdsApiTest, EmptyListenersUpdate) { @@ -226,11 +151,9 @@ TEST_F(LdsApiTest, EmptyListenersUpdate) { std::vector> existing_listeners; EXPECT_CALL(listener_manager_, listeners()).WillOnce(Return(existing_listeners)); - EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(request_, cancel()); - lds_->onConfigUpdate(listeners, ""); + lds_callbacks_->onConfigUpdate(listeners, ""); } TEST_F(LdsApiTest, ListenerCreationContinuesEvenAfterException) { @@ -257,10 +180,9 @@ TEST_F(LdsApiTest, ListenerCreationContinuesEvenAfterException) { EXPECT_CALL(init_watcher_, ready()); - EXPECT_THROW_WITH_MESSAGE(lds_->onConfigUpdate(listeners, ""), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(lds_callbacks_->onConfigUpdate(listeners, ""), EnvoyException, "Error adding/updating listener(s) invalid-listener-1: something is " "wrong, invalid-listener-2: something else is wrong"); - EXPECT_CALL(request_, cancel()); } // Validate onConfigUpdate throws EnvoyException with duplicate listeners. @@ -280,38 +202,9 @@ TEST_F(LdsApiTest, ValidateDuplicateListeners) { EXPECT_CALL(listener_manager_, addOrUpdateListener(_, _, true)).WillOnce(Return(true)); EXPECT_CALL(init_watcher_, ready()); - EXPECT_THROW_WITH_MESSAGE(lds_->onConfigUpdate(listeners, ""), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(lds_callbacks_->onConfigUpdate(listeners, ""), EnvoyException, "Error adding/updating listener(s) duplicate_listener: duplicate " "listener duplicate_listener found"); - EXPECT_CALL(request_, cancel()); -} - -TEST_F(LdsApiTest, BadLocalInfo) { - interval_timer_ = new Event::MockTimer(&dispatcher_); - const std::string config_yaml = R"EOF( -api_config_source: - api_type: REST - cluster_names: - - foo_cluster - refresh_delay: 1s - )EOF"; - - envoy::api::v2::core::ConfigSource lds_config; - TestUtility::loadFromYaml(config_yaml, lds_config); - Upstream::ClusterManager::ClusterInfoMap cluster_map; - Upstream::MockClusterMockPrioritySet cluster; - cluster_map.emplace("foo_cluster", cluster); - EXPECT_CALL(cluster_manager_, clusters()).WillOnce(Return(cluster_map)); - EXPECT_CALL(cluster, info()).Times(2); - EXPECT_CALL(*cluster.info_, addedViaApi()); - EXPECT_CALL(*cluster.info_, type()); - ON_CALL(local_info_, clusterName()).WillByDefault(ReturnRef(EMPTY_STRING)); - EXPECT_THROW_WITH_MESSAGE( - LdsApiImpl(lds_config, cluster_manager_, dispatcher_, random_, init_manager_, local_info_, - store_, listener_manager_, validation_visitor_, *api_), - EnvoyException, - "lds: node 'id' and 'cluster' are required. Set it either in 'node' config or via " - "--service-node and --service-cluster options."); } TEST_F(LdsApiTest, Basic) { @@ -338,22 +231,15 @@ TEST_F(LdsApiTest, Basic) { ] } )EOF"; - - Http::MessagePtr message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - message->body() = std::make_unique(response1_json); + auto response1 = TestUtility::parseYaml(response1_json); makeListenersAndExpectCall({}); expectAdd("listener1", "0", true); expectAdd("listener2", "0", true); EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + lds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); EXPECT_EQ("0", lds_->versionInfo()); - EXPECT_EQ(7148434200721666028U, lds_version_.value()); - expectRequest(); - interval_timer_->callback_(); const std::string response2_json = R"EOF( { @@ -374,22 +260,14 @@ TEST_F(LdsApiTest, Basic) { ] } )EOF"; - - message = std::make_unique( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}); - message->body() = std::make_unique(response2_json); + auto response2 = TestUtility::parseYaml(response2_json); makeListenersAndExpectCall({"listener1", "listener2"}); EXPECT_CALL(listener_manager_, removeListener("listener2")).WillOnce(Return(true)); expectAdd("listener1", "1", false); expectAdd("listener3", "1", true); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + lds_callbacks_->onConfigUpdate(response2.resources(), response2.version_info()); EXPECT_EQ("1", lds_->versionInfo()); - - EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_attempt").value()); - EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_success").value()); - EXPECT_EQ(13237225503670494420U, lds_version_.value()); } // Regression test against only updating versionInfo() if at least one listener @@ -412,20 +290,14 @@ TEST_F(LdsApiTest, UpdateVersionOnListenerRemove) { ] } )EOF"; - - Http::MessagePtr message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - message->body() = std::make_unique(response1_json); + auto response1 = TestUtility::parseYaml(response1_json); makeListenersAndExpectCall({}); expectAdd("listener1", "0", true); EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + lds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); EXPECT_EQ("0", lds_->versionInfo()); - expectRequest(); - interval_timer_->callback_(); const std::string response2_json = R"EOF( { @@ -433,22 +305,12 @@ TEST_F(LdsApiTest, UpdateVersionOnListenerRemove) { "resources": [] } )EOF"; - - message = std::make_unique( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}); - message->body() = std::make_unique(response2_json); + auto response2 = TestUtility::parseYaml(response2_json); makeListenersAndExpectCall({"listener1"}); EXPECT_CALL(listener_manager_, removeListener("listener1")).WillOnce(Return(true)); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + lds_callbacks_->onConfigUpdate(response2.resources(), response2.version_info()); EXPECT_EQ("1", lds_->versionInfo()); - - EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_attempt").value()); - EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_success").value()); - EXPECT_EQ( - 13237225503670494420U, - store_.gauge("listener_manager.lds.version", Stats::Gauge::ImportMode::NeverImport).value()); } // Regression test issue #2188 where an empty ca_cert_file field was created and caused the LDS @@ -471,19 +333,12 @@ TEST_F(LdsApiTest, TlsConfigWithoutCaCert) { ] } )EOF"; - - Http::MessagePtr message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - message->body() = std::make_unique(response1_json); + auto response1 = TestUtility::parseYaml(response1_json); makeListenersAndExpectCall({"listener0"}); expectAdd("listener0", {}, true); EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); - - expectRequest(); - interval_timer_->callback_(); + lds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); std::string response2_basic = R"EOF( {{ @@ -511,26 +366,24 @@ TEST_F(LdsApiTest, TlsConfigWithoutCaCert) { fmt::format(response2_basic, TestEnvironment::runfilesPath("/test/config/integration/certs/servercert.pem"), TestEnvironment::runfilesPath("/test/config/integration/certs/serverkey.pem")); + auto response2 = TestUtility::parseYaml(response2_json); - message = std::make_unique( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}); - message->body() = std::make_unique(response2_json); makeListenersAndExpectCall({ "listener-8080", }); // Can't check version here because of bazel sandbox paths for the certs. expectAdd("listener-8080", {}, true); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - EXPECT_NO_THROW(callbacks_->onSuccess(std::move(message))); + EXPECT_NO_THROW(lds_callbacks_->onConfigUpdate(response2.resources(), response2.version_info())); } -TEST_F(LdsApiTest, Failure) { +// Validate behavior when the config is delivered but it fails PGV validation. +TEST_F(LdsApiTest, FailureInvalidConfig) { InSequence s; setup(); // To test the case of valid JSON with invalid config, create a listener with no address. - const std::string response_json = R"EOF( + const std::string response1_json = R"EOF( { "version_info": "1", "resources": [ @@ -542,30 +395,25 @@ TEST_F(LdsApiTest, Failure) { ] } )EOF"; - - Http::MessagePtr message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - message->body() = std::make_unique(response_json); + auto response1 = TestUtility::parseYaml(response1_json); std::vector> existing_listeners; EXPECT_CALL(listener_manager_, listeners()).WillOnce(Return(existing_listeners)); - EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + EXPECT_THROW(lds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()), + EnvoyException); + EXPECT_EQ("", lds_->versionInfo()); +} - expectRequest(); - interval_timer_->callback_(); +// Validate behavior when the config fails delivery at the subscription level. +TEST_F(LdsApiTest, FailureSubscription) { + InSequence s; - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onFailure(Http::AsyncClient::FailureReason::Reset); - EXPECT_EQ("", lds_->versionInfo()); + setup(); - EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_attempt").value()); - EXPECT_EQ(1UL, store_.counter("listener_manager.lds.update_failure").value()); - // Validate that the schema error increments update_rejected stat. - EXPECT_EQ(1UL, store_.counter("listener_manager.lds.update_failure").value()); - EXPECT_EQ(0UL, lds_version_.value()); + EXPECT_CALL(init_watcher_, ready()); + lds_callbacks_->onConfigUpdateFailed({}); + EXPECT_EQ("", lds_->versionInfo()); } TEST_F(LdsApiTest, ReplacingListenerWithSameAddress) { @@ -592,22 +440,15 @@ TEST_F(LdsApiTest, ReplacingListenerWithSameAddress) { ] } )EOF"; - - Http::MessagePtr message(new Http::ResponseMessageImpl( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); - message->body() = std::make_unique(response1_json); + auto response1 = TestUtility::parseYaml(response1_json); makeListenersAndExpectCall({}); expectAdd("listener1", "0", true); expectAdd("listener2", "0", true); EXPECT_CALL(init_watcher_, ready()); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); + lds_callbacks_->onConfigUpdate(response1.resources(), response1.version_info()); EXPECT_EQ("0", lds_->versionInfo()); - EXPECT_EQ(7148434200721666028U, lds_version_.value()); - expectRequest(); - interval_timer_->callback_(); const std::string response2_json = R"EOF( { @@ -628,22 +469,13 @@ TEST_F(LdsApiTest, ReplacingListenerWithSameAddress) { ] } )EOF"; - - message = std::make_unique( - Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}}); - message->body() = std::make_unique(response2_json); + auto response2 = TestUtility::parseYaml(response2_json); makeListenersAndExpectCall({"listener1", "listener2"}); EXPECT_CALL(listener_manager_, removeListener("listener2")).WillOnce(Return(true)); expectAdd("listener1", "1", false); expectAdd("listener3", "1", true); - EXPECT_CALL(*interval_timer_, enableTimer(_)); - callbacks_->onSuccess(std::move(message)); - - EXPECT_EQ("1", lds_->versionInfo()); - EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_attempt").value()); - EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_success").value()); - EXPECT_EQ(13237225503670494420U, lds_version_.value()); + lds_callbacks_->onConfigUpdate(response2.resources(), response2.version_info()); } } // namespace