Commit c9a56dac authored by Stan Hu's avatar Stan Hu

Merge branch '19572-storage-path-validations-fail-incorrectly-for-some-patterns' into 'master'

Fix a false positive where similar un-nested storage paths were reported as nested

Closes #19572

See merge request !5145
parents e599afac 89589007
...@@ -3,22 +3,27 @@ def storage_name_valid?(name) ...@@ -3,22 +3,27 @@ def storage_name_valid?(name)
end end
def find_parent_path(name, path) def find_parent_path(name, path)
parent = Pathname.new(path).realpath.parent
Gitlab.config.repositories.storages.detect do |n, p| 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
end end
def error(message) def storage_validation_error(message)
raise "#{message}. Please fix this in your gitlab.yml before starting GitLab." raise "#{message}. Please fix this in your gitlab.yml before starting GitLab."
end end
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| 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) parent_name, _parent_path = find_parent_path(name, path)
if parent_name 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 end
end end
validate_storages unless Rails.env.test?
require 'spec_helper' require 'spec_helper'
require_relative '../../config/initializers/6_validations.rb'
describe '6_validations', lib: true do 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 context 'with correct settings' do
before 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 end
it 'passes through' do it 'passes through' do
expect { load_validations }.not_to raise_error expect { validate_storages }.not_to raise_error
end end
end end
context 'with invalid storage names' do context 'with invalid storage names' do
before do before do
mock_storages('name with spaces' => '/a/b/c') mock_storages('name with spaces' => 'tmp/tests/paths/a/b/c')
end end
it 'throws an error' do 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
end end
context 'with nested storage paths' do context 'with nested storage paths' do
before 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 end
it 'throws an error' do 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
end end
def mock_storages(storages) context 'with similar but un-nested storage paths' do
allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) before do
mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/c2')
end
it 'passes through' do
expect { validate_storages }.not_to raise_error
end
end end
def load_validations def mock_storages(storages)
load File.join(__dir__, '../../config/initializers/6_validations.rb') allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment