diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index be386c9ede3c1add77a7a26a848e01e4d8133996..fbc18a42b0f6c1ca6ea7acdfaff6a163628e5e40 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.33.0 +=5d2f27592bf5c43b58559ccb57f05c0e03b792da diff --git a/app/models/repository.rb b/app/models/repository.rb index e0a021ed2d2f03084185739742cc5d391de0f51b..b3527dd9e18e47770b5088fae76a3a7a3c7b1ab9 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1094,7 +1094,7 @@ def remove_remote(name) end def fetch_remote(remote, forced: false, ssh_auth: nil, no_tags: false) - gitlab_shell.fetch_remote(repository_storage_path, disk_path, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags) + gitlab_shell.fetch_remote(raw_repository, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags) end def fetch_ref(source_path, source_ref, target_ref) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index eb3731ba35a8c244a62e4771721ac79bf4f33345..c4e1d2e53353ea7c57c79e1a9442e108d2d0efea 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -46,6 +46,9 @@ def create(storage_path, name, bare: true, symlink_hooks_to: nil) # Directory name of repo attr_reader :name + # Relative path of repo + attr_reader :relative_path + # Rugged repo object attr_reader :rugged diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index a74a6dc6e78a968ebd2f07fd2b3702b49018878c..177a1284f3806a35f111a54197827c3759319415 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -37,6 +37,22 @@ def apply_gitattributes(revision) request = Gitaly::ApplyGitattributesRequest.new(repository: @gitaly_repo, revision: revision) GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request) end + + def fetch_remote(remote, ssh_auth: nil, forced: false, no_tags: false) + request = Gitaly::FetchRemoteRequest.new(repository: @gitaly_repo, remote: remote, force: forced, no_tags: no_tags) + + if ssh_auth&.ssh_import? + if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? + request.ssh_key = ssh_auth.ssh_private_key + end + + if ssh_auth.ssh_known_hosts.present? + request.known_hosts = ssh_auth.ssh_known_hosts + end + end + + GitalyClient.call(@storage, :repository_service, :fetch_remote, request) + end end end end diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 842421fc93863c495c13cb083eaac91dd3264394..b2e64a53aa5517747d9023a954077de7fb70ed24 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -126,34 +126,25 @@ def list_remote_tags(storage, name, remote) # Fetch remote for repository # - # name - project path with namespace + # repository - an instance of Git::Repository # remote - remote name # ssh_auth - SSH known_hosts data and a private key to use for public-key authentication # forced - should we use --force flag? # no_tags - should we use --no-tags flag? # # Ex. - # fetch_remote("gitlab/gitlab-ci", "upstream") + # fetch_remote(my_repo, "upstream") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 - def fetch_remote(storage, name, remote, ssh_auth: nil, forced: false, no_tags: false) - args = [gitlab_shell_projects_path, 'fetch-remote', storage, "#{name}.git", remote, "#{Gitlab.config.gitlab_shell.git_timeout}"] - args << '--force' if forced - args << '--no-tags' if no_tags - - vars = {} - - if ssh_auth&.ssh_import? - if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? - vars['GITLAB_SHELL_SSH_KEY'] = ssh_auth.ssh_private_key - end - - if ssh_auth.ssh_known_hosts.present? - vars['GITLAB_SHELL_KNOWN_HOSTS'] = ssh_auth.ssh_known_hosts + def fetch_remote(repository, remote, ssh_auth: nil, forced: false, no_tags: false) + gitaly_migrate(:fetch_remote) do |is_enabled| + if is_enabled + gitaly_repository_client(repository).fetch_remote(remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags) + else + storage_path = Gitlab.config.repositories.storages[repository.storage]["path"] + local_fetch_remote(storage_path, repository.relative_path, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags) end end - - gitlab_shell_fast_execute_raise_error(args, vars) end # Move repository @@ -458,6 +449,26 @@ def authorized_keys_enabled? private + def local_fetch_remote(storage, name, remote, ssh_auth: nil, forced: false, no_tags: false) + args = [gitlab_shell_projects_path, 'fetch-remote', storage, name, remote, "#{Gitlab.config.gitlab_shell.git_timeout}"] + args << '--force' if forced + args << '--no-tags' if no_tags + + vars = {} + + if ssh_auth&.ssh_import? + if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? + vars['GITLAB_SHELL_SSH_KEY'] = ssh_auth.ssh_private_key + end + + if ssh_auth.ssh_known_hosts.present? + vars['GITLAB_SHELL_KNOWN_HOSTS'] = ssh_auth.ssh_known_hosts + end + end + + gitlab_shell_fast_execute_raise_error(args, vars) + end + def gitlab_shell_fast_execute(cmd) output, status = gitlab_shell_fast_execute_helper(cmd) @@ -481,5 +492,17 @@ def gitlab_shell_fast_execute_helper(cmd, vars = {}) # from wasting I/O by searching through GEM_PATH Bundler.with_original_env { Popen.popen(cmd, nil, vars) } end + + def gitaly_repository_client(repo) + Gitlab::GitalyClient::RepositoryService.new(repo) + end + + def gitaly_migrate(method, &block) + Gitlab::GitalyClient.migrate(method, &block) + rescue GRPC::NotFound, GRPC::BadStatus => e + # Old Popen code returns [Error, output] to the caller, so we + # need to do the same here... + raise Error, e + end end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 7b6bd8b18a1b14f80d212f853234262acfc4301a..93202f6a4da3c304a37cd5e6887e3a5d54eeeef2 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -525,22 +525,47 @@ end end - describe '#fetch_remote' do + shared_examples 'fetch_remote' do |gitaly_on| + let(:project2) { create(:project, :repository) } + let(:repository) { project2.repository } + def fetch_remote(ssh_auth = nil) - gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage', ssh_auth: ssh_auth) + gitlab_shell.fetch_remote(repository.raw_repository, 'new/storage', ssh_auth: ssh_auth) end - def expect_popen(vars = {}) + def expect_popen(fail = false, vars = {}) popen_args = [ projects_path, 'fetch-remote', - 'current/storage', - 'project/path.git', + TestEnv.repos_path, + repository.relative_path, 'new/storage', Gitlab.config.gitlab_shell.git_timeout.to_s ] - expect(Gitlab::Popen).to receive(:popen).with(popen_args, nil, popen_vars.merge(vars)) + if fail + expect(Gitlab::Popen).to receive(:popen).with(popen_args, nil, popen_vars.merge(vars)).and_return(["error", 1]) + else + expect(Gitlab::Popen).to receive(:popen).with(popen_args, nil, popen_vars.merge(vars)).and_return([nil, 0]) + end + end + + def expect_gitaly_call(fail, vars = {}) + if fail + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fetch_remote).and_raise(GRPC::NotFound) + else + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fetch_remote).and_return(true) + end + end + + if gitaly_on + def expect_call(fail, vars = {}) + expect_gitaly_call(fail, vars) + end + else + def expect_call(fail, vars = {}) + expect_popen(fail, vars) + end end def build_ssh_auth(opts = {}) @@ -555,20 +580,20 @@ def build_ssh_auth(opts = {}) end it 'returns true when the command succeeds' do - expect_popen.and_return([nil, 0]) + expect_call(false) expect(fetch_remote).to be_truthy end it 'raises an exception when the command fails' do - expect_popen.and_return(["error", 1]) + expect_call(true) - expect { fetch_remote }.to raise_error(Gitlab::Shell::Error, "error") + expect { fetch_remote }.to raise_error(Gitlab::Shell::Error) end context 'SSH auth' do it 'passes the SSH key if specified' do - expect_popen('GITLAB_SHELL_SSH_KEY' => 'foo').and_return([nil, 0]) + expect_call(false, 'GITLAB_SHELL_SSH_KEY' => 'foo') ssh_auth = build_ssh_auth(ssh_key_auth?: true, ssh_private_key: 'foo') @@ -576,7 +601,7 @@ def build_ssh_auth(opts = {}) end it 'does not pass an empty SSH key' do - expect_popen.and_return([nil, 0]) + expect_call(false) ssh_auth = build_ssh_auth(ssh_key_auth: true, ssh_private_key: '') @@ -584,7 +609,7 @@ def build_ssh_auth(opts = {}) end it 'does not pass the key unless SSH key auth is to be used' do - expect_popen.and_return([nil, 0]) + expect_call(false) ssh_auth = build_ssh_auth(ssh_key_auth: false, ssh_private_key: 'foo') @@ -592,7 +617,7 @@ def build_ssh_auth(opts = {}) end it 'passes the known_hosts data if specified' do - expect_popen('GITLAB_SHELL_KNOWN_HOSTS' => 'foo').and_return([nil, 0]) + expect_call(false, 'GITLAB_SHELL_KNOWN_HOSTS' => 'foo') ssh_auth = build_ssh_auth(ssh_known_hosts: 'foo') @@ -600,7 +625,7 @@ def build_ssh_auth(opts = {}) end it 'does not pass empty known_hosts data' do - expect_popen.and_return([nil, 0]) + expect_call(false) ssh_auth = build_ssh_auth(ssh_known_hosts: '') @@ -608,7 +633,7 @@ def build_ssh_auth(opts = {}) end it 'does not pass known_hosts data unless SSH is to be used' do - expect_popen(popen_vars).and_return([nil, 0]) + expect_call(false, popen_vars) ssh_auth = build_ssh_auth(ssh_import?: false, ssh_known_hosts: 'foo') @@ -617,6 +642,14 @@ def build_ssh_auth(opts = {}) end end + describe '#fetch_remote local', skip_gitaly_mock: true do + it_should_behave_like 'fetch_remote', false + end + + describe '#fetch_remote gitaly' do + it_should_behave_like 'fetch_remote', true + end + describe '#import_repository' do it 'returns true when the command succeeds' do expect(Gitlab::Popen).to receive(:popen)