Merge branch 'mark-legacy-git-access' into 'master'

Route path lookups through legacy_disk_path

See merge request gitlab-org/gitlab-ce!17743

lib/system_check/orphans/repository_check.rb

@@ -6,10 +6,12 @@ module SystemCheck
       def multi_check
         Gitlab.config.repositories.storages.each do |storage_name, repository_storage|
+          storage_path = repository_storage.legacy_disk_path
+
           $stdout.puts
-          $stdout.puts "* Storage: #{storage_name} (#{repository_storage['path']})".color(:yellow)
-          repositories = disk_repositories(repository_storage['path'])
+          $stdout.puts "* Storage: #{storage_name} (#{storage_path})".color(:yellow)
+          repositories = disk_repositories(storage_path)
           orphans = (repositories - fetch_repositories(storage_name))
           print_orphans(orphans, storage_name)

lib/tasks/gitlab/check.rake

@@ -61,7 +61,7 @@ namespace :gitlab do
       puts "Repo base directory exists?"
       Gitlab.config.repositories.storages.each do |name, repository_storage|
-        repo_base_path = repository_storage['path']
+        repo_base_path = repository_storage.legacy_disk_path
         print "#{name}... "
         if File.exist?(repo_base_path)
@@ -86,7 +86,7 @@ namespace :gitlab do
       puts "Repo storage directories are symlinks?"
       Gitlab.config.repositories.storages.each do |name, repository_storage|
-        repo_base_path = repository_storage['path']
+        repo_base_path = repository_storage.legacy_disk_path
         print "#{name}... "
         unless File.exist?(repo_base_path)
@@ -110,7 +110,7 @@ namespace :gitlab do
       puts "Repo paths access is drwxrws---?"
       Gitlab.config.repositories.storages.each do |name, repository_storage|
-        repo_base_path = repository_storage['path']
+        repo_base_path = repository_storage.legacy_disk_path
         print "#{name}... "
         unless File.exist?(repo_base_path)
@@ -140,7 +140,7 @@ namespace :gitlab do
       puts "Repo paths owned by #{gitlab_shell_ssh_user}:root, or #{gitlab_shell_ssh_user}:#{Gitlab.config.gitlab_shell.owner_group}?"
       Gitlab.config.repositories.storages.each do |name, repository_storage|
-        repo_base_path = repository_storage['path']
+        repo_base_path = repository_storage.legacy_disk_path
         print "#{name}... "
         unless File.exist?(repo_base_path)

lib/tasks/gitlab/cleanup.rake

@@ -12,7 +12,7 @@ namespace :gitlab do
       namespaces  = Namespace.pluck(:path)
       namespaces << HASHED_REPOSITORY_NAME  # add so that it will be ignored
       Gitlab.config.repositories.storages.each do |name, repository_storage|
-        git_base_path = repository_storage['path']
+        git_base_path = repository_storage.legacy_disk_path
         all_dirs = Dir.glob(git_base_path + '/*')
         puts git_base_path.color(:yellow)
@@ -54,7 +54,7 @@ namespace :gitlab do
       move_suffix = "+orphaned+#{Time.now.to_i}"
       Gitlab.config.repositories.storages.each do |name, repository_storage|
-        repo_root = repository_storage['path']
+        repo_root = repository_storage.legacy_disk_path
         # Look for global repos (legacy, depth 1) and normal repos (depth 2)
         IO.popen(%W(find #{repo_root} -mindepth 1 -maxdepth 2 -name *.git)) do |find|
           find.each_line do |path|

lib/tasks/gitlab/info.rake

@@ -68,7 +68,7 @@ namespace :gitlab do
       puts "Version:\t#{gitlab_shell_version || "unknown".color(:red)}"
       puts "Repository storage paths:"
       Gitlab.config.repositories.storages.each do |name, repository_storage|
-        puts "- #{name}: \t#{repository_storage['path']}"
+        puts "- #{name}: \t#{repository_storage.legacy_disk_path}"
       end
       puts "Hooks:\t\t#{Gitlab.config.gitlab_shell.hooks_path}"
       puts "Git:\t\t#{Gitlab.config.git.bin_path}"

spec/initializers/6_validations_spec.rb

@@ -15,7 +15,7 @@ describe '6_validations' do
   describe 'validate_storages_config' do
     context 'with correct settings' do
       before do
-        mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/d' })
+        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
@@ -25,7 +25,7 @@ describe '6_validations' do
     context 'when one of the settings is incorrect' do
       before do
-        mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c', 'failure_count_threshold' => 'not a number' })
+        mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c', 'failure_count_threshold' => 'not a number'))
       end
       it 'throws an error' do
@@ -35,7 +35,7 @@ describe '6_validations' do
     context 'with invalid storage names' do
       before do
-        mock_storages('name with spaces' => { 'path' => 'tmp/tests/paths/a/b/c' })
+        mock_storages('name with spaces' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'))
       end
       it 'throws an error' do
@@ -67,7 +67,7 @@ describe '6_validations' do
   describe 'validate_storages_paths' do
     context 'with correct settings' do
       before do
-        mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/d' })
+        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
@@ -77,7 +77,7 @@ describe '6_validations' do
     context 'with nested storage paths' do
       before do
-        mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c/d' })
+        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
@@ -87,7 +87,7 @@ describe '6_validations' do
     context 'with similar but un-nested storage paths' do
       before do
-        mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c2' })
+        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
@@ -97,7 +97,7 @@ describe '6_validations' do
     describe 'inaccessible storage' do
       before do
-        mock_storages('foo' => { 'path' => 'tmp/tests/a/path/that/does/not/exist' })
+        mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/a/path/that/does/not/exist'))
       end
       it 'passes through with a warning' do

spec/initializers/settings_spec.rb

 require 'spec_helper'
-require_relative '../../config/initializers/1_settings'
+require_relative '../../config/initializers/1_settings' unless defined?(Settings)
 describe Settings do
   describe '#ldap' do

spec/lib/backup/repository_spec.rb

@@ -33,7 +33,7 @@ describe Backup::Repository do
     let(:timestamp) { Time.utc(2017, 3, 22) }
     let(:temp_dirs) do
       Gitlab.config.repositories.storages.map do |name, storage|
-        File.join(storage['path'], '..', 'repositories.old.' + timestamp.to_i.to_s)
+        File.join(storage.legacy_disk_path, '..', 'repositories.old.' + timestamp.to_i.to_s)
       end
     end

spec/lib/gitlab/bare_repository_import/repository_spec.rb

@@ -54,7 +54,7 @@ describe ::Gitlab::BareRepositoryImport::Repository do
   context 'hashed storage' do
     let(:gitlab_shell) { Gitlab::Shell.new }
     let(:repository_storage) { 'default' }
-    let(:root_path) { Gitlab.config.repositories.storages[repository_storage]['path'] }
+    let(:root_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path }
     let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' }
     let(:hashed_path) { "@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b" }
     let(:repo_path) { File.join(root_path, "#{hashed_path}.git") }

spec/lib/gitlab/health_checks/fs_shards_check_spec.rb

@@ -26,7 +26,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
   let(:storages_paths) do
     {
-      default: { path: tmp_dir }
+      default: Gitlab::GitalyClient::StorageSettings.new('path' => tmp_dir)
     }.with_indifferent_access
   end
@@ -56,7 +56,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
       context 'storage points to not existing folder' do
         let(:storages_paths) do
           {
-            default: { path: 'tmp/this/path/doesnt/exist' }
+            default: Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/this/path/doesnt/exist')
           }.with_indifferent_access
         end
@@ -102,7 +102,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
       context 'storage points to not existing folder' do
         let(:storages_paths) do
           {
-            default: { path: 'tmp/this/path/doesnt/exist' }
+            default: Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/this/path/doesnt/exist')
           }.with_indifferent_access
         end

spec/lib/gitlab/repo_path_spec.rb

@@ -48,8 +48,8 @@ describe ::Gitlab::RepoPath do
   describe '.strip_storage_path' do
     before do
       allow(Gitlab.config.repositories).to receive(:storages).and_return({
-        'storage1' => { 'path' => '/foo' },
-        'storage2' => { 'path' => '/bar' }
+        'storage1' => Gitlab::GitalyClient::StorageSettings.new('path' => '/foo'),
+        'storage2' => Gitlab::GitalyClient::StorageSettings.new('path' => '/bar')
       })
     end

spec/lib/gitlab/shell_spec.rb

@@ -405,7 +405,7 @@ describe Gitlab::Shell do
     describe '#create_repository' do
       shared_examples '#create_repository' do
         let(:repository_storage) { 'default' }
-        let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] }
+        let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path }
         let(:repo_name) { 'project/path' }
         let(:created_path) { File.join(repository_storage_path, repo_name + '.git') }
@@ -679,7 +679,7 @@ describe Gitlab::Shell do
   describe 'namespace actions' do
     subject { described_class.new }
-    let(:storage_path) { Gitlab.config.repositories.storages.default.path }
+    let(:storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path }
     describe '#add_namespace' do
       it 'creates a namespace' do

spec/migrations/remove_dot_git_from_usernames_spec.rb

@@ -29,7 +29,9 @@ describe RemoveDotGitFromUsernames do
         update_namespace(user, 'test.git')
         update_namespace(user2, 'test_git')
-        storages = { 'default' => 'tmp/tests/custom_repositories' }
+        default_hash = Gitlab.config.repositories.storages.default.to_h
+        default_hash['path'] = 'tmp/tests/custom_repositories'
+        storages = { 'default' => Gitlab::GitalyClient::StorageSettings.new(default_hash) }
         allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
         allow(migration).to receive(:route_exists?).with('test_git').and_return(true)

spec/models/namespace_spec.rb

@@ -305,7 +305,7 @@ describe Namespace do
   end
   describe '#rm_dir', 'callback' do
-    let(:repository_storage_path) { Gitlab.config.repositories.storages.default['path'] }
+    let(:repository_storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path }
     let(:path_in_dir) { File.join(repository_storage_path, namespace.full_path) }
     let(:deleted_path) { namespace.full_path.gsub(namespace.path, "#{namespace.full_path}+#{namespace.id}+deleted") }
     let(:deleted_path_in_dir) { File.join(repository_storage_path, deleted_path) }

spec/models/project_spec.rb

@@ -1101,8 +1101,8 @@ describe Project do
     before do
       storages = {
-        'default' => { 'path' => 'tmp/tests/repositories' },
-        'picked'  => { 'path' => 'tmp/tests/repositories' }
+        'default' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories'),
+        'picked'  => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories')
       }
       allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
     end

spec/services/projects/create_service_spec.rb

@@ -153,7 +153,7 @@ describe Projects::CreateService, '#execute' do
     context 'when another repository already exists on disk' do
       let(:repository_storage) { 'default' }
-      let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] }
+      let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path }
       let(:opts) do
         {

spec/services/projects/fork_service_spec.rb

@@ -105,7 +105,7 @@ describe Projects::ForkService do
       context 'repository already exists' do
         let(:repository_storage) { 'default' }
-        let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] }
+        let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path }
         before do
           gitlab_shell.create_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}")

spec/services/projects/transfer_service_spec.rb

@@ -146,7 +146,7 @@ describe Projects::TransferService do
   context 'namespace which contains orphan repository with same projects path name' do
     let(:repository_storage) { 'default' }
-    let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] }
+    let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path }
     before do
       group.add_owner(user)

spec/services/projects/update_service_spec.rb

@@ -190,7 +190,7 @@ describe Projects::UpdateService do
     context 'when renaming a project' do
       let(:repository_storage) { 'default' }
-      let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] }
+      let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path }
       context 'with legacy storage' do
         let(:project) { create(:project, :legacy_storage, :repository, creator: user, namespace: user.namespace) }

spec/support/stored_repositories.rb

@@ -4,7 +4,7 @@ RSpec.configure do |config|
   end
   config.before(:all, :broken_storage) do
-    FileUtils.rm_rf Gitlab.config.repositories.storages.broken['path']
+    FileUtils.rm_rf Gitlab.config.repositories.storages.broken.legacy_disk_path
   end
   config.before(:each, :broken_storage) do

spec/support/stub_configuration.rb

@@ -50,8 +50,12 @@ module StubConfiguration
     # Default storage is always required
     messages['default'] ||= Gitlab.config.repositories.storages.default
-    messages.each do |storage_name, storage_settings|
-      storage_settings['path'] = TestEnv.repos_path unless storage_settings.key?('path')
+    messages.each do |storage_name, storage_hash|
+      if !storage_hash.key?('path') || storage_hash['path'] == Gitlab::GitalyClient::StorageSettings::Deprecated
+        storage_hash['path'] = TestEnv.repos_path
+      end
+
+      messages[storage_name] = Gitlab::GitalyClient::StorageSettings.new(storage_hash.to_h)
     end
     allow(Gitlab.config.repositories).to receive(:storages).and_return(Settingslogic.new(messages))