From 662ebe43006442c66eb75201ea705ba2c419e800 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 5 Sep 2019 17:20:43 -0700 Subject: [PATCH 01/11] Refactor redis auth call to redis client factory. Make the redis health checker also call auth if auth password is configured. Signed-off-by: Henry Yang --- include/envoy/server/health_checker_config.h | 5 + .../common/upstream/cluster_factory_impl.cc | 3 +- source/common/upstream/health_checker_impl.cc | 21 ++-- source/common/upstream/health_checker_impl.h | 12 +-- .../upstream/health_discovery_service.cc | 10 +- .../upstream/health_discovery_service.h | 3 +- .../clusters/redis/redis_cluster.cc | 10 +- .../filters/network/common/redis/BUILD | 1 + .../filters/network/common/redis/client.h | 9 +- .../network/common/redis/client_impl.cc | 26 ++++- .../network/common/redis/client_impl.h | 4 +- .../filters/network/redis_proxy/BUILD | 1 + .../filters/network/redis_proxy/config.h | 12 +++ .../network/redis_proxy/conn_pool_impl.cc | 25 +---- source/extensions/health_checkers/redis/BUILD | 1 + .../health_checkers/redis/config.cc | 2 +- .../extensions/health_checkers/redis/redis.cc | 9 +- .../extensions/health_checkers/redis/redis.h | 5 +- test/common/upstream/BUILD | 1 + .../upstream/health_checker_impl_test.cc | 15 +-- test/extensions/clusters/redis/BUILD | 1 - .../clusters/redis/redis_cluster_test.cc | 3 +- .../network/common/redis/client_impl_test.cc | 71 +++++++++++++- .../filters/network/common/redis/mocks.h | 1 + .../redis_proxy/conn_pool_impl_test.cc | 98 +------------------ test/extensions/health_checkers/redis/BUILD | 2 + .../health_checkers/redis/redis_test.cc | 16 +-- 27 files changed, 194 insertions(+), 173 deletions(-) diff --git a/include/envoy/server/health_checker_config.h b/include/envoy/server/health_checker_config.h index 163a1d7c2801..638a0362adcc 100644 --- a/include/envoy/server/health_checker_config.h +++ b/include/envoy/server/health_checker_config.h @@ -43,6 +43,11 @@ class HealthCheckerFactoryContext { * messages. */ virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE; + + /** + * @return Api::Api& the API used by the server. + */ + virtual Api::Api& api() PURE; }; /** diff --git a/source/common/upstream/cluster_factory_impl.cc b/source/common/upstream/cluster_factory_impl.cc index 43c6d5581977..bebf10b93326 100644 --- a/source/common/upstream/cluster_factory_impl.cc +++ b/source/common/upstream/cluster_factory_impl.cc @@ -109,7 +109,8 @@ ClusterFactoryImplBase::create(const envoy::api::v2::Cluster& cluster, } else { new_cluster_pair.first->setHealthChecker(HealthCheckerFactory::create( cluster.health_checks()[0], *new_cluster_pair.first, context.runtime(), context.random(), - context.dispatcher(), context.logManager(), context.messageValidationVisitor())); + context.dispatcher(), context.logManager(), context.messageValidationVisitor(), + context.api())); } } diff --git a/source/common/upstream/health_checker_impl.cc b/source/common/upstream/health_checker_impl.cc index 708f5e132981..ddd52756eaee 100644 --- a/source/common/upstream/health_checker_impl.cc +++ b/source/common/upstream/health_checker_impl.cc @@ -29,9 +29,11 @@ class HealthCheckerFactoryContextImpl : public Server::Configuration::HealthChec Envoy::Runtime::RandomGenerator& random, Event::Dispatcher& dispatcher, HealthCheckEventLoggerPtr&& event_logger, - ProtobufMessage::ValidationVisitor& validation_visitor) + ProtobufMessage::ValidationVisitor& validation_visitor, + Api::Api& api) : cluster_(cluster), runtime_(runtime), random_(random), dispatcher_(dispatcher), - event_logger_(std::move(event_logger)), validation_visitor_(validation_visitor) {} + event_logger_(std::move(event_logger)), validation_visitor_(validation_visitor), api_(api) { + } Upstream::Cluster& cluster() override { return cluster_; } Envoy::Runtime::Loader& runtime() override { return runtime_; } Envoy::Runtime::RandomGenerator& random() override { return random_; } @@ -40,6 +42,7 @@ class HealthCheckerFactoryContextImpl : public Server::Configuration::HealthChec ProtobufMessage::ValidationVisitor& messageValidationVisitor() override { return validation_visitor_; } + Api::Api& api() override { return api_; } private: Upstream::Cluster& cluster_; @@ -48,14 +51,14 @@ class HealthCheckerFactoryContextImpl : public Server::Configuration::HealthChec Event::Dispatcher& dispatcher_; HealthCheckEventLoggerPtr event_logger_; ProtobufMessage::ValidationVisitor& validation_visitor_; + Api::Api& api_; }; -HealthCheckerSharedPtr -HealthCheckerFactory::create(const envoy::api::v2::core::HealthCheck& health_check_config, - Upstream::Cluster& cluster, Runtime::Loader& runtime, - Runtime::RandomGenerator& random, Event::Dispatcher& dispatcher, - AccessLog::AccessLogManager& log_manager, - ProtobufMessage::ValidationVisitor& validation_visitor) { +HealthCheckerSharedPtr HealthCheckerFactory::create( + const envoy::api::v2::core::HealthCheck& health_check_config, Upstream::Cluster& cluster, + Runtime::Loader& runtime, Runtime::RandomGenerator& random, Event::Dispatcher& dispatcher, + AccessLog::AccessLogManager& log_manager, + ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) { HealthCheckEventLoggerPtr event_logger; if (!health_check_config.event_log_path().empty()) { event_logger = std::make_unique( @@ -81,7 +84,7 @@ HealthCheckerFactory::create(const envoy::api::v2::core::HealthCheck& health_che health_check_config.custom_health_check().name()); std::unique_ptr context( new HealthCheckerFactoryContextImpl(cluster, runtime, random, dispatcher, - std::move(event_logger), validation_visitor)); + std::move(event_logger), validation_visitor, api)); return factory.createCustomHealthChecker(health_check_config, *context); } default: diff --git a/source/common/upstream/health_checker_impl.h b/source/common/upstream/health_checker_impl.h index 6ca106d947b9..c16a4393df24 100644 --- a/source/common/upstream/health_checker_impl.h +++ b/source/common/upstream/health_checker_impl.h @@ -11,6 +11,7 @@ #include "common/stream_info/stream_info_impl.h" #include "common/upstream/health_checker_base_impl.h" +#include "include/envoy/api/_virtual_includes/api_interface/envoy/api/api.h" #include "src/proto/grpc/health/v1/health.pb.h" namespace Envoy { @@ -32,12 +33,11 @@ class HealthCheckerFactory : public Logger::Loggable * @param validation_visitor message validation visitor instance. * @return a health checker. */ - static HealthCheckerSharedPtr create(const envoy::api::v2::core::HealthCheck& health_check_config, - Upstream::Cluster& cluster, Runtime::Loader& runtime, - Runtime::RandomGenerator& random, - Event::Dispatcher& dispatcher, - AccessLog::AccessLogManager& log_manager, - ProtobufMessage::ValidationVisitor& validation_visitor); + static HealthCheckerSharedPtr + create(const envoy::api::v2::core::HealthCheck& health_check_config, Upstream::Cluster& cluster, + Runtime::Loader& runtime, Runtime::RandomGenerator& random, Event::Dispatcher& dispatcher, + AccessLog::AccessLogManager& log_manager, + ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); }; /** diff --git a/source/common/upstream/health_discovery_service.cc b/source/common/upstream/health_discovery_service.cc index 8fb6f44b1f15..894be7a4c9b6 100644 --- a/source/common/upstream/health_discovery_service.cc +++ b/source/common/upstream/health_discovery_service.cc @@ -152,7 +152,8 @@ void HdsDelegate::processMessage( info_factory_, cm_, local_info_, dispatcher_, random_, singleton_manager_, tls_, validation_visitor_, api_)); - hds_clusters_.back()->startHealthchecks(access_log_manager_, runtime_, random_, dispatcher_); + hds_clusters_.back()->startHealthchecks(access_log_manager_, runtime_, random_, dispatcher_, + api_); } } @@ -243,10 +244,11 @@ ProdClusterInfoFactory::createClusterInfo(const CreateClusterInfoParams& params) void HdsCluster::startHealthchecks(AccessLog::AccessLogManager& access_log_manager, Runtime::Loader& runtime, Runtime::RandomGenerator& random, - Event::Dispatcher& dispatcher) { + Event::Dispatcher& dispatcher, Api::Api& api) { for (auto& health_check : cluster_.health_checks()) { - health_checkers_.push_back(Upstream::HealthCheckerFactory::create( - health_check, *this, runtime, random, dispatcher, access_log_manager, validation_visitor_)); + health_checkers_.push_back( + Upstream::HealthCheckerFactory::create(health_check, *this, runtime, random, dispatcher, + access_log_manager, validation_visitor_, api)); health_checkers_.back()->start(); } } diff --git a/source/common/upstream/health_discovery_service.h b/source/common/upstream/health_discovery_service.h index b1c889a30c23..250b915896b4 100644 --- a/source/common/upstream/health_discovery_service.h +++ b/source/common/upstream/health_discovery_service.h @@ -59,7 +59,8 @@ class HdsCluster : public Cluster, Logger::Loggable { // Creates and starts healthcheckers to its endpoints void startHealthchecks(AccessLog::AccessLogManager& access_log_manager, Runtime::Loader& runtime, - Runtime::RandomGenerator& random, Event::Dispatcher& dispatcher); + Runtime::RandomGenerator& random, Event::Dispatcher& dispatcher, + Api::Api& api); std::vector healthCheckers() { return health_checkers_; }; diff --git a/source/extensions/clusters/redis/redis_cluster.cc b/source/extensions/clusters/redis/redis_cluster.cc index dc41651b0ca1..0e9c3a24888f 100644 --- a/source/extensions/clusters/redis/redis_cluster.cc +++ b/source/extensions/clusters/redis/redis_cluster.cc @@ -249,16 +249,10 @@ void RedisCluster::RedisDiscoverySession::startResolveRedis() { if (!client) { client = std::make_unique(*this); client->host_ = current_host_address_; - client->client_ = client_factory_.create(host, dispatcher_, *this); - client->client_->addConnectionCallbacks(*client); std::string auth_password = Envoy::Config::DataSource::read(parent_.auth_password_datasource_, true, parent_.api_); - if (!auth_password.empty()) { - // Send an AUTH command to the upstream server. - client->client_->makeRequest( - Extensions::NetworkFilters::Common::Redis::Utility::makeAuthCommand(auth_password), - null_pool_callbacks); - } + client->client_ = client_factory_.create(host, dispatcher_, *this, auth_password); + client->client_->addConnectionCallbacks(*client); } current_request_ = client->client_->makeRequest(ClusterSlotsRequest::instance_, *this); diff --git a/source/extensions/filters/network/common/redis/BUILD b/source/extensions/filters/network/common/redis/BUILD index 1757c2a7b9b4..55a6a8bcef6e 100644 --- a/source/extensions/filters/network/common/redis/BUILD +++ b/source/extensions/filters/network/common/redis/BUILD @@ -57,6 +57,7 @@ envoy_cc_library( deps = [ ":client_interface", ":codec_lib", + ":utility_lib", "//include/envoy/router:router_interface", "//include/envoy/thread_local:thread_local_interface", "//include/envoy/upstream:cluster_manager_interface", diff --git a/source/extensions/filters/network/common/redis/client.h b/source/extensions/filters/network/common/redis/client.h index e20df148fb82..28c3d26733f5 100644 --- a/source/extensions/filters/network/common/redis/client.h +++ b/source/extensions/filters/network/common/redis/client.h @@ -95,6 +95,12 @@ class Client : public Event::DeferredDeletable { * for some reason. */ virtual PoolRequest* makeRequest(const RespValue& request, PoolCallbacks& callbacks) PURE; + + /** + * Initialize the connection. Issue the auth command and readonly command as needed. + * @param auth password for upstream host. + */ + virtual void initialize(const std::string& auth_password) PURE; }; using ClientPtr = std::unique_ptr; @@ -181,10 +187,11 @@ class ClientFactory { * @param host supplies the upstream host. * @param dispatcher supplies the owning thread's dispatcher. * @param config supplies the connection pool configuration. + * @param auth password for upstream host. * @return ClientPtr a new connection pool client. */ virtual ClientPtr create(Upstream::HostConstSharedPtr host, Event::Dispatcher& dispatcher, - const Config& config) PURE; + const Config& config, const std::string& auth_password) PURE; }; } // namespace Client diff --git a/source/extensions/filters/network/common/redis/client_impl.cc b/source/extensions/filters/network/common/redis/client_impl.cc index 6f941d223ca0..622143780e24 100644 --- a/source/extensions/filters/network/common/redis/client_impl.cc +++ b/source/extensions/filters/network/common/redis/client_impl.cc @@ -6,6 +6,9 @@ namespace NetworkFilters { namespace Common { namespace Redis { namespace Client { +namespace { +Common::Redis::Client::DoNothingPoolCallbacks null_pool_callbacks; +} // namespace ConfigImpl::ConfigImpl( const envoy::config::filter::network::redis_proxy::v2::RedisProxy::ConnPoolSettings& config) @@ -49,7 +52,6 @@ ConfigImpl::ConfigImpl( ClientPtr ClientImpl::create(Upstream::HostConstSharedPtr host, Event::Dispatcher& dispatcher, EncoderPtr&& encoder, DecoderFactory& decoder_factory, const Config& config) { - std::unique_ptr client( new ClientImpl(host, dispatcher, std::move(encoder), decoder_factory, config)); client->connection_ = host->createConnection(dispatcher, nullptr, nullptr).connection_; @@ -245,12 +247,28 @@ void ClientImpl::PendingRequest::cancel() { canceled_ = true; } +void ClientImpl::initialize(const std::string& auth_password) { + if (!auth_password.empty()) { + // Send an AUTH command to the upstream server. + makeRequest(Utility::makeAuthCommand(auth_password), null_pool_callbacks); + } + // Any connection to replica requires the READONLY command in order to perform read. + // Also the READONLY command is a no-opt for the master. + // We only need to send the READONLY command iff it's possible that the host is a replica. + if (config_.readPolicy() != Common::Redis::Client::ReadPolicy::Master) { + makeRequest(Utility::ReadOnlyRequest::instance(), null_pool_callbacks); + } +} + ClientFactoryImpl ClientFactoryImpl::instance_; ClientPtr ClientFactoryImpl::create(Upstream::HostConstSharedPtr host, - Event::Dispatcher& dispatcher, const Config& config) { - return ClientImpl::create(host, dispatcher, EncoderPtr{new EncoderImpl()}, decoder_factory_, - config); + Event::Dispatcher& dispatcher, const Config& config, + const std::string& auth_password) { + ClientPtr client = + ClientImpl::create(host, dispatcher, EncoderPtr{new EncoderImpl()}, decoder_factory_, config); + client->initialize(auth_password); + return client; } } // namespace Client diff --git a/source/extensions/filters/network/common/redis/client_impl.h b/source/extensions/filters/network/common/redis/client_impl.h index 9522482fb022..17e4b2e94aba 100644 --- a/source/extensions/filters/network/common/redis/client_impl.h +++ b/source/extensions/filters/network/common/redis/client_impl.h @@ -15,6 +15,7 @@ #include "common/upstream/upstream_impl.h" #include "extensions/filters/network/common/redis/client.h" +#include "extensions/filters/network/common/redis/utility.h" namespace Envoy { namespace Extensions { @@ -77,6 +78,7 @@ class ClientImpl : public Client, public DecoderCallbacks, public Network::Conne PoolRequest* makeRequest(const RespValue& request, PoolCallbacks& callbacks) override; bool active() override { return !pending_requests_.empty(); } void flushBufferAndResetTimer(); + void initialize(const std::string& auth_password) override; private: friend class RedisClientImplTest; @@ -135,7 +137,7 @@ class ClientFactoryImpl : public ClientFactory { public: // RedisProxy::ConnPool::ClientFactoryImpl ClientPtr create(Upstream::HostConstSharedPtr host, Event::Dispatcher& dispatcher, - const Config& config) override; + const Config& config, const std::string& auth_password) override; static ClientFactoryImpl instance_; diff --git a/source/extensions/filters/network/redis_proxy/BUILD b/source/extensions/filters/network/redis_proxy/BUILD index bbc18dff95d2..255d453fe484 100644 --- a/source/extensions/filters/network/redis_proxy/BUILD +++ b/source/extensions/filters/network/redis_proxy/BUILD @@ -116,6 +116,7 @@ envoy_cc_library( hdrs = ["config.h"], deps = [ "//include/envoy/registry", + "//include/envoy/upstream:upstream_interface", "//source/common/config:filter_json_lib", "//source/extensions/filters/network:well_known_names", "//source/extensions/filters/network/common:factory_base_lib", diff --git a/source/extensions/filters/network/redis_proxy/config.h b/source/extensions/filters/network/redis_proxy/config.h index 37d6a7c0203c..30bf184f527b 100644 --- a/source/extensions/filters/network/redis_proxy/config.h +++ b/source/extensions/filters/network/redis_proxy/config.h @@ -2,9 +2,12 @@ #include +#include "envoy/api/api.h" #include "envoy/config/filter/network/redis_proxy/v2/redis_proxy.pb.h" #include "envoy/config/filter/network/redis_proxy/v2/redis_proxy.pb.validate.h" +#include "envoy/upstream/upstream.h" +#include "common/common/empty_string.h" #include "common/config/datasource.h" #include "extensions/filters/network/common/factory_base.h" @@ -29,6 +32,15 @@ class ProtocolOptionsConfigImpl : public Upstream::ProtocolOptionsConfig { return auth_password_; } + static const std::string auth_password(Upstream::ClusterInfoConstSharedPtr info, Api::Api& api) { + auto options = info->extensionProtocolOptionsTyped( + NetworkFilterNames::get().RedisProxy); + if (options) { + return options->auth_password(api); + } + return EMPTY_STRING; + } + private: envoy::api::v2::core::DataSource auth_password_; }; diff --git a/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc b/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc index a097924ea734..38dfe3bc28e0 100644 --- a/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc +++ b/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc @@ -15,9 +15,6 @@ namespace Extensions { namespace NetworkFilters { namespace RedisProxy { namespace ConnPool { -namespace { -Common::Redis::Client::DoNothingPoolCallbacks null_pool_callbacks; -} // namespace InstanceImpl::InstanceImpl( const std::string& cluster_name, Upstream::ClusterManager& cm, @@ -54,11 +51,7 @@ InstanceImpl::ThreadLocalPool::ThreadLocalPool(InstanceImpl& parent, Event::Disp cluster_update_handle_ = parent_.cm_.addThreadLocalClusterUpdateCallbacks(*this); Upstream::ThreadLocalCluster* cluster = parent_.cm_.get(cluster_name_); if (cluster != nullptr) { - auto options = cluster->info()->extensionProtocolOptionsTyped( - NetworkFilterNames::get().RedisProxy); - if (options) { - auth_password_ = options->auth_password(parent_.api_); - } + auth_password_ = ProtocolOptionsConfigImpl::auth_password(cluster->info(), parent_.api_); onClusterAddOrUpdateNonVirtual(*cluster); } } @@ -195,21 +188,9 @@ InstanceImpl::ThreadLocalPool::threadLocalActiveClient(Upstream::HostConstShared if (!client) { client = std::make_unique(*this); client->host_ = host; - client->redis_client_ = parent_.client_factory_.create(host, dispatcher_, parent_.config_); + client->redis_client_ = + parent_.client_factory_.create(host, dispatcher_, parent_.config_, auth_password_); client->redis_client_->addConnectionCallbacks(*client); - // TODO(hyang): should the auth command and readonly command be moved to the factory method? - if (!auth_password_.empty()) { - // Send an AUTH command to the upstream server. - client->redis_client_->makeRequest(Common::Redis::Utility::makeAuthCommand(auth_password_), - null_pool_callbacks); - } - // Any connection to replica requires the READONLY command in order to perform read. - // Also the READONLY command is a no-opt for the master. - // We only need to send the READONLY command iff it's possible that the host is a replica. - if (parent_.config_.readPolicy() != Common::Redis::Client::ReadPolicy::Master) { - client->redis_client_->makeRequest(Common::Redis::Utility::ReadOnlyRequest::instance(), - null_pool_callbacks); - } } return client; } diff --git a/source/extensions/health_checkers/redis/BUILD b/source/extensions/health_checkers/redis/BUILD index 1c92f366295c..5c3c7bdffe8b 100644 --- a/source/extensions/health_checkers/redis/BUILD +++ b/source/extensions/health_checkers/redis/BUILD @@ -17,6 +17,7 @@ envoy_cc_library( deps = [ "//source/common/upstream:health_checker_base_lib", "//source/extensions/filters/network/common/redis:client_lib", + "//source/extensions/filters/network/redis_proxy:config", "//source/extensions/filters/network/redis_proxy:conn_pool_lib", "@envoy_api//envoy/api/v2/core:health_check_cc", "@envoy_api//envoy/config/health_checker/redis/v2:redis_cc", diff --git a/source/extensions/health_checkers/redis/config.cc b/source/extensions/health_checkers/redis/config.cc index 6892781ea8dd..a3bdb6eab6de 100644 --- a/source/extensions/health_checkers/redis/config.cc +++ b/source/extensions/health_checkers/redis/config.cc @@ -17,7 +17,7 @@ Upstream::HealthCheckerSharedPtr RedisHealthCheckerFactory::createCustomHealthCh return std::make_shared( context.cluster(), config, getRedisHealthCheckConfig(config, context.messageValidationVisitor()), context.dispatcher(), - context.runtime(), context.random(), context.eventLogger(), + context.runtime(), context.random(), context.eventLogger(), context.api(), NetworkFilters::Common::Redis::Client::ClientFactoryImpl::instance_); }; diff --git a/source/extensions/health_checkers/redis/redis.cc b/source/extensions/health_checkers/redis/redis.cc index 9f130824639c..cea3c36fa82d 100644 --- a/source/extensions/health_checkers/redis/redis.cc +++ b/source/extensions/health_checkers/redis/redis.cc @@ -9,10 +9,12 @@ RedisHealthChecker::RedisHealthChecker( const Upstream::Cluster& cluster, const envoy::api::v2::core::HealthCheck& config, const envoy::config::health_checker::redis::v2::Redis& redis_config, Event::Dispatcher& dispatcher, Runtime::Loader& runtime, Runtime::RandomGenerator& random, - Upstream::HealthCheckEventLoggerPtr&& event_logger, + Upstream::HealthCheckEventLoggerPtr&& event_logger, Api::Api& api, Extensions::NetworkFilters::Common::Redis::Client::ClientFactory& client_factory) : HealthCheckerImplBase(cluster, config, dispatcher, runtime, random, std::move(event_logger)), - client_factory_(client_factory), key_(redis_config.key()) { + client_factory_(client_factory), key_(redis_config.key()), + auth_password_(NetworkFilters::RedisProxy::ProtocolOptionsConfigImpl::auth_password( + cluster.info(), api)) { if (!key_.empty()) { type_ = Type::Exists; } else { @@ -51,7 +53,8 @@ void RedisHealthChecker::RedisActiveHealthCheckSession::onEvent(Network::Connect void RedisHealthChecker::RedisActiveHealthCheckSession::onInterval() { if (!client_) { - client_ = parent_.client_factory_.create(host_, parent_.dispatcher_, *this); + client_ = + parent_.client_factory_.create(host_, parent_.dispatcher_, *this, parent_.auth_password_); client_->addConnectionCallbacks(*this); } diff --git a/source/extensions/health_checkers/redis/redis.h b/source/extensions/health_checkers/redis/redis.h index 4c7cc320e01f..18c11e6918e4 100644 --- a/source/extensions/health_checkers/redis/redis.h +++ b/source/extensions/health_checkers/redis/redis.h @@ -2,11 +2,13 @@ #include +#include "envoy/api/api.h" #include "envoy/config/health_checker/redis/v2/redis.pb.validate.h" #include "common/upstream/health_checker_base_impl.h" #include "extensions/filters/network/common/redis/client_impl.h" +#include "extensions/filters/network/redis_proxy/config.h" #include "extensions/filters/network/redis_proxy/conn_pool_impl.h" namespace Envoy { @@ -23,7 +25,7 @@ class RedisHealthChecker : public Upstream::HealthCheckerImplBase { const Upstream::Cluster& cluster, const envoy::api::v2::core::HealthCheck& config, const envoy::config::health_checker::redis::v2::Redis& redis_config, Event::Dispatcher& dispatcher, Runtime::Loader& runtime, Runtime::RandomGenerator& random, - Upstream::HealthCheckEventLoggerPtr&& event_logger, + Upstream::HealthCheckEventLoggerPtr&& event_logger, Api::Api& api, Extensions::NetworkFilters::Common::Redis::Client::ClientFactory& client_factory); static const NetworkFilters::Common::Redis::RespValue& pingHealthCheckRequest() { @@ -116,6 +118,7 @@ class RedisHealthChecker : public Upstream::HealthCheckerImplBase { Extensions::NetworkFilters::Common::Redis::Client::ClientFactory& client_factory_; Type type_; const std::string key_; + const std::string auth_password_; }; } // namespace RedisHealthChecker diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 4e5d8b4679ba..ee43445582b8 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -119,6 +119,7 @@ envoy_cc_test( "//source/common/upstream:upstream_lib", "//test/common/http:common_lib", "//test/mocks/access_log:access_log_mocks", + "//test/mocks/api:api_mocks", "//test/mocks/network:network_mocks", "//test/mocks/protobuf:protobuf_mocks", "//test/mocks/runtime:runtime_mocks", diff --git a/test/common/upstream/health_checker_impl_test.cc b/test/common/upstream/health_checker_impl_test.cc index 4753cf9da7be..c9cecab00675 100644 --- a/test/common/upstream/health_checker_impl_test.cc +++ b/test/common/upstream/health_checker_impl_test.cc @@ -18,6 +18,7 @@ #include "test/common/http/common.h" #include "test/common/upstream/utility.h" #include "test/mocks/access_log/mocks.h" +#include "test/mocks/api/mocks.h" #include "test/mocks/network/mocks.h" #include "test/mocks/protobuf/mocks.h" #include "test/mocks/runtime/mocks.h" @@ -64,10 +65,11 @@ TEST(HealthCheckerFactoryTest, GrpcHealthCheckHTTP2NotConfiguredException) { Event::MockDispatcher dispatcher; AccessLog::MockAccessLogManager log_manager; NiceMock validation_visitor; + Api::MockApi api; EXPECT_THROW_WITH_MESSAGE( HealthCheckerFactory::create(createGrpcHealthCheckConfig(), cluster, runtime, random, - dispatcher, log_manager, validation_visitor), + dispatcher, log_manager, validation_visitor, api), EnvoyException, "fake_cluster cluster must support HTTP/2 for gRPC healthchecking"); } @@ -82,12 +84,13 @@ TEST(HealthCheckerFactoryTest, CreateGrpc) { Event::MockDispatcher dispatcher; AccessLog::MockAccessLogManager log_manager; NiceMock validation_visitor; + Api::MockApi api; - EXPECT_NE(nullptr, - dynamic_cast( - HealthCheckerFactory::create(createGrpcHealthCheckConfig(), cluster, runtime, - random, dispatcher, log_manager, validation_visitor) - .get())); + EXPECT_NE(nullptr, dynamic_cast( + HealthCheckerFactory::create(createGrpcHealthCheckConfig(), cluster, + runtime, random, dispatcher, log_manager, + validation_visitor, api) + .get())); } class TestHttpHealthCheckerImpl : public HttpHealthCheckerImpl { diff --git a/test/extensions/clusters/redis/BUILD b/test/extensions/clusters/redis/BUILD index 25495a9d7d95..7c39f26b53d6 100644 --- a/test/extensions/clusters/redis/BUILD +++ b/test/extensions/clusters/redis/BUILD @@ -82,7 +82,6 @@ envoy_extension_cc_test( ) envoy_extension_cc_test( - name = "redis_cluster_integration_test", size = "small", srcs = ["redis_cluster_integration_test.cc"], extension_name = "envoy.clusters.redis", diff --git a/test/extensions/clusters/redis/redis_cluster_test.cc b/test/extensions/clusters/redis/redis_cluster_test.cc index 39f23ef1be78..8d8e6b750da6 100644 --- a/test/extensions/clusters/redis/redis_cluster_test.cc +++ b/test/extensions/clusters/redis/redis_cluster_test.cc @@ -63,7 +63,8 @@ class RedisClusterTest : public testing::Test, // ClientFactory Extensions::NetworkFilters::Common::Redis::Client::ClientPtr create(Upstream::HostConstSharedPtr host, Event::Dispatcher&, - const Extensions::NetworkFilters::Common::Redis::Client::Config&) override { + const Extensions::NetworkFilters::Common::Redis::Client::Config&, + const std::string&) override { EXPECT_EQ(22120, host->address()->ip()->port()); return Extensions::NetworkFilters::Common::Redis::Client::ClientPtr{ create_(host->address()->asString())}; diff --git a/test/extensions/filters/network/common/redis/client_impl_test.cc b/test/extensions/filters/network/common/redis/client_impl_test.cc index a6646c86741a..3b19dd49e7df 100644 --- a/test/extensions/filters/network/common/redis/client_impl_test.cc +++ b/test/extensions/filters/network/common/redis/client_impl_test.cc @@ -5,6 +5,7 @@ #include "common/upstream/upstream_impl.h" #include "extensions/filters/network/common/redis/client_impl.h" +#include "extensions/filters/network/common/redis/utility.h" #include "test/extensions/filters/network/common/redis/mocks.h" #include "test/extensions/filters/network/common/redis/test_utils.h" @@ -99,6 +100,28 @@ class RedisClientImplTest : public testing::Test, public Common::Redis::DecoderF client_impl->onRespValue(std::move(response1)); } + void testInitializeReadPolicy( + envoy::config::filter::network::redis_proxy::v2::RedisProxy::ConnPoolSettings::ReadPolicy + read_policy) { + InSequence s; + + setup(std::make_unique(createConnPoolSettings(20, true, true, 100, read_policy))); + + Common::Redis::RespValue readonly_request = Utility::ReadOnlyRequest::instance(); + EXPECT_CALL(*encoder_, encode(Eq(readonly_request), _)); + EXPECT_CALL(*flush_timer_, enabled()).WillOnce(Return(false)); + client_->initialize(auth_password_); + + EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_total_.value()); + EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(1UL, host_->stats_.rq_total_.value()); + EXPECT_EQ(1UL, host_->stats_.rq_active_.value()); + + EXPECT_CALL(*upstream_connection_, close(Network::ConnectionCloseType::NoFlush)); + EXPECT_CALL(*connect_or_op_timer_, disableTimer()); + client_->close(); + } + const std::string cluster_name_{"foo"}; std::shared_ptr host_{new NiceMock()}; Event::MockDispatcher dispatcher_; @@ -111,6 +134,7 @@ class RedisClientImplTest : public testing::Test, public Common::Redis::DecoderF Network::ReadFilterSharedPtr upstream_read_filter_; std::unique_ptr config_; ClientPtr client_; + std::string auth_password_; }; TEST_F(RedisClientImplTest, BatchWithZeroBufferAndTimeout) { @@ -256,6 +280,8 @@ TEST_F(RedisClientImplTest, Basic) { setup(); + client_->initialize(auth_password_); + Common::Redis::RespValue request1; MockPoolCallbacks callbacks1; EXPECT_CALL(*encoder_, encode(Ref(request1), _)); @@ -301,6 +327,47 @@ TEST_F(RedisClientImplTest, Basic) { client_->close(); } +TEST_F(RedisClientImplTest, InitializedWithAuthPassword) { + InSequence s; + + setup(); + + auth_password_ = "testing password"; + Common::Redis::RespValue auth_request = Utility::makeAuthCommand(auth_password_); + EXPECT_CALL(*encoder_, encode(Eq(auth_request), _)); + EXPECT_CALL(*flush_timer_, enabled()).WillOnce(Return(false)); + client_->initialize(auth_password_); + + EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_total_.value()); + EXPECT_EQ(1UL, host_->cluster_.stats_.upstream_rq_active_.value()); + EXPECT_EQ(1UL, host_->stats_.rq_total_.value()); + EXPECT_EQ(1UL, host_->stats_.rq_active_.value()); + + EXPECT_CALL(*upstream_connection_, close(Network::ConnectionCloseType::NoFlush)); + EXPECT_CALL(*connect_or_op_timer_, disableTimer()); + client_->close(); +} + +TEST_F(RedisClientImplTest, InitializedWithPreferMasterReadPolicy) { + testInitializeReadPolicy(envoy::config::filter::network::redis_proxy::v2:: + RedisProxy_ConnPoolSettings_ReadPolicy_PREFER_MASTER); +} + +TEST_F(RedisClientImplTest, InitializedWithReplicaReadPolicy) { + testInitializeReadPolicy(envoy::config::filter::network::redis_proxy::v2:: + RedisProxy_ConnPoolSettings_ReadPolicy_REPLICA); +} + +TEST_F(RedisClientImplTest, InitializedWithPreferReplicaReadPolicy) { + testInitializeReadPolicy(envoy::config::filter::network::redis_proxy::v2:: + RedisProxy_ConnPoolSettings_ReadPolicy_PREFER_REPLICA); +} + +TEST_F(RedisClientImplTest, InitializedWithAnyReadPolicy) { + testInitializeReadPolicy( + envoy::config::filter::network::redis_proxy::v2::RedisProxy_ConnPoolSettings_ReadPolicy_ANY); +} + TEST_F(RedisClientImplTest, Cancel) { InSequence s; @@ -875,10 +942,10 @@ TEST(RedisClientFactoryImplTest, Basic) { EXPECT_CALL(*host, createConnection_(_, _)).WillOnce(Return(conn_info)); NiceMock dispatcher; ConfigImpl config(createConnPoolSettings()); - ClientPtr client = factory.create(host, dispatcher, config); + const std::string auth_password; + ClientPtr client = factory.create(host, dispatcher, config, auth_password); client->close(); } - } // namespace Client } // namespace Redis } // namespace Common diff --git a/test/extensions/filters/network/common/redis/mocks.h b/test/extensions/filters/network/common/redis/mocks.h index 859ea03cf42b..a44e41ef63e9 100644 --- a/test/extensions/filters/network/common/redis/mocks.h +++ b/test/extensions/filters/network/common/redis/mocks.h @@ -73,6 +73,7 @@ class MockClient : public Client { MOCK_METHOD0(close, void()); MOCK_METHOD2(makeRequest, PoolRequest*(const Common::Redis::RespValue& request, PoolCallbacks& callbacks)); + MOCK_METHOD1(initialize, void(const std::string& password)); std::list callbacks_; }; 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 9dd249ec6002..a68414c43c26 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 @@ -94,9 +94,6 @@ class RedisConnPoolImplTest : public testing::Test, public Common::Redis::Client Common::Redis::RespValue value; Common::Redis::Client::MockPoolCallbacks callbacks; Common::Redis::Client::MockPoolRequest active_request; - if (create_client && !auth_password_.empty()) { - EXPECT_CALL(*client_, makeRequest(_, _)).WillOnce(Return(nullptr)); - } EXPECT_CALL(*cm_.thread_local_cluster_.lb_.host_, address()) .WillRepeatedly(Return(test_address_)); EXPECT_CALL(*client_, makeRequest(Ref(value), Ref(callbacks))) @@ -155,7 +152,9 @@ class RedisConnPoolImplTest : public testing::Test, public Common::Redis::Client // Common::Redis::Client::ClientFactory Common::Redis::Client::ClientPtr create(Upstream::HostConstSharedPtr host, Event::Dispatcher&, - const Common::Redis::Client::Config&) override { + const Common::Redis::Client::Config&, + const std::string& password) override { + EXPECT_EQ(auth_password_, password); return Common::Redis::Client::ClientPtr{create_(host)}; } @@ -185,10 +184,6 @@ class RedisConnPoolImplTest : public testing::Test, public Common::Redis::Client return cm_.thread_local_cluster_.lb_.host_; })); EXPECT_CALL(*this, create_(_)).WillOnce(Return(client)); - EXPECT_CALL( - *client, - makeRequest(Eq(NetworkFilters::Common::Redis::Utility::ReadOnlyRequest::instance()), _)) - .WillOnce(Return(&readonly_request)); EXPECT_CALL(*cm_.thread_local_cluster_.lb_.host_, address()) .WillRepeatedly(Return(test_address_)); EXPECT_CALL(*client, makeRequest(Ref(value), Ref(callbacks))).WillOnce(Return(&active_request)); @@ -247,40 +242,6 @@ TEST_F(RedisConnPoolImplTest, Basic) { tls_.shutdownThread(); }; -TEST_F(RedisConnPoolImplTest, BasicWithAuthPassword) { - InSequence s; - - auth_password_ = "testing password"; - setup(); - - Common::Redis::RespValue value; - Common::Redis::Client::MockPoolRequest auth_request, active_request; - Common::Redis::Client::MockPoolCallbacks callbacks; - Common::Redis::Client::MockClient* client = new NiceMock(); - - EXPECT_CALL(cm_.thread_local_cluster_.lb_, chooseHost(_)) - .WillOnce(Invoke([&](Upstream::LoadBalancerContext* context) -> Upstream::HostConstSharedPtr { - EXPECT_EQ(context->computeHashKey().value(), MurmurHash::murmurHash2_64("hash_key")); - EXPECT_EQ(context->metadataMatchCriteria(), nullptr); - EXPECT_EQ(context->downstreamConnection(), nullptr); - return cm_.thread_local_cluster_.lb_.host_; - })); - EXPECT_CALL(*this, create_(_)).WillOnce(Return(client)); - EXPECT_CALL( - *client, - makeRequest(Eq(NetworkFilters::Common::Redis::Utility::makeAuthCommand(auth_password_)), _)) - .WillOnce(Return(&auth_request)); - EXPECT_CALL(*cm_.thread_local_cluster_.lb_.host_, address()) - .WillRepeatedly(Return(test_address_)); - EXPECT_CALL(*client, makeRequest(Ref(value), Ref(callbacks))).WillOnce(Return(&active_request)); - Common::Redis::Client::PoolRequest* request = - conn_pool_->makeRequest("hash_key", value, callbacks); - EXPECT_EQ(&active_request, request); - - EXPECT_CALL(*client, close()); - tls_.shutdownThread(); -}; - TEST_F(RedisConnPoolImplTest, BasicWithReadPolicy) { testReadPolicy(envoy::config::filter::network::redis_proxy::v2:: RedisProxy_ConnPoolSettings_ReadPolicy_PREFER_MASTER, @@ -598,59 +559,6 @@ TEST_F(RedisConnPoolImplTest, MakeRequestToHostWithZeroMaxUnknownUpstreamConnect tls_.shutdownThread(); } -TEST_F(RedisConnPoolImplTest, MakeRequestToHostWithAuthPassword) { - InSequence s; - - auth_password_ = "superduperpassword"; - setup(false); - - Common::Redis::RespValue value; - Common::Redis::Client::MockPoolRequest auth_request1, active_request1; - Common::Redis::Client::MockPoolRequest auth_request2, active_request2; - Common::Redis::Client::MockPoolCallbacks callbacks1; - Common::Redis::Client::MockPoolCallbacks callbacks2; - Common::Redis::Client::MockClient* client1 = new NiceMock(); - Common::Redis::Client::MockClient* client2 = new NiceMock(); - Upstream::HostConstSharedPtr host1; - Upstream::HostConstSharedPtr host2; - - // There is no cluster yet, so makeRequestToHost() should fail. - EXPECT_EQ(nullptr, conn_pool_->makeRequestToHost("10.0.0.1:3000", value, callbacks1)); - // Add the cluster now. - update_callbacks_->onClusterAddOrUpdate(cm_.thread_local_cluster_); - - EXPECT_CALL(*this, create_(_)).WillOnce(DoAll(SaveArg<0>(&host1), Return(client1))); - EXPECT_CALL( - *client1, - makeRequest(Eq(NetworkFilters::Common::Redis::Utility::makeAuthCommand(auth_password_)), _)) - .WillOnce(Return(&auth_request1)); - EXPECT_CALL(*client1, makeRequest(Ref(value), Ref(callbacks1))) - .WillOnce(Return(&active_request1)); - Common::Redis::Client::PoolRequest* request1 = - conn_pool_->makeRequestToHost("10.0.0.1:3000", value, callbacks1); - EXPECT_EQ(&active_request1, request1); - EXPECT_EQ(host1->address()->asString(), "10.0.0.1:3000"); - - // IPv6 address returned from Redis server will not have square brackets - // around it, while Envoy represents Address::Ipv6Instance addresses with square brackets around - // the address. - EXPECT_CALL(*this, create_(_)).WillOnce(DoAll(SaveArg<0>(&host2), Return(client2))); - EXPECT_CALL( - *client2, - makeRequest(Eq(NetworkFilters::Common::Redis::Utility::makeAuthCommand(auth_password_)), _)) - .WillOnce(Return(&auth_request2)); - EXPECT_CALL(*client2, makeRequest(Ref(value), Ref(callbacks2))) - .WillOnce(Return(&active_request2)); - Common::Redis::Client::PoolRequest* request2 = - conn_pool_->makeRequestToHost("2001:470:813B:0:0:0:0:1:3333", value, callbacks2); - EXPECT_EQ(&active_request2, request2); - EXPECT_EQ(host2->address()->asString(), "[2001:470:813b::1]:3333"); - - EXPECT_CALL(*client2, close()); - EXPECT_CALL(*client1, close()); - tls_.shutdownThread(); -} - // This test forces the creation of 2 hosts (one with an IPv4 address, and the other with an IPv6 // address) and pending requests using makeRequestToHost(). After their creation, "new" hosts are // discovered, and the original hosts are put aside to drain. The test then verifies the drain diff --git a/test/extensions/health_checkers/redis/BUILD b/test/extensions/health_checkers/redis/BUILD index 015f73f5e4f4..8f38c2dfe32d 100644 --- a/test/extensions/health_checkers/redis/BUILD +++ b/test/extensions/health_checkers/redis/BUILD @@ -16,6 +16,7 @@ envoy_extension_cc_test( srcs = ["redis_test.cc"], extension_name = "envoy.health_checkers.redis", deps = [ + "//source/common/api:api_lib", "//source/extensions/health_checkers/redis", "//source/extensions/health_checkers/redis:utility", "//test/common/upstream:utility_lib", @@ -25,6 +26,7 @@ envoy_extension_cc_test( "//test/mocks/network:network_mocks", "//test/mocks/runtime:runtime_mocks", "//test/mocks/upstream:upstream_mocks", + "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/health_checkers/redis/redis_test.cc b/test/extensions/health_checkers/redis/redis_test.cc index 2d721cacf56d..e5ab3df747ef 100644 --- a/test/extensions/health_checkers/redis/redis_test.cc +++ b/test/extensions/health_checkers/redis/redis_test.cc @@ -1,5 +1,7 @@ #include +#include "envoy/api/api.h" + #include "extensions/health_checkers/redis/redis.h" #include "extensions/health_checkers/redis/utility.h" @@ -31,7 +33,8 @@ class RedisHealthCheckerTest public: RedisHealthCheckerTest() : cluster_(new NiceMock()), - event_logger_(new Upstream::MockHealthCheckEventLogger()) {} + event_logger_(new Upstream::MockHealthCheckEventLogger()), + api_(Api::createApiForTest()) {} void setup() { const std::string yaml = R"EOF( @@ -52,7 +55,7 @@ class RedisHealthCheckerTest health_checker_.reset( new RedisHealthChecker(*cluster_, health_check_config, redis_config, dispatcher_, runtime_, - random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *this)); + random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); } void setupAlwaysLogHealthCheckFailures() { @@ -75,7 +78,7 @@ class RedisHealthCheckerTest health_checker_.reset( new RedisHealthChecker(*cluster_, health_check_config, redis_config, dispatcher_, runtime_, - random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *this)); + random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); } void setupExistsHealthcheck() { @@ -98,7 +101,7 @@ class RedisHealthCheckerTest health_checker_.reset( new RedisHealthChecker(*cluster_, health_check_config, redis_config, dispatcher_, runtime_, - random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *this)); + random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); } void setupDontReuseConnection() { @@ -121,12 +124,12 @@ class RedisHealthCheckerTest health_checker_.reset( new RedisHealthChecker(*cluster_, health_check_config, redis_config, dispatcher_, runtime_, - random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *this)); + random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); } Extensions::NetworkFilters::Common::Redis::Client::ClientPtr create(Upstream::HostConstSharedPtr, Event::Dispatcher&, - const Extensions::NetworkFilters::Common::Redis::Client::Config&) override { + const Extensions::NetworkFilters::Common::Redis::Client::Config&, const std::string&) override { return Extensions::NetworkFilters::Common::Redis::Client::ClientPtr{create_()}; } @@ -182,6 +185,7 @@ class RedisHealthCheckerTest Extensions::NetworkFilters::Common::Redis::Client::MockPoolRequest pool_request_; Extensions::NetworkFilters::Common::Redis::Client::PoolCallbacks* pool_callbacks_{}; std::shared_ptr health_checker_; + Api::ApiPtr api_; }; TEST_F(RedisHealthCheckerTest, PingAndVariousFailures) { From fc133dabda40ab7e0c7247b99a83eed7ad0fed82 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 5 Sep 2019 19:06:53 -0700 Subject: [PATCH 02/11] fix format Signed-off-by: Henry Yang --- .../health_checkers/redis/redis_test.cc | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/extensions/health_checkers/redis/redis_test.cc b/test/extensions/health_checkers/redis/redis_test.cc index e5ab3df747ef..f88077659773 100644 --- a/test/extensions/health_checkers/redis/redis_test.cc +++ b/test/extensions/health_checkers/redis/redis_test.cc @@ -33,8 +33,7 @@ class RedisHealthCheckerTest public: RedisHealthCheckerTest() : cluster_(new NiceMock()), - event_logger_(new Upstream::MockHealthCheckEventLogger()), - api_(Api::createApiForTest()) {} + event_logger_(new Upstream::MockHealthCheckEventLogger()), api_(Api::createApiForTest()) {} void setup() { const std::string yaml = R"EOF( @@ -53,9 +52,9 @@ class RedisHealthCheckerTest const auto& redis_config = getRedisHealthCheckConfig( health_check_config, ProtobufMessage::getStrictValidationVisitor()); - health_checker_.reset( - new RedisHealthChecker(*cluster_, health_check_config, redis_config, dispatcher_, runtime_, - random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); + health_checker_.reset(new RedisHealthChecker( + *cluster_, health_check_config, redis_config, dispatcher_, runtime_, random_, + Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); } void setupAlwaysLogHealthCheckFailures() { @@ -76,9 +75,9 @@ class RedisHealthCheckerTest const auto& redis_config = getRedisHealthCheckConfig( health_check_config, ProtobufMessage::getStrictValidationVisitor()); - health_checker_.reset( - new RedisHealthChecker(*cluster_, health_check_config, redis_config, dispatcher_, runtime_, - random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); + health_checker_.reset(new RedisHealthChecker( + *cluster_, health_check_config, redis_config, dispatcher_, runtime_, random_, + Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); } void setupExistsHealthcheck() { @@ -99,9 +98,9 @@ class RedisHealthCheckerTest const auto& redis_config = getRedisHealthCheckConfig( health_check_config, ProtobufMessage::getStrictValidationVisitor()); - health_checker_.reset( - new RedisHealthChecker(*cluster_, health_check_config, redis_config, dispatcher_, runtime_, - random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); + health_checker_.reset(new RedisHealthChecker( + *cluster_, health_check_config, redis_config, dispatcher_, runtime_, random_, + Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); } void setupDontReuseConnection() { @@ -122,14 +121,15 @@ class RedisHealthCheckerTest const auto& redis_config = getRedisHealthCheckConfig( health_check_config, ProtobufMessage::getStrictValidationVisitor()); - health_checker_.reset( - new RedisHealthChecker(*cluster_, health_check_config, redis_config, dispatcher_, runtime_, - random_, Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); + health_checker_.reset(new RedisHealthChecker( + *cluster_, health_check_config, redis_config, dispatcher_, runtime_, random_, + Upstream::HealthCheckEventLoggerPtr(event_logger_), *api_, *this)); } Extensions::NetworkFilters::Common::Redis::Client::ClientPtr create(Upstream::HostConstSharedPtr, Event::Dispatcher&, - const Extensions::NetworkFilters::Common::Redis::Client::Config&, const std::string&) override { + const Extensions::NetworkFilters::Common::Redis::Client::Config&, + const std::string&) override { return Extensions::NetworkFilters::Common::Redis::Client::ClientPtr{create_()}; } From 7a2f64d5a1fd76a58c2ec83577c64e775136e6f2 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 5 Sep 2019 21:55:00 -0700 Subject: [PATCH 03/11] Add release note. Signed-off-by: Henry Yang --- docs/root/intro/version_history.rst | 1 + test/extensions/clusters/redis/BUILD | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 147c65f5ce60..482738ec716a 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -36,6 +36,7 @@ Version history * performance: stats symbol table implementation (disabled by default; to test it, add "--use-fake-symbol-table 0" to the command-line arguments when starting Envoy). * rbac: added support for DNS SAN as :ref:`principal_name `. * redis: added :ref:`read_policy ` to allow reading from redis replicas for Redis Cluster deployments. +* redis: fix a bug where the redis health checker ignored the upstream auth password. * regex: introduce new :ref:`RegexMatcher ` type that provides a safe regex implementation for untrusted user input. This type is now used in all configuration that processes user provided input. See :ref:`deprecated configuration details diff --git a/test/extensions/clusters/redis/BUILD b/test/extensions/clusters/redis/BUILD index 7c39f26b53d6..25495a9d7d95 100644 --- a/test/extensions/clusters/redis/BUILD +++ b/test/extensions/clusters/redis/BUILD @@ -82,6 +82,7 @@ envoy_extension_cc_test( ) envoy_extension_cc_test( + name = "redis_cluster_integration_test", size = "small", srcs = ["redis_cluster_integration_test.cc"], extension_name = "envoy.clusters.redis", From 36591d6776e6c6aa09944c83945b37094bfdbb91 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 5 Sep 2019 23:27:02 -0700 Subject: [PATCH 04/11] Fix failed tests Signed-off-by: Henry Yang --- test/extensions/health_checkers/redis/config_test.cc | 3 ++- test/mocks/server/mocks.cc | 1 + test/mocks/server/mocks.h | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/test/extensions/health_checkers/redis/config_test.cc b/test/extensions/health_checkers/redis/config_test.cc index 2dcb9a439127..bcc6ecb5a01e 100644 --- a/test/extensions/health_checkers/redis/config_test.cc +++ b/test/extensions/health_checkers/redis/config_test.cc @@ -106,11 +106,12 @@ TEST(HealthCheckerFactoryTest, CreateRedisViaUpstreamHealthCheckerFactory) { Runtime::MockRandomGenerator random; Event::MockDispatcher dispatcher; AccessLog::MockAccessLogManager log_manager; + NiceMock api; EXPECT_NE(nullptr, dynamic_cast( Upstream::HealthCheckerFactory::create( Upstream::parseHealthCheckFromV2Yaml(yaml), cluster, runtime, random, - dispatcher, log_manager, ProtobufMessage::getStrictValidationVisitor()) + dispatcher, log_manager, ProtobufMessage::getStrictValidationVisitor(), api) .get())); } } // namespace diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 42bcd95947a8..c63523948992 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -224,6 +224,7 @@ MockHealthCheckerFactoryContext::MockHealthCheckerFactoryContext() { ON_CALL(*this, eventLogger_()).WillByDefault(Return(event_logger_)); ON_CALL(*this, messageValidationVisitor()) .WillByDefault(ReturnRef(ProtobufMessage::getStrictValidationVisitor())); + ON_CALL(*this, api()).WillByDefault(ReturnRef(api_)); } MockHealthCheckerFactoryContext::~MockHealthCheckerFactoryContext() = default; diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index fd0ee9b27ef6..de7d678c383b 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -546,6 +546,7 @@ class MockHealthCheckerFactoryContext : public virtual HealthCheckerFactoryConte MOCK_METHOD0(runtime, Envoy::Runtime::Loader&()); MOCK_METHOD0(eventLogger_, Upstream::HealthCheckEventLogger*()); MOCK_METHOD0(messageValidationVisitor, ProtobufMessage::ValidationVisitor&()); + MOCK_METHOD0(api, Api::Api&()); Upstream::HealthCheckEventLoggerPtr eventLogger() override { return Upstream::HealthCheckEventLoggerPtr(eventLogger_()); } @@ -555,6 +556,7 @@ class MockHealthCheckerFactoryContext : public virtual HealthCheckerFactoryConte testing::NiceMock random_; testing::NiceMock runtime_; testing::NiceMock* event_logger_{}; + testing::NiceMock api_{}; }; } // namespace Configuration From 4898669a00ffb138c8487ac988ceb009f0e9d044 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 5 Sep 2019 23:30:52 -0700 Subject: [PATCH 05/11] fix format Signed-off-by: Henry Yang --- test/extensions/health_checkers/redis/config_test.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/extensions/health_checkers/redis/config_test.cc b/test/extensions/health_checkers/redis/config_test.cc index bcc6ecb5a01e..8b38426fc652 100644 --- a/test/extensions/health_checkers/redis/config_test.cc +++ b/test/extensions/health_checkers/redis/config_test.cc @@ -108,11 +108,12 @@ TEST(HealthCheckerFactoryTest, CreateRedisViaUpstreamHealthCheckerFactory) { AccessLog::MockAccessLogManager log_manager; NiceMock api; - EXPECT_NE(nullptr, dynamic_cast( - Upstream::HealthCheckerFactory::create( - Upstream::parseHealthCheckFromV2Yaml(yaml), cluster, runtime, random, - dispatcher, log_manager, ProtobufMessage::getStrictValidationVisitor(), api) - .get())); + EXPECT_NE(nullptr, + dynamic_cast( + Upstream::HealthCheckerFactory::create( + Upstream::parseHealthCheckFromV2Yaml(yaml), cluster, runtime, random, + dispatcher, log_manager, ProtobufMessage::getStrictValidationVisitor(), api) + .get())); } } // namespace } // namespace RedisHealthChecker From bc294299731f587dec2698a9b7beac89e574ae4e Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Mon, 9 Sep 2019 13:59:08 -0700 Subject: [PATCH 06/11] Avoid READONLY commands for the connections used for CLUSTER SLOTS. Signed-off-by: Henry Yang --- source/extensions/clusters/redis/redis_cluster.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/extensions/clusters/redis/redis_cluster.h b/source/extensions/clusters/redis/redis_cluster.h index f4038b7629f1..b7d0026f4f76 100644 --- a/source/extensions/clusters/redis/redis_cluster.h +++ b/source/extensions/clusters/redis/redis_cluster.h @@ -214,10 +214,10 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { uint32_t maxBufferSizeBeforeFlush() const override { return 0; } std::chrono::milliseconds bufferFlushTimeoutInMs() const override { return buffer_timeout_; } uint32_t maxUpstreamUnknownConnections() const override { return 0; } - // This is effectively not in used for making the "Cluster Slots" calls. - // since we call cluster slots on both the master and slaves, ANY is more appropriate here. + // This is effectively not in used for making the "Cluster Slots" calls, using Master to avoid + // the READONLY command when establishing connection. Extensions::NetworkFilters::Common::Redis::Client::ReadPolicy readPolicy() const override { - return Extensions::NetworkFilters::Common::Redis::Client::ReadPolicy::Any; + return Extensions::NetworkFilters::Common::Redis::Client::ReadPolicy::Master; } // Extensions::NetworkFilters::Common::Redis::Client::PoolCallbacks From 4c8ab4542b01076a716baca9d28cdd505565bad6 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Mon, 9 Sep 2019 16:34:13 -0700 Subject: [PATCH 07/11] Fix merge issue. Signed-off-by: Henry Yang --- test/extensions/filters/network/common/redis/client_impl_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/extensions/filters/network/common/redis/client_impl_test.cc b/test/extensions/filters/network/common/redis/client_impl_test.cc index 3903e29155b9..9b63c9676ec4 100644 --- a/test/extensions/filters/network/common/redis/client_impl_test.cc +++ b/test/extensions/filters/network/common/redis/client_impl_test.cc @@ -16,6 +16,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::Eq; using testing::InSequence; using testing::Invoke; using testing::Ref; From 6531a4285ee98d6c2338e358ce2d6461605e9e4f Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Tue, 10 Sep 2019 13:46:14 -0700 Subject: [PATCH 08/11] Fix review feedback and cache auth_password used in redis cluster Signed-off-by: Henry Yang --- source/extensions/clusters/redis/redis_cluster.cc | 13 ++----------- source/extensions/clusters/redis/redis_cluster.h | 9 +++++---- .../extensions/filters/network/redis_proxy/config.h | 2 +- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/source/extensions/clusters/redis/redis_cluster.cc b/source/extensions/clusters/redis/redis_cluster.cc index 0e9c3a24888f..c1c6a49cc887 100644 --- a/source/extensions/clusters/redis/redis_cluster.cc +++ b/source/extensions/clusters/redis/redis_cluster.cc @@ -34,7 +34,7 @@ RedisCluster::RedisCluster( : Config::Utility::translateClusterHosts(cluster.hosts())), local_info_(factory_context.localInfo()), random_(factory_context.random()), redis_discovery_session_(*this, redis_client_factory), lb_factory_(std::move(lb_factory)), - api_(api) { + auth_password_(NetworkFilters::RedisProxy::ProtocolOptionsConfigImpl::auth_password(info(), api)) { const auto& locality_lb_endpoints = load_assignment_.endpoints(); for (const auto& locality_lb_endpoint : locality_lb_endpoints) { for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) { @@ -43,13 +43,6 @@ RedisCluster::RedisCluster( *this, host.socket_address().address(), host.socket_address().port_value())); } } - - auto options = - info()->extensionProtocolOptionsTyped( - NetworkFilters::NetworkFilterNames::get().RedisProxy); - if (options) { - auth_password_datasource_ = options->auth_password_datasource(); - } } void RedisCluster::startPreInit() { @@ -249,9 +242,7 @@ void RedisCluster::RedisDiscoverySession::startResolveRedis() { if (!client) { client = std::make_unique(*this); client->host_ = current_host_address_; - std::string auth_password = - Envoy::Config::DataSource::read(parent_.auth_password_datasource_, true, parent_.api_); - client->client_ = client_factory_.create(host, dispatcher_, *this, auth_password); + client->client_ = client_factory_.create(host, dispatcher_, *this, parent_.auth_password_); client->client_->addConnectionCallbacks(*client); } diff --git a/source/extensions/clusters/redis/redis_cluster.h b/source/extensions/clusters/redis/redis_cluster.h index b7d0026f4f76..2ca4c123784b 100644 --- a/source/extensions/clusters/redis/redis_cluster.h +++ b/source/extensions/clusters/redis/redis_cluster.h @@ -214,8 +214,10 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { uint32_t maxBufferSizeBeforeFlush() const override { return 0; } std::chrono::milliseconds bufferFlushTimeoutInMs() const override { return buffer_timeout_; } uint32_t maxUpstreamUnknownConnections() const override { return 0; } - // This is effectively not in used for making the "Cluster Slots" calls, using Master to avoid - // the READONLY command when establishing connection. + // For any readPolicy other than Master, the RedisClientFactory will send a READONLY command when + // establishing a new connection. Since we're only using this for making the "cluster slots" commands, + // the READONLY command is not relevant in this context. We're setting it to Master to avoid the + // additional READONLY command. Extensions::NetworkFilters::Common::Redis::Client::ReadPolicy readPolicy() const override { return Extensions::NetworkFilters::Common::Redis::Client::ReadPolicy::Master; } @@ -259,8 +261,7 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { Upstream::HostVector hosts_; Upstream::HostMap all_hosts_; - envoy::api::v2::core::DataSource auth_password_datasource_; - Api::Api& api_; + const std::string auth_password_; }; class RedisClusterFactory : public Upstream::ConfigurableClusterFactoryBase< diff --git a/source/extensions/filters/network/redis_proxy/config.h b/source/extensions/filters/network/redis_proxy/config.h index 30bf184f527b..a25a8afd0589 100644 --- a/source/extensions/filters/network/redis_proxy/config.h +++ b/source/extensions/filters/network/redis_proxy/config.h @@ -32,7 +32,7 @@ class ProtocolOptionsConfigImpl : public Upstream::ProtocolOptionsConfig { return auth_password_; } - static const std::string auth_password(Upstream::ClusterInfoConstSharedPtr info, Api::Api& api) { + static const std::string auth_password(const Upstream::ClusterInfoConstSharedPtr info, Api::Api& api) { auto options = info->extensionProtocolOptionsTyped( NetworkFilterNames::get().RedisProxy); if (options) { From 73d02c80ffeeaabae1aa5b0c484fecf98a1e7daa Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Tue, 10 Sep 2019 13:59:12 -0700 Subject: [PATCH 09/11] Fix format. Signed-off-by: Henry Yang --- source/extensions/clusters/redis/redis_cluster.cc | 3 ++- source/extensions/clusters/redis/redis_cluster.h | 8 ++++---- source/extensions/filters/network/redis_proxy/config.h | 3 ++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/source/extensions/clusters/redis/redis_cluster.cc b/source/extensions/clusters/redis/redis_cluster.cc index c1c6a49cc887..bd699a3c51e4 100644 --- a/source/extensions/clusters/redis/redis_cluster.cc +++ b/source/extensions/clusters/redis/redis_cluster.cc @@ -34,7 +34,8 @@ RedisCluster::RedisCluster( : Config::Utility::translateClusterHosts(cluster.hosts())), local_info_(factory_context.localInfo()), random_(factory_context.random()), redis_discovery_session_(*this, redis_client_factory), lb_factory_(std::move(lb_factory)), - auth_password_(NetworkFilters::RedisProxy::ProtocolOptionsConfigImpl::auth_password(info(), api)) { + auth_password_( + NetworkFilters::RedisProxy::ProtocolOptionsConfigImpl::auth_password(info(), api)) { const auto& locality_lb_endpoints = load_assignment_.endpoints(); for (const auto& locality_lb_endpoint : locality_lb_endpoints) { for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) { diff --git a/source/extensions/clusters/redis/redis_cluster.h b/source/extensions/clusters/redis/redis_cluster.h index 2ca4c123784b..ea5e6822aadc 100644 --- a/source/extensions/clusters/redis/redis_cluster.h +++ b/source/extensions/clusters/redis/redis_cluster.h @@ -214,10 +214,10 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { uint32_t maxBufferSizeBeforeFlush() const override { return 0; } std::chrono::milliseconds bufferFlushTimeoutInMs() const override { return buffer_timeout_; } uint32_t maxUpstreamUnknownConnections() const override { return 0; } - // For any readPolicy other than Master, the RedisClientFactory will send a READONLY command when - // establishing a new connection. Since we're only using this for making the "cluster slots" commands, - // the READONLY command is not relevant in this context. We're setting it to Master to avoid the - // additional READONLY command. + // For any readPolicy other than Master, the RedisClientFactory will send a READONLY command + // when establishing a new connection. Since we're only using this for making the "cluster + // slots" commands, the READONLY command is not relevant in this context. We're setting it to + // Master to avoid the additional READONLY command. Extensions::NetworkFilters::Common::Redis::Client::ReadPolicy readPolicy() const override { return Extensions::NetworkFilters::Common::Redis::Client::ReadPolicy::Master; } diff --git a/source/extensions/filters/network/redis_proxy/config.h b/source/extensions/filters/network/redis_proxy/config.h index a25a8afd0589..5a72b8ad9c4e 100644 --- a/source/extensions/filters/network/redis_proxy/config.h +++ b/source/extensions/filters/network/redis_proxy/config.h @@ -32,7 +32,8 @@ class ProtocolOptionsConfigImpl : public Upstream::ProtocolOptionsConfig { return auth_password_; } - static const std::string auth_password(const Upstream::ClusterInfoConstSharedPtr info, Api::Api& api) { + static const std::string auth_password(const Upstream::ClusterInfoConstSharedPtr info, + Api::Api& api) { auto options = info->extensionProtocolOptionsTyped( NetworkFilterNames::get().RedisProxy); if (options) { From 92cd158dd34445f16e85d6a9024b523c2a2c42ce Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Tue, 10 Sep 2019 17:13:44 -0700 Subject: [PATCH 10/11] Fix format Signed-off-by: Henry Yang --- .../extensions/filters/network/common/redis/client_impl.cc | 5 ++--- .../filters/network/redis_proxy/conn_pool_impl.cc | 5 +++-- source/extensions/health_checkers/redis/redis.cc | 6 +++--- .../filters/network/common/redis/client_impl_test.cc | 3 ++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/source/extensions/filters/network/common/redis/client_impl.cc b/source/extensions/filters/network/common/redis/client_impl.cc index 6397b6aaaf9c..631221a39496 100644 --- a/source/extensions/filters/network/common/redis/client_impl.cc +++ b/source/extensions/filters/network/common/redis/client_impl.cc @@ -297,9 +297,8 @@ ClientPtr ClientFactoryImpl::create(Upstream::HostConstSharedPtr host, Event::Dispatcher& dispatcher, const Config& config, const RedisCommandStatsSharedPtr& redis_command_stats, Stats::Scope& scope, const std::string& auth_password) { - ClientPtr client = - ClientImpl::create(host, dispatcher, EncoderPtr{new EncoderImpl()}, decoder_factory_, - config, redis_command_stats, scope); + ClientPtr client = ClientImpl::create(host, dispatcher, EncoderPtr{new EncoderImpl()}, + decoder_factory_, config, redis_command_stats, scope); client->initialize(auth_password); return client; } diff --git a/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc b/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc index 78e09d9ce4a1..7304ef902325 100644 --- a/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc +++ b/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc @@ -190,8 +190,9 @@ InstanceImpl::ThreadLocalPool::threadLocalActiveClient(Upstream::HostConstShared if (!client) { client = std::make_unique(*this); client->host_ = host; - client->redis_client_ = parent_.client_factory_.create( - host, dispatcher_, parent_.config_, parent_.redis_command_stats_, *parent_.stats_scope_, auth_password_); + client->redis_client_ = parent_.client_factory_.create(host, dispatcher_, parent_.config_, + parent_.redis_command_stats_, + *parent_.stats_scope_, auth_password_); client->redis_client_->addConnectionCallbacks(*client); } return client; diff --git a/source/extensions/health_checkers/redis/redis.cc b/source/extensions/health_checkers/redis/redis.cc index ce4d474c3972..b1578e33370f 100644 --- a/source/extensions/health_checkers/redis/redis.cc +++ b/source/extensions/health_checkers/redis/redis.cc @@ -58,9 +58,9 @@ void RedisHealthChecker::RedisActiveHealthCheckSession::onEvent(Network::Connect void RedisHealthChecker::RedisActiveHealthCheckSession::onInterval() { if (!client_) { - client_ = - parent_.client_factory_.create(host_, parent_.dispatcher_, *this, redis_command_stats_, - parent_.cluster_.info()->statsScope(), parent_.auth_password_); + client_ = parent_.client_factory_.create( + host_, parent_.dispatcher_, *this, redis_command_stats_, + parent_.cluster_.info()->statsScope(), parent_.auth_password_); client_->addConnectionCallbacks(*this); } diff --git a/test/extensions/filters/network/common/redis/client_impl_test.cc b/test/extensions/filters/network/common/redis/client_impl_test.cc index dfbef0eb1ffd..54c67ba9a5bf 100644 --- a/test/extensions/filters/network/common/redis/client_impl_test.cc +++ b/test/extensions/filters/network/common/redis/client_impl_test.cc @@ -951,7 +951,8 @@ TEST(RedisClientFactoryImplTest, Basic) { auto redis_command_stats = Common::Redis::RedisCommandStats::createRedisCommandStats(stats_.symbolTable()); const std::string auth_password; - ClientPtr client = factory.create(host, dispatcher, config, redis_command_stats, stats_, auth_password); + ClientPtr client = + factory.create(host, dispatcher, config, redis_command_stats, stats_, auth_password); client->close(); } } // namespace Client From ff82723f7030987113e5afc8df5f878cbba8b290 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Wed, 11 Sep 2019 11:12:31 -0700 Subject: [PATCH 11/11] Fix merge issue. Signed-off-by: Henry Yang --- source/extensions/health_checkers/redis/redis.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/source/extensions/health_checkers/redis/redis.cc b/source/extensions/health_checkers/redis/redis.cc index b1578e33370f..22a7bd9be07f 100644 --- a/source/extensions/health_checkers/redis/redis.cc +++ b/source/extensions/health_checkers/redis/redis.cc @@ -1,5 +1,4 @@ #include "extensions/health_checkers/redis/redis.h" -#include "source/extensions/health_checkers/redis/redis.h" namespace Envoy { namespace Extensions {