diff --git a/Gemfile b/Gemfile index 88c1ed19a30e1e05e840eca898b353e1bbb5b6e8..553c59cfb061000ff38a208d256cf0a706edd042 100644 --- a/Gemfile +++ b/Gemfile @@ -455,7 +455,7 @@ group :ed25519 do end # Gitaly GRPC protocol definitions -gem 'gitaly', '~> 12.9.0.pre.rc4' +gem 'gitaly', '~> 13.0.0.pre.rc1' gem 'grpc', '~> 1.24.0' diff --git a/Gemfile.lock b/Gemfile.lock index 9f60cdcd4ad0588aba940ffa0a4e5c4ae70a7962..c34643e26f812f2e0e104c7b6d259ed0da28c0cb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -392,7 +392,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) git (1.5.0) - gitaly (12.9.0.pre.rc4) + gitaly (13.0.0.pre.rc1) grpc (~> 1.0) github-markup (1.7.0) gitlab-chronic (0.10.5) @@ -1247,7 +1247,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly (~> 12.9.0.pre.rc4) + gitaly (~> 13.0.0.pre.rc1) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) gitlab-labkit (= 0.12.0) diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index 0334d63dd369057a0902a652c9ffdcffd74546c0..8e7612e63c8902b5d8525af279b55c0cb5cd9c0d 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -106,7 +106,23 @@ def update_in_progress? update_status == 'started' end - def update_repository(options) + def update_repository + Gitlab::Git::RemoteMirror.new( + project.repository.raw, + remote_name, + **options_for_update + ).update + end + + def options_for_update + options = { + keep_divergent_refs: keep_divergent_refs? + } + + if only_protected_branches? + options[:only_branches_matching] = project.protected_branches.pluck(:name) + end + if ssh_mirror_url? if ssh_key_auth? && ssh_private_key.present? options[:ssh_key] = ssh_private_key @@ -117,13 +133,7 @@ def update_repository(options) end end - options[:keep_divergent_refs] = keep_divergent_refs? - - Gitlab::Git::RemoteMirror.new( - project.repository.raw, - remote_name, - **options - ).update + options end def sync? diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index 13a467a3ef9b1ba593c734fc40bc9e4f61acc60f..e554bed681967334fd7e1d344c89027656188a5b 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -29,14 +29,16 @@ def update_mirror(remote_mirror) remote_mirror.ensure_remote! repository.fetch_remote(remote_mirror.remote_name, ssh_auth: remote_mirror, no_tags: true) - opts = {} - if remote_mirror.only_protected_branches? - opts[:only_branches_matching] = project.protected_branches.select(:name).map(&:name) - end + response = remote_mirror.update_repository - remote_mirror.update_repository(opts) + if response.divergent_refs.any? + message = "Some refs have diverged and have not been updated on the remote:" + message += "\n\n#{response.divergent_refs.join("\n")}" - remote_mirror.update_finish! + remote_mirror.mark_as_failed!(message) + else + remote_mirror.update_finish! + end end def retry_or_fail(mirror, message, tries) diff --git a/spec/factories/remote_mirrors.rb b/spec/factories/remote_mirrors.rb index 124c0510cabb296d721179ea4e2043dbb270fe06..aa0ace30d90c595333b0d2a55a06d3436b9d8650 100644 --- a/spec/factories/remote_mirrors.rb +++ b/spec/factories/remote_mirrors.rb @@ -4,5 +4,10 @@ factory :remote_mirror, class: 'RemoteMirror' do association :project, :repository url { "http://foo:bar@test.com" } + + trait :ssh do + url { 'ssh://git@test.com:foo/bar.git' } + auth_method { 'ssh_public_key' } + end end end diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 15b162ae87a8b8677bfd1c0e134fd819bd0ed087..356b0e18559f7b061b68cd0c0e62b5c0cc37a5e1 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -143,22 +143,54 @@ end describe '#update_repository' do - let(:git_remote_mirror) { spy } + it 'performs update including options' do + git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy) + mirror = build(:remote_mirror) - before do - stub_const('Gitlab::Git::RemoteMirror', git_remote_mirror) + expect(mirror).to receive(:options_for_update).and_return(options: true) + mirror.update_repository + + expect(git_remote_mirror).to have_received(:new).with( + mirror.project.repository.raw, + mirror.remote_name, + options: true + ) + expect(git_remote_mirror).to have_received(:update) end + end - it 'includes the `keep_divergent_refs` setting' do + describe '#options_for_update' do + it 'includes the `keep_divergent_refs` option' do mirror = build_stubbed(:remote_mirror, keep_divergent_refs: true) - mirror.update_repository({}) + options = mirror.options_for_update - expect(git_remote_mirror).to have_received(:new).with( - anything, - mirror.remote_name, - hash_including(keep_divergent_refs: true) - ) + expect(options).to include(keep_divergent_refs: true) + end + + it 'includes the `only_branches_matching` option' do + branch = create(:protected_branch) + mirror = build_stubbed(:remote_mirror, project: branch.project, only_protected_branches: true) + + options = mirror.options_for_update + + expect(options).to include(only_branches_matching: [branch.name]) + end + + it 'includes the `ssh_key` option' do + mirror = build(:remote_mirror, :ssh, ssh_private_key: 'private-key') + + options = mirror.options_for_update + + expect(options).to include(ssh_key: 'private-key') + end + + it 'includes the `known_hosts` option' do + mirror = build(:remote_mirror, :ssh, ssh_known_hosts: 'known-hosts') + + options = mirror.options_for_update + + expect(options).to include(known_hosts: 'known-hosts') end end diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index 4396ccab58456b242d7359ed7a26ce334f3e763f..38c2dc0780e5ff2e3b16caafed306c135a8b4a54 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -5,7 +5,7 @@ describe Projects::UpdateRemoteMirrorService do let(:project) { create(:project, :repository) } let(:remote_project) { create(:forked_project_with_submodules) } - let(:remote_mirror) { project.remote_mirrors.create!(url: remote_project.http_url_to_repo, enabled: true, only_protected_branches: false) } + let(:remote_mirror) { create(:remote_mirror, project: project, enabled: true) } let(:remote_name) { remote_mirror.remote_name } subject(:service) { described_class.new(project, project.creator) } @@ -16,7 +16,9 @@ before do project.repository.add_branch(project.owner, 'existing-branch', 'master') - allow(remote_mirror).to receive(:update_repository).and_return(true) + allow(remote_mirror) + .to receive(:update_repository) + .and_return(double(divergent_refs: [])) end it 'ensures the remote exists' do @@ -53,7 +55,7 @@ it 'marks the mirror as failed and raises the error when an unexpected error occurs' do allow(project.repository).to receive(:fetch_remote).and_raise('Badly broken') - expect { execute! }.to raise_error /Badly broken/ + expect { execute! }.to raise_error(/Badly broken/) expect(remote_mirror).to be_failed expect(remote_mirror.last_error).to include('Badly broken') @@ -83,32 +85,21 @@ end end - context 'when syncing all branches' do - it 'push all the branches the first time' do + context 'when there are divergent refs' do + before do stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) - - expect(remote_mirror).to receive(:update_repository).with({}) - - execute! end - end - context 'when only syncing protected branches' do - it 'sync updated protected branches' do - stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) - protected_branch = create_protected_branch(project) - remote_mirror.only_protected_branches = true - - expect(remote_mirror) - .to receive(:update_repository) - .with(only_branches_matching: [protected_branch.name]) + it 'marks the mirror as failed and sets an error message' do + response = double(divergent_refs: %w[refs/heads/master refs/heads/develop]) + expect(remote_mirror).to receive(:update_repository).and_return(response) execute! - end - def create_protected_branch(project) - branch_name = project.repository.branch_names.find { |n| n != 'existing-branch' } - create(:protected_branch, project: project, name: branch_name) + expect(remote_mirror).to be_failed + expect(remote_mirror.last_error).to include("Some refs have diverged") + expect(remote_mirror.last_error).to include("refs/heads/master\n") + expect(remote_mirror.last_error).to include("refs/heads/develop") end end end