From f9126fbe0a73aa6d8d61be2eb249260fa29ac461 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 25 Oct 2016 15:50:41 +0200 Subject: [PATCH] Updated mail_room and added sentinel support to Reply by Email --- CHANGELOG.md | 1 + Gemfile | 2 +- Gemfile.lock | 6 ++--- config/mail_room.yml | 17 +++++++++++- lib/gitlab/mail_room.rb | 7 ++++- lib/gitlab/redis.rb | 8 ++++++ spec/config/mail_room_spec.rb | 26 +++++++++++++++++-- spec/lib/gitlab/redis_spec.rb | 49 +++++++++++++++++++++++++++++++++++ 8 files changed, 108 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e25f0b2ceb..921578ad9a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix HipChat notifications rendering (airatshigapov, eisnerd) - Add hover to trash icon in notes !7008 (blackst0ne) - Escape ref and path for relative links !6050 (winniehell) + - 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: Backup restore doesn't clear cache - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method diff --git a/Gemfile b/Gemfile index 46245ab62d1..b810f6e8685 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index 442184b9228..536435d3cf6 100644 --- a/Gemfile.lock +++ b/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 diff --git a/config/mail_room.yml b/config/mail_room.yml index 68697bd1dc4..b026d510f1b 100644 --- a/config/mail_room.yml +++ b/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 %> diff --git a/lib/gitlab/mail_room.rb b/lib/gitlab/mail_room.rb index 12999a90a29..a5220d92312 100644 --- a/lib/gitlab/mail_room.rb +++ b/lib/gitlab/mail_room.rb @@ -33,7 +33,12 @@ module Gitlab 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 diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index c649da8c426..9226da2d6b1 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -63,6 +63,14 @@ module Gitlab raw_config_hash[:url] end + def sentinels + raw_config_hash[:sentinels] + end + + def sentinels? + sentinels && !sentinels.empty? + end + private def redis_store_options diff --git a/spec/config/mail_room_spec.rb b/spec/config/mail_room_spec.rb index c5d3cd70acc..22bf3055538 100644 --- a/spec/config/mail_room_spec.rb +++ b/spec/config/mail_room_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' 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 @@ describe 'mail_room.yml' do 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 @@ describe 'mail_room.yml' do 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 @@ describe 'mail_room.yml' do 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 diff --git a/spec/lib/gitlab/redis_spec.rb b/spec/lib/gitlab/redis_spec.rb index 74ff85e132a..e5406fb2d33 100644 --- a/spec/lib/gitlab/redis_spec.rb +++ b/spec/lib/gitlab/redis_spec.rb @@ -116,6 +116,55 @@ describe Gitlab::Redis do 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 }