From 10d86fbb35e87369778a494b7776db920e9018f8 Mon Sep 17 00:00:00 2001 From: Nusret Ozates Date: Thu, 20 Oct 2022 03:25:16 +0300 Subject: [PATCH] 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 Co-authored-by: Sojan Jose --- .env.example | 5 ++++ lib/redis/config.rb | 11 ++++++-- spec/lib/redis/config_spec.rb | 53 ++++++++++++++++++++++++++++------- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/.env.example b/.env.example index 12262576e..830ee7bce 100644 --- a/.env.example +++ b/.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 diff --git a/lib/redis/config.rb b/lib/redis/config.rb index 8e91b5bb0..f35c32c24 100644 --- a/lib/redis/config.rb +++ b/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) - { host: host, port: port.presence || DEFAULT_SENTINEL_PORT, password: base_config[:password] } + sentinel_url_config(sentinel_url) end # over-write redis url as redis://:@/ when using sentinel diff --git a/spec/lib/redis/config_spec.rb b/spec/lib/redis/config_spec.rb index a9093c341..02e31339c 100644 --- a/spec/lib/redis/config_spec.rb +++ b/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) - allow(ENV).to receive(:fetch).with('REDIS_PASSWORD', nil).and_return(redis_pasword) - allow(ENV).to receive(:fetch).with('REDIS_SENTINELS', nil).and_return('') - allow(ENV).to receive(:fetch).with('REDIS_SENTINEL_MASTER_NAME', 'mymaster').and_return('') - described_class.config + with_modified_env REDIS_URL: redis_url, REDIS_PASSWORD: redis_pasword, REDIS_SENTINELS: '', REDIS_SENTINEL_MASTER_NAME: '' do + described_class.config + end 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) - allow(ENV).to receive(:fetch).with('REDIS_PASSWORD', nil).and_return(redis_pasword) - allow(ENV).to receive(:fetch).with('REDIS_SENTINELS', nil).and_return(redis_sentinels) - allow(ENV).to receive(:fetch).with('REDIS_SENTINEL_MASTER_NAME', 'mymaster').and_return(redis_master_name) - described_class.config + with_modified_env REDIS_URL: redis_url, REDIS_PASSWORD: redis_pasword, REDIS_SENTINELS: redis_sentinels, + REDIS_SENTINEL_MASTER_NAME: redis_master_name do + described_class.config + end 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