Merge branch 'dm-fix-mailroom-config' into 'master'

Fix reply by email by fixing config/mail_room.yml to be interpretable without ActiveSupport

See merge request !9979

3 files changed, 70 insertions, 58 deletions

diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb
index 62dbd429156..bc5370de32a 100644
--- a/lib/gitlab/redis.rb
+++ b/lib/gitlab/redis.rb
@@ -1,6 +1,7 @@
 # This file should not have any direct dependency on Rails environment
 # please require all dependencies below:
 require 'active_support/core_ext/hash/keys'
+require 'active_support/core_ext/module/delegation'
 
 module Gitlab
   class Redis
@@ -9,7 +10,6 @@ module Gitlab
     SIDEKIQ_NAMESPACE = 'resque:gitlab'.freeze
     MAILROOM_NAMESPACE = 'mail_room:gitlab'.freeze
     DEFAULT_REDIS_URL = 'redis://localhost:6379'.freeze
-    CONFIG_FILE = File.expand_path('../../config/resque.yml', __dir__)
 
     class << self
       delegate :params, :url, to: :new
@@ -33,13 +33,17 @@ module Gitlab
         return @_raw_config if defined?(@_raw_config)
 
         begin
-          @_raw_config = ERB.new(File.read(CONFIG_FILE)).result.freeze
+          @_raw_config = ERB.new(File.read(config_file)).result.freeze
         rescue Errno::ENOENT
           @_raw_config = false
         end
 
         @_raw_config
       end
+
+      def config_file
+        ENV['GITLAB_REDIS_CONFIG_FILE'] || File.expand_path('../../config/resque.yml', __dir__)
+      end
     end
 
     def initialize(rails_env = nil)
diff --git a/spec/config/mail_room_spec.rb b/spec/config/mail_room_spec.rb
index 0b8ff006d22..092048a6259 100644
--- a/spec/config/mail_room_spec.rb
+++ b/spec/config/mail_room_spec.rb
@@ -1,20 +1,36 @@
 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 }
+  include StubENV
 
-  context 'when incoming email is disabled' do
-    before do
-      ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] = Rails.root.join('spec/fixtures/config/mail_room_disabled.yml').to_s
-      Gitlab::MailRoom.reset_config!
-    end
+  let(:mailroom_config_path) { 'config/mail_room.yml' }
+  let(:gitlab_config_path) { 'config/mail_room.yml' }
+  let(:redis_config_path) { 'config/resque.yml' }
 
-    after do
-      ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] = nil
-    end
+  let(:configuration) do
+    vars = {
+      'MAIL_ROOM_GITLAB_CONFIG_FILE' => absolute_path(gitlab_config_path),
+      'GITLAB_REDIS_CONFIG_FILE' => absolute_path(redis_config_path)
+    }
+    cmd = "puts ERB.new(File.read(#{absolute_path(mailroom_config_path).inspect})).result"
+
+    output, status = Gitlab::Popen.popen(%W(ruby -rerb -e #{cmd}), absolute_path('config'), vars)
+    raise "Error interpreting #{mailroom_config_path}: #{output}" unless status.zero?
+
+    YAML.load(output)
+  end
+
+  before(:each) do
+    stub_env('GITLAB_REDIS_CONFIG_FILE', absolute_path(redis_config_path))
+    clear_redis_raw_config
+  end
+
+  after(:each) do
+    clear_redis_raw_config
+  end
+
+  context 'when incoming email is disabled' do
+    let(:gitlab_config_path) { 'spec/fixtures/config/mail_room_disabled.yml' }
 
     it 'contains no configuration' do
       expect(configuration[:mailboxes]).to be_nil
@@ -22,21 +38,12 @@ 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/config/mail_room_enabled.yml').to_s
-      Gitlab::MailRoom.reset_config!
-    end
+    let(:gitlab_config_path) { 'spec/fixtures/config/mail_room_enabled.yml' }
+    let(:redis_config_path) { 'spec/fixtures/config/redis_new_format_host.yml' }
 
-    after do
-      ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] = nil
-    end
+    let(:gitlab_redis) { Gitlab::Redis.new(Rails.env) }
 
     it 'contains the intended configuration' do
-      stub_const('Gitlab::Redis::CONFIG_FILE', redis_config)
-
       expect(configuration[:mailboxes].length).to eq(1)
       mailbox = configuration[:mailboxes].first
 
@@ -66,9 +73,13 @@ describe 'mail_room.yml' do
     end
   end
 
-  def clear_raw_config
+  def clear_redis_raw_config
     Gitlab::Redis.remove_instance_variable(:@_raw_config)
   rescue NameError
     # raised if @_raw_config was not set; ignore
   end
+
+  def absolute_path(path)
+    Rails.root.join(path).to_s
+  end
 end
diff --git a/spec/lib/gitlab/redis_spec.rb b/spec/lib/gitlab/redis_spec.rb
index 917c5c46db1..8b77c925705 100644
--- a/spec/lib/gitlab/redis_spec.rb
+++ b/spec/lib/gitlab/redis_spec.rb
@@ -3,8 +3,16 @@ require 'spec_helper'
 describe Gitlab::Redis do
   include StubENV
 
-  before(:each) { clear_raw_config }
-  after(:each) { clear_raw_config }
+  let(:config) { 'config/resque.yml' }
+
+  before(:each) do
+    stub_env('GITLAB_REDIS_CONFIG_FILE', Rails.root.join(config).to_s)
+    clear_raw_config
+  end
+
+  after(:each) do
+    clear_raw_config
+  end
 
   describe '.params' do
     subject { described_class.params }
@@ -18,22 +26,22 @@ describe Gitlab::Redis do
     end
 
     context 'when url contains unix socket reference' do
-      let(:config_old) { Rails.root.join('spec/fixtures/config/redis_old_format_socket.yml').to_s }
-      let(:config_new) { Rails.root.join('spec/fixtures/config/redis_new_format_socket.yml').to_s }
+      let(:config_old) { 'spec/fixtures/config/redis_old_format_socket.yml' }
+      let(:config_new) { 'spec/fixtures/config/redis_new_format_socket.yml' }
 
       context 'with old format' do
-        it 'returns path key instead' do
-          stub_const("#{described_class}::CONFIG_FILE", config_old)
+        let(:config) { config_old }
 
+        it 'returns path key instead' do
           is_expected.to include(path: '/path/to/old/redis.sock')
           is_expected.not_to have_key(:url)
         end
       end
 
       context 'with new format' do
-        it 'returns path key instead' do
-          stub_const("#{described_class}::CONFIG_FILE", config_new)
+        let(:config) { config_new }
 
+        it 'returns path key instead' do
           is_expected.to include(path: '/path/to/redis.sock')
           is_expected.not_to have_key(:url)
         end
@@ -41,22 +49,22 @@ describe Gitlab::Redis do
     end
 
     context 'when url is host based' do
-      let(:config_old) { Rails.root.join('spec/fixtures/config/redis_old_format_host.yml') }
-      let(:config_new) { Rails.root.join('spec/fixtures/config/redis_new_format_host.yml') }
+      let(:config_old) { 'spec/fixtures/config/redis_old_format_host.yml' }
+      let(:config_new) { 'spec/fixtures/config/redis_new_format_host.yml' }
 
       context 'with old format' do
-        it 'returns hash with host, port, db, and password' do
-          stub_const("#{described_class}::CONFIG_FILE", config_old)
+        let(:config) { config_old }
 
+        it 'returns hash with host, port, db, and password' do
           is_expected.to include(host: 'localhost', password: 'mypassword', port: 6379, db: 99)
           is_expected.not_to have_key(:url)
         end
       end
 
       context 'with new format' do
-        it 'returns hash with host, port, db, and password' do
-          stub_const("#{described_class}::CONFIG_FILE", config_new)
+        let(:config) { config_new }
 
+        it 'returns hash with host, port, db, and password' do
           is_expected.to include(host: 'localhost', password: 'mynewpassword', port: 6379, db: 99)
           is_expected.not_to have_key(:url)
         end
@@ -74,15 +82,13 @@ describe Gitlab::Redis do
     end
 
     context 'when yml file with env variable' do
-      let(:redis_config) { Rails.root.join('spec/fixtures/config/redis_config_with_env.yml') }
+      let(:config) { 'spec/fixtures/config/redis_config_with_env.yml' }
 
       before  do
         stub_env('TEST_GITLAB_REDIS_URL', 'redis://redishost:6379')
       end
 
       it 'reads redis url from env variable' do
-        stub_const("#{described_class}::CONFIG_FILE", redis_config)
-
         expect(described_class.url).to eq 'redis://redishost:6379'
       end
     end
@@ -90,14 +96,13 @@ describe Gitlab::Redis do
 
   describe '._raw_config' do
     subject { described_class._raw_config }
+    let(:config) { '/var/empty/doesnotexist' }
 
     it 'should be frozen' do
       expect(subject).to be_frozen
     end
 
     it 'returns false when the file does not exist' do
-      stub_const("#{described_class}::CONFIG_FILE", '/var/empty/doesnotexist')
-
       expect(subject).to eq(false)
     end
   end
@@ -134,22 +139,18 @@ describe Gitlab::Redis 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') }
+      let(:config) { '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') }
+      let(:config) { '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
@@ -159,21 +160,17 @@ describe Gitlab::Redis 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') }
+      let(:config) { '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') }
+      let(:config) { '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