diff options
author | Nick Thomas <nick@gitlab.com> | 2019-04-04 14:16:19 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-04-04 14:16:19 +0000 |
commit | 456b4a26ce6b188eb0dd0e8768bf647a257f7b2d (patch) | |
tree | b69113a89dc6f80cafaca5dfaa0dc9c63873bb2f | |
parent | a3bdb3ef8ccbb0601c79882480b48bde85611509 (diff) | |
parent | 5cd13175c94912651202a61ab755cfead33a3ee9 (diff) | |
download | gitlab-shell-456b4a26ce6b188eb0dd0e8768bf647a257f7b2d.tar.gz |
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
-rw-r--r-- | lib/action/custom.rb | 4 | ||||
-rw-r--r-- | lib/console_helper.rb | 17 | ||||
-rw-r--r-- | lib/gitlab_access_status.rb | 10 | ||||
-rw-r--r-- | lib/gitlab_shell.rb | 13 | ||||
-rw-r--r-- | spec/console_helper_spec.rb | 38 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 17 |
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 |