Skip to content

G
gitlab-shell
Commit 306b6b5b authored by Nick Thomas's avatar Nick Thomas
Browse files
Options

Merge branch 'zj-cleanup-exec' into 'master' 

Cleanup `git-upload-*` and `git-receive-*` related code

Closes gitaly#1300

See merge request gitlab-org/gitlab-shell!232
parents abb55c83 911e49e1
Hide whitespace changes
Inline Side-by-side
Showing with 52 additions and 223 deletions
config.yml.example
View file @ 306b6b5b
......@@ -49,10 +49,3 @@ log_level: INFO
# Set to true to see real usernames in the logs instead of key ids, which is easier to follow, but
# incurs an extra API call on every gitlab-shell command.
audit_usernames: false
# Git trace log file.
# If set, git commands receive GIT_TRACE* environment variables
# See https://git-scm.com/book/es/v2/Git-Internals-Environment-Variables#Debugging for documentation
# An absolute path starting with / – the trace output will be appended to that file.
# It needs to exist so we can check permissions and avoid to throwing warnings to the users.
git_trace_log_file:
lib/gitlab_config.rb
View file @ 306b6b5b
......@@ -50,10 +50,6 @@ class GitlabConfig
@config['audit_usernames'] ||= false
end
def git_trace_log_file
@config['git_trace_log_file']
end
def metrics_log_file
@config['metrics_log_file'] ||= File.join(ROOT_PATH, 'gitlab-shell-metrics.log')
end
......
lib/gitlab_shell.rb
View file @ 306b6b5b
# frozen_string_literal: true
require 'shellwords'
require 'pathname'
......@@ -9,14 +11,15 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
class DisallowedCommandError < StandardError; end
class InvalidRepositoryPathError < StandardError; end
GIT_COMMANDS = %w(git-upload-pack git-receive-pack git-upload-archive git-lfs-authenticate).freeze
GITALY_MIGRATED_COMMANDS = {
GITALY_COMMANDS = {
'git-upload-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-upload-pack'),
'git-upload-archive' => File.join(ROOT_PATH, 'bin', 'gitaly-upload-archive'),
'git-receive-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-receive-pack')
}.freeze
GIT_COMMANDS = (GITALY_COMMANDS.keys + ['git-lfs-authenticate']).freeze
API_COMMANDS = %w(2fa_recovery_codes).freeze
GL_PROTOCOL = 'ssh'.freeze
GL_PROTOCOL = 'ssh'
attr_accessor :gl_id, :gl_repository, :repo_name, :command, :git_access, :git_protocol
......@@ -136,64 +139,31 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
return
end
executable = @command
args = []
if GITALY_MIGRATED_COMMANDS.key?(executable) && @gitaly
executable = GITALY_MIGRATED_COMMANDS[executable]
gitaly_address = @gitaly['address']
# The entire gitaly_request hash should be built in gitlab-ce and passed
# on as-is. For now we build a fake one on the spot.
gitaly_request = {
'repository' => @gitaly['repository'],
'gl_repository' => @gl_repository,
'gl_id' => @gl_id,
'gl_username' => @username,
'git_config_options' => @git_config_options,
'git_protocol' => @git_protocol
}
args = [gitaly_address, JSON.dump(gitaly_request)]
end
# TODO: instead of building from pieces here in gitlab-shell, build the
# entire gitaly_request in gitlab-ce and pass on as-is here.
args = JSON.dump(
'repository' => @gitaly['repository'],
'gl_repository' => @gl_repository,
'gl_id' => @gl_id,
'gl_username' => @username,
'git_config_options' => @git_config_options,
'git_protocol' => @git_protocol
)
args_string = [File.basename(executable), *args].join(' ')
gitaly_address = @gitaly['address']
executable = GITALY_COMMANDS.fetch(@command)
gitaly_bin = File.basename(executable)
args_string = [gitaly_bin, gitaly_address, args].join(' ')
$logger.info('executing git command', command: args_string, user: log_username)
exec_cmd(executable, *args)
exec_cmd(executable, gitaly_address: gitaly_address, token: @gitaly['token'], json_args: args)
end
# This method is not covered by Rspec because it ends the current Ruby process.
def exec_cmd(*args)
# If you want to call a command without arguments, use
# exec_cmd(['my_command', 'my_command']) . Otherwise use
# exec_cmd('my_command', 'my_argument', ...).
if args.count == 1 && !args.first.is_a?(Array)
raise DisallowedCommandError
end
env = {
'HOME' => ENV['HOME'],
'PATH' => ENV['PATH'],
'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'],
'LANG' => ENV['LANG'],
'GL_ID' => @gl_id,
'GL_PROTOCOL' => GL_PROTOCOL,
'GL_REPOSITORY' => @gl_repository,
'GL_USERNAME' => @username
}
if @gitaly && @gitaly.include?('token')
env['GITALY_TOKEN'] = @gitaly['token']
end
if git_trace_available?
env.merge!(
'GIT_TRACE' => @config.git_trace_log_file,
'GIT_TRACE_PACKET' => @config.git_trace_log_file,
'GIT_TRACE_PERFORMANCE' => @config.git_trace_log_file
)
end
def exec_cmd(executable, gitaly_address:, token:, json_args:)
env = { 'GITALY_TOKEN' => token }
args = [executable, gitaly_address, json_args]
# We use 'chdir: ROOT_PATH' to let the next executable know where config.yml is.
Kernel.exec(env, *args, unsetenv_others: true, chdir: ROOT_PATH)
end
......@@ -274,21 +244,4 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
"#{resp['message']}"
end
end
def git_trace_available?
return false unless @config.git_trace_log_file
if Pathname(@config.git_trace_log_file).relative?
$logger.warn('git trace log path must be absolute, ignoring', git_trace_log_file: @config.git_trace_log_file)
return false
end
begin
File.open(@config.git_trace_log_file, 'a') { nil }
return true
rescue => ex
$logger.warn('Failed to open git trace log file', git_trace_log_file: @config.git_trace_log_file, error: ex.to_s)
return false
end
end
end
spec/gitlab_shell_spec.rb
View file @ 306b6b5b
......@@ -201,19 +201,13 @@ describe GitlabShell do
end
end
context 'git-upload-pack' do
it_behaves_like 'upload-pack', 'git-upload-pack'
end
context 'git upload-pack' do
it_behaves_like 'upload-pack', 'git upload-pack'
end
context 'gitaly-upload-pack' do
let(:ssh_cmd) { "git-upload-pack gitlab-ci.git" }
before do
allow(api).to receive(:check_access).and_return(gitaly_check_access)
end
after { subject.exec(ssh_cmd) }
it "should process the command" do
......@@ -221,12 +215,13 @@ describe GitlabShell do
end
it "should execute the command" do
expect(subject).to receive(:exec_cmd).with(File.join(ROOT_PATH, "bin/gitaly-upload-pack"), 'unix:gitaly.socket', gitaly_message)
expect(subject).to receive(:exec_cmd).with(File.join(ROOT_PATH, "bin/gitaly-upload-pack"), gitaly_address: 'unix:gitaly.socket', json_args: gitaly_message, token: nil)
end
it "should log the command execution" do
message = "executing git command"
user_string = "user with id #{gl_id}"
expect($logger).to receive(:info).with(message, command: "gitaly-upload-pack unix:gitaly.socket #{gitaly_message}", user: user_string)
end
......@@ -238,6 +233,11 @@ describe GitlabShell do
context 'git-receive-pack' do
let(:ssh_cmd) { "git-receive-pack gitlab-ci.git" }
before do
allow(api).to receive(:check_access).and_return(gitaly_check_access)
end
after { subject.exec(ssh_cmd) }
it "should process the command" do
......@@ -245,13 +245,13 @@ describe GitlabShell do
end
it "should execute the command" do
expect(subject).to receive(:exec_cmd).with('git-receive-pack')
expect(subject).to receive(:exec_cmd).with(File.join(ROOT_PATH, "bin/gitaly-receive-pack"), gitaly_address: 'unix:gitaly.socket', json_args: gitaly_message, token: nil)
end
it "should log the command execution" do
message = "executing git command"
user_string = "user with id #{gl_id}"
expect($logger).to receive(:info).with(message, command: "git-receive-pack", user: user_string)
expect($logger).to receive(:info).with(message, command: "gitaly-receive-pack unix:gitaly.socket #{gitaly_message}", user: user_string)
end
end
......@@ -267,7 +267,7 @@ describe GitlabShell do
end
it "should execute the command" do
expect(subject).to receive(:exec_cmd).with(File.join(ROOT_PATH, "bin/gitaly-receive-pack"), 'unix:gitaly.socket', gitaly_message)
expect(subject).to receive(:exec_cmd).with(File.join(ROOT_PATH, "bin/gitaly-receive-pack"), gitaly_address: 'unix:gitaly.socket', json_args: gitaly_message, token: nil)
end
it "should log the command execution" do
......@@ -284,7 +284,6 @@ describe GitlabShell do
shared_examples_for 'upload-archive' do |command|
let(:ssh_cmd) { "#{command} gitlab-ci.git" }
let(:exec_cmd_params) { ['git-upload-archive'] }
let(:exec_cmd_log_params) { exec_cmd_params }
after { subject.exec(ssh_cmd) }
......@@ -309,14 +308,6 @@ describe GitlabShell do
end
end
context 'git-upload-archive' do
it_behaves_like 'upload-archive', 'git-upload-archive'
end
context 'git upload-archive' do
it_behaves_like 'upload-archive', 'git upload-archive'
end
context 'gitaly-upload-archive' do
before do
allow(api).to receive(:check_access).and_return(gitaly_check_access)
......@@ -327,8 +318,7 @@ describe GitlabShell do
let(:exec_cmd_params) do
[
File.join(ROOT_PATH, "bin", gitaly_executable),
'unix:gitaly.socket',
gitaly_message
{ gitaly_address: 'unix:gitaly.socket', json_args: gitaly_message, token: nil }
]
end
let(:exec_cmd_log_params) do
......@@ -424,6 +414,19 @@ describe GitlabShell do
let(:ssh_cmd) { "git-upload-pack gitlab-ci.git" }
describe 'check access with api' do
before do
allow(api).to receive(:check_access).and_return(
GitAccessStatus.new(
false,
'denied',
gl_repository: nil,
gl_id: nil,
gl_username: nil,
git_config_options: nil,
gitaly: nil,
git_protocol: nil))
end
after { subject.exec(ssh_cmd) }
it "should call api.check_access" do
......@@ -431,126 +434,10 @@ describe GitlabShell do
end
it "should disallow access and log the attempt if check_access returns false status" do
allow(api).to receive(:check_access).and_return(GitAccessStatus.new(
false,
'denied',
gl_repository: nil,
gl_id: nil,
gl_username: nil,
git_config_options: nil,
gitaly: nil,
git_protocol: nil))
message = 'Access denied'
user_string = "user with id #{gl_id}"
expect($logger).to receive(:warn).with(message, command: 'git-upload-pack gitlab-ci.git', user: user_string)
end
end
end
describe :exec_cmd do
let(:shell) { GitlabShell.new(gl_id) }
let(:env) do
{
'HOME' => ENV['HOME'],
'PATH' => ENV['PATH'],
'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'],
'LANG' => ENV['LANG'],
'GL_ID' => gl_id,
'GL_PROTOCOL' => 'ssh',
'GL_REPOSITORY' => gl_repository,
'GL_USERNAME' => 'testuser'
}
end
let(:exec_options) { { unsetenv_others: true, chdir: ROOT_PATH } }
before do
allow(Kernel).to receive(:exec)
shell.gl_repository = gl_repository
shell.git_protocol = git_protocol
shell.instance_variable_set(:@username, gl_username)
end
it "uses Kernel::exec method" do
expect(Kernel).to receive(:exec).with(env, 1, 2, exec_options).once
shell.send :exec_cmd, 1, 2
end
it "refuses to execute a lone non-array argument" do
expect { shell.send :exec_cmd, 1 }.to raise_error(GitlabShell::DisallowedCommandError)
end
it "allows one argument if it is an array" do
expect(Kernel).to receive(:exec).with(env, [1, 2], exec_options).once
shell.send :exec_cmd, [1, 2]
end
context "when specifying a git_tracing log file" do
let(:git_trace_log_file) { '/tmp/git_trace_performance.log' }
before do
allow_any_instance_of(GitlabConfig).to receive(:git_trace_log_file).and_return(git_trace_log_file)
shell
end
it "uses GIT_TRACE_PERFORMANCE" do
expected_hash = hash_including(
'GIT_TRACE' => git_trace_log_file,
'GIT_TRACE_PACKET' => git_trace_log_file,
'GIT_TRACE_PERFORMANCE' => git_trace_log_file
)
expect(Kernel).to receive(:exec).with(expected_hash, [1, 2], exec_options).once
shell.send :exec_cmd, [1, 2]
end
context "when provides a relative path" do
let(:git_trace_log_file) { 'git_trace_performance.log' }
it "does not uses GIT_TRACE*" do
# If we try to use it we'll show a warning to the users
expected_hash = hash_excluding(
'GIT_TRACE', 'GIT_TRACE_PACKET', 'GIT_TRACE_PERFORMANCE'
)
expect(Kernel).to receive(:exec).with(expected_hash, [1, 2], exec_options).once
shell.send :exec_cmd, [1, 2]
end
it "writes an entry on the log" do
message = 'git trace log path must be absolute, ignoring'
expect($logger).to receive(:warn).
with(message, git_trace_log_file: git_trace_log_file)
expect(Kernel).to receive(:exec).with(env, [1, 2], exec_options).once
shell.send :exec_cmd, [1, 2]
end
end
context "when provides a file not writable" do
before do
expect(File).to receive(:open).with(git_trace_log_file, 'a').and_raise(Errno::EACCES)
end
it "does not uses GIT_TRACE*" do
# If we try to use it we'll show a warning to the users
expected_hash = hash_excluding(
'GIT_TRACE', 'GIT_TRACE_PACKET', 'GIT_TRACE_PERFORMANCE'
)
expect(Kernel).to receive(:exec).with(expected_hash, [1, 2], exec_options).once
shell.send :exec_cmd, [1, 2]
end
it "writes an entry on the log" do
message = 'Failed to open git trace log file'
error = 'Permission denied'
expect($logger).to receive(:warn).
with(message, git_trace_log_file: git_trace_log_file, error: error)
expect(Kernel).to receive(:exec).with(env, [1, 2], exec_options).once
shell.send :exec_cmd, [1, 2]
end
expect($logger).to receive(:warn).with(message, command: 'git-upload-pack gitlab-ci.git', user: user_string)
end
end
end
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7