Skip to content

Commit 9fd73bb

Browse files
tysiong.daudov
authored andcommitted
feat redis: add sentinel_password option and make sure that it work for non-subscribe cases
This PR adds support for password authentication when connecting to Redis Sentinel. Closes: userver-framework#924 Allows configuring Sentinel password via `sentinel_password` option in secdist config. Example: ```json { "redis_settings": { "taxi-tmp": { "password": "redispwd", "sentinel_password": "sentpwd", /* <---- `AUTH sentpwd` will be executed on connection to sentinel` */ "database_index": 5, "secure_connection": false, "shards": [ { "name": "mymaster" } ], "sentinels": [ { "host": "127.0.0.1", "port": 26379 } ] } } } ``` Tests: протестировано CI Pull Request resolved: userver-framework#925 commit_hash:84ec2914e9b505c1655600cf3ce6ccf2281ae75c
1 parent cbb6019 commit 9fd73bb

File tree

6 files changed

+183
-4
lines changed

6 files changed

+183
-4
lines changed

redis/include/userver/storages/redis/component.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ namespace components {
9999
/// "redis_settings": {
100100
/// "some_name_of_your_database": {
101101
/// "password": "the_password_of_your_database",
102+
/// "sentinel_password": "the_password_for_sentinels_if_any",
102103
/// "sentinels": [
103104
/// {"host": "the_host1_of_your_database", "port": 11564}
104105
/// ],

redis/src/storages/redis/impl/secdist_redis.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ struct RedisSettings {
2121
std::vector<std::string> shards;
2222
std::vector<HostPort> sentinels;
2323
storages::redis::Password password{std::string()};
24+
storages::redis::Password sentinel_password{std::string()};
2425
storages::redis::ConnectionSecurity secure_connection{storages::redis::ConnectionSecurity::kNone};
2526
std::size_t database_index{0};
2627
};

redis/src/storages/redis/impl/sentinel.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ std::shared_ptr<Sentinel> Sentinel::CreateSentinel(
147147
const testsuite::RedisControl& testsuite_redis_control
148148
) {
149149
const auto& password = settings.password;
150+
const auto& sentinel_password = settings.sentinel_password;
150151

151152
const std::vector<std::string>& shards = settings.shards;
152153
LOG_DEBUG() << "shards.size() = " << shards.size();
@@ -157,13 +158,12 @@ std::shared_ptr<Sentinel> Sentinel::CreateSentinel(
157158
LOG_DEBUG() << "sentinels.size() = " << settings.sentinels.size();
158159
for (const auto& sentinel : settings.sentinels) {
159160
LOG_DEBUG() << "sentinel: host = " << sentinel.host << " port = " << sentinel.port;
160-
// SENTINEL MASTERS/SLAVES works without auth, sentinel has no AUTH command.
161161
// CLUSTER SLOTS works after auth only. Masters and slaves used instead of
162162
// sentinels in cluster mode.
163163
conns.emplace_back(
164164
sentinel.host,
165165
sentinel.port,
166-
(key_shard_factory.IsClusterStrategy() ? password : Password("")),
166+
(key_shard_factory.IsClusterStrategy() ? password : sentinel_password),
167167
false,
168168
settings.secure_connection
169169
);

redis/src/storages/redis/impl/server_test.cpp

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,14 @@
77
#include <storages/redis/impl/command.hpp>
88
#include <storages/redis/impl/secdist_redis.hpp>
99
#include <storages/redis/impl/sentinel.hpp>
10+
#include <storages/redis/impl/subscribe_sentinel.hpp>
1011
#include <storages/redis/impl/thread_pools.hpp>
12+
#include <storages/redis/subscribe_client_impl.hpp>
13+
#include <userver/dynamic_config/test_helpers.hpp>
14+
#include <userver/storages/redis/subscribe_client.hpp>
15+
#include <userver/storages/redis/subscription_token.hpp>
16+
17+
#include <userver/utest/utest.hpp>
1118

1219
USERVER_NAMESPACE_BEGIN
1320

@@ -44,6 +51,52 @@ bool IsConnected(const storages::redis::impl::Redis& redis) {
4451
return redis.GetState() == storages::redis::RedisState::kConnected;
4552
}
4653

54+
struct MockSentinelServers {
55+
static constexpr size_t kRedisThreadCount = 1;
56+
static constexpr std::string_view kRedisName = "redis_name";
57+
58+
void RegisterSentinelMastersSlaves() {
59+
std::vector<MockRedisServer::SlaveInfo> slave_infos;
60+
std::string redis_name{kRedisName};
61+
for (const auto& slave : slaves) {
62+
slave_infos.emplace_back(redis_name, kLocalhost, slave.GetPort());
63+
}
64+
65+
for (auto& sentinel : sentinels) {
66+
sentinel.RegisterSentinelMastersHandler({{redis_name, kLocalhost, masters[0].GetPort()}});
67+
sentinel.RegisterSentinelSlavesHandler(redis_name, slave_infos);
68+
}
69+
}
70+
71+
template <class Function>
72+
void ForEachServer(const Function& visitor) {
73+
for (auto& server : masters) {
74+
visitor(server);
75+
}
76+
for (auto& server : slaves) {
77+
visitor(server);
78+
}
79+
for (auto& server : sentinels) {
80+
visitor(server);
81+
}
82+
}
83+
84+
MockRedisServer masters[1] = {
85+
MockRedisServer{"master0"},
86+
};
87+
MockRedisServer slaves[2] = {
88+
MockRedisServer{"slave0"},
89+
MockRedisServer{"slave1"},
90+
};
91+
MockRedisServer sentinels[3] = {
92+
MockRedisServer{"sentinel0"},
93+
MockRedisServer{"sentinel1"},
94+
MockRedisServer{"sentinel2"},
95+
};
96+
std::shared_ptr<storages::redis::impl::ThreadPools> thread_pool =
97+
std::make_shared<storages::redis::impl::ThreadPools>(1, kRedisThreadCount);
98+
};
99+
47100
} // namespace
48101

49102
TEST(Redis, NoPassword) {
@@ -101,6 +154,124 @@ TEST(Redis, AuthTimeout) {
101154
PeriodicCheck([&] { return !IsConnected(*redis); });
102155
}
103156

157+
UTEST(Redis, SentinelAuth) {
158+
MockSentinelServers mock;
159+
mock.RegisterSentinelMastersSlaves();
160+
mock.ForEachServer([](auto& server) { server.RegisterPingHandler(); });
161+
auto& [masters, slaves, sentinels, thread_pool] = mock;
162+
163+
secdist::RedisSettings settings;
164+
settings.shards = {std::string{MockSentinelServers::kRedisName}};
165+
settings.sentinel_password = storages::redis::Password("pass");
166+
settings.sentinels.reserve(std::size(sentinels));
167+
for (const auto& sentinel : sentinels) {
168+
settings.sentinels.emplace_back(kLocalhost, sentinel.GetPort());
169+
}
170+
171+
std::vector<MockRedisServer::HandlerPtr> auth_handlers;
172+
auth_handlers.reserve(std::size(sentinels));
173+
for (auto& sentinel : sentinels) {
174+
auth_handlers.push_back(sentinel.RegisterStatusReplyHandler("AUTH", "OK"));
175+
}
176+
std::vector<MockRedisServer::HandlerPtr> no_auth_handlers;
177+
no_auth_handlers.reserve(std::size(masters) + std::size(slaves));
178+
for (auto& server : masters) {
179+
no_auth_handlers.push_back(server.RegisterStatusReplyHandler("AUTH", "FAIL"));
180+
}
181+
for (auto& server : slaves) {
182+
no_auth_handlers.push_back(server.RegisterStatusReplyHandler("AUTH", "FAIL"));
183+
}
184+
185+
auto sentinel_client = storages::redis::impl::Sentinel::CreateSentinel(
186+
thread_pool, settings, "test_shard_group_name", dynamic_config::GetDefaultSource(), "test_client_name", {""}
187+
);
188+
sentinel_client->WaitConnectedDebug(std::empty(slaves));
189+
190+
for (auto& handler : auth_handlers) {
191+
EXPECT_TRUE(handler->WaitForFirstReply(kSmallPeriod));
192+
}
193+
194+
for (auto& handler : no_auth_handlers) {
195+
EXPECT_FALSE(handler->WaitForFirstReply(kWaitPeriod));
196+
}
197+
198+
for (const auto& sentinel : sentinels) {
199+
EXPECT_TRUE(sentinel.WaitForFirstPingReply(kSmallPeriod));
200+
}
201+
}
202+
203+
// TODO: TAXICOMMON-10834. Looks like AUTH to sentinel is not sent in case of SUBSCRIBE
204+
UTEST(Redis, DISABLED_SentinelAuthSubscribe) {
205+
MockSentinelServers mock;
206+
mock.RegisterSentinelMastersSlaves();
207+
mock.ForEachServer([](auto& server) { server.RegisterPingHandler(); });
208+
auto& [masters, slaves, sentinels, thread_pool] = mock;
209+
// Sentinels do NOT receive SUBSCRIBE
210+
std::vector<MockRedisServer::HandlerPtr> subscribe_handlers;
211+
for (auto& server : masters) {
212+
subscribe_handlers.push_back(server.RegisterHandlerWithConstReply("SUBSCRIBE", 1));
213+
}
214+
for (auto& server : slaves) {
215+
subscribe_handlers.push_back(server.RegisterHandlerWithConstReply("SUBSCRIBE", 1));
216+
}
217+
218+
secdist::RedisSettings settings;
219+
settings.shards = {std::string{MockSentinelServers::kRedisName}};
220+
settings.sentinel_password = storages::redis::Password("pass");
221+
settings.sentinels.reserve(std::size(sentinels));
222+
for (const auto& sentinel : sentinels) {
223+
settings.sentinels.emplace_back(kLocalhost, sentinel.GetPort());
224+
}
225+
226+
std::vector<MockRedisServer::HandlerPtr> auth_handlers;
227+
auth_handlers.reserve(std::size(sentinels));
228+
for (auto& sentinel : sentinels) {
229+
auth_handlers.push_back(sentinel.RegisterStatusReplyHandler("AUTH", "OK"));
230+
}
231+
std::vector<MockRedisServer::HandlerPtr> no_auth_handlers;
232+
no_auth_handlers.reserve(std::size(masters) + std::size(slaves));
233+
for (auto& server : masters) {
234+
no_auth_handlers.push_back(server.RegisterStatusReplyHandler("AUTH", "FAIL"));
235+
}
236+
for (auto& server : slaves) {
237+
no_auth_handlers.push_back(server.RegisterStatusReplyHandler("AUTH", "FAIL"));
238+
}
239+
240+
storages::redis::CommandControl cc{};
241+
testsuite::RedisControl redis_control{};
242+
auto subscribe_sentinel = storages::redis::impl::SubscribeSentinel::Create(
243+
thread_pool,
244+
settings,
245+
"test_shard_group_name",
246+
dynamic_config::GetDefaultSource(),
247+
"test_client_name",
248+
{""},
249+
cc,
250+
redis_control
251+
);
252+
subscribe_sentinel->WaitConnectedDebug(std::empty(slaves));
253+
std::shared_ptr<storages::redis::SubscribeClient> client =
254+
std::make_shared<storages::redis::SubscribeClientImpl>(std::move(subscribe_sentinel));
255+
256+
storages::redis::SubscriptionToken::OnMessageCb callback = [](const std::string& channel,
257+
const std::string& message) {
258+
EXPECT_TRUE(false) << "Should not be called. Channel = " << channel << ", message = " << message;
259+
};
260+
auto subscription = client->Subscribe("channel_name", std::move(callback));
261+
262+
for (auto& handler : subscribe_handlers) {
263+
EXPECT_TRUE(handler->WaitForFirstReply(utest::kMaxTestWaitTime));
264+
}
265+
266+
for (auto& handler : auth_handlers) {
267+
EXPECT_TRUE(handler->WaitForFirstReply(kSmallPeriod));
268+
}
269+
270+
for (auto& handler : no_auth_handlers) {
271+
EXPECT_FALSE(handler->WaitForFirstReply(kWaitPeriod));
272+
}
273+
}
274+
104275
TEST(Redis, Select) {
105276
MockRedisServer server{"redis_db"};
106277
auto ping_handler = server.RegisterPingHandler();

redis/src/storages/redis/impl/subscribe_sentinel.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ std::shared_ptr<SubscribeSentinel> SubscribeSentinel::Create(
8484
const testsuite::RedisControl& testsuite_redis_control
8585
) {
8686
const auto& password = settings.password;
87+
const auto& sentinel_password = settings.password;
8788

8889
const std::vector<std::string>& shards = settings.shards;
8990
LOG_DEBUG() << "shards.size() = " << shards.size();
@@ -96,11 +97,14 @@ std::shared_ptr<SubscribeSentinel> SubscribeSentinel::Create(
9697
LOG_DEBUG() << "sentinels.size() = " << settings.sentinels.size();
9798
for (const auto& sentinel : settings.sentinels) {
9899
LOG_DEBUG() << "sentinel: host = " << sentinel.host << " port = " << sentinel.port;
99-
// SENTINEL MASTERS/SLAVES works without auth, sentinel has no AUTH command.
100100
// CLUSTER SLOTS works after auth only. Masters and slaves used instead of
101101
// sentinels in cluster mode.
102102
conns.emplace_back(
103-
sentinel.host, sentinel.port, (is_cluster_mode ? password : Password("")), false, settings.secure_connection
103+
sentinel.host,
104+
sentinel.port,
105+
(is_cluster_mode ? password : sentinel_password),
106+
false,
107+
settings.secure_connection
104108
);
105109
}
106110
LOG_DEBUG() << "redis command_control: " << command_control.ToString();

redis/src/storages/redis/redis_secdist.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ RedisMapSettings::RedisMapSettings(const formats::json::Value& doc) {
3535

3636
USERVER_NAMESPACE::secdist::RedisSettings settings;
3737
settings.password = storages::redis::Password(GetString(client_settings, "password"));
38+
settings.sentinel_password =
39+
storages::redis::Password(client_settings["sentinel_password"].As<std::string>(""));
3840
settings.secure_connection = GetValue<bool>(client_settings, "secure_connection", false)
3941
? USERVER_NAMESPACE::storages::redis::ConnectionSecurity::kTLS
4042
: USERVER_NAMESPACE::storages::redis::ConnectionSecurity::kNone;

0 commit comments

Comments
 (0)