diff options
author | Robert Speicher <rspeicher@gmail.com> | 2017-07-11 13:38:27 -0400 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2017-07-11 13:38:27 -0400 |
commit | a97c1a4b6595e17bc4fbe6050618dba9c629fc1b (patch) | |
tree | c294ad39d25e77314ec46c1de11484e4cf445616 | |
parent | 2a5d2ecfbfa4494b1b58e869a9d8988af2f8a2b4 (diff) | |
download | gitlab-ce-rs-issue-34941.tar.gz |
Make `Redis::Wrapper#_raw_config` and `#fetch_config` more resilientrs-issue-34941
These two methods now handle two additional real-world possibilities:
1. `config/resque.yml` not being present
2. `config/resque.yml` being present but not containing YAML
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/34941
-rw-r--r-- | lib/gitlab/redis/wrapper.rb | 26 | ||||
-rw-r--r-- | spec/support/redis/redis_shared_examples.rb | 21 |
2 files changed, 35 insertions, 12 deletions
diff --git a/lib/gitlab/redis/wrapper.rb b/lib/gitlab/redis/wrapper.rb index 10c16a962ca..70b64c4863d 100644 --- a/lib/gitlab/redis/wrapper.rb +++ b/lib/gitlab/redis/wrapper.rb @@ -33,11 +33,16 @@ module Gitlab def _raw_config return @_raw_config if defined?(@_raw_config) - begin - @_raw_config = ERB.new(File.read(config_file_name)).result.freeze - rescue Errno::ENOENT - @_raw_config = false - end + @_raw_config = + begin + if filename = config_file_name + ERB.new(File.read(filename)).result.freeze + else + false + end + rescue Errno::ENOENT + false + end @_raw_config end @@ -116,7 +121,16 @@ module Gitlab end def fetch_config - self.class._raw_config ? YAML.load(self.class._raw_config)[@rails_env] : false + return false unless self.class._raw_config + + yaml = YAML.load(self.class._raw_config) + + # If the file has content but it's invalid YAML, `load` returns false + if yaml + yaml.fetch(@rails_env, false) + else + false + end end end end diff --git a/spec/support/redis/redis_shared_examples.rb b/spec/support/redis/redis_shared_examples.rb index 95a5df181c1..f4ebffee9c8 100644 --- a/spec/support/redis/redis_shared_examples.rb +++ b/spec/support/redis/redis_shared_examples.rb @@ -66,11 +66,8 @@ RSpec.shared_examples "redis_shared_examples" do describe '.url' do it 'withstands mutation' do - url1 = described_class.url - url2 = described_class.url - url1 << 'foobar' - - expect(url2).not_to end_with('foobar') + expect { described_class.url << 'foobar' } + .to raise_error(RuntimeError, "can't modify frozen String") end context 'when yml file with env variable' do @@ -97,6 +94,12 @@ RSpec.shared_examples "redis_shared_examples" do it 'returns false when the file does not exist' do expect(subject).to eq(false) end + + it "returns false when the filename can't be determined" do + expect(described_class).to receive(:config_file_name).and_return(nil) + + expect(subject).to eq(false) + end end describe '.with' do @@ -192,7 +195,13 @@ RSpec.shared_examples "redis_shared_examples" do it 'returns false when no config file is present' do allow(described_class).to receive(:_raw_config) { false } - expect(subject.send(:fetch_config)).to be_falsey + expect(subject.send(:fetch_config)).to eq false + end + + it 'returns false when config file is present but has invalid YAML' do + allow(described_class).to receive(:_raw_config) { "# development: true" } + + expect(subject.send(:fetch_config)).to eq false end end |