From 15ccb81c4b8ea2af312845c30de6398632d2a1ae Mon Sep 17 00:00:00 2001 From: DJ Mountney Date: Thu, 16 Mar 2017 00:44:09 +0000 Subject: 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 --- lib/gitlab/redis.rb | 8 ++++-- spec/config/mail_room_spec.rb | 61 +++++++++++++++++++++++++------------------ spec/lib/gitlab/redis_spec.rb | 59 ++++++++++++++++++++--------------------- 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 -- cgit v1.2.1