Commit 562d7eb4 authored by Douwe Maan's avatar Douwe Maan

Show nice error message when internal API is unreachable.

parent f92a9c5a
...@@ -8,13 +8,18 @@ require_relative '../lib/gitlab_net' ...@@ -8,13 +8,18 @@ require_relative '../lib/gitlab_net'
# #
print "Check GitLab API access: " print "Check GitLab API access: "
resp = GitlabNet.new.check begin
if resp.code == "200" resp = GitlabNet.new.check
if resp.code == "200"
print 'OK' print 'OK'
else else
abort "FAILED. code: #{resp.code}" abort "FAILED. code: #{resp.code}"
end
rescue GitlabNet::ApiUnreachableError
abort "FAILED: Failed to connect to internal API"
end end
puts "\nCheck directories and files: " puts "\nCheck directories and files: "
config = GitlabConfig.new config = GitlabConfig.new
......
...@@ -18,16 +18,21 @@ class GitlabAccess ...@@ -18,16 +18,21 @@ class GitlabAccess
end end
def exec def exec
begin
status = api.check_access('git-receive-pack', @repo_name, @actor, @changes) status = api.check_access('git-receive-pack', @repo_name, @actor, @changes)
if status.allowed?
true return true if status.allowed?
else
message = status.message
rescue GitlabNet::ApiUnreachableError
message = "Failed to authorize your Git request: internal API unreachable"
end
# reset GL_ID env since we stop git push here # reset GL_ID env since we stop git push here
ENV['GL_ID'] = nil ENV['GL_ID'] = nil
puts "GitLab: #{status.message}" puts "GitLab: #{message}"
false false
end end
end
protected protected
......
...@@ -7,6 +7,8 @@ require_relative 'gitlab_logger' ...@@ -7,6 +7,8 @@ require_relative 'gitlab_logger'
require_relative 'gitlab_access' require_relative 'gitlab_access'
class GitlabNet class GitlabNet
class ApiUnreachableError < StandardError; end
def check_access(cmd, repo, actor, changes) def check_access(cmd, repo, actor, changes)
project_name = repo.gsub("'", "") project_name = repo.gsub("'", "")
project_name = project_name.gsub(/\.git\Z/, "") project_name = project_name.gsub(/\.git\Z/, "")
...@@ -97,7 +99,11 @@ class GitlabNet ...@@ -97,7 +99,11 @@ class GitlabNet
http = http_client_for(uri) http = http_client_for(uri)
request = http_request_for(method, uri, params) request = http_request_for(method, uri, params)
begin
response = http.start { http.request(request) } response = http.start { http.request(request) }
rescue
raise ApiUnreachableError
end
if response.code == "200" if response.code == "200"
$logger.debug "Received response #{response.code} => <#{response.body}>." $logger.debug "Received response #{response.code} => <#{response.body}>."
......
...@@ -18,10 +18,14 @@ class GitlabPostReceive ...@@ -18,10 +18,14 @@ class GitlabPostReceive
update_redis update_redis
if broadcast_message = GitlabNet.new.broadcast_message begin
broadcast_message = GitlabNet.new.broadcast_message
if broadcast_message
puts puts
print_broadcast_message(broadcast_message["message"]) print_broadcast_message(broadcast_message["message"])
end end
rescue GitlabNet::ApiUnreachableError
end
end end
protected protected
......
...@@ -21,12 +21,13 @@ class GitlabShell ...@@ -21,12 +21,13 @@ class GitlabShell
if git_cmds.include?(@git_cmd) if git_cmds.include?(@git_cmd)
ENV['GL_ID'] = @key_id ENV['GL_ID'] = @key_id
if validate_access access = api.check_access(@git_cmd, @repo_name, @key_id, '_any')
if access.allowed?
process_cmd process_cmd
else else
message = "gitlab-shell: Access denied for git command <#{@origin_cmd}> by #{log_username}." message = "gitlab-shell: Access denied for git command <#{@origin_cmd}> by #{log_username}."
$logger.warn message $logger.warn message
$stderr.puts "Access denied." puts access.message
end end
else else
raise DisallowedCommandError raise DisallowedCommandError
...@@ -34,10 +35,13 @@ class GitlabShell ...@@ -34,10 +35,13 @@ class GitlabShell
else else
puts "Welcome to GitLab, #{username}!" puts "Welcome to GitLab, #{username}!"
end end
rescue GitlabNet::ApiUnreachableError => ex
$logger.warn "gitlab-shell: Failed to connect to internal API"
puts "Failed to authorize your Git request: internal API unreachable"
rescue DisallowedCommandError => ex rescue DisallowedCommandError => ex
message = "gitlab-shell: Attempt to execute disallowed command <#{@origin_cmd}> by #{log_username}." message = "gitlab-shell: Attempt to execute disallowed command <#{@origin_cmd}> by #{log_username}."
$logger.warn message $logger.warn message
puts 'Not allowed command' puts 'Disallowed command'
end end
protected protected
...@@ -59,10 +63,6 @@ class GitlabShell ...@@ -59,10 +63,6 @@ class GitlabShell
exec_cmd(@git_cmd, repo_full_path) exec_cmd(@git_cmd, repo_full_path)
end end
def validate_access
api.check_access(@git_cmd, @repo_name, @key_id, '_any').allowed?
end
# 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' => ENV['GL_ID']}, *args, unsetenv_others: true)
...@@ -73,11 +73,12 @@ class GitlabShell ...@@ -73,11 +73,12 @@ class GitlabShell
end end
def user def user
# Can't use "@user ||=" because that will keep hitting the API when @user is really nil! return @user if defined?(@user)
if instance_variable_defined?('@user')
@user begin
else
@user = api.discover(@key_id) @user = api.discover(@key_id)
rescue GitlabNet::ApiUnreachableError
@user = nil
end end
end end
......
...@@ -5,15 +5,56 @@ describe GitlabAccess do ...@@ -5,15 +5,56 @@ describe GitlabAccess do
let(:repository_path) { "/home/git/repositories" } let(:repository_path) { "/home/git/repositories" }
let(:repo_name) { 'dzaporozhets/gitlab-ci' } let(:repo_name) { 'dzaporozhets/gitlab-ci' }
let(:repo_path) { File.join(repository_path, repo_name) + ".git" } let(:repo_path) { File.join(repository_path, repo_name) + ".git" }
let(:gitlab_access) { GitlabAccess.new(repo_path, 'key-123', 'wow') } let(:api) do
double(GitlabNet).tap do |api|
api.stub(check_access: GitAccessStatus.new(true))
end
end
subject do
GitlabAccess.new(repo_path, 'key-123', 'wow').tap do |access|
access.stub(exec_cmd: :exec_called)
access.stub(api: api)
end
end
before do before do
GitlabConfig.any_instance.stub(repos_path: repository_path) GitlabConfig.any_instance.stub(repos_path: repository_path)
end end
describe :initialize do describe :initialize do
it { gitlab_access.repo_name.should == repo_name } it { subject.repo_name.should == repo_name }
it { gitlab_access.repo_path.should == repo_path } it { subject.repo_path.should == repo_path }
it { gitlab_access.changes.should == ['wow'] } it { subject.changes.should == ['wow'] }
end
describe "#exec" do
context "access is granted" do
it "returns true" do
expect(subject.exec).to be_true
end
end
context "access is denied" do
before do
api.stub(check_access: GitAccessStatus.new(false))
end
it "returns false" do
expect(subject.exec).to be_false
end
end
context "API connection fails" do
before do
api.stub(:check_access).and_raise(GitlabNet::ApiUnreachableError)
end
it "returns false" do
expect(subject.exec).to be_false
end
end
end end
end end
...@@ -26,6 +26,11 @@ describe GitlabNet, vcr: true do ...@@ -26,6 +26,11 @@ describe GitlabNet, vcr: true do
gitlab_net.check gitlab_net.check
end end
end end
it "raises an exception if the connection fails" do
Net::HTTP.any_instance.stub(:request).and_raise(StandardError)
expect { gitlab_net.check }.to raise_error(GitlabNet::ApiUnreachableError)
end
end end
describe :discover do describe :discover do
...@@ -42,6 +47,13 @@ describe GitlabNet, vcr: true do ...@@ -42,6 +47,13 @@ describe GitlabNet, vcr: true do
gitlab_net.discover('key-126') gitlab_net.discover('key-126')
end end
end end
it "raises an exception if the connection fails" do
VCR.use_cassette("discover-ok") do
Net::HTTP.any_instance.stub(:request).and_raise(StandardError)
expect { gitlab_net.discover('key-126') }.to raise_error(GitlabNet::ApiUnreachableError)
end
end
end end
describe :broadcast_message do describe :broadcast_message do
...@@ -110,6 +122,13 @@ describe GitlabNet, vcr: true do ...@@ -110,6 +122,13 @@ describe GitlabNet, vcr: true do
end end
end end
end end
it "raises an exception if the connection fails" do
Net::HTTP.any_instance.stub(:request).and_raise(StandardError)
expect {
gitlab_net.check_access('git-upload-pack', 'gitlab/gitlabhq.git', 'user-1', changes)
}.to raise_error(GitlabNet::ApiUnreachableError)
end
end end
describe :host do describe :host do
......
...@@ -135,6 +135,27 @@ describe GitlabShell do ...@@ -135,6 +135,27 @@ describe GitlabShell do
api.should_receive(:discover).with(key_id) api.should_receive(:discover).with(key_id)
end end
end end
context "failed connection" do
before {
ssh_cmd 'git-upload-pack gitlab-ci.git'
api.stub(:check_access).and_raise(GitlabNet::ApiUnreachableError)
}
after { subject.exec }
it "should not process the command" do
subject.should_not_receive(:process_cmd)
end
it "should not execute the command" do
subject.should_not_receive(:exec_cmd)
end
it "should log the failed connection" do
message = "gitlab-shell: Failed to connect to internal API"
$logger.should_receive(:warn).with(message)
end
end
end end
describe :validate_access do describe :validate_access do
......
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