From 008a6a6ce6fa943adcfecf3a606b845cfa282680 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 14 Mar 2018 14:42:49 +0100 Subject: Route path lookups through legacy_disk_path --- app/helpers/application_settings_helper.rb | 2 +- app/models/project.rb | 2 +- config/initializers/1_settings.rb | 9 ++---- config/initializers/6_validations.rb | 6 ++-- ...124141322_migrate_process_commit_worker_jobs.rb | 2 +- ...161220141214_remove_dot_git_from_group_names.rb | 2 +- ...20161226122833_remove_dot_git_from_usernames.rb | 4 +-- lib/backup/repository.rb | 4 +-- .../v1/migration_classes.rb | 2 +- lib/gitlab/git/gitlab_projects.rb | 2 +- lib/gitlab/git/repository.rb | 2 +- lib/gitlab/git/storage/checker.rb | 2 +- lib/gitlab/git/storage/circuit_breaker.rb | 2 +- lib/gitlab/gitaly_client/storage_settings.rb | 35 ++++++++++++++++++++++ lib/gitlab/health_checks/fs_shards_check.rb | 2 +- lib/gitlab/repo_path.rb | 4 +-- lib/gitlab/setup_helper.rb | 2 +- lib/gitlab/shell.rb | 6 ++-- lib/gitlab/task_helpers.rb | 4 +-- lib/system_check/orphans/namespace_check.rb | 4 +-- lib/system_check/orphans/repository_check.rb | 6 ++-- lib/tasks/gitlab/check.rake | 8 ++--- lib/tasks/gitlab/cleanup.rake | 4 +-- lib/tasks/gitlab/info.rake | 2 +- spec/initializers/6_validations_spec.rb | 14 ++++----- spec/initializers/settings_spec.rb | 2 +- spec/lib/backup/repository_spec.rb | 2 +- .../bare_repository_import/repository_spec.rb | 2 +- .../gitlab/health_checks/fs_shards_check_spec.rb | 6 ++-- spec/lib/gitlab/repo_path_spec.rb | 4 +-- spec/lib/gitlab/shell_spec.rb | 4 +-- .../remove_dot_git_from_usernames_spec.rb | 4 ++- spec/models/namespace_spec.rb | 2 +- spec/models/project_spec.rb | 4 +-- spec/services/projects/create_service_spec.rb | 2 +- spec/services/projects/fork_service_spec.rb | 2 +- spec/services/projects/transfer_service_spec.rb | 2 +- spec/services/projects/update_service_spec.rb | 2 +- spec/support/stored_repositories.rb | 2 +- spec/support/stub_configuration.rb | 8 +++-- spec/support/test_env.rb | 2 +- spec/tasks/gitlab/backup_rake_spec.rb | 15 ++++++++-- spec/tasks/gitlab/cleanup_rake_spec.rb | 7 +++-- spec/tasks/gitlab/git_rake_spec.rb | 7 +++-- spec/tasks/gitlab/gitaly_rake_spec.rb | 10 +++---- spec/tasks/gitlab/shell_rake_spec.rb | 2 +- 46 files changed, 138 insertions(+), 85 deletions(-) create mode 100644 lib/gitlab/gitaly_client/storage_settings.rb diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 4c4d7cca8a5..f44f5165c03 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -96,7 +96,7 @@ module ApplicationSettingsHelper def repository_storages_options_for_select(selected) options = Gitlab.config.repositories.storages.map do |name, storage| - ["#{name} - #{storage['path']}", name] + ["#{name} - #{storage.legacy_disk_path}", name] end options_for_select(options, selected) diff --git a/app/models/project.rb b/app/models/project.rb index 5f9d9785d64..f60660d00a4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -498,7 +498,7 @@ class Project < ActiveRecord::Base end def repository_storage_path - Gitlab.config.repositories.storages[repository_storage].try(:[], 'path') + Gitlab.config.repositories.storages[repository_storage]&.legacy_disk_path end def team diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index ea0dee7af53..53cf0010d8e 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -467,12 +467,7 @@ unless Settings.repositories.storages['default'] end Settings.repositories.storages.each do |key, storage| - storage = Settingslogic.new(storage) - - # Expand relative paths - storage['path'] = Settings.absolute(storage['path']) - - Settings.repositories.storages[key] = storage + Settings.repositories.storages[key] = Gitlab::GitalyClient::StorageSettings.new(storage) end # @@ -486,7 +481,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']) -if repository_downloads_path.blank? || repositories_storages.any? { |rs| [repository_downloads_path, repository_downloads_full_path].include?(rs['path'].gsub(%r{/$}, '')) } +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') end diff --git a/config/initializers/6_validations.rb b/config/initializers/6_validations.rb index f8e67ce04c9..d92cdb97766 100644 --- a/config/initializers/6_validations.rb +++ b/config/initializers/6_validations.rb @@ -5,7 +5,7 @@ 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['path']).realpath == parent + name != n && Pathname.new(rs.legacy_disk_path).realpath == parent end rescue Errno::EIO, Errno::ENOENT => e warning = "WARNING: couldn't verify #{path} (#{name}). "\ @@ -33,7 +33,7 @@ def validate_storages_config "If you're using the Gitlab Development Kit, you can update your configuration running `gdk reconfigure`.\n" end - if !repository_storage.is_a?(Hash) || repository_storage['path'].nil? + if !repository_storage.is_a?(Gitlab::GitalyClient::StorageSettings) || repository_storage.legacy_disk_path.nil? storage_validation_error("#{name} is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example") end @@ -50,7 +50,7 @@ end def validate_storages_paths Gitlab.config.repositories.storages.each do |name, repository_storage| - parent_name, _parent_path = find_parent_path(name, repository_storage['path']) + 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 diff --git a/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb b/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb index bcdae272209..a96ea7d9db4 100644 --- a/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb +++ b/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb @@ -12,7 +12,7 @@ class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration end def repository_storage_path - Gitlab.config.repositories.storages[repository_storage]['path'] + Gitlab.config.repositories.storages[repository_storage].legacy_disk_path end def repository_path diff --git a/db/migrate/20161220141214_remove_dot_git_from_group_names.rb b/db/migrate/20161220141214_remove_dot_git_from_group_names.rb index 8fb1f9d5e73..bddc234db25 100644 --- a/db/migrate/20161220141214_remove_dot_git_from_group_names.rb +++ b/db/migrate/20161220141214_remove_dot_git_from_group_names.rb @@ -60,7 +60,7 @@ class RemoveDotGitFromGroupNames < ActiveRecord::Migration def move_namespace(group_id, path_was, path) repository_storage_paths = select_all("SELECT distinct(repository_storage) FROM projects WHERE namespace_id = #{group_id}").map do |row| - Gitlab.config.repositories.storages[row['repository_storage']]['path'] + Gitlab.config.repositories.storages[row['repository_storage']].legacy_disk_path end.compact # Move the namespace directory in all storages paths used by member projects diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index 61dcc8c54f5..7c28d934c29 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -71,7 +71,7 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration route_exists = route_exists?(path) Gitlab.config.repositories.storages.each_value do |storage| - if route_exists || path_exists?(path, storage['path']) + if route_exists || path_exists?(path, storage.legacy_disk_path) counter += 1 path = "#{base}#{counter}" @@ -84,7 +84,7 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration def move_namespace(namespace_id, path_was, path) repository_storage_paths = select_all("SELECT distinct(repository_storage) FROM projects WHERE namespace_id = #{namespace_id}").map do |row| - Gitlab.config.repositories.storages[row['repository_storage']]['path'] + Gitlab.config.repositories.storages[row['repository_storage']].legacy_disk_path end.compact # Move the namespace directory in all storages paths used by member projects diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 6715159a1aa..88a7f2a4235 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -65,7 +65,7 @@ module Backup def restore Gitlab.config.repositories.storages.each do |name, repository_storage| - path = repository_storage['path'] + path = repository_storage.legacy_disk_path next unless File.exist?(path) # Move repos dir to 'repositories.old' dir @@ -200,7 +200,7 @@ module Backup end def repository_storage_paths_args - Gitlab.config.repositories.storages.values.map { |rs| rs['path'] } + Gitlab.config.repositories.storages.values.map { |rs| rs.legacy_disk_path } end def progress diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb index fd4a8832ec2..62d4d0a92a6 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb @@ -74,7 +74,7 @@ module Gitlab }.freeze def repository_storage_path - Gitlab.config.repositories.storages[repository_storage]['path'] + Gitlab.config.repositories.storages[repository_storage].legacy_disk_path end # Overridden to have the correct `source_type` for the `route` relation diff --git a/lib/gitlab/git/gitlab_projects.rb b/lib/gitlab/git/gitlab_projects.rb index 5e1e22ae65c..2d13435a90c 100644 --- a/lib/gitlab/git/gitlab_projects.rb +++ b/lib/gitlab/git/gitlab_projects.rb @@ -212,7 +212,7 @@ module Gitlab end def shard_name_from_shard_path(shard_path) - Gitlab.config.repositories.storages.find { |_, info| info['path'] == shard_path }&.first || + Gitlab.config.repositories.storages.find { |_, info| info.legacy_disk_path == shard_path }&.first || raise(ShardNameNotFoundError, "no shard found for path '#{shard_path}'") end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index fbc93542619..17379ac36d4 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -93,7 +93,7 @@ module Gitlab @relative_path = relative_path @gl_repository = gl_repository - storage_path = Gitlab.config.repositories.storages[@storage]['path'] + storage_path = Gitlab.config.repositories.storages[@storage].legacy_disk_path @gitlab_projects = Gitlab::Git::GitlabProjects.new( storage_path, relative_path, diff --git a/lib/gitlab/git/storage/checker.rb b/lib/gitlab/git/storage/checker.rb index d3c37f82101..2f611cef37b 100644 --- a/lib/gitlab/git/storage/checker.rb +++ b/lib/gitlab/git/storage/checker.rb @@ -35,7 +35,7 @@ module Gitlab def initialize(storage, logger = Rails.logger) @storage = storage config = Gitlab.config.repositories.storages[@storage] - @storage_path = config['path'] + @storage_path = config.legacy_disk_path @logger = logger @hostname = Gitlab::Environment.hostname diff --git a/lib/gitlab/git/storage/circuit_breaker.rb b/lib/gitlab/git/storage/circuit_breaker.rb index 898bb1b65be..e35054466ff 100644 --- a/lib/gitlab/git/storage/circuit_breaker.rb +++ b/lib/gitlab/git/storage/circuit_breaker.rb @@ -25,7 +25,7 @@ module Gitlab if !config.present? NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Storage '#{storage}' is not configured")) - elsif !config['path'].present? + elsif !config.legacy_disk_path.present? NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Path for storage '#{storage}' is not configured")) else new(storage, hostname) diff --git a/lib/gitlab/gitaly_client/storage_settings.rb b/lib/gitlab/gitaly_client/storage_settings.rb new file mode 100644 index 00000000000..8668caf0c55 --- /dev/null +++ b/lib/gitlab/gitaly_client/storage_settings.rb @@ -0,0 +1,35 @@ +module Gitlab + module GitalyClient + # This is a chokepoint that is meant to help us stop remove all places + # where production code (app, config, db, lib) touches Git repositories + # directly. + class StorageSettings + DirectPathAccessError = Class.new(StandardError) + + # This class will give easily recognizable NoMethodErrors + Deprecated = Class.new + + attr_reader :legacy_disk_path + + def initialize(storage) + raise "expected a Hash, got a #{storage.class.name}" unless storage.is_a?(Hash) + + # Support a nil 'path' field because some of the circuit breaker tests use it. + @legacy_disk_path = File.expand_path(storage['path'], Rails.root) if storage['path'] + + storage['path'] = Deprecated + @hash = storage + end + + def gitaly_address + @hash.fetch(:gitaly_address) + end + + private + + def method_missing(m, *args, &block) + @hash.public_send(m, *args, &block) # rubocop:disable GitlabSecurity/PublicSend + end + end + end +end diff --git a/lib/gitlab/health_checks/fs_shards_check.rb b/lib/gitlab/health_checks/fs_shards_check.rb index afaa59b1018..6e554383270 100644 --- a/lib/gitlab/health_checks/fs_shards_check.rb +++ b/lib/gitlab/health_checks/fs_shards_check.rb @@ -77,7 +77,7 @@ module Gitlab end def storage_path(storage_name) - storages_paths&.dig(storage_name, 'path') + storages_paths[storage_name]&.legacy_disk_path end # All below test methods use shell commands to perform actions on storage volumes. diff --git a/lib/gitlab/repo_path.rb b/lib/gitlab/repo_path.rb index 79265cf952d..1fa2a19b0af 100644 --- a/lib/gitlab/repo_path.rb +++ b/lib/gitlab/repo_path.rb @@ -21,11 +21,11 @@ module Gitlab result = repo_path storage = Gitlab.config.repositories.storages.values.find do |params| - repo_path.start_with?(params['path']) + repo_path.start_with?(params.legacy_disk_path) end if storage - result = result.sub(storage['path'], '') + result = result.sub(storage.legacy_disk_path, '') elsif fail_on_not_found raise NotFoundError.new("No known storage path matches #{repo_path.inspect}") end diff --git a/lib/gitlab/setup_helper.rb b/lib/gitlab/setup_helper.rb index 07d7c91cb5d..e5c02dd8ecc 100644 --- a/lib/gitlab/setup_helper.rb +++ b/lib/gitlab/setup_helper.rb @@ -24,7 +24,7 @@ module Gitlab address = val['gitaly_address'] end - storages << { name: key, path: val['path'] } + storages << { name: key, path: val.legacy_disk_path } end if Rails.env.test? diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index dda7afc0999..c016909b856 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -81,7 +81,7 @@ module Gitlab repository.gitaly_repository_client.create_repository true else - repo_path = File.join(Gitlab.config.repositories.storages[storage]['path'], relative_path) + repo_path = File.join(Gitlab.config.repositories.storages[storage].legacy_disk_path, relative_path) Gitlab::Git::Repository.create(repo_path, bare: true, symlink_hooks_to: gitlab_shell_hooks_path) end end @@ -130,7 +130,7 @@ module Gitlab if is_enabled repository.gitaly_repository_client.fetch_remote(remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, timeout: git_timeout, prune: prune) else - storage_path = Gitlab.config.repositories.storages[repository.storage]["path"] + storage_path = Gitlab.config.repositories.storages[repository.storage].legacy_disk_path local_fetch_remote(storage_path, repository.relative_path, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, prune: prune) end end @@ -477,7 +477,7 @@ module Gitlab def gitaly_namespace_client(storage_path) storage, _value = Gitlab.config.repositories.storages.find do |storage, value| - value['path'] == storage_path + value.legacy_disk_path == storage_path end Gitlab::GitalyClient::NamespaceService.new(storage) diff --git a/lib/gitlab/task_helpers.rb b/lib/gitlab/task_helpers.rb index 34bee6fecbe..42be301fd9b 100644 --- a/lib/gitlab/task_helpers.rb +++ b/lib/gitlab/task_helpers.rb @@ -129,7 +129,7 @@ module Gitlab def all_repos Gitlab.config.repositories.storages.each_value do |repository_storage| - IO.popen(%W(find #{repository_storage['path']} -mindepth 2 -type d -name *.git)) do |find| + IO.popen(%W(find #{repository_storage.legacy_disk_path} -mindepth 2 -type d -name *.git)) do |find| find.each_line do |path| yield path.chomp end @@ -138,7 +138,7 @@ module Gitlab end def repository_storage_paths_args - Gitlab.config.repositories.storages.values.map { |rs| rs['path'] } + Gitlab.config.repositories.storages.values.map { |rs| rs.legacy_disk_path } end def user_home diff --git a/lib/system_check/orphans/namespace_check.rb b/lib/system_check/orphans/namespace_check.rb index b8446300f72..b5f443abe06 100644 --- a/lib/system_check/orphans/namespace_check.rb +++ b/lib/system_check/orphans/namespace_check.rb @@ -6,8 +6,8 @@ module SystemCheck def multi_check Gitlab.config.repositories.storages.each do |storage_name, repository_storage| $stdout.puts - $stdout.puts "* Storage: #{storage_name} (#{repository_storage['path']})".color(:yellow) - toplevel_namespace_dirs = disk_namespaces(repository_storage['path']) + $stdout.puts "* Storage: #{storage_name} (#{repository_storage.legacy_disk_path})".color(:yellow) + toplevel_namespace_dirs = disk_namespaces(repository_storage.legacy_disk_path) orphans = (toplevel_namespace_dirs - existing_namespaces) print_orphans(orphans, storage_name) diff --git a/lib/system_check/orphans/repository_check.rb b/lib/system_check/orphans/repository_check.rb index 9b6b2429783..5ef0b93ad08 100644 --- a/lib/system_check/orphans/repository_check.rb +++ b/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) + $stdout.puts "* Storage: #{storage_name} (#{storage_path})".color(:yellow) - repositories = disk_repositories(repository_storage['path']) + repositories = disk_repositories(storage_path) orphans = (repositories - fetch_repositories(storage_name)) print_orphans(orphans, storage_name) diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 2403f57f05a..abef8cd2bcc 100644 --- a/lib/tasks/gitlab/check.rake +++ b/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) diff --git a/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake index 2453079911d..d6d15285489 100644 --- a/lib/tasks/gitlab/cleanup.rake +++ b/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| diff --git a/lib/tasks/gitlab/info.rake b/lib/tasks/gitlab/info.rake index 45e9a1a1c72..47ed522aec3 100644 --- a/lib/tasks/gitlab/info.rake +++ b/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}" diff --git a/spec/initializers/6_validations_spec.rb b/spec/initializers/6_validations_spec.rb index 83283f03940..1dc307ea922 100644 --- a/spec/initializers/6_validations_spec.rb +++ b/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 diff --git a/spec/initializers/settings_spec.rb b/spec/initializers/settings_spec.rb index 838ca9fabef..57f5adbbc40 100644 --- a/spec/initializers/settings_spec.rb +++ b/spec/initializers/settings_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' -require_relative '../../config/initializers/1_settings' +require_relative '../../config/initializers/1_settings' unless defined?(Settings) describe Settings do describe '#ldap' do diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb index a9b5ed1112a..03573c304aa 100644 --- a/spec/lib/backup/repository_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 9f42cf1dfca..d5226f479fb 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/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") } diff --git a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb index 4c1ca4349ea..9dcf272d25e 100644 --- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/repo_path_spec.rb b/spec/lib/gitlab/repo_path_spec.rb index b67bcc77bd4..f030f371372 100644 --- a/spec/lib/gitlab/repo_path_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 56b45d8da3c..00c95b9e5d6 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -405,7 +405,7 @@ describe Gitlab::Shell do describe '#add_repository' do shared_examples '#add_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 diff --git a/spec/migrations/remove_dot_git_from_usernames_spec.rb b/spec/migrations/remove_dot_git_from_usernames_spec.rb index 129374cb38c..3a88a66a476 100644 --- a/spec/migrations/remove_dot_git_from_usernames_spec.rb +++ b/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) diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index e626efd054d..3238e56a6c8 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -281,7 +281,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) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e970cd7dfdb..2a27ce1a38a 100644 --- a/spec/models/project_spec.rb +++ b/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 diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 9a44dfde41b..89845293e1f 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/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 { diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 409d5de8d43..2ccc114b69f 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/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.add_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}") diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index ae0e22e3dc0..68864edb42f 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/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) diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index d454ac0bda5..951e4c749f0 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/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) } diff --git a/spec/support/stored_repositories.rb b/spec/support/stored_repositories.rb index 52e47ae2d34..21995c89a6e 100644 --- a/spec/support/stored_repositories.rb +++ b/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 diff --git a/spec/support/stub_configuration.rb b/spec/support/stub_configuration.rb index 9f08c139322..bad1d34df3a 100644 --- a/spec/support/stub_configuration.rb +++ b/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)) diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 01321989f01..f14e69b1041 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -225,7 +225,7 @@ module TestEnv end def repos_path - Gitlab.config.repositories.storages.default['path'] + Gitlab.config.repositories.storages.default.legacy_disk_path end def backup_path diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 168facd51a6..0d24782f317 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -195,14 +195,23 @@ describe 'gitlab:app namespace rake task' do end context 'multiple repository storages' do - let(:gitaly_address) { Gitlab.config.repositories.storages.default.gitaly_address } + let(:storage_default) do + Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/default_storage')) + end + let(:test_second_storage) do + Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/custom_storage')) + end let(:storages) do { - 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage'), 'gitaly_address' => gitaly_address }, - 'test_second_storage' => { 'path' => Settings.absolute('tmp/tests/custom_storage'), 'gitaly_address' => gitaly_address } + 'default' => storage_default, + 'test_second_storage' => test_second_storage } end + before(:all) do + @default_storage_hash = Gitlab.config.repositories.storages.default.to_h + end + before do # We only need a backup of the repositories for this test stub_env('SKIP', 'db,uploads,builds,artifacts,lfs,registry') diff --git a/spec/tasks/gitlab/cleanup_rake_spec.rb b/spec/tasks/gitlab/cleanup_rake_spec.rb index 9e746ceddd6..2bf873c923f 100644 --- a/spec/tasks/gitlab/cleanup_rake_spec.rb +++ b/spec/tasks/gitlab/cleanup_rake_spec.rb @@ -6,13 +6,16 @@ describe 'gitlab:cleanup rake tasks' do end describe 'cleanup' do - let(:gitaly_address) { Gitlab.config.repositories.storages.default.gitaly_address } let(:storages) do { - 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage'), 'gitaly_address' => gitaly_address } + 'default' => Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/default_storage')) } end + before(:all) do + @default_storage_hash = Gitlab.config.repositories.storages.default.to_h + end + before do FileUtils.mkdir(Settings.absolute('tmp/tests/default_storage')) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb index 9aebf7b0b4a..1efaecc63a5 100644 --- a/spec/tasks/gitlab/git_rake_spec.rb +++ b/spec/tasks/gitlab/git_rake_spec.rb @@ -1,10 +1,13 @@ require 'rake_helper' describe 'gitlab:git rake tasks' do + before(:all) do + @default_storage_hash = Gitlab.config.repositories.storages.default.to_h + end + before do Rake.application.rake_require 'tasks/gitlab/git' - - storages = { 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage') } } + storages = { 'default' => Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/default_storage')) } FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git')) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) diff --git a/spec/tasks/gitlab/gitaly_rake_spec.rb b/spec/tasks/gitlab/gitaly_rake_spec.rb index 1f4053ff9ad..1e507c0236e 100644 --- a/spec/tasks/gitlab/gitaly_rake_spec.rb +++ b/spec/tasks/gitlab/gitaly_rake_spec.rb @@ -99,14 +99,14 @@ describe 'gitlab:gitaly namespace rake task' do describe 'storage_config' do it 'prints storage configuration in a TOML format' do config = { - 'default' => { + 'default' => Gitlab::GitalyClient::StorageSettings.new( 'path' => '/path/to/default', 'gitaly_address' => 'unix:/path/to/my.socket' - }, - 'nfs_01' => { + ), + 'nfs_01' => Gitlab::GitalyClient::StorageSettings.new( 'path' => '/path/to/nfs_01', 'gitaly_address' => 'unix:/path/to/my.socket' - } + ) } allow(Gitlab.config.repositories).to receive(:storages).and_return(config) allow(Rails.env).to receive(:test?).and_return(false) @@ -134,7 +134,7 @@ describe 'gitlab:gitaly namespace rake task' do parsed_output = TomlRB.parse(expected_output) config.each do |name, params| - expect(parsed_output['storage']).to include({ 'name' => name, 'path' => params['path'] }) + expect(parsed_output['storage']).to include({ 'name' => name, 'path' => params.legacy_disk_path }) end end end diff --git a/spec/tasks/gitlab/shell_rake_spec.rb b/spec/tasks/gitlab/shell_rake_spec.rb index 65155cb044d..4a756c5742d 100644 --- a/spec/tasks/gitlab/shell_rake_spec.rb +++ b/spec/tasks/gitlab/shell_rake_spec.rb @@ -11,7 +11,7 @@ describe 'gitlab:shell rake tasks' do it 'invokes create_hooks task' do expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke) - storages = Gitlab.config.repositories.storages.values.map { |rs| rs['path'] } + storages = Gitlab.config.repositories.storages.values.map(&:legacy_disk_path) expect(Kernel).to receive(:system).with('bin/install', *storages).and_call_original expect(Kernel).to receive(:system).with('bin/compile').and_call_original -- cgit v1.2.1