Commit f9cd192b authored by Patrick Steinhardt's avatar Patrick Steinhardt Committed by Nikola Milojevic

Remove `ref_name` parameter in `RemoteMirror`

On-disk remotes are deprecated, and in fact we don't create any remotes
in GitLab anymore in favor of using in-memory remotes, where we just
pass the URL to Gitaly. We thus don't need to pass information about
remote names anymore given that it's unused nowadays.

Remove the `ref_name` parameter in `RemoteMirror` to reflect this. Note
that this parameter is misnamed, and should really have been called
`remote_name`. Furthermore, drop now-unneeded infrastructure which was
required to track the remote name.
parent 65444a95
...@@ -22,8 +22,6 @@ class RemoteMirror < ApplicationRecord ...@@ -22,8 +22,6 @@ class RemoteMirror < ApplicationRecord
validates :url, presence: true, public_url: { schemes: %w(ssh git http https), allow_blank: true, enforce_user: true } validates :url, presence: true, public_url: { schemes: %w(ssh git http https), allow_blank: true, enforce_user: true }
before_save :set_new_remote_name, if: :mirror_url_changed?
after_save :set_override_remote_mirror_available, unless: -> { Gitlab::CurrentSettings.current_application_settings.mirror_available } after_save :set_override_remote_mirror_available, unless: -> { Gitlab::CurrentSettings.current_application_settings.mirror_available }
after_update :reset_fields, if: :saved_change_to_mirror_url? after_update :reset_fields, if: :saved_change_to_mirror_url?
...@@ -85,10 +83,6 @@ class RemoteMirror < ApplicationRecord ...@@ -85,10 +83,6 @@ class RemoteMirror < ApplicationRecord
end end
end end
def remote_name
super || fallback_remote_name
end
def update_failed? def update_failed?
update_status == 'failed' update_status == 'failed'
end end
...@@ -100,7 +94,6 @@ class RemoteMirror < ApplicationRecord ...@@ -100,7 +94,6 @@ class RemoteMirror < ApplicationRecord
def update_repository def update_repository
Gitlab::Git::RemoteMirror.new( Gitlab::Git::RemoteMirror.new(
project.repository.raw, project.repository.raw,
remote_name,
remote_url, remote_url,
**options_for_update **options_for_update
).update ).update
...@@ -268,12 +261,6 @@ class RemoteMirror < ApplicationRecord ...@@ -268,12 +261,6 @@ class RemoteMirror < ApplicationRecord
super super
end end
def fallback_remote_name
return unless id
"remote_mirror_#{id}"
end
def recently_scheduled? def recently_scheduled?
return false unless self.last_update_started_at return false unless self.last_update_started_at
...@@ -296,10 +283,6 @@ class RemoteMirror < ApplicationRecord ...@@ -296,10 +283,6 @@ class RemoteMirror < ApplicationRecord
project.update(remote_mirror_available_overridden: enabled) project.update(remote_mirror_available_overridden: enabled)
end end
def set_new_remote_name
self.remote_name = "remote_mirror_#{SecureRandom.hex}"
end
def mirror_url_changed? def mirror_url_changed?
url_changed? || attribute_changed?(:credentials) url_changed? || attribute_changed?(:credentials)
end end
......
...@@ -5,11 +5,10 @@ module Gitlab ...@@ -5,11 +5,10 @@ module Gitlab
class RemoteMirror class RemoteMirror
include Gitlab::Git::WrapsGitalyErrors include Gitlab::Git::WrapsGitalyErrors
attr_reader :repository, :ref_name, :remote_url, :only_branches_matching, :ssh_key, :known_hosts, :keep_divergent_refs attr_reader :repository, :remote_url, :only_branches_matching, :ssh_key, :known_hosts, :keep_divergent_refs
def initialize(repository, ref_name, remote_url, only_branches_matching: [], ssh_key: nil, known_hosts: nil, keep_divergent_refs: false) def initialize(repository, remote_url, only_branches_matching: [], ssh_key: nil, known_hosts: nil, keep_divergent_refs: false)
@repository = repository @repository = repository
@ref_name = ref_name
@remote_url = remote_url @remote_url = remote_url
@only_branches_matching = only_branches_matching @only_branches_matching = only_branches_matching
@ssh_key = ssh_key @ssh_key = ssh_key
...@@ -20,7 +19,6 @@ module Gitlab ...@@ -20,7 +19,6 @@ module Gitlab
def update def update
wrapped_gitaly_errors do wrapped_gitaly_errors do
repository.gitaly_remote_client.update_remote_mirror( repository.gitaly_remote_client.update_remote_mirror(
ref_name,
remote_url, remote_url,
only_branches_matching, only_branches_matching,
ssh_key: ssh_key, ssh_key: ssh_key,
......
...@@ -37,18 +37,13 @@ module Gitlab ...@@ -37,18 +37,13 @@ module Gitlab
encode_utf8(response.ref) encode_utf8(response.ref)
end end
def update_remote_mirror(ref_name, remote_url, only_branches_matching, ssh_key: nil, known_hosts: nil, keep_divergent_refs: false) def update_remote_mirror(remote_url, only_branches_matching, ssh_key: nil, known_hosts: nil, keep_divergent_refs: false)
req_enum = Enumerator.new do |y| req_enum = Enumerator.new do |y|
first_request = Gitaly::UpdateRemoteMirrorRequest.new( first_request = Gitaly::UpdateRemoteMirrorRequest.new(
repository: @gitaly_repo repository: @gitaly_repo
) )
if remote_url first_request.remote = Gitaly::UpdateRemoteMirrorRequest::Remote.new(url: remote_url)
first_request.remote = Gitaly::UpdateRemoteMirrorRequest::Remote.new(url: remote_url)
else
first_request.ref_name = ref_name
end
first_request.ssh_key = ssh_key if ssh_key.present? first_request.ssh_key = ssh_key if ssh_key.present?
first_request.known_hosts = known_hosts if known_hosts.present? first_request.known_hosts = known_hosts if known_hosts.present?
first_request.keep_divergent_refs = keep_divergent_refs first_request.keep_divergent_refs = keep_divergent_refs
......
...@@ -265,7 +265,6 @@ FactoryBot.define do ...@@ -265,7 +265,6 @@ FactoryBot.define do
trait :remote_mirror do trait :remote_mirror do
transient do transient do
remote_name { "remote_mirror_#{SecureRandom.hex}" }
url { "http://foo.com" } url { "http://foo.com" }
enabled { true } enabled { true }
end end
......
...@@ -6,30 +6,17 @@ RSpec.describe Gitlab::Git::RemoteMirror do ...@@ -6,30 +6,17 @@ RSpec.describe Gitlab::Git::RemoteMirror do
describe '#update' do describe '#update' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:repository) { project.repository } let(:repository) { project.repository }
let(:ref_name) { 'foo' }
let(:url) { 'https://example.com' } let(:url) { 'https://example.com' }
let(:options) { { only_branches_matching: ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true } } let(:options) { { only_branches_matching: ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true } }
subject(:remote_mirror) { described_class.new(repository, ref_name, url, **options) } subject(:remote_mirror) { described_class.new(repository, url, **options) }
shared_examples 'an update' do it 'delegates to the Gitaly client' do
it 'delegates to the Gitaly client' do expect(repository.gitaly_remote_client)
expect(repository.gitaly_remote_client) .to receive(:update_remote_mirror)
.to receive(:update_remote_mirror) .with(url, ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true)
.with(ref_name, url, ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS', keep_divergent_refs: true)
remote_mirror.update # rubocop:disable Rails/SaveBang
end
end
context 'with url' do
it_behaves_like 'an update'
end
context 'without url' do
let(:url) { nil }
it_behaves_like 'an update' remote_mirror.update # rubocop:disable Rails/SaveBang
end end
it 'wraps gitaly errors' do it 'wraps gitaly errors' do
......
...@@ -35,34 +35,19 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do ...@@ -35,34 +35,19 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do
end end
describe '#update_remote_mirror' do describe '#update_remote_mirror' do
let(:ref_name) { 'remote_mirror_1' }
let(:only_branches_matching) { %w[my-branch master] } let(:only_branches_matching) { %w[my-branch master] }
let(:ssh_key) { 'KEY' } let(:ssh_key) { 'KEY' }
let(:known_hosts) { 'KNOWN HOSTS' } let(:known_hosts) { 'KNOWN HOSTS' }
let(:url) { 'http:://git.example.com/my-repo.git' }
let(:expected_params) { { remote: Gitaly::UpdateRemoteMirrorRequest::Remote.new(url: url) } }
shared_examples 'an update' do it 'sends an update_remote_mirror message' do
it 'sends an update_remote_mirror message' do expect_any_instance_of(Gitaly::RemoteService::Stub)
expect_any_instance_of(Gitaly::RemoteService::Stub) .to receive(:update_remote_mirror)
.to receive(:update_remote_mirror) .with(array_including(gitaly_request_with_params(expected_params)), kind_of(Hash))
.with(array_including(gitaly_request_with_params(expected_params)), kind_of(Hash)) .and_return(double(:update_remote_mirror_response))
.and_return(double(:update_remote_mirror_response))
client.update_remote_mirror(ref_name, url, only_branches_matching, ssh_key: ssh_key, known_hosts: known_hosts, keep_divergent_refs: true)
end
end
context 'with remote name' do
let(:url) { nil }
let(:expected_params) { { ref_name: ref_name } }
it_behaves_like 'an update'
end
context 'with remote URL' do
let(:url) { 'http:://git.example.com/my-repo.git' }
let(:expected_params) { { remote: Gitaly::UpdateRemoteMirrorRequest::Remote.new(url: url) } }
it_behaves_like 'an update' client.update_remote_mirror(url, only_branches_matching, ssh_key: ssh_key, known_hosts: known_hosts, keep_divergent_refs: true)
end end
end end
......
...@@ -105,35 +105,6 @@ RSpec.describe RemoteMirror, :mailer do ...@@ -105,35 +105,6 @@ RSpec.describe RemoteMirror, :mailer do
end end
end end
describe '#remote_name' do
context 'when remote name is persisted in the database' do
it 'returns remote name with random value' do
allow(SecureRandom).to receive(:hex).and_return('secret')
remote_mirror = create(:remote_mirror)
expect(remote_mirror.remote_name).to eq('remote_mirror_secret')
end
end
context 'when remote name is not persisted in the database' do
it 'returns remote name with remote mirror id' do
remote_mirror = create(:remote_mirror)
remote_mirror.remote_name = nil
expect(remote_mirror.remote_name).to eq("remote_mirror_#{remote_mirror.id}")
end
end
context 'when remote is not persisted in the database' do
it 'returns nil' do
remote_mirror = build(:remote_mirror, remote_name: nil)
expect(remote_mirror.remote_name).to be_nil
end
end
end
describe '#bare_url' do describe '#bare_url' do
it 'returns the URL without any credentials' do it 'returns the URL without any credentials' do
remote_mirror = build(:remote_mirror, url: 'http://user:pass@example.com/foo') remote_mirror = build(:remote_mirror, url: 'http://user:pass@example.com/foo')
...@@ -158,7 +129,6 @@ RSpec.describe RemoteMirror, :mailer do ...@@ -158,7 +129,6 @@ RSpec.describe RemoteMirror, :mailer do
expect(git_remote_mirror).to have_received(:new).with( expect(git_remote_mirror).to have_received(:new).with(
mirror.project.repository.raw, mirror.project.repository.raw,
mirror.remote_name,
mirror.url, mirror.url,
keep_divergent_refs: true keep_divergent_refs: true
) )
......
...@@ -7,8 +7,6 @@ RSpec.describe Projects::UpdateRemoteMirrorService do ...@@ -7,8 +7,6 @@ RSpec.describe Projects::UpdateRemoteMirrorService do
let_it_be(:remote_project) { create(:forked_project_with_submodules) } let_it_be(:remote_project) { create(:forked_project_with_submodules) }
let_it_be(:remote_mirror) { create(:remote_mirror, project: project, enabled: true) } let_it_be(:remote_mirror) { create(:remote_mirror, project: project, enabled: true) }
let(:remote_name) { remote_mirror.remote_name }
subject(:service) { described_class.new(project, project.creator) } subject(:service) { described_class.new(project, project.creator) }
describe '#execute' do describe '#execute' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment