Add feature flag to use gitaly-ssh mirroring when cloning internal repos

This also allows us to simplify the naming since we can make some
fetching methods private.

3 files changed, 28 insertions, 38 deletions

diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 1468069a991..b1a6dbfe8d3 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -20,6 +20,7 @@ module Gitlab
       SEARCH_CONTEXT_LINES = 3
       REBASE_WORKTREE_PREFIX = 'rebase'.freeze
       SQUASH_WORKTREE_PREFIX = 'squash'.freeze
+      GITALY_INTERNAL_URL = 'ssh://gitaly/internal.git'.freeze
 
       NoRepository = Class.new(StandardError)
       InvalidBlobName = Class.new(StandardError)
@@ -1140,12 +1141,25 @@ module Gitlab
         @has_visible_content = has_local_branches?
       end
 
-      # Like all public `Gitlab::Git::Repository` methods, this method is part
-      # of `Repository`'s interface through `method_missing`.
-      # `Repository` has its own `fetch_remote` which uses `gitlab-shell` and
-      # takes some extra attributes, so we qualify this method name to prevent confusion.
-      def fetch_remote_without_shell(remote = 'origin')
-        run_git(['fetch', remote]).last.zero?
+      def fetch_repository_as_mirror(repository)
+        remote_name = "tmp-#{SecureRandom.hex}"
+
+        # Notice that this feature flag is not for `fetch_repository_as_mirror`
+        # as a whole but for the fetching mechanism (file path or gitaly-ssh).
+        url, env = gitaly_migrate(:fetch_internal) do |is_enabled|
+          if is_enabled
+            repository = RemoteRepository.new(repository) unless repository.is_a?(RemoteRepository)
+            [GITALY_INTERNAL_URL, repository.fetch_env]
+          else
+            [repository.path, nil]
+          end
+        end
+
+        add_remote(remote_name, url)
+        set_remote_as_mirror(remote_name)
+        fetch_remote(remote_name, env: env)
+      ensure
+        remove_remote(remote_name)
       end
 
       def blob_at(sha, path)
@@ -1849,7 +1863,7 @@ module Gitlab
       end
 
       def gitaly_fetch_ref(source_repository, source_ref:, target_ref:)
-        args = %W(fetch --no-tags -f ssh://gitaly/internal.git #{source_ref}:#{target_ref})
+        args = %W(fetch --no-tags -f #{GITALY_INTERNAL_URL} #{source_ref}:#{target_ref})
 
         run_git(args, env: source_repository.fetch_env)
       end
@@ -1869,6 +1883,10 @@ module Gitlab
       rescue Rugged::ReferenceError
         raise ArgumentError, 'Invalid merge source'
       end
+
+      def fetch_remote(remote_name = 'origin', env: nil)
+        run_git(['fetch', remote_name], env: env).last.zero?
+      end
     end
   end
 end
diff --git a/lib/gitlab/git/repository_mirroring.rb b/lib/gitlab/git/repository_mirroring.rb
index 392bef69e80..30c1e522e8e 100644
--- a/lib/gitlab/git/repository_mirroring.rb
+++ b/lib/gitlab/git/repository_mirroring.rb
@@ -31,19 +31,6 @@ module Gitlab
         end
       end
 
-      # Like all_refs public `Gitlab::Git::Repository` methods, this method is part
-      # of `Repository`'s interface through `method_missing`.
-      # `Repository` has its own `fetch_as_mirror` which uses `gitlab-shell` and
-      # takes some extra attributes, so we qualify this method name to prevent confusion.
-      def fetch_as_mirror_without_shell(url)
-        remote_name = "tmp-#{SecureRandom.hex}"
-        add_remote(remote_name, url)
-        set_remote_as_mirror(remote_name)
-        fetch_remote_without_shell(remote_name)
-      ensure
-        remove_remote(remote_name) if remote_name
-      end
-
       def remote_tags(remote)
         # Each line has this format: "dc872e9fa6963f8f03da6c8f6f264d0845d6b092\trefs/tags/v1.10.0\n"
         # We want to convert it to: [{ 'v1.10.0' => 'dc872e9fa6963f8f03da6c8f6f264d0845d6b092' }, ...]
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 08dd6ea80ff..4f24ca3c25f 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -588,12 +588,12 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe '#fetch_as_mirror_without_shell' do
+  describe '#fetch_repository_as_mirror' do
     let(:new_repository) do
       Gitlab::Git::Repository.new('default', 'my_project.git', '')
     end
 
-    subject { new_repository.fetch_as_mirror_without_shell(repository.path) }
+    subject { new_repository.fetch_repository_as_mirror(repository) }
 
     before do
       Gitlab::Shell.new.add_repository('default', 'my_project')
@@ -603,7 +603,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
       Gitlab::Shell.new.remove_repository(TestEnv.repos_path, 'my_project')
     end
 
-    it 'fetches a url as a mirror remote' do
+    it 'fetches a repository as a mirror remote' do
       subject
 
       expect(refs(new_repository.path)).to eq(refs(repository.path))
@@ -1662,21 +1662,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe '#fetch_remote_without_shell' do
-    let(:git_path) { Gitlab.config.git.bin_path }
-    let(:remote_name) { 'my_remote' }
-
-    subject { repository.fetch_remote_without_shell(remote_name) }
-
-    it 'fetches the remote and returns true if the command was successful' do
-      expect(repository).to receive(:popen)
-        .with(%W(#{git_path} fetch #{remote_name}), repository.path, {})
-        .and_return(['', 0])
-
-      expect(subject).to be(true)
-    end
-  end
-
   describe '#merge' do
     let(:repository) do
       Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')