Merge branch 'sh-remove-geo-node-ssh-keys' into 'master'

Remove special case treatment of Geo nodes for SSH

Closes #115

See merge request gitlab-org/gitlab-shell!179

CHANGELOG

+v6.0.0
+  - Remove bin/gitlab_projects (!180)
+  - Remove direct redis integration (!181)
+  - Remove support unhiding of all references for Geo nodes (!179)
+
 v5.11.0
   - Introduce a more-complete implementation of bin/authorized_keys (!178)
 

VERSION

-5.11.0
+6.0.0

lib/gitlab_access_status.rb

 require 'json'
 
 class GitAccessStatus
-  attr_reader :message, :gl_repository, :gl_username, :repository_path, :gitaly, :geo_node
+  attr_reader :message, :gl_repository, :gl_username, :repository_path, :gitaly
 
-  def initialize(status, message, gl_repository:, gl_username:, repository_path:, gitaly:, geo_node:)
+  def initialize(status, message, gl_repository:, gl_username:, repository_path:, gitaly:)
     @status = status
     @message = message
     @gl_repository = gl_repository
     @gl_username = gl_username
     @repository_path = repository_path
     @gitaly = gitaly
-    @geo_node = geo_node
   end
 
   def self.create_from_json(json)
@@ -20,8 +19,7 @@ class GitAccessStatus
              gl_repository: values["gl_repository"],
              gl_username: values["gl_username"],
              repository_path: values["repository_path"],
-             gitaly: values["gitaly"],
-             geo_node: values["geo_node"])
+             gitaly: values["gitaly"])
   end
 
   def allowed?

lib/gitlab_net.rb

@@ -44,8 +44,7 @@ class GitlabNet
                           gl_repository: nil,
                           gl_username: nil,
                           repository_path: nil,
-                          gitaly: nil,
-                          geo_node: false)
+                          gitaly: nil)
     end
   end
 

lib/gitlab_shell.rb

@@ -16,11 +16,8 @@ class GitlabShell
   }
   API_COMMANDS = %w(2fa_recovery_codes)
   GL_PROTOCOL = 'ssh'.freeze
-  # We have to use a negative transfer.hideRefs since this is the only way
-  # to undo an already set parameter: https://www.spinics.net/lists/git/msg256772.html
-  GIT_CONFIG_SHOW_ALL_REFS = "transfer.hideRefs=!refs".freeze
 
-  attr_accessor :key_id, :gl_repository, :repo_name, :command, :git_access, :show_all_refs, :username
+  attr_accessor :key_id, :gl_repository, :repo_name, :command, :git_access, :username
   attr_reader :repo_path
 
   def initialize(key_id)
@@ -112,7 +109,6 @@ class GitlabShell
     self.repo_path = status.repository_path
     @gl_repository = status.gl_repository
     @gitaly = status.gitaly
-    @show_all_refs = status.geo_node
     @username = status.gl_username
   end
 
@@ -144,8 +140,6 @@ class GitlabShell
         'gl_username' => @username
       }
 
-      gitaly_request['git_config_options'] = [GIT_CONFIG_SHOW_ALL_REFS] if @show_all_refs
-
       args = [gitaly_address, JSON.dump(gitaly_request)]
     end
 
@@ -177,8 +171,6 @@ class GitlabShell
       env['GITALY_TOKEN'] = @gitaly['token']
     end
 
-    env['GIT_CONFIG_PARAMETERS'] = "'#{GIT_CONFIG_SHOW_ALL_REFS}'" if @show_all_refs
-
     if git_trace_available?
       env.merge!({
         'GIT_TRACE' => @config.git_trace_log_file,

spec/gitlab_access_spec.rb

@@ -12,8 +12,7 @@ describe GitlabAccess do
                                                  gl_repository: 'project-1',
                                                  gl_username: 'testuser',
                                                  repository_path: '/home/git/repositories',
-                                                 gitaly: nil,
-                                                 geo_node: nil))
+                                                 gitaly: nil))
     end
   end
   subject do
@@ -50,8 +49,7 @@ describe GitlabAccess do
                   gl_repository: nil,
                   gl_username: nil,
                   repository_path: nil,
-                  gitaly: nil,
-                  geo_node: nil
+                  gitaly: nil
                 ))
       end
 

spec/gitlab_shell_spec.rb

@@ -25,8 +25,7 @@ describe GitlabShell do
     gl_repository: gl_repository,
     gl_username: gl_username,
     repository_path: repo_path,
-    gitaly: { 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default'} , 'address' => 'unix:gitaly.socket' },
-    geo_node: false
+    gitaly: { 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default'} , 'address' => 'unix:gitaly.socket' }
   )
   }
 
@@ -39,8 +38,7 @@ describe GitlabShell do
                 gl_repository: gl_repository,
                 gl_username: gl_username,
                 repository_path: repo_path,
-                gitaly: nil,
-                geo_node: nil))
+                gitaly: nil))
       api.stub(two_factor_recovery_codes: {
         'success' => true,
         'recovery_codes' => ['f67c514de60c4953', '41278385fc00c1e0']
@@ -182,25 +180,6 @@ describe GitlabShell do
       it_behaves_like 'upload-pack', 'git upload-pack'
     end
 
-    context 'gitaly-upload-pack with GeoNode' do
-      let(:ssh_cmd) { "git-upload-pack gitlab-ci.git" }
-      let(:gitaly_check_access_with_geo) { GitAccessStatus.new(
-        true,
-        'ok',
-        gl_repository: gl_repository,
-        gl_username: gl_username,
-        repository_path: repo_path,
-        gitaly: { 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default'} , 'address' => 'unix:gitaly.socket' },
-        geo_node: true) }
-      let(:gitaly_message_with_all_refs) { JSON.dump({ 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default' }, 'gl_repository' => gl_repository , 'gl_id' => key_id, 'gl_username' => gl_username, 'git_config_options' => [GitlabShell::GIT_CONFIG_SHOW_ALL_REFS]}) }
-      before { api.stub(check_access: gitaly_check_access_with_geo) }
-      after { subject.exec(ssh_cmd) }
-
-      it "should execute the command with unhiding refs" do
-        subject.should_receive(:exec_cmd).with(File.join(ROOT_PATH, "bin/gitaly-upload-pack"), 'unix:gitaly.socket', gitaly_message_with_all_refs)
-      end
-    end
-
     context 'gitaly-upload-pack' do
       let(:ssh_cmd) { "git-upload-pack gitlab-ci.git" }
       before {
@@ -376,8 +355,7 @@ describe GitlabShell do
                   gl_repository: nil,
                   gl_username: nil,
                   repository_path: nil,
-                  gitaly: nil,
-                  geo_node: nil))
+                  gitaly: nil))
         message = "gitlab-shell: Access denied for git command <git-upload-pack gitlab-ci.git> "
         message << "by user with key #{key_id}."
         $logger.should_receive(:warn).with(message)
@@ -439,20 +417,6 @@ describe GitlabShell do
       shell.send :exec_cmd, [1, 2]
     end
 
-    context "when show_all_refs is enabled" do
-      before { shell.show_all_refs = true }
-
-      it 'sets local git parameters' do
-        expected_hash = hash_including(
-          'GIT_CONFIG_PARAMETERS' => "'transfer.hideRefs=!refs'"
-        )
-
-        Kernel.should_receive(:exec).with(expected_hash, [1, 2], exec_options).once
-
-        shell.send :exec_cmd, [1, 2]
-      end
-    end
-
     context "when specifying a git_tracing log file" do
       let(:git_trace_log_file) { '/tmp/git_trace_performance.log' }