diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index 08b1c3a7d7a..db48bb7e8b8 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -23,10 +23,11 @@ class RepositoryForkWorker source_repository_storage_path, source_disk_path = *args - source_repository_storage_name = Gitlab.config.repositories.storages.find do |_, info| - info.legacy_disk_path == source_repository_storage_path - end&.first || raise("no shard found for path '#{source_repository_storage_path}'") - + source_repository_storage_name = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.find do |_, info| + info.legacy_disk_path == source_repository_storage_path + end&.first || raise("no shard found for path '#{source_repository_storage_path}'") + end fork_repository(target_project, source_repository_storage_name, source_disk_path) end end diff --git a/lib/gitlab/git/gitlab_projects.rb b/lib/gitlab/git/gitlab_projects.rb index 00c943fdb25..8475645971e 100644 --- a/lib/gitlab/git/gitlab_projects.rb +++ b/lib/gitlab/git/gitlab_projects.rb @@ -53,24 +53,11 @@ module Gitlab # Import project via git clone --bare # URL must be publicly cloneable def import_project(source, timeout) - Gitlab::GitalyClient.migrate(:import_repository, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_import_repository(source) - else - git_import_repository(source, timeout) - end - end + git_import_repository(source, timeout) end def fork_repository(new_shard_name, new_repository_relative_path) - Gitlab::GitalyClient.migrate(:fork_repository, - status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_fork_repository(new_shard_name, new_repository_relative_path) - else - git_fork_repository(new_shard_name, new_repository_relative_path) - end - end + git_fork_repository(new_shard_name, new_repository_relative_path) end def fetch_remote(name, timeout, force:, tags:, ssh_key: nil, known_hosts: nil, prune: true) @@ -241,16 +228,6 @@ module Gitlab true end - def gitaly_import_repository(source) - raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil) - - Gitlab::GitalyClient::RepositoryService.new(raw_repository).import_repository(source) - true - rescue GRPC::BadStatus => e - @output << e.message - false - end - def git_fork_repository(new_shard_name, new_repository_relative_path) from_path = repository_absolute_path new_shard_path = Gitlab.config.repositories.storages.fetch(new_shard_name).legacy_disk_path @@ -270,16 +247,6 @@ module Gitlab run(cmd, nil) && Gitlab::Git::Repository.create_hooks(to_path, global_hooks_path) end - - def gitaly_fork_repository(new_shard_name, new_repository_relative_path) - target_repository = Gitlab::Git::Repository.new(new_shard_name, new_repository_relative_path, nil) - raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil) - - Gitlab::GitalyClient::RepositoryService.new(target_repository).fork_repository(raw_repository) - rescue GRPC::BadStatus => e - logger.error "fork-repository failed: #{e.message}" - false - end end end end diff --git a/lib/gitlab/setup_helper.rb b/lib/gitlab/setup_helper.rb index e5c02dd8ecc..4a87f43597e 100644 --- a/lib/gitlab/setup_helper.rb +++ b/lib/gitlab/setup_helper.rb @@ -24,7 +24,9 @@ module Gitlab address = val['gitaly_address'] end - storages << { name: key, path: val.legacy_disk_path } + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + storages << { name: key, path: val.legacy_disk_path } + end end if Rails.env.test? diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 4a691d640b3..4b8aae4f5a2 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -106,10 +106,17 @@ module Gitlab raise Error.new("don't use disk paths with import_repository: #{url.inspect}") end - # The timeout ensures the subprocess won't hang forever - cmd = gitlab_projects(storage, "#{name}.git") - success = cmd.import_project(url, git_timeout) + relative_path = "#{name}.git" + cmd = gitaly_migrate(:import_repository, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| + if is_enabled + GitalyGitlabProjects.new(storage, relative_path) + else + # The timeout ensures the subprocess won't hang forever + gitlab_projects(storage, relative_path) + end + end + success = cmd.import_project(url, git_timeout) raise Error, cmd.output unless success success @@ -165,8 +172,16 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/817 def fork_repository(forked_from_storage, forked_from_disk_path, forked_to_storage, forked_to_disk_path) - gitlab_projects(forked_from_storage, "#{forked_from_disk_path}.git") - .fork_repository(forked_to_storage, "#{forked_to_disk_path}.git") + forked_from_relative_path = "#{forked_from_disk_path}.git" + fork_args = [forked_to_storage, "#{forked_to_disk_path}.git"] + + gitaly_migrate(:fork_repository, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| + if is_enabled + GitalyGitlabProjects.new(forked_from_storage, forked_from_relative_path).fork_repository(*fork_args) + else + gitlab_projects(forked_from_storage, forked_from_relative_path).fork_repository(*fork_args) + end + end end # Removes a repository from file system, using rm_diretory which is an alias @@ -452,5 +467,39 @@ module Gitlab # need to do the same here... raise Error, e end + + class GitalyGitlabProjects + attr_reader :shard_name, :repository_relative_path, :output + + def initialize(shard_name, repository_relative_path) + @shard_name = shard_name + @repository_relative_path = repository_relative_path + @output = '' + end + + def import_project(source, _timeout) + raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil) + + Gitlab::GitalyClient::RepositoryService.new(raw_repository).import_repository(source) + true + rescue GRPC::BadStatus => e + @output = e.message + false + end + + def fork_repository(new_shard_name, new_repository_relative_path) + target_repository = Gitlab::Git::Repository.new(new_shard_name, new_repository_relative_path, nil) + raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil) + + Gitlab::GitalyClient::RepositoryService.new(target_repository).fork_repository(raw_repository) + rescue GRPC::BadStatus => e + logger.error "fork-repository failed: #{e.message}" + false + end + + def logger + Rails.logger + end + end end end diff --git a/lib/gitlab/task_helpers.rb b/lib/gitlab/task_helpers.rb index 723e655c150..922418966e9 100644 --- a/lib/gitlab/task_helpers.rb +++ b/lib/gitlab/task_helpers.rb @@ -140,7 +140,9 @@ module Gitlab end def repository_storage_paths_args - Gitlab.config.repositories.storages.values.map { |rs| rs.legacy_disk_path } + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.values.map { |rs| rs.legacy_disk_path } + end end def user_home diff --git a/lib/system_check/orphans/namespace_check.rb b/lib/system_check/orphans/namespace_check.rb index b5f443abe06..09b57c7b408 100644 --- a/lib/system_check/orphans/namespace_check.rb +++ b/lib/system_check/orphans/namespace_check.rb @@ -4,13 +4,15 @@ module SystemCheck set_name 'Orphaned namespaces:' def multi_check - Gitlab.config.repositories.storages.each do |storage_name, repository_storage| - $stdout.puts - $stdout.puts "* Storage: #{storage_name} (#{repository_storage.legacy_disk_path})".color(:yellow) - toplevel_namespace_dirs = disk_namespaces(repository_storage.legacy_disk_path) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.each do |storage_name, repository_storage| + $stdout.puts + $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) + orphans = (toplevel_namespace_dirs - existing_namespaces) + print_orphans(orphans, storage_name) + end end clear_namespaces! # releases memory when check finishes diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index c04dae7446f..a8acafa9cd9 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -44,11 +44,13 @@ namespace :gitlab do start_checking "GitLab Shell" check_gitlab_shell - check_repo_base_exists - check_repo_base_is_not_symlink - check_repo_base_user_and_group - check_repo_base_permissions - check_repos_hooks_directory_is_link + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + check_repo_base_exists + check_repo_base_is_not_symlink + check_repo_base_user_and_group + check_repo_base_permissions + check_repos_hooks_directory_is_link + end check_gitlab_shell_self_test finished_checking "GitLab Shell" diff --git a/lib/tasks/gitlab/info.rake b/lib/tasks/gitlab/info.rake index 289aa5d9060..6de739e9515 100644 --- a/lib/tasks/gitlab/info.rake +++ b/lib/tasks/gitlab/info.rake @@ -67,8 +67,10 @@ namespace :gitlab do puts "GitLab Shell".color(:yellow) 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.legacy_disk_path}" + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.each do |name, repository_storage| + puts "- #{name}: \t#{repository_storage.legacy_disk_path}" + end end puts "Hooks:\t\t#{Gitlab.config.gitlab_shell.hooks_path}" puts "Git:\t\t#{Gitlab.config.git.bin_path}" diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 16e025618a6..f6b05bac0e8 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -151,11 +151,6 @@ FactoryBot.define do trait :empty_repo do after(:create) do |project| raise "Failed to create repository!" unless project.create_repository - - # We delete hooks so that gitlab-shell will not try to authenticate with - # an API that isn't running - project.gitlab_shell.rm_directory(project.repository_storage, - File.join("#{project.disk_path}.git", 'hooks')) end end @@ -180,13 +175,6 @@ FactoryBot.define do trait :wiki_repo do after(:create) do |project| raise 'Failed to create wiki repository!' unless project.create_wiki - - # We delete hooks so that gitlab-shell will not try to authenticate with - # an API that isn't running - project.gitlab_shell.rm_directory( - project.repository_storage, - File.join("#{project.wiki.repository.disk_path}.git", "hooks") - ) end end diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index f0e83ccfc7a..5dfbb8e71f8 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -79,7 +79,9 @@ describe Gitlab::Diff::File do let(:diffs) { commit.diffs } before do - info_dir_path = File.join(project.repository.path_to_repo, 'info') + info_dir_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(project.repository.path_to_repo, 'info') + end FileUtils.mkdir(info_dir_path) unless File.exist?(info_dir_path) File.write(File.join(info_dir_path, 'attributes'), "*.md -diff\n") diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index 94eaf86ef80..6015086f002 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -149,7 +149,9 @@ describe Gitlab::Git::Blob, seed_helper: true do it 'limits the size of a large file' do blob_size = Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE + 1 buffer = Array.new(blob_size, 0) - rugged_blob = Rugged::Blob.from_buffer(repository.rugged, buffer.join('')) + rugged_blob = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Rugged::Blob.from_buffer(repository.rugged, buffer.join('')) + end blob = Gitlab::Git::Blob.raw(repository, rugged_blob) expect(blob.size).to eq(blob_size) @@ -164,7 +166,9 @@ describe Gitlab::Git::Blob, seed_helper: true do context 'when sha references a tree' do it 'returns nil' do - tree = repository.rugged.rev_parse('master^{tree}') + tree = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged.rev_parse('master^{tree}') + end blob = Gitlab::Git::Blob.raw(repository, tree.oid) @@ -278,7 +282,11 @@ describe Gitlab::Git::Blob, seed_helper: true do end describe '.batch_lfs_pointers' do - let(:tree_object) { repository.rugged.rev_parse('master^{tree}') } + let(:tree_object) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged.rev_parse('master^{tree}') + end + end let(:non_lfs_blob) do Gitlab::Git::Blob.find( diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index a19155ed5b0..ec1a684cfbc 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -69,7 +69,9 @@ describe Gitlab::Git::Branch, seed_helper: true do Gitlab::Git.committer_hash(email: user.email, name: user.name) end let(:params) do - parents = [repository.rugged.head.target] + parents = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + [repository.rugged.head.target] + end tree = parents.first.tree { diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 89be8a1b7f2..5af982c7a54 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -4,12 +4,15 @@ describe Gitlab::Git::Commit, seed_helper: true do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } let(:rugged_commit) do - repository.rugged.lookup(SeedRepo::Commit::ID) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged.lookup(SeedRepo::Commit::ID) + end end - describe "Commit info" do before do - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged + repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged + end @committer = { email: 'mike@smith.com', @@ -58,7 +61,9 @@ describe Gitlab::Git::Commit, seed_helper: true do after do # Erase the new commit so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged + repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged + end repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end end @@ -115,7 +120,9 @@ describe Gitlab::Git::Commit, seed_helper: true do describe '.find' do it "should return first head commit if without params" do expect(described_class.last(repository).id).to eq( - repository.rugged.head.target.oid + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged.head.target.oid + end ) end diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 4a7b06003fc..3bb0b5be15b 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -27,8 +27,10 @@ EOT too_large: false } - @rugged_diff = repository.rugged.diff("5937ac0a7beb003549fc5fd26fc247adbce4a52e^", "5937ac0a7beb003549fc5fd26fc247adbce4a52e", paths: - [".gitmodules"]).patches.first + @rugged_diff = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged.diff("5937ac0a7beb003549fc5fd26fc247adbce4a52e^", "5937ac0a7beb003549fc5fd26fc247adbce4a52e", paths: + [".gitmodules"]).patches.first + end end describe '.new' do diff --git a/spec/lib/gitlab/git/gitlab_projects_spec.rb b/spec/lib/gitlab/git/gitlab_projects_spec.rb index 8b715d717c1..f5d8503c30c 100644 --- a/spec/lib/gitlab/git/gitlab_projects_spec.rb +++ b/spec/lib/gitlab/git/gitlab_projects_spec.rb @@ -5,6 +5,13 @@ describe Gitlab::Git::GitlabProjects do TestEnv.clean_test_path end + around do |example| + # TODO move this spec to gitaly-ruby. GitlabProjects is not used in gitlab-ce + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + let(:project) { create(:project, :repository) } if $VERBOSE @@ -190,36 +197,30 @@ describe Gitlab::Git::GitlabProjects do end end - context 'when Gitaly import_repository feature is enabled' do - it_behaves_like 'importing repository' + describe 'logging' do + it 'imports a repo' do + message = "Importing project from <#{import_url}> to <#{tmp_repo_path}>." + expect(logger).to receive(:info).with(message) + + subject + end end - context 'when Gitaly import_repository feature is disabled', :disable_gitaly do - describe 'logging' do - it 'imports a repo' do - message = "Importing project from <#{import_url}> to <#{tmp_repo_path}>." - expect(logger).to receive(:info).with(message) + context 'timeout' do + it 'does not import a repo' do + stub_spawn_timeout(cmd, timeout, nil) - subject - end + message = "Importing project from <#{import_url}> to <#{tmp_repo_path}> failed." + expect(logger).to receive(:error).with(message) + + is_expected.to be_falsy + + expect(gl_projects.output).to eq("Timed out\n") + expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_falsy end - - context 'timeout' do - it 'does not import a repo' do - stub_spawn_timeout(cmd, timeout, nil) - - message = "Importing project from <#{import_url}> to <#{tmp_repo_path}> failed." - expect(logger).to receive(:error).with(message) - - is_expected.to be_falsy - - expect(gl_projects.output).to eq("Timed out\n") - expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_falsy - end - end - - it_behaves_like 'importing repository' end + + it_behaves_like 'importing repository' end describe '#fork_repository' do @@ -232,9 +233,6 @@ describe Gitlab::Git::GitlabProjects do before do FileUtils.mkdir_p(dest_repos_path) - - # Undo spec_helper stub that deletes hooks - allow_any_instance_of(described_class).to receive(:fork_repository).and_call_original end after do @@ -258,51 +256,45 @@ describe Gitlab::Git::GitlabProjects do end end - context 'when Gitaly fork_repository feature is enabled' do - it_behaves_like 'forking a repository' + it_behaves_like 'forking a repository' + + # We seem to be stuck to having only one working Gitaly storage in tests, changing + # that is not very straight-forward so I'm leaving this test here for now till + # https://gitlab.com/gitlab-org/gitlab-ce/issues/41393 is fixed. + context 'different storages' do + let(:dest_repos) { 'alternative' } + let(:dest_repos_path) { File.join(File.dirname(tmp_repos_path), dest_repos) } + + before do + stub_storage_settings(dest_repos => { 'path' => dest_repos_path }) + end + + it 'forks the repo' do + is_expected.to be_truthy + + expect(File.exist?(dest_repo)).to be_truthy + expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy + expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy + end end - context 'when Gitaly fork_repository feature is disabled', :disable_gitaly do - it_behaves_like 'forking a repository' + describe 'log messages' do + describe 'successful fork' do + it do + message = "Forking repository from <#{tmp_repo_path}> to <#{dest_repo}>." + expect(logger).to receive(:info).with(message) - # We seem to be stuck to having only one working Gitaly storage in tests, changing - # that is not very straight-forward so I'm leaving this test here for now till - # https://gitlab.com/gitlab-org/gitlab-ce/issues/41393 is fixed. - context 'different storages' do - let(:dest_repos) { 'alternative' } - let(:dest_repos_path) { File.join(File.dirname(tmp_repos_path), dest_repos) } - - before do - stub_storage_settings(dest_repos => { 'path' => dest_repos_path }) - end - - it 'forks the repo' do - is_expected.to be_truthy - - expect(File.exist?(dest_repo)).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy + subject end end - describe 'log messages' do - describe 'successful fork' do - it do - message = "Forking repository from <#{tmp_repo_path}> to <#{dest_repo}>." - expect(logger).to receive(:info).with(message) + describe 'failed fork due existing destination' do + it do + FileUtils.mkdir_p(dest_repo) + message = "fork-repository failed: destination repository <#{dest_repo}> already exists." + expect(logger).to receive(:error).with(message) - subject - end - end - - describe 'failed fork due existing destination' do - it do - FileUtils.mkdir_p(dest_repo) - message = "fork-repository failed: destination repository <#{dest_repo}> already exists." - expect(logger).to receive(:error).with(message) - - subject - end + subject end end end diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb index 2fe1f5603ce..c8a5052200c 100644 --- a/spec/lib/gitlab/git/hook_spec.rb +++ b/spec/lib/gitlab/git/hook_spec.rb @@ -8,6 +8,13 @@ describe Gitlab::Git::Hook do allow_any_instance_of(described_class).to receive(:trigger).and_call_original end + around do |example| + # TODO move hook tests to gitaly-ruby. Hook will disappear from gitlab-ce + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + describe "#trigger" do let(:project) { create(:project, :repository) } let(:repository) { project.repository.raw_repository } diff --git a/spec/lib/gitlab/git/index_spec.rb b/spec/lib/gitlab/git/index_spec.rb index 73fbc6a6afa..16e6bd35449 100644 --- a/spec/lib/gitlab/git/index_spec.rb +++ b/spec/lib/gitlab/git/index_spec.rb @@ -8,6 +8,13 @@ describe Gitlab::Git::Index, seed_helper: true do index.read_tree(repository.lookup('master').tree) end + around do |example| + # TODO move these specs to gitaly-ruby. The Index class will disappear from gitlab-ce + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + describe '#create' do let(:options) do { diff --git a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb index b793636c4d6..68eaa70e6b6 100644 --- a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb +++ b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb @@ -19,7 +19,9 @@ describe Gitlab::ImportExport::MergeRequestParser do end after do - FileUtils.rm_rf(project.repository.path_to_repo) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + FileUtils.rm_rf(project.repository.path_to_repo) + end end it 'has a source branch' do diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 14eae22a2ec..155e1663298 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -498,16 +498,34 @@ describe Gitlab::Shell do ) end - it 'returns true when the command succeeds' do - expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { true } + context 'with gitaly' do + it 'returns true when the command succeeds' do + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository) + .with(repository.raw_repository) { :gitaly_response_object } - is_expected.to be_truthy + is_expected.to be_truthy + end + + it 'return false when the command fails' do + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository) + .with(repository.raw_repository) { raise GRPC::BadStatus, 'bla' } + + is_expected.to be_falsy + end end - it 'return false when the command fails' do - expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { false } + context 'without gitaly', :disable_gitaly do + it 'returns true when the command succeeds' do + expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { true } - is_expected.to be_falsy + is_expected.to be_truthy + end + + it 'return false when the command fails' do + expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { false } + + is_expected.to be_falsy + end end end @@ -662,21 +680,43 @@ describe Gitlab::Shell do describe '#import_repository' do let(:import_url) { 'https://gitlab.com/gitlab-org/gitlab-ce.git' } - it 'returns true when the command succeeds' do - expect(gitlab_projects).to receive(:import_project).with(import_url, timeout) { true } + context 'with gitaly' do + it 'returns true when the command succeeds' do + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository).with(import_url) - result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) - expect(result).to be_truthy + expect(result).to be_truthy + end + + it 'raises an exception when the command fails' do + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository) + .with(import_url) { raise GRPC::BadStatus, 'bla' } + expect_any_instance_of(Gitlab::Shell::GitalyGitlabProjects).to receive(:output) { 'error'} + + expect do + gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + end.to raise_error(Gitlab::Shell::Error, "error") + end end - it 'raises an exception when the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:import_project) { false } + context 'without gitaly', :disable_gitaly do + it 'returns true when the command succeeds' do + expect(gitlab_projects).to receive(:import_project).with(import_url, timeout) { true } - expect do - gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) - end.to raise_error(Gitlab::Shell::Error, "error") + result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + + expect(result).to be_truthy + end + + it 'raises an exception when the command fails' do + allow(gitlab_projects).to receive(:output) { 'error' } + expect(gitlab_projects).to receive(:import_project) { false } + + expect do + gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + end.to raise_error(Gitlab::Shell::Error, "error") + end end end end diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index 33405d7a7ec..92159e1e372 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -118,7 +118,9 @@ describe GitTagPushService do before do # Create the lightweight tag - project.repository.raw_repository.rugged.tags.create(tag_name, newrev) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.raw_repository.rugged.tags.create(tag_name, newrev) + end # Clear tag list cache project.repository.expire_tags_cache diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb index bd884787425..ded17fa92a4 100644 --- a/spec/services/merge_requests/squash_service_spec.rb +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -63,7 +63,9 @@ describe MergeRequests::SquashService do end it 'has the same diff as the merge request, but a different SHA' do - rugged = project.repository.rugged + rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged + end mr_diff = rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha) squash_diff = rugged.diff(merge_request.diff_start_sha, squash_sha) diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb index c6678fc1f5c..cd52bc88f4c 100644 --- a/spec/services/projects/after_import_service_spec.rb +++ b/spec/services/projects/after_import_service_spec.rb @@ -32,7 +32,7 @@ describe Projects::AfterImportService do end it 'removes refs/pull/**/*' do - expect(repository.rugged.references.map(&:name)) + expect(rugged.references.map(&:name)) .not_to include(%r{\Arefs/pull/}) end end @@ -46,10 +46,14 @@ describe Projects::AfterImportService do end it "does not remove refs/#{name}/tmp" do - expect(repository.rugged.references.map(&:name)) + expect(rugged.references.map(&:name)) .to include("refs/#{name}/tmp") end end end + + def rugged + Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.rugged } + end end end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index b63f409579e..38660ad7a01 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -5,7 +5,11 @@ describe Projects::DestroyService do let!(:user) { create(:user) } let!(:project) { create(:project, :repository, namespace: user.namespace) } - let!(:path) { project.repository.path_to_repo } + let!(:path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path_to_repo + end + end let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") } let!(:async) { false } # execute or async_execute diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 7dca81eb59e..ed4930313c5 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -37,7 +37,11 @@ describe Projects::HashedStorage::MigrateRepositoryService do it 'writes project full path to .git/config' do service.execute - expect(project.repository.rugged.config['gitlab.fullpath']).to eq project.full_path + rugged_config = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged.config['gitlab.fullpath'] + end + + expect(rugged_config).to eq project.full_path end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e093444121a..8417b340de5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -107,19 +107,6 @@ RSpec.configure do |config| end config.before(:example) do - # Skip pre-receive hook check so we can use the web editor and merge. - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) - - allow_any_instance_of(Gitlab::Git::GitlabProjects).to receive(:fork_repository).and_wrap_original do |m, *args| - m.call(*args) - - shard_name, repository_relative_path = args - # We can't leave the hooks in place after a fork, as those would fail in tests - # The "internal" API is not available - Gitlab::Shell.new.rm_directory(shard_name, - File.join(repository_relative_path, 'hooks')) - end - # Enable all features by default for testing allow(Feature).to receive(:enabled?) { true } end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 1fef50a52ec..05a8e6206ae 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -135,6 +135,16 @@ module TestEnv install_dir: Gitlab.config.gitlab_shell.path, version: Gitlab::Shell.version_required, task: 'gitlab:shell:install') + + create_fake_git_hooks + end + + def create_fake_git_hooks + # gitlab-shell hooks don't work in our test environment because they try to make internal API calls + hooks_dir = File.join(Gitlab.config.gitlab_shell.path, 'hooks') + %w[pre-receive post-receive update].each do |hook| + File.open(File.join(hooks_dir, hook), 'w', 0755) { |f| f.puts '#!/bin/sh' } + end end def setup_gitaly diff --git a/spec/tasks/gitlab/gitaly_rake_spec.rb b/spec/tasks/gitlab/gitaly_rake_spec.rb index 1e507c0236e..4545226d78c 100644 --- a/spec/tasks/gitlab/gitaly_rake_spec.rb +++ b/spec/tasks/gitlab/gitaly_rake_spec.rb @@ -134,7 +134,9 @@ 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.legacy_disk_path }) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + expect(parsed_output['storage']).to include({ 'name' => name, 'path' => params.legacy_disk_path }) + end end end end diff --git a/spec/tasks/gitlab/shell_rake_spec.rb b/spec/tasks/gitlab/shell_rake_spec.rb index 4a756c5742d..0ed5d3e27b9 100644 --- a/spec/tasks/gitlab/shell_rake_spec.rb +++ b/spec/tasks/gitlab/shell_rake_spec.rb @@ -7,11 +7,17 @@ describe 'gitlab:shell rake tasks' do stub_warn_user_is_not_gitlab end + after do + TestEnv.create_fake_git_hooks + end + describe 'install task' do it 'invokes create_hooks task' do expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke) - storages = Gitlab.config.repositories.storages.values.map(&:legacy_disk_path) + storages = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.values.map(&:legacy_disk_path) + end expect(Kernel).to receive(:system).with('bin/install', *storages).and_call_original expect(Kernel).to receive(:system).with('bin/compile').and_call_original diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index f44b4edc305..807d1b8c084 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -218,7 +218,9 @@ describe GitGarbageCollectWorker do # Create a new commit on a random new branch def create_objects(project) - rugged = project.repository.rugged + rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged + end old_commit = rugged.branches.first.target new_commit_sha = Rugged::Commit.create( rugged, @@ -237,7 +239,9 @@ describe GitGarbageCollectWorker do end def packs(project) - Dir["#{project.repository.path_to_repo}/objects/pack/*.pack"] + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Dir["#{project.repository.path_to_repo}/objects/pack/*.pack"] + end end def packed_refs(project) diff --git a/spec/workers/project_destroy_worker_spec.rb b/spec/workers/project_destroy_worker_spec.rb index f19c9dff941..42e1d86e3bb 100644 --- a/spec/workers/project_destroy_worker_spec.rb +++ b/spec/workers/project_destroy_worker_spec.rb @@ -2,7 +2,11 @@ require 'spec_helper' describe ProjectDestroyWorker do let(:project) { create(:project, :repository, pending_delete: true) } - let(:path) { project.repository.path_to_repo } + let(:path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path_to_repo + end + end subject { described_class.new } diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 4b3c1736ea0..f5f1b60ace8 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -89,6 +89,9 @@ describe RepositoryForkWorker do it_behaves_like 'RepositoryForkWorker performing' it 'logs a message about forking with old-style arguments' do + allow(subject).to receive(:gitlab_shell).and_return(shell) + expect(shell).to receive(:fork_repository) { true } + allow(Rails.logger).to receive(:info).with(anything) # To compensate for other logs expect(Rails.logger).to receive(:info).with("Project #{fork_project.id} is being forked using old-style arguments.")