Update sentinel auth with explicit kwargs

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Unreleased
 
 - Fix `Redis#without_reconnect` for sentinel clients. Fix #1212.
+- Add `sentinel_username`, `sentinel_password` for sentinel clients. Bump `redis-client` to `>=0.17.0`. See #1213
 
 # 5.0.7
 

README.md

@@ -120,29 +120,39 @@ but a few so that if one is down the client will try the next one. The client
 is able to remember the last Sentinel that was able to reply correctly and will
 use it for the next requests.
 
-To [authenticate](https://redis.io/docs/management/sentinel/#configuring-sentinel-instances-with-authentication) Sentinel itself, you can specify the `username` and `password` options per instance. Exclude the `username` option if you're using password-only authentication.
+To [authenticate](https://redis.io/docs/management/sentinel/#configuring-sentinel-instances-with-authentication) Sentinel itself, you can specify the `sentinel_username` and `sentinel_password`. Exclude the `sentinel_username` option if you're using password-only authentication.
 
 ```ruby
-SENTINELS = [{ host: '127.0.0.1', port: 26380, username: 'appuser', password: 'mysecret' },
-             { host: '127.0.0.1', port: 26381, username: 'appuser', password: 'mysecret' }]
+SENTINELS = [{ host: '127.0.0.1', port: 26380},
+             { host: '127.0.0.1', port: 26381}]
 
-redis = Redis.new(name: 'mymaster', sentinels: SENTINELS, role: :master)
+redis = Redis.new(name: 'mymaster', sentinels: SENTINELS, sentinel_username: 'appuser', sentinel_password: 'mysecret', role: :master)
 ```
 
-If you specify a username and/or password at the top level for your main Redis instance, Sentinel will default to using those credentials. You can pass nil or override them for each sentinel.
+If you specify a username and/or password at the top level for your main Redis instance, Sentinel *will not* using thouse credentials
 
 ```ruby
 # Use 'mysecret' to authenticate against the mymaster instance, but skip authentication for the sentinels:
-SENTINELS = [{ host: '127.0.0.1', port: 26380, password: nil },
-             { host: '127.0.0.1', port: 26381, password: nil }]
+SENTINELS = [{ host: '127.0.0.1', port: 26380 },
+             { host: '127.0.0.1', port: 26381 }]
 
 redis = Redis.new(name: 'mymaster', sentinels: SENTINELS, role: :master, password: 'mysecret')
 ```
 
-Also the name can be passed as an url:
+So you have to provide Sentinel credential and Redis explictly even they are the same
 
 ```ruby
-redis = Redis.new(name: "redis://mymaster", sentinels: SENTINELS, role: :master)
+# Use 'mysecret' to authenticate against the mymaster instance and sentinel
+SENTINELS = [{ host: '127.0.0.1', port: 26380 },
+             { host: '127.0.0.1', port: 26381 }]
+
+redis = Redis.new(name: 'mymaster', sentinels: SENTINELS, role: :master, password: 'mysecret', sentinel_password: 'mysecret')
+```
+
+Also the `name`, `password`, `username` and `db` for Redis instance can be passed as an url:
+
+```ruby
+redis = Redis.new(url: "redis://appuser:mysecret@mymaster/10", sentinels: SENTINELS, role: :master)
 ```
 
 ## Cluster support

lib/redis.rb

@@ -137,21 +137,6 @@ def initialize_client(options)
     end
 
     if options.key?(:sentinels)
-      if url = options.delete(:url)
-        uri = URI.parse(url)
-        if !options.key?(:name) && uri.host
-          options[:name] = uri.host
-        end
-
-        if !options.key?(:password) && uri.password && !uri.password.empty?
-          options[:password] = uri.password
-        end
-
-        if !options.key?(:username) && uri.user && !uri.user.empty?
-          options[:username] = uri.user
-        end
-      end
-
       Client.sentinel(**options).new_client
     else
       Client.config(**options).new_client

redis.gemspec

@@ -45,5 +45,5 @@ Gem::Specification.new do |s|
 
   s.required_ruby_version = '>= 2.5.0'
 
-  s.add_runtime_dependency('redis-client', '>= 0.16.0')
+  s.add_runtime_dependency('redis-client', '>= 0.17.0')
 end

test/sentinel/sentinel_test.rb

@@ -205,7 +205,7 @@ def test_sentinel_with_non_sentinel_options
       end
     end
 
-    assert_equal [%w[auth foo], %w[get-master-addr-by-name master1], ["sentinels", "master1"]], commands[:s1]
+    assert_equal [%w[get-master-addr-by-name master1], ["sentinels", "master1"]], commands[:s1]
     assert_equal [%w[auth foo], %w[role]], commands[:m1]
   end
 
@@ -252,8 +252,8 @@ def test_authentication_for_sentinel
 
     RedisMock.start(master) do |master_port|
       RedisMock.start(sentinel.call(master_port)) do |sen_port|
-        s = [{ host: '127.0.0.1', port: sen_port, password: 'foo' }]
-        r = Redis.new(name: 'master1', sentinels: s, role: :master)
+        s = [{ host: '127.0.0.1', port: sen_port }]
+        r = Redis.new(name: 'master1', sentinels: s, role: :master, sentinel_password: 'foo')
         assert r.ping
       end
     end
@@ -308,8 +308,8 @@ def test_authentication_for_sentinel_and_redis
 
     RedisMock.start(master) do |master_port|
       RedisMock.start(sentinel.call(master_port)) do |sen_port|
-        s = [{ host: '127.0.0.1', port: sen_port, password: 'foo' }]
-        r = Redis.new(name: 'master1', sentinels: s, role: :master, password: 'bar')
+        s = [{ host: '127.0.0.1', port: sen_port }]
+        r = Redis.new(name: 'master1', sentinels: s, role: :master, password: 'bar', sentinel_password: 'foo')
         assert r.ping
       end
     end
@@ -361,8 +361,8 @@ def test_authentication_with_acl
 
     RedisMock.start(master) do |master_port|
       RedisMock.start(sentinel.call(master_port)) do |sen_port|
-        s = [{ host: '127.0.0.1', port: sen_port, username: 'bob', password: 'foo' }]
-        r = Redis.new(name: 'master1', sentinels: s, role: :master, username: 'alice', password: 'bar')
+        s = [{ host: '127.0.0.1', port: sen_port }]
+        r = Redis.new(name: 'master1', sentinels: s, role: :master, username: 'alice', password: 'bar', sentinel_username: 'bob', sentinel_password: 'foo')
         assert r.ping
       end
     end

test/support/conf/redis-7.2.conf

@@ -0,0 +1,3 @@
+appendonly no
+save ""
+enable-debug-command yes