Commit bf34fa78 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'gl-id-refactor' into 'master'

Refactor logic around GL_ID.

Builds upon !8.

See merge request !9
parents fd677db7 3c2ad9cb
...@@ -5,6 +5,9 @@ unless ENV['SSH_CONNECTION'] ...@@ -5,6 +5,9 @@ unless ENV['SSH_CONNECTION']
exit exit
end end
key_id = /key-[0-9]+/.match(ARGV.join).to_s
original_cmd = ENV['SSH_ORIGINAL_COMMAND']
require_relative '../lib/gitlab_init' require_relative '../lib/gitlab_init'
# #
...@@ -14,7 +17,7 @@ require_relative '../lib/gitlab_init' ...@@ -14,7 +17,7 @@ require_relative '../lib/gitlab_init'
# #
require File.join(ROOT_PATH, 'lib', 'gitlab_shell') require File.join(ROOT_PATH, 'lib', 'gitlab_shell')
if GitlabShell.new.exec if GitlabShell.new(key_id, original_cmd).exec
exit 0 exit 0
else else
exit 1 exit 1
......
...@@ -7,6 +7,9 @@ refs = ARGF.read ...@@ -7,6 +7,9 @@ refs = ARGF.read
key_id = ENV['GL_ID'] key_id = ENV['GL_ID']
repo_path = Dir.pwd repo_path = Dir.pwd
# reset GL_ID env since we already got its value
ENV['GL_ID'] = nil
require_relative '../lib/gitlab_custom_hook' require_relative '../lib/gitlab_custom_hook'
require_relative '../lib/gitlab_post_receive' require_relative '../lib/gitlab_post_receive'
......
...@@ -4,24 +4,21 @@ class GitlabCustomHook ...@@ -4,24 +4,21 @@ class GitlabCustomHook
def pre_receive(changes, repo_path) def pre_receive(changes, repo_path)
hook = hook_file('pre-receive', repo_path) hook = hook_file('pre-receive', repo_path)
return true if hook.nil? return true if hook.nil?
if call_receive_hook(hook, changes)
return true call_receive_hook(hook, changes)
else
# reset GL_ID env since we stop git push here
ENV['GL_ID'] = nil
return false
end
end end
def post_receive(changes, repo_path) def post_receive(changes, repo_path)
hook = hook_file('post-receive', repo_path) hook = hook_file('post-receive', repo_path)
return true if hook.nil? return true if hook.nil?
call_receive_hook(hook, changes) ? true : false
call_receive_hook(hook, changes)
end end
def update(ref_name, old_value, new_value, repo_path) def update(ref_name, old_value, new_value, repo_path)
hook = hook_file('update', repo_path) hook = hook_file('update', repo_path)
return true if hook.nil? return true if hook.nil?
system(hook, ref_name, old_value, new_value) system(hook, ref_name, old_value, new_value)
end end
......
...@@ -13,10 +13,6 @@ class GitlabPostReceive ...@@ -13,10 +13,6 @@ class GitlabPostReceive
end end
def exec def exec
# reset GL_ID env since we already
# get value from it
ENV['GL_ID'] = nil
result = update_redis result = update_redis
begin begin
......
...@@ -7,11 +7,13 @@ class GitlabShell ...@@ -7,11 +7,13 @@ class GitlabShell
class DisallowedCommandError < StandardError; end class DisallowedCommandError < StandardError; end
class InvalidRepositoryPathError < StandardError; end class InvalidRepositoryPathError < StandardError; end
GIT_COMMANDS = %w(git-upload-pack git-receive-pack git-upload-archive git-annex-shell).freeze
attr_accessor :key_id, :repo_name, :git_cmd, :repos_path, :repo_name attr_accessor :key_id, :repo_name, :git_cmd, :repos_path, :repo_name
def initialize def initialize(key_id, origin_cmd)
@key_id = /key-[0-9]+/.match(ARGV.join).to_s @key_id = key_id
@origin_cmd = ENV['SSH_ORIGINAL_COMMAND'] @origin_cmd = origin_cmd
@config = GitlabConfig.new @config = GitlabConfig.new
@repos_path = @config.repos_path @repos_path = @config.repos_path
end end
...@@ -24,12 +26,7 @@ class GitlabShell ...@@ -24,12 +26,7 @@ class GitlabShell
parse_cmd parse_cmd
raise DisallowedCommandError unless git_cmds.include?(@git_cmd) verify_access
ENV['GL_ID'] = @key_id
status = api.check_access(@git_cmd, @repo_name, @key_id, '_any')
raise AccessDeniedError, status.message unless status.allowed?
process_cmd process_cmd
...@@ -60,6 +57,8 @@ class GitlabShell ...@@ -60,6 +57,8 @@ class GitlabShell
args = Shellwords.shellwords(@origin_cmd) args = Shellwords.shellwords(@origin_cmd)
@git_cmd = args.first @git_cmd = args.first
raise DisallowedCommandError unless GIT_COMMANDS.include?(@git_cmd)
if @git_cmd == 'git-annex-shell' if @git_cmd == 'git-annex-shell'
raise DisallowedCommandError unless @config.git_annex_enabled? raise DisallowedCommandError unless @config.git_annex_enabled?
...@@ -73,8 +72,10 @@ class GitlabShell ...@@ -73,8 +72,10 @@ class GitlabShell
end end
end end
def git_cmds def verify_access
%w(git-upload-pack git-receive-pack git-upload-archive git-annex-shell) status = api.check_access(@git_cmd, @repo_name, @key_id, '_any')
raise AccessDeniedError, status.message unless status.allowed?
end end
def process_cmd def process_cmd
...@@ -105,7 +106,7 @@ class GitlabShell ...@@ -105,7 +106,7 @@ class GitlabShell
# This method is not covered by Rspec because it ends the current Ruby process. # This method is not covered by Rspec because it ends the current Ruby process.
def exec_cmd(*args) def exec_cmd(*args)
Kernel::exec({ 'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'GL_ID' => ENV['GL_ID'] }, *args, unsetenv_others: true) Kernel::exec({ 'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'GL_ID' => @key_id }, *args, unsetenv_others: true)
end end
def api def api
......
...@@ -24,14 +24,6 @@ describe GitlabPostReceive do ...@@ -24,14 +24,6 @@ describe GitlabPostReceive do
allow(gitlab_post_receive).to receive(:system).and_return(true) allow(gitlab_post_receive).to receive(:system).and_return(true)
end end
it "resets the GL_ID environment variable" do
ENV["GL_ID"] = actor
gitlab_post_receive.exec
expect(ENV["GL_ID"]).to be_nil
end
it "prints the broadcast message" do it "prints the broadcast message" do
expect(gitlab_post_receive).to receive(:puts).ordered expect(gitlab_post_receive).to receive(:puts).ordered
expect(gitlab_post_receive).to receive(:puts).with( expect(gitlab_post_receive).to receive(:puts).with(
......
...@@ -13,7 +13,7 @@ describe GitlabShell do ...@@ -13,7 +13,7 @@ describe GitlabShell do
subject do subject do
ARGV[0] = key_id ARGV[0] = key_id
GitlabShell.new.tap do |shell| GitlabShell.new(key_id, ssh_cmd).tap do |shell|
shell.stub(exec_cmd: :exec_called) shell.stub(exec_cmd: :exec_called)
shell.stub(api: api) shell.stub(api: api)
end end
...@@ -27,6 +27,7 @@ describe GitlabShell do ...@@ -27,6 +27,7 @@ describe GitlabShell do
end end
let(:key_id) { "key-#{rand(100) + 100}" } let(:key_id) { "key-#{rand(100) + 100}" }
let(:ssh_cmd) { nil }
let(:tmp_repos_path) { File.join(ROOT_PATH, 'tmp', 'repositories') } let(:tmp_repos_path) { File.join(ROOT_PATH, 'tmp', 'repositories') }
before do before do
...@@ -34,7 +35,7 @@ describe GitlabShell do ...@@ -34,7 +35,7 @@ describe GitlabShell do
end end
describe :initialize do describe :initialize do
before { ssh_cmd 'git-receive-pack' } let(:ssh_cmd) { 'git-receive-pack' }
its(:key_id) { should == key_id } its(:key_id) { should == key_id }
its(:repos_path) { should == tmp_repos_path } its(:repos_path) { should == tmp_repos_path }
...@@ -43,8 +44,9 @@ describe GitlabShell do ...@@ -43,8 +44,9 @@ describe GitlabShell do
describe :parse_cmd do describe :parse_cmd do
describe 'git' do describe 'git' do
context 'w/o namespace' do context 'w/o namespace' do
let(:ssh_cmd) { 'git-upload-pack gitlab-ci.git' }
before do before do
ssh_cmd 'git-upload-pack gitlab-ci.git'
subject.send :parse_cmd subject.send :parse_cmd
end end
...@@ -53,8 +55,9 @@ describe GitlabShell do ...@@ -53,8 +55,9 @@ describe GitlabShell do
end end
context 'namespace' do context 'namespace' do
let(:ssh_cmd) { 'git-upload-pack dmitriy.zaporozhets/gitlab-ci.git' }
before do before do
ssh_cmd 'git-upload-pack dmitriy.zaporozhets/gitlab-ci.git'
subject.send :parse_cmd subject.send :parse_cmd
end end
...@@ -63,7 +66,7 @@ describe GitlabShell do ...@@ -63,7 +66,7 @@ describe GitlabShell do
end end
context 'with an invalid number of arguments' do context 'with an invalid number of arguments' do
before { ssh_cmd 'foobar' } let(:ssh_cmd) { 'foobar' }
it "should raise an DisallowedCommandError" do it "should raise an DisallowedCommandError" do
expect { subject.send :parse_cmd }.to raise_error(GitlabShell::DisallowedCommandError) expect { subject.send :parse_cmd }.to raise_error(GitlabShell::DisallowedCommandError)
...@@ -74,6 +77,8 @@ describe GitlabShell do ...@@ -74,6 +77,8 @@ describe GitlabShell do
describe 'git-annex' do describe 'git-annex' do
let(:repo_path) { File.join(tmp_repos_path, 'dzaporozhets/gitlab.git') } let(:repo_path) { File.join(tmp_repos_path, 'dzaporozhets/gitlab.git') }
let(:ssh_cmd) { 'git-annex-shell inannex /~/dzaporozhets/gitlab.git SHA256E' }
before do before do
GitlabConfig.any_instance.stub(git_annex_enabled?: true) GitlabConfig.any_instance.stub(git_annex_enabled?: true)
...@@ -82,7 +87,6 @@ describe GitlabShell do ...@@ -82,7 +87,6 @@ describe GitlabShell do
cmd = %W(git --git-dir=#{repo_path} init --bare) cmd = %W(git --git-dir=#{repo_path} init --bare)
system(*cmd) system(*cmd)
ssh_cmd 'git-annex-shell inannex /~/dzaporozhets/gitlab.git SHA256E'
subject.send :parse_cmd subject.send :parse_cmd
end end
...@@ -97,7 +101,7 @@ describe GitlabShell do ...@@ -97,7 +101,7 @@ describe GitlabShell do
describe :exec do describe :exec do
context 'git-upload-pack' do context 'git-upload-pack' do
before { ssh_cmd 'git-upload-pack gitlab-ci.git' } let(:ssh_cmd) { 'git-upload-pack gitlab-ci.git' }
after { subject.exec } after { subject.exec }
it "should process the command" do it "should process the command" do
...@@ -108,10 +112,6 @@ describe GitlabShell do ...@@ -108,10 +112,6 @@ describe GitlabShell do
subject.should_receive(:exec_cmd).with("git-upload-pack", File.join(tmp_repos_path, 'gitlab-ci.git')) subject.should_receive(:exec_cmd).with("git-upload-pack", File.join(tmp_repos_path, 'gitlab-ci.git'))
end end
it "should set the GL_ID environment variable" do
ENV.should_receive("[]=").with("GL_ID", key_id)
end
it "should log the command execution" do it "should log the command execution" do
message = "gitlab-shell: executing git command " message = "gitlab-shell: executing git command "
message << "<git-upload-pack #{File.join(tmp_repos_path, 'gitlab-ci.git')}> " message << "<git-upload-pack #{File.join(tmp_repos_path, 'gitlab-ci.git')}> "
...@@ -126,7 +126,7 @@ describe GitlabShell do ...@@ -126,7 +126,7 @@ describe GitlabShell do
end end
context 'git-receive-pack' do context 'git-receive-pack' do
before { ssh_cmd 'git-receive-pack gitlab-ci.git' } let(:ssh_cmd) { 'git-receive-pack gitlab-ci.git' }
after { subject.exec } after { subject.exec }
it "should process the command" do it "should process the command" do
...@@ -146,7 +146,7 @@ describe GitlabShell do ...@@ -146,7 +146,7 @@ describe GitlabShell do
end end
context 'arbitrary command' do context 'arbitrary command' do
before { ssh_cmd 'arbitrary command' } let(:ssh_cmd) { 'arbitrary command' }
after { subject.exec } after { subject.exec }
it "should not process the command" do it "should not process the command" do
...@@ -164,7 +164,6 @@ describe GitlabShell do ...@@ -164,7 +164,6 @@ describe GitlabShell do
end end
context 'no command' do context 'no command' do
before { ssh_cmd nil }
after { subject.exec } after { subject.exec }
it "should call api.discover" do it "should call api.discover" do
...@@ -173,8 +172,9 @@ describe GitlabShell do ...@@ -173,8 +172,9 @@ describe GitlabShell do
end end
context "failed connection" do context "failed connection" do
let(:ssh_cmd) { 'git-upload-pack gitlab-ci.git' }
before { before {
ssh_cmd 'git-upload-pack gitlab-ci.git'
api.stub(:check_access).and_raise(GitlabNet::ApiUnreachableError) api.stub(:check_access).and_raise(GitlabNet::ApiUnreachableError)
} }
after { subject.exec } after { subject.exec }
...@@ -189,9 +189,10 @@ describe GitlabShell do ...@@ -189,9 +189,10 @@ describe GitlabShell do
end end
describe 'git-annex' do describe 'git-annex' do
let(:ssh_cmd) { 'git-annex-shell commit /~/gitlab-ci.git SHA256' }
before do before do
GitlabConfig.any_instance.stub(git_annex_enabled?: true) GitlabConfig.any_instance.stub(git_annex_enabled?: true)
ssh_cmd 'git-annex-shell commit /~/gitlab-ci.git SHA256'
end end
after { subject.exec } after { subject.exec }
...@@ -203,7 +204,7 @@ describe GitlabShell do ...@@ -203,7 +204,7 @@ describe GitlabShell do
end end
describe :validate_access do describe :validate_access do
before { ssh_cmd 'git-upload-pack gitlab-ci.git' } let(:ssh_cmd) { 'git-upload-pack gitlab-ci.git' }
after { subject.exec } after { subject.exec }
it "should call api.check_access" do it "should call api.check_access" do
...@@ -220,7 +221,7 @@ describe GitlabShell do ...@@ -220,7 +221,7 @@ describe GitlabShell do
end end
describe :exec_cmd do describe :exec_cmd do
let(:shell) { GitlabShell.new } let(:shell) { GitlabShell.new(key_id, ssh_cmd) }
before { Kernel.stub!(:exec) } before { Kernel.stub!(:exec) }
it "uses Kernel::exec method" do it "uses Kernel::exec method" do
...@@ -230,21 +231,17 @@ describe GitlabShell do ...@@ -230,21 +231,17 @@ describe GitlabShell do
end end
describe :api do describe :api do
let(:shell) { GitlabShell.new } let(:shell) { GitlabShell.new(key_id, ssh_cmd) }
subject { shell.send :api } subject { shell.send :api }
it { should be_a(GitlabNet) } it { should be_a(GitlabNet) }
end end
describe :escape_path do describe :escape_path do
let(:shell) { GitlabShell.new } let(:shell) { GitlabShell.new(key_id, ssh_cmd) }
before { File.stub(:absolute_path) { 'y' } } before { File.stub(:absolute_path) { 'y' } }
subject { -> { shell.send(:escape_path, 'z') } } subject { -> { shell.send(:escape_path, 'z') } }
it { should raise_error(GitlabShell::InvalidRepositoryPathError) } it { should raise_error(GitlabShell::InvalidRepositoryPathError) }
end end
def ssh_cmd(cmd)
ENV['SSH_ORIGINAL_COMMAND'] = cmd
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