Merge branch 'pks-config-cleanups' into 'master'

Remove direct modifications of gitconfig

See merge request gitlab-org/gitlab!66822

app/models/repository.rb

@@ -938,8 +938,8 @@ class Repository
     end
   end
 
-  def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil, prune: true)
-    fetch_remote(remote_name, url: url, refmap: refmap, forced: forced, prune: prune)
+  def fetch_as_mirror(url, forced: false, refmap: :all_refs, prune: true, http_authorization_header: "")
+    fetch_remote(url, refmap: refmap, forced: forced, prune: prune, http_authorization_header: http_authorization_header)
   end
 
   def fetch_source_branch!(source_repository, source_branch, local_ref)

ee/app/models/concerns/geo/repository_replicator_strategy.rb

@@ -61,11 +61,9 @@ module Geo
     end
 
     def jwt_authentication_header
-      authorization = ::Gitlab::Geo::RepoSyncRequest.new(
+      ::Gitlab::Geo::RepoSyncRequest.new(
         scope: repository.full_path
       ).authorization
-
-      { "http.#{remote_url}.extraHeader" => "Authorization: #{authorization}" }
     end
 
     def deleted_params

ee/app/models/ee/repository.rb

@@ -17,15 +17,6 @@ module EE
       delegate :checksum, :find_remote_root_ref, to: :raw_repository
     end
 
-    # Transiently sets a configuration variable
-    def with_config(values = {})
-      raw_repository.set_config(values)
-
-      yield
-    ensure
-      raw_repository.delete_config(*values.keys)
-    end
-
     # Runs code after a repository has been synced.
     def after_sync
       expire_all_method_caches
@@ -35,8 +26,7 @@ module EE
 
     def fetch_upstream(url, forced: false, check_tags_changed: false)
       fetch_remote(
-        MIRROR_REMOTE,
-        url: url,
+        url,
         refmap: ["+refs/heads/*:refs/remotes/#{MIRROR_REMOTE}/*"],
         ssh_auth: project&.import_data,
         forced: forced,
@@ -96,8 +86,8 @@ module EE
     end
 
     # Update the default branch querying the remote to determine its HEAD
-    def update_root_ref(remote, remote_url, authorization)
-      root_ref = find_remote_root_ref(remote, remote_url, authorization)
+    def update_root_ref(remote_url, authorization)
+      root_ref = find_remote_root_ref(remote_url, authorization)
       change_head(root_ref) if root_ref.present?
     rescue ::Gitlab::Git::Repository::NoRepository => e
       ::Gitlab::AppLogger.error("Error updating root ref for repository #{full_path} (#{container.id}): #{e.message}.")

ee/app/services/geo/framework_repository_sync_service.rb

@@ -15,7 +15,6 @@ module Geo
 
     delegate :registry, to: :replicator
 
-    GEO_REMOTE_NAME  = 'geo'
     LEASE_TIMEOUT    = 8.hours
     LEASE_KEY_PREFIX = 'geo_sync_ssf_service'
     RETRIES_BEFORE_REDOWNLOAD = 5
@@ -112,9 +111,7 @@ module Geo
 
     def fetch_geo_mirror(repository)
       # Fetch the repository, using a JWT header for authentication
-      repository.with_config(replicator.jwt_authentication_header) do
-        repository.fetch_as_mirror(replicator.remote_url, remote_name: GEO_REMOTE_NAME, forced: true)
-      end
+      repository.fetch_as_mirror(replicator.remote_url, forced: true, http_authorization_header: replicator.jwt_authentication_header)
     end
 
     # Use snapshotting for redownloads *only* when enabled.
@@ -265,7 +262,7 @@ module Geo
         scope: repository.full_path
       ).authorization
 
-      repository.update_root_ref(GEO_REMOTE_NAME, replicator.remote_url, authorization)
+      repository.update_root_ref(replicator.remote_url, authorization)
     end
   end
 end

ee/app/services/geo/repository_base_sync_service.rb

@@ -15,7 +15,6 @@ module Geo
 
     attr_reader :project
 
-    GEO_REMOTE_NAME  = 'geo'
     LEASE_TIMEOUT    = 8.hours
     LEASE_KEY_PREFIX = 'geo_sync_service'
 
@@ -96,18 +95,14 @@ module Geo
 
     def fetch_geo_mirror(repository)
       # Fetch the repository, using a JWT header for authentication
-      repository.with_config(jwt_authentication_header) do
-        repository.fetch_as_mirror(remote_url, remote_name: GEO_REMOTE_NAME, forced: true)
-      end
+      repository.fetch_as_mirror(remote_url, forced: true, http_authorization_header: jwt_authentication_header)
     end
 
     # Build a JWT header for authentication
     def jwt_authentication_header
-      authorization = ::Gitlab::Geo::RepoSyncRequest.new(
+      ::Gitlab::Geo::RepoSyncRequest.new(
         scope: repository.full_path
       ).authorization
-
-      { "http.#{remote_url}.extraHeader" => "Authorization: #{authorization}" }
     end
 
     def remote_url

ee/app/services/geo/repository_sync_service.rb

@@ -54,7 +54,7 @@ module Geo
         scope: repository.full_path
       ).authorization
 
-      repository.update_root_ref(GEO_REMOTE_NAME, remote_url, authorization)
+      repository.update_root_ref(remote_url, authorization)
     end
 
     def execute_housekeeping

ee/spec/models/repository_spec.rb

@@ -55,42 +55,13 @@ RSpec.describe Repository do
     it 'fetches the URL without creating a remote' do
       expect(repository)
         .to receive(:fetch_remote)
-        .with(described_class::MIRROR_REMOTE, url: url, refmap: ['+refs/heads/*:refs/remotes/upstream/*'], ssh_auth: nil, forced: true, check_tags_changed: true)
+        .with(url, refmap: ['+refs/heads/*:refs/remotes/upstream/*'], ssh_auth: nil, forced: true, check_tags_changed: true)
         .and_return(nil)
 
       repository.fetch_upstream(url, forced: true, check_tags_changed: true)
     end
   end
 
-  describe '#with_config' do
-    let(:rugged) { rugged_repo(repository) }
-    let(:entries) do
-      {
-        'test.foo1' => 'hello',
-        'test.foo2' => 'world',
-        'http.http://gitlab-primary.geo/gitlab-qa-sandbox-group/qa-test-10-07-2018-07-22-41/geo-project-ac55ec2cd134afea.wiki.git.extraHeader' => 'Authorization: blabla'
-      }
-    end
-
-    it 'sets config only during the block' do
-      keys_should_not_be_set
-
-      repository.with_config(entries) do
-        entries.each do |key, value|
-          expect(rugged.config[key]).to eq(value)
-        end
-      end
-
-      keys_should_not_be_set
-    end
-
-    def keys_should_not_be_set
-      entries.each do |key, value|
-        expect(rugged.config[key]).to be_blank
-      end
-    end
-  end
-
   describe "Elastic search", :elastic do
     before do
       stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
@@ -250,7 +221,7 @@ RSpec.describe Repository do
     it 'updates the default branch when HEAD has changed' do
       stub_find_remote_root_ref(repository, ref: 'feature')
 
-      expect { repository.update_root_ref('origin', url, auth) }
+      expect { repository.update_root_ref(url, auth) }
         .to change { project.default_branch }
         .from('master')
         .to('feature')
@@ -261,7 +232,7 @@ RSpec.describe Repository do
 
       expect(repository).to receive(:change_head).with('master').and_call_original
 
-      repository.update_root_ref('origin', url, auth)
+      repository.update_root_ref(url, auth)
 
       expect(project.default_branch).to eq('master')
     end
@@ -269,22 +240,22 @@ RSpec.describe Repository do
     it 'does not update the default branch when HEAD does not exist' do
       stub_find_remote_root_ref(repository, ref: 'foo')
 
-      expect { repository.update_root_ref('origin', url, auth) }
+      expect { repository.update_root_ref(url, auth) }
         .not_to change { project.default_branch }
     end
 
     it 'does not raise error when repository does not exist' do
       allow(repository).to receive(:find_remote_root_ref)
-        .with('origin', url, auth)
+        .with(url, auth)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
-      expect { repository.update_root_ref('origin', url, auth) }.not_to raise_error
+      expect { repository.update_root_ref(url, auth) }.not_to raise_error
     end
 
     def stub_find_remote_root_ref(repository, ref:)
       allow(repository)
         .to receive(:find_remote_root_ref)
-        .with('origin', url, auth)
+        .with(url, auth)
         .and_return(ref)
     end
   end

ee/spec/services/geo/design_repository_sync_service_spec.rb

@@ -40,7 +40,7 @@ RSpec.describe Geo::DesignRepositorySyncService do
 
       allow_any_instance_of(Repository)
         .to receive(:find_remote_root_ref)
-        .with('geo')
+        .with(url_to_repo)
         .and_return('master')
 
       allow_any_instance_of(Geo::ProjectHousekeepingService).to receive(:execute)
@@ -53,13 +53,8 @@ RSpec.describe Geo::DesignRepositorySyncService do
     include_context 'lease handling'
 
     it 'fetches project repository with JWT credentials' do
-      expect(repository).to receive(:with_config)
-        .with("http.#{url_to_repo}.extraHeader" => anything)
-        .once
-        .and_call_original
-
       expect(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .once
 
       subject.execute
@@ -81,7 +76,7 @@ RSpec.describe Geo::DesignRepositorySyncService do
 
     it 'rescues when Gitlab::Shell::Error is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Shell::Error)
 
       expect { subject.execute }.not_to raise_error
@@ -89,7 +84,7 @@ RSpec.describe Geo::DesignRepositorySyncService do
 
     it 'rescues exception when Gitlab::Git::Repository::NoRepository is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-      .with(url_to_repo, remote_name: 'geo', forced: true)
+      .with(url_to_repo, forced: true, http_authorization_header: anything)
       .and_raise(Gitlab::Git::Repository::NoRepository)
 
       expect { subject.execute }.not_to raise_error
@@ -97,7 +92,7 @@ RSpec.describe Geo::DesignRepositorySyncService do
 
     it 'increases retry count when Gitlab::Git::Repository::NoRepository is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
       subject.execute
@@ -111,7 +106,7 @@ RSpec.describe Geo::DesignRepositorySyncService do
       registry = create(:geo_design_registry, project: project)
 
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Shell::Error.new(Gitlab::GitAccessDesign::ERROR_MESSAGES[:no_repo]))
 
       subject.execute
@@ -128,7 +123,7 @@ RSpec.describe Geo::DesignRepositorySyncService do
       expect(Geo::DesignRegistry.last.state).to eq 'synced'
 
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
       subject.execute

ee/spec/services/geo/framework_repository_sync_service_spec.rb

@@ -53,19 +53,15 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
 
       allow_any_instance_of(Repository)
         .to receive(:find_remote_root_ref)
-        .with('geo', url_to_repo, anything)
+        .with(url_to_repo, anything)
         .and_return('master')
     end
 
     include_context 'lease handling'
 
     it 'fetches project repository with JWT credentials' do
-      expect(repository).to receive(:with_config)
-        .with("http.#{url_to_repo}.extraHeader" => anything)
-        .and_call_original
-
       expect(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .once
 
       subject.execute
@@ -87,7 +83,7 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
 
     it 'rescues when Gitlab::Shell::Error is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Shell::Error)
 
       expect { subject.execute }.not_to raise_error
@@ -95,7 +91,7 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
 
     it 'rescues exception and fires after_create hook when Gitlab::Git::Repository::NoRepository is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-      .with(url_to_repo, remote_name: 'geo', forced: true)
+      .with(url_to_repo, forced: true, http_authorization_header: anything)
       .and_raise(Gitlab::Git::Repository::NoRepository)
 
       expect(repository).to receive(:after_create)
@@ -107,7 +103,7 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
       registry.save!
 
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
       subject.execute
@@ -120,7 +116,7 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
 
     it 'marks sync as successful if no repository found' do
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Shell::Error.new(Gitlab::GitAccessSnippet::ERROR_MESSAGES[:no_repo]))
 
       subject.execute
@@ -137,7 +133,7 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
       expect(registry.synced?).to be true
 
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
       subject.execute
@@ -178,7 +174,7 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
             before do
               allow(repository)
                 .to receive(:find_remote_root_ref)
-                .with('geo', url_to_repo, anything)
+                .with(url_to_repo, anything)
                 .and_return('feature')
             end
 
@@ -189,11 +185,6 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
             end
 
             it 'updates the default branch' do
-              expect(repository).to receive(:with_config)
-                .with("http.#{url_to_repo}.extraHeader" => anything)
-                .and_call_original
-                .once
-
               expect(repository).to receive(:change_head).with('feature').once
 
               subject.execute
@@ -208,11 +199,6 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
             end
 
             it 'updates the default branch' do
-              expect(repository).to receive(:with_config)
-                .with("http.#{url_to_repo}.extraHeader" => anything)
-                .and_call_original
-                .once
-
               expect(repository).to receive(:change_head).with('master').once
 
               subject.execute
@@ -224,7 +210,7 @@ RSpec.describe Geo::FrameworkRepositorySyncService, :geo do
       context 'when repository sync fail' do
         before do
           allow(repository).to receive(:fetch_as_mirror)
-            .with(url_to_repo, remote_name: 'geo', forced: true)
+            .with(url_to_repo, forced: true, http_authorization_header: anything)
             .and_raise(Gitlab::Shell::Error.new('shell error'))
         end
 

ee/spec/services/geo/repository_sync_service_spec.rb

@@ -36,7 +36,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
 
       allow_any_instance_of(Repository)
         .to receive(:find_remote_root_ref)
-        .with('geo', url_to_repo, anything)
+        .with(url_to_repo, anything)
         .and_return('master')
 
       allow_any_instance_of(Geo::ProjectHousekeepingService).to receive(:execute)
@@ -46,12 +46,8 @@ RSpec.describe Geo::RepositorySyncService, :geo do
     include_context 'lease handling'
 
     it 'fetches project repository with JWT credentials' do
-      expect(repository).to receive(:with_config)
-        .with("http.#{url_to_repo}.extraHeader" => anything)
-        .and_call_original
-
       expect(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .once
 
       subject.execute
@@ -73,7 +69,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
 
     it 'rescues when Gitlab::Shell::Error is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Shell::Error)
 
       expect { subject.execute }.not_to raise_error
@@ -81,7 +77,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
 
     it 'rescues exception and fires after_create hook when Gitlab::Git::Repository::NoRepository is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-      .with(url_to_repo, remote_name: 'geo', forced: true)
+      .with(url_to_repo, forced: true, http_authorization_header: anything)
       .and_raise(Gitlab::Git::Repository::NoRepository)
 
       expect(repository).to receive(:after_create)
@@ -91,7 +87,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
 
     it 'increases retry count when Gitlab::Git::Repository::NoRepository is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
       subject.execute
@@ -106,7 +102,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
       registry = create(:geo_project_registry, project: project)
 
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Shell::Error.new(Gitlab::GitAccess::ERROR_MESSAGES[:no_repo]))
 
       subject.execute
@@ -124,7 +120,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
       expect(Geo::ProjectRegistry.last.resync_repository).to be false
 
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
       subject.execute
@@ -138,7 +134,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
         create(:repository_state, :repository_verified, project: project)
 
         allow(repository).to receive(:fetch_as_mirror)
-          .with(url_to_repo, remote_name: 'geo', forced: true)
+          .with(url_to_repo, forced: true, http_authorization_header: anything)
           .and_raise(Gitlab::Shell::Error.new(Gitlab::GitAccess::ERROR_MESSAGES[:no_repo]))
 
         subject.execute
@@ -237,7 +233,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
             before do
               allow(project.repository)
                 .to receive(:find_remote_root_ref)
-                .with('geo', url_to_repo, anything)
+                .with(url_to_repo, anything)
                 .and_return('feature')
             end
 
@@ -248,11 +244,6 @@ RSpec.describe Geo::RepositorySyncService, :geo do
             end
 
             it 'updates the default branch' do
-              expect(repository).to receive(:with_config)
-                .with("http.#{url_to_repo}.extraHeader" => anything)
-                .and_call_original
-                .once
-
               expect(repository).to receive(:change_head).with('feature').once
 
               subject.execute
@@ -267,11 +258,6 @@ RSpec.describe Geo::RepositorySyncService, :geo do
             end
 
             it 'updates the default branch' do
-              expect(repository).to receive(:with_config)
-                .with("http.#{url_to_repo}.extraHeader" => anything)
-                .and_call_original
-                .once
-
               expect(repository).to receive(:change_head).with('master').once
 
               subject.execute
@@ -285,7 +271,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
 
         before do
           allow(repository).to receive(:fetch_as_mirror)
-            .with(url_to_repo, remote_name: 'geo', forced: true)
+            .with(url_to_repo, forced: true, http_authorization_header: anything)
             .and_raise(Gitlab::Shell::Error.new('shell error'))
         end
 

ee/spec/services/geo/wiki_sync_service_spec.rb

@@ -37,9 +37,8 @@ RSpec.describe Geo::WikiSyncService, :geo do
     include_context 'lease handling'
 
     it 'fetches wiki repository with JWT credentials' do
-      expect(repository).to receive(:with_config).with("http.#{url_to_repo}.extraHeader" => anything).and_call_original
       expect(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .once
 
       subject.execute
@@ -53,7 +52,7 @@ RSpec.describe Geo::WikiSyncService, :geo do
 
     it 'rescues exception when Gitlab::Shell::Error is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Shell::Error)
 
       expect { subject.execute }.not_to raise_error
@@ -61,7 +60,7 @@ RSpec.describe Geo::WikiSyncService, :geo do
 
     it 'rescues exception when Gitlab::Git::Repository::NoRepository is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
       expect { subject.execute }.not_to raise_error
@@ -69,7 +68,7 @@ RSpec.describe Geo::WikiSyncService, :geo do
 
     it 'increases retry count when Gitlab::Git::Repository::NoRepository is raised' do
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
       subject.execute
@@ -84,7 +83,7 @@ RSpec.describe Geo::WikiSyncService, :geo do
       registry = create(:geo_project_registry, project: project)
 
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Shell::Error.new(Gitlab::GitAccessWiki::ERROR_MESSAGES[:no_repo]))
 
       subject.execute
@@ -100,7 +99,7 @@ RSpec.describe Geo::WikiSyncService, :geo do
       described_class.new(project).execute
 
       allow(repository).to receive(:fetch_as_mirror)
-        .with(url_to_repo, remote_name: 'geo', forced: true)
+        .with(url_to_repo, forced: true, http_authorization_header: anything)
         .and_raise(Gitlab::Git::Repository::NoRepository)
 
       subject.execute
@@ -114,7 +113,7 @@ RSpec.describe Geo::WikiSyncService, :geo do
         create(:repository_state, :wiki_verified, project: project)
 
         allow(repository).to receive(:fetch_as_mirror)
-          .with(url_to_repo, remote_name: 'geo', forced: true)
+          .with(url_to_repo, forced: true, http_authorization_header: anything)
           .and_raise(Gitlab::Shell::Error.new(Gitlab::GitAccessWiki::ERROR_MESSAGES[:no_repo]))
 
         subject.execute
@@ -203,7 +202,7 @@ RSpec.describe Geo::WikiSyncService, :geo do
 
         before do
           allow(repository).to receive(:fetch_as_mirror)
-            .with(url_to_repo, remote_name: 'geo', forced: true)
+            .with(url_to_repo, forced: true, http_authorization_header: anything)
             .and_raise(Gitlab::Shell::Error.new('shell error'))
         end
 

ee/spec/support/shared_examples/services/base_sync_service_shared_examples.rb

@@ -165,7 +165,7 @@ RSpec.shared_context 'lease handling' do
 
   it 'returns the lease when sync fail' do
     allow(repository).to receive(:fetch_as_mirror)
-      .with(url_to_repo, remote_name: 'geo', forced: true)
+      .with(url_to_repo, forced: true, http_authorization_header: anything)
       .and_raise(Gitlab::Shell::Error)
 
     expect_to_cancel_exclusive_lease(lease_key, lease_uuid)

lib/gitlab/bitbucket_server_import/importer.rb

@@ -7,7 +7,6 @@ module Gitlab
       attr_reader :project, :project_key, :repository_slug, :client, :errors, :users, :already_imported_cache_key
       attr_accessor :logger
 
-      REMOTE_NAME = 'bitbucket_server'
       BATCH_SIZE = 100
       # The base cache key to use for tracking already imported objects.
       ALREADY_IMPORTED_CACHE_KEY =
@@ -142,7 +141,7 @@ module Gitlab
         log_info(stage: 'import_repository', message: 'starting import')
 
         project.ensure_repository
-        project.repository.fetch_as_mirror(project.import_url, refmap: self.class.refmap, remote_name: REMOTE_NAME)
+        project.repository.fetch_as_mirror(project.import_url, refmap: self.class.refmap)
 
         log_info(stage: 'import_repository', message: 'finished import')
       rescue Gitlab::Shell::Error => e

lib/gitlab/git/repository.rb

@@ -699,11 +699,11 @@ module Gitlab
         write_ref(ref, start_point)
       end
 
-      def find_remote_root_ref(remote_name, remote_url, authorization = nil)
-        return unless remote_name.present? && remote_url.present?
+      def find_remote_root_ref(remote_url, authorization = nil)
+        return unless remote_url.present?
 
         wrapped_gitaly_errors do
-          gitaly_remote_client.find_remote_root_ref(remote_name, remote_url, authorization)
+          gitaly_remote_client.find_remote_root_ref(remote_url, authorization)
         end
       end
 
@@ -803,18 +803,18 @@ module Gitlab
       # no_tags - should we use --no-tags flag?
       # prune - should we use --prune flag?
       # check_tags_changed - should we ask gitaly to calculate whether any tags changed?
-      def fetch_remote(remote, url: nil, refmap: nil, ssh_auth: nil, forced: false, no_tags: false, prune: true, check_tags_changed: false)
+      def fetch_remote(url, refmap: nil, ssh_auth: nil, forced: false, no_tags: false, prune: true, check_tags_changed: false, http_authorization_header: "")
         wrapped_gitaly_errors do
           gitaly_repository_client.fetch_remote(
-            remote,
-            url: url,
+            url,
             refmap: refmap,
             ssh_auth: ssh_auth,
             forced: forced,
             no_tags: no_tags,
             prune: prune,
             check_tags_changed: check_tags_changed,
-            timeout: GITLAB_PROJECTS_TIMEOUT
+            timeout: GITLAB_PROJECTS_TIMEOUT,
+            http_authorization_header: http_authorization_header
           )
         end
       end
@@ -924,12 +924,6 @@ module Gitlab
         end
       end
 
-      def delete_config(*keys)
-        wrapped_gitaly_errors do
-          gitaly_repository_client.delete_config(keys)
-        end
-      end
-
       def disconnect_alternates
         wrapped_gitaly_errors do
           gitaly_repository_client.disconnect_alternates

lib/gitlab/gitaly_client/remote_service.rb

@@ -26,8 +26,7 @@ module Gitlab
         @storage = repository.storage
       end
 
-      # The remote_name parameter is deprecated and will be removed soon.
-      def find_remote_root_ref(remote_name, remote_url, authorization)
+      def find_remote_root_ref(remote_url, authorization)
         request = Gitaly::FindRemoteRootRefRequest.new(repository: @gitaly_repo,
                                                        remote_url: remote_url,
                                                        http_authorization_header: authorization)

lib/gitlab/gitaly_client/repository_service.rb

@@ -73,17 +73,20 @@ module Gitlab
       # rubocop: disable Metrics/ParameterLists
       # The `remote` parameter is going away soonish anyway, at which point the
       # Rubocop warning can be enabled again.
-      def fetch_remote(remote, url:, refmap:, ssh_auth:, forced:, no_tags:, timeout:, prune: true, check_tags_changed: false)
+      def fetch_remote(url, refmap:, ssh_auth:, forced:, no_tags:, timeout:, prune: true, check_tags_changed: false, http_authorization_header: "")
         request = Gitaly::FetchRemoteRequest.new(
-          repository: @gitaly_repo, remote: remote, force: forced,
-          no_tags: no_tags, timeout: timeout, no_prune: !prune,
-          check_tags_changed: check_tags_changed
+          repository: @gitaly_repo,
+          force: forced,
+          no_tags: no_tags,
+          timeout: timeout,
+          no_prune: !prune,
+          check_tags_changed: check_tags_changed,
+          remote_params: Gitaly::Remote.new(
+            url: url,
+            mirror_refmaps: Array.wrap(refmap).map(&:to_s),
+            http_authorization_header: http_authorization_header
+          )
         )
-
-        if url
-          request.remote_params = Gitaly::Remote.new(url: url,
-                                                     mirror_refmaps: Array.wrap(refmap).map(&:to_s))
-        end
 
         if ssh_auth&.ssh_mirror_url?
           if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present?
@@ -297,22 +300,6 @@ module Gitlab
         nil
       end
 
-      def delete_config(keys)
-        return if keys.empty?
-
-        request = Gitaly::DeleteConfigRequest.new(repository: @gitaly_repo, keys: keys)
-
-        GitalyClient.call(
-          @storage,
-          :repository_service,
-          :delete_config,
-          request,
-          timeout: GitalyClient.fast_timeout
-        )
-
-        nil
-      end
-
       def license_short_name
         request = Gitaly::FindLicenseRequest.new(repository: @gitaly_repo)
 

lib/gitlab/github_import/importer/pull_requests_importer.rb

@@ -40,7 +40,7 @@ module Gitlab
           # updating the timestamp.
           project.update_column(:last_repository_updated_at, Time.zone.now)
 
-          project.repository.fetch_remote('github', url: project.import_url, refmap: Gitlab::GithubImport.refmap, forced: false)
+          project.repository.fetch_remote(project.import_url, refmap: Gitlab::GithubImport.refmap, forced: false)
 
           pname = project.path_with_namespace
 

spec/lib/gitlab/bitbucket_server_import/importer_spec.rb

@@ -32,8 +32,7 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do
       expect(subject).to receive(:delete_temp_branches)
       expect(project.repository).to receive(:fetch_as_mirror)
                                      .with('http://bitbucket:test@my-bitbucket',
-                                           refmap: [:heads, :tags, '+refs/pull-requests/*/to:refs/merge-requests/*/head'],
-                                           remote_name: 'bitbucket_server')
+                                           refmap: [:heads, :tags, '+refs/pull-requests/*/to:refs/merge-requests/*/head'])
 
       subject.execute
     end

spec/lib/gitlab/git/repository_spec.rb

@@ -491,6 +491,8 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
   end
 
   describe '#fetch_remote' do
+    let(:url) { 'http://example.clom' }
+
     it 'delegates to the gitaly RepositoryService' do
       ssh_auth = double(:ssh_auth)
       expected_opts = {
@@ -500,17 +502,17 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
         timeout: described_class::GITLAB_PROJECTS_TIMEOUT,
         prune: false,
         check_tags_changed: false,
-        url: nil,
-        refmap: nil
+        refmap: nil,
+        http_authorization_header: ""
       }
 
-      expect(repository.gitaly_repository_client).to receive(:fetch_remote).with('remote-name', expected_opts)
+      expect(repository.gitaly_repository_client).to receive(:fetch_remote).with(url, expected_opts)
 
-      repository.fetch_remote('remote-name', ssh_auth: ssh_auth, forced: true, no_tags: true, prune: false, check_tags_changed: false)
+      repository.fetch_remote(url, ssh_auth: ssh_auth, forced: true, no_tags: true, prune: false, check_tags_changed: false)
     end
 
     it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RepositoryService, :fetch_remote do
-      subject { repository.fetch_remote('remote-name') }
+      subject { repository.fetch_remote(url) }
     end
   end
 
@@ -584,29 +586,29 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
       expect_any_instance_of(Gitlab::GitalyClient::RemoteService)
         .to receive(:find_remote_root_ref).and_call_original
 
-      expect(repository.find_remote_root_ref('origin', SeedHelper::GITLAB_GIT_TEST_REPO_URL)).to eq 'master'
+      expect(repository.find_remote_root_ref(SeedHelper::GITLAB_GIT_TEST_REPO_URL)).to eq 'master'
     end
 
     it 'returns UTF-8' do
-      expect(repository.find_remote_root_ref('origin', SeedHelper::GITLAB_GIT_TEST_REPO_URL)).to be_utf8
+      expect(repository.find_remote_root_ref(SeedHelper::GITLAB_GIT_TEST_REPO_URL)).to be_utf8
     end
 
     it 'returns nil when remote name is nil' do
       expect_any_instance_of(Gitlab::GitalyClient::RemoteService)
         .not_to receive(:find_remote_root_ref)
 
-      expect(repository.find_remote_root_ref(nil, nil)).to be_nil
+      expect(repository.find_remote_root_ref(nil)).to be_nil
     end
 
     it 'returns nil when remote name is empty' do
       expect_any_instance_of(Gitlab::GitalyClient::RemoteService)
         .not_to receive(:find_remote_root_ref)
 
-      expect(repository.find_remote_root_ref('', '')).to be_nil
+      expect(repository.find_remote_root_ref('')).to be_nil
     end
 
     it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RemoteService, :find_remote_root_ref do
-      subject { repository.find_remote_root_ref('origin', SeedHelper::GITLAB_GIT_TEST_REPO_URL) }
+      subject { repository.find_remote_root_ref(SeedHelper::GITLAB_GIT_TEST_REPO_URL) }
     end
   end
 
@@ -1810,34 +1812,6 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
     end
   end
 
-  describe '#delete_config' do
-    let(:repository) { mutable_repository }
-    let(:entries) do
-      {
-        'test.foo1' => 'bla bla',
-        'test.foo2' => 1234,
-        'test.foo3' => true
-      }
-    end
-
-    it 'can delete config settings' do
-      entries.each do |key, value|
-        repository_rugged.config[key] = value
-      end
-
-      expect(repository.delete_config(*%w[does.not.exist test.foo1 test.foo2])).to be_nil
-
-      # Workaround for https://github.com/libgit2/rugged/issues/785: If
-      # Gitaly changes .gitconfig while Rugged has the file loaded
-      # Rugged::Repository#each_key will report stale values unless a
-      # lookup is done first.
-      expect(repository_rugged.config['test.foo1']).to be_nil
-      config_keys = repository_rugged.config.each_key.to_a
-      expect(config_keys).not_to include('test.foo1')
-      expect(config_keys).not_to include('test.foo2')
-    end
-  end
-
   describe '#merge_to_ref' do
     let(:repository) { mutable_repository }
     let(:branch_head) { '6d394385cf567f80a8fd85055db1ab4c5295806f' }

spec/lib/gitlab/gitaly_client/remote_service_spec.rb

@@ -6,11 +6,9 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do
   let(:project) { create(:project) }
   let(:storage_name) { project.repository_storage }
   let(:relative_path) { project.disk_path + '.git' }
-  let(:remote_name) { 'my-remote' }
   let(:client) { described_class.new(project.repository) }
 
   describe '#find_remote_root_ref' do
-    let(:remote) { 'origin' }
     let(:url) { 'http://git.example.com/my-repo.git' }
     let(:auth) { 'Basic secret' }
     let(:expected_params) { { remote_url: url, http_authorization_header: auth } }
@@ -22,7 +20,7 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do
         .with(gitaly_request_with_params(expected_params), kind_of(Hash))
         .and_return(double(ref: 'master'))
 
-      expect(client.find_remote_root_ref(remote, url, auth)).to eq 'master'
+      expect(client.find_remote_root_ref(url, auth)).to eq 'master'
     end
 
     it 'ensure ref is a valid UTF-8 string' do
@@ -32,7 +30,7 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do
         .with(gitaly_request_with_params(expected_params), kind_of(Hash))
         .and_return(double(ref: "an_invalid_ref_\xE5"))
 
-      expect(client.find_remote_root_ref(remote, url, auth)).to eq "an_invalid_ref_å"
+      expect(client.find_remote_root_ref(url, auth)).to eq "an_invalid_ref_å"
     end
   end
 

spec/lib/gitlab/gitaly_client/repository_service_spec.rb

@@ -122,14 +122,15 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
   end
 
   describe '#fetch_remote' do
-    shared_examples 'a fetch' do
-      it 'sends a fetch_remote_request message' do
-        expected_remote_params = Gitaly::Remote.new(
-          url: url, http_authorization_header: "", mirror_refmaps: [])
+    let(:url) { 'https://example.com/git/repo.git' }
 
+    it 'sends a fetch_remote_request message' do
       expected_request = gitaly_request_with_params(
-          remote: remote,
-          remote_params: url ? expected_remote_params : nil,
+        remote_params: Gitaly::Remote.new(
+          url: url,
+          http_authorization_header: "",
+          mirror_refmaps: []
+        ),
         ssh_key: '',
         known_hosts: '',
         force: false,
@@ -143,7 +144,7 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
         .with(expected_request, kind_of(Hash))
         .and_return(double(value: true))
 
-        client.fetch_remote(remote, url: url, refmap: nil, ssh_auth: nil, forced: false, no_tags: false, timeout: 1, check_tags_changed: false)
+      client.fetch_remote(url, refmap: nil, ssh_auth: nil, forced: false, no_tags: false, timeout: 1, check_tags_changed: false)
     end
 
     context 'SSH auth' do
@@ -170,12 +171,12 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
         end
 
         it do
-            expected_remote_params = Gitaly::Remote.new(
-              url: url, http_authorization_header: "", mirror_refmaps: [])
-
           expected_request = gitaly_request_with_params({
-              remote: remote,
-              remote_params: url ? expected_remote_params : nil,
+            remote_params: Gitaly::Remote.new(
+              url: url,
+              http_authorization_header: "",
+              mirror_refmaps: []
+            ),
             ssh_key: '',
             known_hosts: '',
             force: false,
@@ -188,24 +189,9 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
             .with(expected_request, kind_of(Hash))
             .and_return(double(value: true))
 
-            client.fetch_remote(remote, url: url, refmap: nil, ssh_auth: ssh_auth, forced: false, no_tags: false, timeout: 1)
-          end
-        end
-      end
-    end
-
-    context 'with remote' do
-      it_behaves_like 'a fetch' do
-        let(:remote) { 'remote-name' }
-        let(:url) { nil }
+          client.fetch_remote(url, refmap: nil, ssh_auth: ssh_auth, forced: false, no_tags: false, timeout: 1)
         end
       end
-
-    context 'with URL' do
-      it_behaves_like 'a fetch' do
-        let(:remote) { "" }
-        let(:url) { 'https://example.com/git/repo.git' }
-      end
     end
   end
 

spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb

@@ -164,7 +164,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do
 
       expect(project.repository)
         .to receive(:fetch_remote)
-        .with('github', forced: false, url: url, refmap: Gitlab::GithubImport.refmap)
+        .with(url, forced: false, refmap: Gitlab::GithubImport.refmap)
 
       freeze_time do
         importer.update_repository

spec/models/repository_spec.rb

@@ -1096,15 +1096,14 @@ RSpec.describe Repository do
 
   describe '#fetch_as_mirror' do
     let(:url) { "http://example.com" }
-    let(:remote_name) { "remote-name" }
 
     it 'fetches the URL without creating a remote' do
       expect(repository)
         .to receive(:fetch_remote)
-        .with(remote_name, url: url, forced: false, prune: true, refmap: :all_refs)
+        .with(url, forced: false, prune: true, refmap: :all_refs, http_authorization_header: "")
         .and_return(nil)
 
-      repository.fetch_as_mirror(url, remote_name: remote_name)
+      repository.fetch_as_mirror(url)
     end
   end