Merge branch '21645-mail_room_sentinel'

CHANGELOG.md

@@ -16,6 +16,7 @@ Please view this file on the master branch, on stable branches it's out of date.
   - Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose)
   - Fix filtering of milestones with quotes in title (airatshigapov)
   - Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison)
+  - Update mail_room and enable sentinel support to Reply By Email (!7101)
   - Simpler arguments passed to named_route on toggle_award_url helper method
   - Fix typo in framework css class. !7086 (Daniel Voogsgerd)
   - New issue board list dropdown stays open after adding a new list

Gemfile

@@ -326,7 +326,7 @@ gem 'newrelic_rpm', '~> 3.16'
 
 gem 'octokit', '~> 4.3.0'
 
-gem 'mail_room', '~> 0.8.1'
+gem 'mail_room', '~> 0.9.0'
 
 gem 'email_reply_parser', '~> 0.5.8'
 

Gemfile.lock

@@ -402,7 +402,7 @@ GEM
       systemu (~> 2.6.2)
     mail (2.6.4)
       mime-types (>= 1.16, < 4)
-    mail_room (0.8.1)
+    mail_room (0.9.0)
     method_source (0.8.2)
     mime-types (2.99.3)
     mimemagic (0.3.0)
@@ -893,7 +893,7 @@ DEPENDENCIES
   license_finder (~> 2.1.0)
   licensee (~> 8.0.0)
   loofah (~> 2.0.3)
-  mail_room (~> 0.8.1)
+  mail_room (~> 0.9.0)
   method_source (~> 0.8)
   minitest (~> 5.7.0)
   mousetrap-rails (~> 1.4.6)
@@ -994,4 +994,4 @@ DEPENDENCIES
   wikicloth (= 0.8.1)
 
 BUNDLED WITH
-   1.13.2
+   1.13.3

config/mail_room.yml

@@ -27,10 +27,25 @@
         :namespace: <%= Gitlab::Redis::SIDEKIQ_NAMESPACE %>
         :queue: email_receiver
         :worker: EmailReceiverWorker
+        <% if config[:sentinels] %>
+        :sentinels:
+          <% config[:sentinels].each do |sentinel| %>
+          -
+            :host: <%= sentinel[:host] %>
+            :port: <%= sentinel[:port] %>
+          <% end %>
+        <% end %>
 
       :arbitration_method: redis
       :arbitration_options:
         :redis_url: <%= config[:redis_url].to_json %>
         :namespace: <%= Gitlab::Redis::MAILROOM_NAMESPACE %>
-
+        <% if config[:sentinels] %>
+        :sentinels:
+          <% config[:sentinels].each do |sentinel| %>
+          -
+            :host: <%= sentinel[:host] %>
+            :port: <%= sentinel[:port] %>
+          <% end %>
+        <% end %>
   <% end %>

lib/gitlab/mail_room.rb

@@ -33,7 +33,12 @@ def fetch_config
         config[:mailbox] = 'inbox' if config[:mailbox].nil?
 
         if config[:enabled] && config[:address]
-          config[:redis_url] = Gitlab::Redis.new(rails_env).url
+          gitlab_redis = Gitlab::Redis.new(rails_env)
+          config[:redis_url] = gitlab_redis.url
+
+          if gitlab_redis.sentinels?
+            config[:sentinels] = gitlab_redis.sentinels
+          end
         end
 
         config

lib/gitlab/redis.rb

@@ -63,6 +63,14 @@ def url
       raw_config_hash[:url]
     end
 
+    def sentinels
+      raw_config_hash[:sentinels]
+    end
+
+    def sentinels?
+      sentinels && !sentinels.empty?
+    end
+
     private
 
     def redis_store_options

spec/config/mail_room_spec.rb

@@ -3,6 +3,8 @@
 describe 'mail_room.yml' do
   let(:config_path)   { 'config/mail_room.yml' }
   let(:configuration) { YAML.load(ERB.new(File.read(config_path)).result) }
+  before(:each) { clear_raw_config }
+  after(:each) { clear_raw_config }
 
   context 'when incoming email is disabled' do
     before do
@@ -20,6 +22,9 @@
   end
 
   context 'when incoming email is enabled' do
+    let(:redis_config) { Rails.root.join('spec/fixtures/config/redis_new_format_host.yml') }
+    let(:gitlab_redis) { Gitlab::Redis.new(Rails.env) }
+
     before do
       ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] = Rails.root.join('spec/fixtures/mail_room_enabled.yml').to_s
       Gitlab::MailRoom.reset_config!
@@ -30,8 +35,9 @@
     end
 
     it 'contains the intended configuration' do
-      expect(configuration[:mailboxes].length).to eq(1)
+      stub_const('Gitlab::Redis::CONFIG_FILE', redis_config)
 
+      expect(configuration[:mailboxes].length).to eq(1)
       mailbox = configuration[:mailboxes].first
 
       expect(mailbox[:host]).to eq('imap.gmail.com')
@@ -42,10 +48,26 @@
       expect(mailbox[:password]).to eq('[REDACTED]')
       expect(mailbox[:name]).to eq('inbox')
 
-      redis_url = Gitlab::Redis.url
+      redis_url = gitlab_redis.url
+      sentinels = gitlab_redis.sentinels
 
+      expect(mailbox[:delivery_options][:redis_url]).to be_present
       expect(mailbox[:delivery_options][:redis_url]).to eq(redis_url)
+
+      expect(mailbox[:delivery_options][:sentinels]).to be_present
+      expect(mailbox[:delivery_options][:sentinels]).to eq(sentinels)
+
+      expect(mailbox[:arbitration_options][:redis_url]).to be_present
       expect(mailbox[:arbitration_options][:redis_url]).to eq(redis_url)
+
+      expect(mailbox[:arbitration_options][:sentinels]).to be_present
+      expect(mailbox[:arbitration_options][:sentinels]).to eq(sentinels)
     end
   end
+
+  def clear_raw_config
+    Gitlab::Redis.remove_instance_variable(:@_raw_config)
+  rescue NameError
+    # raised if @_raw_config was not set; ignore
+  end
 end

spec/lib/gitlab/redis_spec.rb

@@ -116,6 +116,55 @@
     end
   end
 
+  describe '#sentinels' do
+    subject { described_class.new(Rails.env).sentinels }
+
+    context 'when sentinels are defined' do
+      let(:config) { Rails.root.join('spec/fixtures/config/redis_new_format_host.yml') }
+
+      it 'returns an array of hashes with host and port keys' do
+        stub_const("#{described_class}::CONFIG_FILE", config)
+
+        is_expected.to include(host: 'localhost', port: 26380)
+        is_expected.to include(host: 'slave2', port: 26381)
+      end
+    end
+
+    context 'when sentinels are not defined' do
+      let(:config) { Rails.root.join('spec/fixtures/config/redis_old_format_host.yml') }
+
+      it 'returns nil' do
+        stub_const("#{described_class}::CONFIG_FILE", config)
+
+        is_expected.to be_nil
+      end
+    end
+  end
+
+  describe '#sentinels?' do
+    subject { described_class.new(Rails.env).sentinels? }
+
+    context 'when sentinels are defined' do
+      let(:config) { Rails.root.join('spec/fixtures/config/redis_new_format_host.yml') }
+
+      it 'returns true' do
+        stub_const("#{described_class}::CONFIG_FILE", config)
+
+        is_expected.to be_truthy
+      end
+    end
+
+    context 'when sentinels are not defined' do
+      let(:config) { Rails.root.join('spec/fixtures/config/redis_old_format_host.yml') }
+
+      it 'returns false' do
+        stub_const("#{described_class}::CONFIG_FILE", config)
+
+        is_expected.to be_falsey
+      end
+    end
+  end
+
   describe '#raw_config_hash' do
     it 'returns default redis url when no config file is present' do
       expect(subject).to receive(:fetch_config) { false }