From 1142e2c32eb0c4f4040c13d40d73fb8f84f35628 Mon Sep 17 00:00:00 2001 From: "Jacob Vosmaer (GitLab)" Date: Wed, 27 Jun 2018 16:20:03 +0000 Subject: Migrate storage nesting check to Gitaly --- GITALY_SERVER_VERSION | 2 +- config/initializers/1_settings.rb | 2 +- config/initializers/6_validations.rb | 27 --------------------- spec/initializers/6_validations_spec.rb | 43 --------------------------------- 4 files changed, 2 insertions(+), 72 deletions(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 3f667dbcd89..8b27ad70f93 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.108.0 +0.109.0 diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 3d3448cb4d6..837e577bc91 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -394,7 +394,7 @@ repositories_storages = Settings.repositories.storages.values repository_downloads_path = Settings.gitlab['repository_downloads_path'].to_s.gsub(%r{/$}, '') repository_downloads_full_path = File.expand_path(repository_downloads_path, Settings.gitlab['user_home']) -# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1237 +# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1255 Gitlab::GitalyClient::StorageSettings.allow_disk_access do if repository_downloads_path.blank? || repositories_storages.any? { |rs| [repository_downloads_path, repository_downloads_full_path].include?(rs.legacy_disk_path.gsub(%r{/$}, '')) } Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive') diff --git a/config/initializers/6_validations.rb b/config/initializers/6_validations.rb index ff6865608f0..bf9e5a50382 100644 --- a/config/initializers/6_validations.rb +++ b/config/initializers/6_validations.rb @@ -2,20 +2,6 @@ def storage_name_valid?(name) !!(name =~ /\A[a-zA-Z0-9\-_]+\z/) end -def find_parent_path(name, path) - parent = Pathname.new(path).realpath.parent - Gitlab.config.repositories.storages.detect do |n, rs| - name != n && Pathname.new(rs.legacy_disk_path).realpath == parent - end -rescue Errno::EIO, Errno::ENOENT => e - warning = "WARNING: couldn't verify #{path} (#{name}). "\ - "If this is an external storage, it might be offline." - message = "#{warning}\n#{e.message}" - Rails.logger.error("#{message}\n\t" + e.backtrace.join("\n\t")) - - nil -end - def storage_validation_error(message) raise "#{message}. Please fix this in your gitlab.yml before starting GitLab." end @@ -37,17 +23,4 @@ def validate_storages_config end end -# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1237 -def validate_storages_paths - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab.config.repositories.storages.each do |name, repository_storage| - parent_name, _parent_path = find_parent_path(name, repository_storage.legacy_disk_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 -end - validate_storages_config -validate_storages_paths unless Rails.env.test? || ENV['SKIP_STORAGE_VALIDATION'] == 'true' diff --git a/spec/initializers/6_validations_spec.rb b/spec/initializers/6_validations_spec.rb index 8d9dc092547..f96e5a2133f 100644 --- a/spec/initializers/6_validations_spec.rb +++ b/spec/initializers/6_validations_spec.rb @@ -44,49 +44,6 @@ describe '6_validations' do end end - describe 'validate_storages_paths' do - context 'with correct settings' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'), 'bar' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/d')) - end - - it 'passes through' do - expect { validate_storages_paths }.not_to raise_error - end - end - - context 'with nested storage paths' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'), 'bar' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c/d')) - end - - it 'throws an error' do - expect { validate_storages_paths }.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 - - context 'with similar but un-nested storage paths' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'), 'bar' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c2')) - end - - it 'passes through' do - expect { validate_storages_paths }.not_to raise_error - end - end - - describe 'inaccessible storage' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/a/path/that/does/not/exist')) - end - - it 'passes through with a warning' do - expect(Rails.logger).to receive(:error) - expect { validate_storages_paths }.not_to raise_error - end - end - end - def mock_storages(storages) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end -- cgit v1.2.3