From 72f59ddf4c9d276bd565892c0cf79d5622906090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 11 Jul 2016 21:29:13 -0400 Subject: [PATCH 1/2] Use Pathname to make the repository storage path validations more robust --- config/initializers/6_validations.rb | 11 ++++++----- spec/initializers/6_validations_spec.rb | 26 ++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/config/initializers/6_validations.rb b/config/initializers/6_validations.rb index 3ba9e36c567..83acdc8ab54 100644 --- a/config/initializers/6_validations.rb +++ b/config/initializers/6_validations.rb @@ -3,22 +3,23 @@ def storage_name_valid?(name) end def find_parent_path(name, path) + parent = Pathname.new(path).realpath.parent Gitlab.config.repositories.storages.detect do |n, p| - name != n && path.chomp('/').start_with?(p.chomp('/')) + name != n && Pathname.new(p).realpath == parent end end -def error(message) +def storage_validation_error(message) raise "#{message}. Please fix this in your gitlab.yml before starting GitLab." end -error('No repository storage path defined') if Gitlab.config.repositories.storages.empty? +storage_validation_error('No repository storage path defined') if Gitlab.config.repositories.storages.empty? Gitlab.config.repositories.storages.each do |name, path| - error("\"#{name}\" is not a valid storage name") unless storage_name_valid?(name) + storage_validation_error("\"#{name}\" is not a valid storage name") unless storage_name_valid?(name) parent_name, _parent_path = find_parent_path(name, path) if parent_name - error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages") + storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages") end end diff --git a/spec/initializers/6_validations_spec.rb b/spec/initializers/6_validations_spec.rb index 5178bd130f4..05d0aa63488 100644 --- a/spec/initializers/6_validations_spec.rb +++ b/spec/initializers/6_validations_spec.rb @@ -1,9 +1,19 @@ require 'spec_helper' describe '6_validations', lib: true do + before :all do + FileUtils.mkdir_p('tmp/tests/paths/a/b/c/d') + FileUtils.mkdir_p('tmp/tests/paths/a/b/c2') + FileUtils.mkdir_p('tmp/tests/paths/a/b/d') + end + + after :all do + FileUtils.rm_rf('tmp/tests/paths') + end + context 'with correct settings' do before do - mock_storages('foo' => '/a/b/c', 'bar' => 'a/b/d') + mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/d') end it 'passes through' do @@ -13,7 +23,7 @@ describe '6_validations', lib: true do context 'with invalid storage names' do before do - mock_storages('name with spaces' => '/a/b/c') + mock_storages('name with spaces' => 'tmp/tests/paths/a/b/c') end it 'throws an error' do @@ -23,7 +33,7 @@ describe '6_validations', lib: true do context 'with nested storage paths' do before do - mock_storages('foo' => '/a/b/c', 'bar' => '/a/b/c/d') + mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/c/d') end it 'throws an error' do @@ -31,6 +41,16 @@ describe '6_validations', lib: true do end end + context 'with similar but un-nested storage paths' do + before do + mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/c2') + end + + it 'passes through' do + expect { load_validations }.not_to raise_error + end + end + def mock_storages(storages) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end From 89589007aee6780c3dea2d44df12e9509a22f975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 18 Jul 2016 17:28:26 -0400 Subject: [PATCH 2/2] Skip repository storage path valitaions on test environment Storage path are not created until `TestEnv.init`, so we must skip their validation on initialization. --- config/initializers/6_validations.rb | 16 ++++++++++------ spec/initializers/6_validations_spec.rb | 13 +++++-------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/config/initializers/6_validations.rb b/config/initializers/6_validations.rb index 83acdc8ab54..37746968675 100644 --- a/config/initializers/6_validations.rb +++ b/config/initializers/6_validations.rb @@ -13,13 +13,17 @@ def storage_validation_error(message) raise "#{message}. Please fix this in your gitlab.yml before starting GitLab." end -storage_validation_error('No repository storage path defined') if Gitlab.config.repositories.storages.empty? +def validate_storages + storage_validation_error('No repository storage path defined') if Gitlab.config.repositories.storages.empty? -Gitlab.config.repositories.storages.each do |name, path| - storage_validation_error("\"#{name}\" is not a valid storage name") unless storage_name_valid?(name) + Gitlab.config.repositories.storages.each do |name, path| + storage_validation_error("\"#{name}\" is not a valid storage name") unless storage_name_valid?(name) - parent_name, _parent_path = find_parent_path(name, path) - if parent_name - storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages") + parent_name, _parent_path = find_parent_path(name, path) + if parent_name + storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages") + end end end + +validate_storages unless Rails.env.test? diff --git a/spec/initializers/6_validations_spec.rb b/spec/initializers/6_validations_spec.rb index 05d0aa63488..baab30f482f 100644 --- a/spec/initializers/6_validations_spec.rb +++ b/spec/initializers/6_validations_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require_relative '../../config/initializers/6_validations.rb' describe '6_validations', lib: true do before :all do @@ -17,7 +18,7 @@ describe '6_validations', lib: true do end it 'passes through' do - expect { load_validations }.not_to raise_error + expect { validate_storages }.not_to raise_error end end @@ -27,7 +28,7 @@ describe '6_validations', lib: true do end it 'throws an error' do - expect { load_validations }.to raise_error('"name with spaces" is not a valid storage name. Please fix this in your gitlab.yml before starting GitLab.') + expect { validate_storages }.to raise_error('"name with spaces" is not a valid storage name. Please fix this in your gitlab.yml before starting GitLab.') end end @@ -37,7 +38,7 @@ describe '6_validations', lib: true do end it 'throws an error' do - expect { load_validations }.to raise_error('bar is a nested path of foo. Nested paths are not supported for repository storages. Please fix this in your gitlab.yml before starting GitLab.') + expect { validate_storages }.to raise_error('bar is a nested path of foo. Nested paths are not supported for repository storages. Please fix this in your gitlab.yml before starting GitLab.') end end @@ -47,15 +48,11 @@ describe '6_validations', lib: true do end it 'passes through' do - expect { load_validations }.not_to raise_error + expect { validate_storages }.not_to raise_error end end def mock_storages(storages) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end - - def load_validations - load File.join(__dir__, '../../config/initializers/6_validations.rb') - end end