chore: Ability to Remove password info from sentinel config (#4550)

Introduce the REDIS_SENTINEL_PASSWORD environment variable to customize the behaviour of sentinel passwords. 

Co-authored-by: EXT02D22861 <nusret.ozates@consultant.turkcell.com.tr>
Co-authored-by: Sojan Jose <sojan@pepalo.com>

.env.example

@@ -34,6 +34,11 @@ REDIS_SENTINELS=
 # You can find list of master using "SENTINEL masters" command
 REDIS_SENTINEL_MASTER_NAME=
 
+# By default Chatwoot will pass REDIS_PASSWORD as the password value for sentinels
+# Use the following environment variable to customize passwords for sentinels. 
+# Use empty string if sentinels are configured with out passwords
+# REDIS_SENTINEL_PASSWORD=
+
 # Redis premium breakage in heroku fix
 # enable the following configuration
 # ref: https://github.com/chatwoot/chatwoot/issues/2420

lib/redis/config.rb

@@ -23,13 +23,20 @@ module Redis::Config
       ENV.fetch('REDIS_SENTINELS', nil).presence
     end
 
+    def sentinel_url_config(sentinel_url)
+      host, port = sentinel_url.split(':').map(&:strip)
+      sentinel_url_config = { host: host, port: port || DEFAULT_SENTINEL_PORT }
+      password = ENV.fetch('REDIS_SENTINEL_PASSWORD', base_config[:password])
+      sentinel_url_config[:password] = password if password.present?
+      sentinel_url_config
+    end
+
     def sentinel_config
       redis_sentinels = ENV.fetch('REDIS_SENTINELS', nil)
 
       # expected format for REDIS_SENTINELS url string is host1:port1, host2:port2
       sentinels = redis_sentinels.split(',').map do |sentinel_url|
-        host, port = sentinel_url.split(':').map(&:strip)
+        sentinel_url_config(sentinel_url)
-        { host: host, port: port.presence || DEFAULT_SENTINEL_PORT, password: base_config[:password] }
       end
 
       # over-write redis url as redis://:<your-redis-password>@<master-name>/ when using sentinel

spec/lib/redis/config_spec.rb

@@ -7,11 +7,9 @@ describe ::Redis::Config do
 
     before do
       described_class.instance_variable_set(:@config, nil)
-      allow(ENV).to receive(:fetch).with('REDIS_URL', 'redis://127.0.0.1:6379').and_return(redis_url)
+      with_modified_env REDIS_URL: redis_url, REDIS_PASSWORD: redis_pasword, REDIS_SENTINELS: '', REDIS_SENTINEL_MASTER_NAME: '' do
-      allow(ENV).to receive(:fetch).with('REDIS_PASSWORD', nil).and_return(redis_pasword)
+        described_class.config
-      allow(ENV).to receive(:fetch).with('REDIS_SENTINELS', nil).and_return('')
+      end
-      allow(ENV).to receive(:fetch).with('REDIS_SENTINEL_MASTER_NAME', 'mymaster').and_return('')
-      described_class.config
     end
 
     it 'checks for app redis config' do
@@ -38,11 +36,10 @@ describe ::Redis::Config do
 
     before do
       described_class.instance_variable_set(:@config, nil)
-      allow(ENV).to receive(:fetch).with('REDIS_URL', 'redis://127.0.0.1:6379').and_return(redis_url)
+      with_modified_env REDIS_URL: redis_url, REDIS_PASSWORD: redis_pasword, REDIS_SENTINELS: redis_sentinels,
-      allow(ENV).to receive(:fetch).with('REDIS_PASSWORD', nil).and_return(redis_pasword)
+                        REDIS_SENTINEL_MASTER_NAME: redis_master_name do
-      allow(ENV).to receive(:fetch).with('REDIS_SENTINELS', nil).and_return(redis_sentinels)
+        described_class.config
-      allow(ENV).to receive(:fetch).with('REDIS_SENTINEL_MASTER_NAME', 'mymaster').and_return(redis_master_name)
+      end
-      described_class.config
     end
 
     it 'checks for app redis config' do
@@ -50,5 +47,41 @@ describe ::Redis::Config do
       expect(described_class.app[:url]).to eq("redis://#{redis_master_name}")
       expect(described_class.app[:sentinels]).to match_array(expected_sentinels)
     end
+
+    context 'when redis sentinel is used with REDIS_SENTINEL_PASSWORD empty string' do
+      let(:redis_sentinel_password) { '' }
+
+      before do
+        described_class.instance_variable_set(:@config, nil)
+        with_modified_env REDIS_URL: redis_url, REDIS_PASSWORD: redis_pasword, REDIS_SENTINELS: redis_sentinels,
+                          REDIS_SENTINEL_MASTER_NAME: redis_master_name, REDIS_SENTINEL_PASSWORD: redis_sentinel_password do
+          described_class.config
+        end
+      end
+
+      it 'checks for app redis config and sentinel passwords will be empty' do
+        expect(described_class.app.keys).to match_array([:url, :password, :sentinels, :network_timeout, :reconnect_attempts, :ssl_params])
+        expect(described_class.app[:url]).to eq("redis://#{redis_master_name}")
+        expect(described_class.app[:sentinels]).to match_array(expected_sentinels.map { |s| s.except(:password) })
+      end
+    end
+
+    context 'when redis sentinel is used with REDIS_SENTINEL_PASSWORD' do
+      let(:redis_sentinel_password) { 'sentinel_password' }
+
+      before do
+        described_class.instance_variable_set(:@config, nil)
+        with_modified_env REDIS_URL: redis_url, REDIS_PASSWORD: redis_pasword, REDIS_SENTINELS: redis_sentinels,
+                          REDIS_SENTINEL_MASTER_NAME: redis_master_name, REDIS_SENTINEL_PASSWORD: redis_sentinel_password do
+          described_class.config
+        end
+      end
+
+      it 'checks for app redis config and redis password is replaced in sentinel config' do
+        expect(described_class.app.keys).to match_array([:url, :password, :sentinels, :network_timeout, :reconnect_attempts, :ssl_params])
+        expect(described_class.app[:url]).to eq("redis://#{redis_master_name}")
+        expect(described_class.app[:sentinels]).to match_array(expected_sentinels.map { |s| s.merge(password: redis_sentinel_password) })
+      end
+    end
   end
 end