Merge branch '9217-warn-on-git-fetch-over-ssh-if-the-secondary-is-lagging-the-primary' into 'master'

Display console messages, if available

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

6 files changed, 90 insertions, 9 deletions

diff --git a/lib/action/custom.rb b/lib/action/custom.rb
index a2f3d59..4efb0a8 100644
--- a/lib/action/custom.rb
+++ b/lib/action/custom.rb
@@ -1,10 +1,12 @@
 require 'base64'
 
 require_relative '../http_helper'
+require_relative '../console_helper'
 
 module Action
   class Custom
     include HTTPHelper
+    include ConsoleHelper
 
     class BaseError < StandardError; end
     class MissingPayloadError < BaseError; end
@@ -105,7 +107,7 @@ module Action
     end
 
     def format_gitlab_output(str)
-      str.split("\n").map { |line| "> GitLab: #{line}" }.join("\n")
+      format_for_stderr(str.split("\n")).join("\n")
     end
 
     def validate!
diff --git a/lib/console_helper.rb b/lib/console_helper.rb
new file mode 100644
index 0000000..5c8bb83
--- /dev/null
+++ b/lib/console_helper.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module ConsoleHelper
+  LINE_PREFACE = '> GitLab:'
+
+  def write_stderr(messages)
+    format_for_stderr(messages).each do |message|
+      $stderr.puts(message)
+    end
+  end
+
+  def format_for_stderr(messages)
+    Array(messages).each_with_object([]) do |message, all|
+      all << "#{LINE_PREFACE} #{message}" unless message.empty?
+    end
+  end
+end
diff --git a/lib/gitlab_access_status.rb b/lib/gitlab_access_status.rb
index dc577f4..22e11a2 100644
--- a/lib/gitlab_access_status.rb
+++ b/lib/gitlab_access_status.rb
@@ -3,12 +3,14 @@ require 'json'
 class GitAccessStatus
   HTTP_MULTIPLE_CHOICES = '300'.freeze
 
-  attr_reader :message, :gl_repository, :gl_project_path, :gl_id, :gl_username, :gitaly, :git_protocol, :git_config_options, :payload
+  attr_reader :message, :gl_repository, :gl_project_path, :gl_id, :gl_username,
+              :gitaly, :git_protocol, :git_config_options, :payload,
+              :gl_console_messages
 
   def initialize(status, status_code, message, gl_repository: nil,
                  gl_project_path: nil, gl_id: nil,
                  gl_username: nil, gitaly: nil, git_protocol: nil,
-                 git_config_options: nil, payload: nil)
+                 git_config_options: nil, payload: nil, gl_console_messages: [])
     @status = status
     @status_code = status_code
     @message = message
@@ -20,6 +22,7 @@ class GitAccessStatus
     @gitaly = gitaly
     @git_protocol = git_protocol
     @payload = payload
+    @gl_console_messages = gl_console_messages
   end
 
   def self.create_from_json(json, status_code)
@@ -34,7 +37,8 @@ class GitAccessStatus
         git_config_options: values["git_config_options"],
         gitaly: values["gitaly"],
         git_protocol: values["git_protocol"],
-        payload: values["payload"])
+        payload: values["payload"],
+        gl_console_messages: values["gl_console_messages"])
   end
 
   def allowed?
diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb
index 2cb76a8..303f4d5 100644
--- a/lib/gitlab_shell.rb
+++ b/lib/gitlab_shell.rb
@@ -6,8 +6,11 @@ require 'pathname'
 require_relative 'gitlab_net'
 require_relative 'gitlab_metrics'
 require_relative 'action'
+require_relative 'console_helper'
 
 class GitlabShell # rubocop:disable Metrics/ClassLength
+  include ConsoleHelper
+
   class AccessDeniedError < StandardError; end
   class DisallowedCommandError < StandardError; end
   class InvalidRepositoryPathError < StandardError; end
@@ -63,6 +66,8 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
       @username = access_status.gl_username
       @git_config_options = access_status.git_config_options
       @gl_id = access_status.gl_id if defined?(@who)
+
+      write_stderr(access_status.gl_console_messages)
     elsif !defined?(@gl_id)
       # We're processing an API command like 2fa_recovery_codes, but
       # don't have a @gl_id yet, that means we're in the "username"
@@ -82,18 +87,18 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
 
     true
   rescue GitlabNet::ApiUnreachableError
-    $stderr.puts "GitLab: Failed to authorize your Git request: internal API unreachable"
+    write_stderr('Failed to authorize your Git request: internal API unreachable')
     false
   rescue AccessDeniedError => ex
     $logger.warn('Access denied', command: origin_cmd, user: log_username)
-    $stderr.puts "GitLab: #{ex.message}"
+    write_stderr(ex.message)
     false
   rescue DisallowedCommandError
     $logger.warn('Denied disallowed command', command: origin_cmd, user: log_username)
-    $stderr.puts "GitLab: Disallowed command"
+    write_stderr('Disallowed command')
     false
   rescue InvalidRepositoryPathError
-    $stderr.puts "GitLab: Invalid repository path"
+    write_stderr('Invalid repository path')
     false
   rescue Action::Custom::BaseError => ex
     $logger.warn('Custom action error', exception: ex.class, message: ex.message,
diff --git a/spec/console_helper_spec.rb b/spec/console_helper_spec.rb
new file mode 100644
index 0000000..cafc7cb
--- /dev/null
+++ b/spec/console_helper_spec.rb
@@ -0,0 +1,38 @@
+require_relative 'spec_helper'
+require_relative '../lib/console_helper'
+
+describe ConsoleHelper do
+  using RSpec::Parameterized::TableSyntax
+
+  class DummyClass
+    include ConsoleHelper
+  end
+
+  subject { DummyClass.new }
+
+  describe '#write_stderr' do
+    where(:messages, :stderr_output) do
+      'test'          | "> GitLab: test\n"
+      %w{test1 test2} | "> GitLab: test1\n> GitLab: test2\n"
+    end
+
+    with_them do
+      it 'puts to $stderr, prefaced with > GitLab:' do
+        expect { subject.write_stderr(messages) }.to output(stderr_output).to_stderr
+      end
+    end
+  end
+
+  describe '#format_for_stderr' do
+    where(:messages, :result) do
+      'test'          | ['> GitLab: test']
+      %w{test1 test2} | ['> GitLab: test1', '> GitLab: test2']
+    end
+
+    with_them do
+      it 'returns message(s), prefaced with > GitLab:' do
+          expect(subject.format_for_stderr(messages)).to eq(result)
+      end
+    end
+  end
+end
diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb
index c261e6f..6a059aa 100644
--- a/spec/gitlab_shell_spec.rb
+++ b/spec/gitlab_shell_spec.rb
@@ -33,7 +33,8 @@ describe GitlabShell do
       gl_username: gl_username,
       git_config_options: git_config_options,
       gitaly: { 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default'} , 'address' => 'unix:gitaly.socket' },
-      git_protocol: git_protocol
+      git_protocol: git_protocol,
+      gl_console_messages:  gl_console_messages
     )
   end
 
@@ -69,6 +70,7 @@ describe GitlabShell do
   let(:gl_username) { 'testuser' }
   let(:git_config_options) { ['receive.MaxInputSize=10000'] }
   let(:git_protocol) { 'version=2' }
+  let(:gl_console_messages) { nil }
 
   before do
     allow_any_instance_of(GitlabConfig).to receive(:audit_usernames).and_return(false)
@@ -438,6 +440,19 @@ describe GitlabShell do
         end
       end
     end
+
+    context 'with a console message' do
+      let(:ssh_cmd) { "git-receive-pack gitlab-ci.git" }
+      let(:gl_console_messages) { 'Very important message' }
+
+      before do
+        allow(api).to receive(:check_access).and_return(gitaly_check_access)
+      end
+
+      it 'displays the message on $stderr' do
+        expect { subject.exec(ssh_cmd) }.to output("> GitLab: #{gl_console_messages}\n").to_stderr
+      end
+    end
   end
 
   describe '#validate_access' do