Commit f2e9148d authored by Douwe Maan's avatar Douwe Maan

Merge branch '42790-improve-feedback-for-internal-git-access-checks-timeouts' into 'master'

Adds trace of each access check when git push times out

Closes #42790

See merge request gitlab-org/gitlab-ce!22265
parents 578064df c6c17f1c
8.4.0 8.4.1
\ No newline at end of file
...@@ -8,6 +8,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController ...@@ -8,6 +8,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
rescue_from Gitlab::GitAccess::UnauthorizedError, with: :render_403 rescue_from Gitlab::GitAccess::UnauthorizedError, with: :render_403
rescue_from Gitlab::GitAccess::NotFoundError, with: :render_404 rescue_from Gitlab::GitAccess::NotFoundError, with: :render_404
rescue_from Gitlab::GitAccess::ProjectCreationError, with: :render_422 rescue_from Gitlab::GitAccess::ProjectCreationError, with: :render_422
rescue_from Gitlab::GitAccess::TimeoutError, with: :render_503
# GET /foo/bar.git/info/refs?service=git-upload-pack (git pull) # GET /foo/bar.git/info/refs?service=git-upload-pack (git pull)
# GET /foo/bar.git/info/refs?service=git-receive-pack (git push) # GET /foo/bar.git/info/refs?service=git-receive-pack (git push)
...@@ -62,6 +63,10 @@ class Projects::GitHttpController < Projects::GitHttpClientController ...@@ -62,6 +63,10 @@ class Projects::GitHttpController < Projects::GitHttpClientController
render plain: exception.message, status: :unprocessable_entity render plain: exception.message, status: :unprocessable_entity
end end
def render_503(exception)
render plain: exception.message, status: :service_unavailable
end
def access def access
@access ||= access_klass.new(access_actor, project, @access ||= access_klass.new(access_actor, project,
'http', authentication_abilities: authentication_abilities, 'http', authentication_abilities: authentication_abilities,
......
---
title: Adds trace of each access check when git push times out
merge_request: 22265
author:
type: added
...@@ -65,6 +65,8 @@ module API ...@@ -65,6 +65,8 @@ module API
result result
rescue Gitlab::GitAccess::UnauthorizedError => e rescue Gitlab::GitAccess::UnauthorizedError => e
break response_with_status(code: 401, success: false, message: e.message) break response_with_status(code: 401, success: false, message: e.message)
rescue Gitlab::GitAccess::TimeoutError => e
break response_with_status(code: 503, success: false, message: e.message)
rescue Gitlab::GitAccess::NotFoundError => e rescue Gitlab::GitAccess::NotFoundError => e
break response_with_status(code: 404, success: false, message: e.message) break response_with_status(code: 404, success: false, message: e.message)
end end
......
...@@ -18,11 +18,24 @@ module Gitlab ...@@ -18,11 +18,24 @@ module Gitlab
lfs_objects_missing: 'LFS objects are missing. Ensure LFS is properly set up or try a manual "git lfs push --all".' lfs_objects_missing: 'LFS objects are missing. Ensure LFS is properly set up or try a manual "git lfs push --all".'
}.freeze }.freeze
attr_reader :user_access, :project, :skip_authorization, :skip_lfs_integrity_check, :protocol, :oldrev, :newrev, :ref, :branch_name, :tag_name LOG_MESSAGES = {
push_checks: "Checking if you are allowed to push...",
delete_default_branch_check: "Checking if default branch is being deleted...",
protected_branch_checks: "Checking if you are force pushing to a protected branch...",
protected_branch_push_checks: "Checking if you are allowed to push to the protected branch...",
protected_branch_deletion_checks: "Checking if you are allowed to delete the protected branch...",
tag_checks: "Checking if you are allowed to change existing tags...",
protected_tag_checks: "Checking if you are creating, updating or deleting a protected tag...",
lfs_objects_exist_check: "Scanning repository for blobs stored in LFS and verifying their files have been uploaded to GitLab...",
commits_check_file_paths_validation: "Validating commits' file paths...",
commits_check: "Validating commit contents..."
}.freeze
attr_reader :user_access, :project, :skip_authorization, :skip_lfs_integrity_check, :protocol, :oldrev, :newrev, :ref, :branch_name, :tag_name, :logger
def initialize( def initialize(
change, user_access:, project:, skip_authorization: false, change, user_access:, project:, skip_authorization: false,
skip_lfs_integrity_check: false, protocol: skip_lfs_integrity_check: false, protocol:, logger:
) )
@oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref) @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
@branch_name = Gitlab::Git.branch_name(@ref) @branch_name = Gitlab::Git.branch_name(@ref)
...@@ -32,6 +45,9 @@ module Gitlab ...@@ -32,6 +45,9 @@ module Gitlab
@skip_authorization = skip_authorization @skip_authorization = skip_authorization
@skip_lfs_integrity_check = skip_lfs_integrity_check @skip_lfs_integrity_check = skip_lfs_integrity_check
@protocol = protocol @protocol = protocol
@logger = logger
@logger.append_message("Running checks for ref: #{@branch_name || @tag_name}")
end end
def exec(skip_commits_check: false) def exec(skip_commits_check: false)
...@@ -49,27 +65,33 @@ module Gitlab ...@@ -49,27 +65,33 @@ module Gitlab
protected protected
def push_checks def push_checks
logger.log_timed(LOG_MESSAGES[__method__]) do
unless can_push? unless can_push?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_code] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_code]
end end
end end
end
def branch_checks def branch_checks
return unless branch_name return unless branch_name
logger.log_timed(LOG_MESSAGES[:delete_default_branch_check]) do
if deletion? && branch_name == project.default_branch if deletion? && branch_name == project.default_branch
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:delete_default_branch] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:delete_default_branch]
end end
end
protected_branch_checks protected_branch_checks
end end
def protected_branch_checks def protected_branch_checks
return unless ProtectedBranch.protected?(project, branch_name) logger.log_timed(LOG_MESSAGES[__method__]) do
return unless ProtectedBranch.protected?(project, branch_name) # rubocop:disable Cop/AvoidReturnFromBlocks
if forced_push? if forced_push?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:force_push_protected_branch] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:force_push_protected_branch]
end end
end
if deletion? if deletion?
protected_branch_deletion_checks protected_branch_deletion_checks
...@@ -79,6 +101,7 @@ module Gitlab ...@@ -79,6 +101,7 @@ module Gitlab
end end
def protected_branch_deletion_checks def protected_branch_deletion_checks
logger.log_timed(LOG_MESSAGES[__method__]) do
unless user_access.can_delete_branch?(branch_name) unless user_access.can_delete_branch?(branch_name)
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_master_delete_protected_branch] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_master_delete_protected_branch]
end end
...@@ -87,8 +110,10 @@ module Gitlab ...@@ -87,8 +110,10 @@ module Gitlab
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_web_delete_protected_branch] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:non_web_delete_protected_branch]
end end
end end
end
def protected_branch_push_checks def protected_branch_push_checks
logger.log_timed(LOG_MESSAGES[__method__]) do
if matching_merge_request? if matching_merge_request?
unless user_access.can_merge_to_branch?(branch_name) || user_access.can_push_to_branch?(branch_name) unless user_access.can_merge_to_branch?(branch_name) || user_access.can_push_to_branch?(branch_name)
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:merge_protected_branch] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:merge_protected_branch]
...@@ -99,19 +124,23 @@ module Gitlab ...@@ -99,19 +124,23 @@ module Gitlab
end end
end end
end end
end
def tag_checks def tag_checks
return unless tag_name return unless tag_name
logger.log_timed(LOG_MESSAGES[__method__]) do
if tag_exists? && user_access.cannot_do_action?(:admin_project) if tag_exists? && user_access.cannot_do_action?(:admin_project)
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:change_existing_tags] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:change_existing_tags]
end end
end
protected_tag_checks protected_tag_checks
end end
def protected_tag_checks def protected_tag_checks
return unless ProtectedTag.protected?(project, tag_name) logger.log_timed(LOG_MESSAGES[__method__]) do
return unless ProtectedTag.protected?(project, tag_name) # rubocop:disable Cop/AvoidReturnFromBlocks
raise(GitAccess::UnauthorizedError, ERROR_MESSAGES[:update_protected_tag]) if update? raise(GitAccess::UnauthorizedError, ERROR_MESSAGES[:update_protected_tag]) if update?
raise(GitAccess::UnauthorizedError, ERROR_MESSAGES[:delete_protected_tag]) if deletion? raise(GitAccess::UnauthorizedError, ERROR_MESSAGES[:delete_protected_tag]) if deletion?
...@@ -120,20 +149,27 @@ module Gitlab ...@@ -120,20 +149,27 @@ module Gitlab
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:create_protected_tag] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:create_protected_tag]
end end
end end
end
def commits_check def commits_check
return if deletion? || newrev.nil? return if deletion? || newrev.nil?
return unless should_run_commit_validations? return unless should_run_commit_validations?
logger.log_timed(LOG_MESSAGES[__method__]) do
# n+1: https://gitlab.com/gitlab-org/gitlab-ee/issues/3593 # n+1: https://gitlab.com/gitlab-org/gitlab-ee/issues/3593
::Gitlab::GitalyClient.allow_n_plus_1_calls do ::Gitlab::GitalyClient.allow_n_plus_1_calls do
commits.each do |commit| commits.each do |commit|
logger.check_timeout_reached
commit_check.validate(commit, validations_for_commit(commit)) commit_check.validate(commit, validations_for_commit(commit))
end end
end end
end
logger.log_timed(LOG_MESSAGES[:commits_check_file_paths_validation]) do
commit_check.validate_file_paths commit_check.validate_file_paths
end end
end
# Method overwritten in EE to inject custom validations # Method overwritten in EE to inject custom validations
def validations_for_commit(_) def validations_for_commit(_)
...@@ -194,12 +230,14 @@ module Gitlab ...@@ -194,12 +230,14 @@ module Gitlab
end end
def lfs_objects_exist_check def lfs_objects_exist_check
lfs_check = Checks::LfsIntegrity.new(project, newrev) logger.log_timed(LOG_MESSAGES[__method__]) do
lfs_check = Checks::LfsIntegrity.new(project, newrev, logger.time_left)
if lfs_check.objects_missing? if lfs_check.objects_missing?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:lfs_objects_missing] raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:lfs_objects_missing]
end end
end end
end
def commit_check def commit_check
@commit_check ||= Gitlab::Checks::CommitCheck.new(project, user_access.user, newrev, oldrev) @commit_check ||= Gitlab::Checks::CommitCheck.new(project, user_access.user, newrev, oldrev)
......
...@@ -3,9 +3,10 @@ ...@@ -3,9 +3,10 @@
module Gitlab module Gitlab
module Checks module Checks
class LfsIntegrity class LfsIntegrity
def initialize(project, newrev) def initialize(project, newrev, time_left)
@project = project @project = project
@newrev = newrev @newrev = newrev
@time_left = time_left
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
...@@ -13,7 +14,7 @@ module Gitlab ...@@ -13,7 +14,7 @@ module Gitlab
return false unless @newrev && @project.lfs_enabled? return false unless @newrev && @project.lfs_enabled?
new_lfs_pointers = Gitlab::Git::LfsChanges.new(@project.repository, @newrev) new_lfs_pointers = Gitlab::Git::LfsChanges.new(@project.repository, @newrev)
.new_pointers(object_limit: ::Gitlab::Git::Repository::REV_LIST_COMMIT_LIMIT) .new_pointers(object_limit: ::Gitlab::Git::Repository::REV_LIST_COMMIT_LIMIT, dynamic_timeout: @time_left)
return false unless new_lfs_pointers.present? return false unless new_lfs_pointers.present?
......
# frozen_string_literal: true
module Gitlab
module Checks
class TimedLogger
TimeoutError = Class.new(StandardError)
attr_reader :start_time, :header, :log, :timeout
def initialize(start_time: Time.now, log: [], header: "", timeout:)
@start_time = start_time
@timeout = timeout
@header = header
@log = log
end
# Adds trace of method being tracked with
# the correspondent time it took to run it.
# We make use of the start default argument
# on unit tests related to this method
#
def log_timed(log_message, start = Time.now)
check_timeout_reached
timed = true
yield
append_message(log_message + time_suffix_message(start: start))
rescue GRPC::DeadlineExceeded, TimeoutError
args = { cancelled: true }
args[:start] = start if timed
append_message(log_message + time_suffix_message(args))
raise TimeoutError
end
def check_timeout_reached
return unless time_expired?
raise TimeoutError
end
def time_left
(start_time + timeout.seconds) - Time.now
end
def full_message
header + log.join("\n")
end
# We always want to append in-place on the log
def append_message(message)
log << message
end
private
def time_expired?
time_left <= 0
end
def time_suffix_message(cancelled: false, start: nil)
return " (#{elapsed_time(start)}ms)" unless cancelled
if start
" (cancelled after #{elapsed_time(start)}ms)"
else
" (cancelled)"
end
end
def elapsed_time(start)
to_ms(Time.now - start)
end
def to_ms(elapsed)
(elapsed.to_f * 1000).round(2)
end
end
end
end
...@@ -6,8 +6,8 @@ module Gitlab ...@@ -6,8 +6,8 @@ module Gitlab
@newrev = newrev @newrev = newrev
end end
def new_pointers(object_limit: nil, not_in: nil) def new_pointers(object_limit: nil, not_in: nil, dynamic_timeout: nil)
@repository.gitaly_blob_client.get_new_lfs_pointers(@newrev, object_limit, not_in) @repository.gitaly_blob_client.get_new_lfs_pointers(@newrev, object_limit, not_in, dynamic_timeout)
end end
def all_pointers def all_pointers
......
...@@ -9,6 +9,7 @@ module Gitlab ...@@ -9,6 +9,7 @@ module Gitlab
UnauthorizedError = Class.new(StandardError) UnauthorizedError = Class.new(StandardError)
NotFoundError = Class.new(StandardError) NotFoundError = Class.new(StandardError)
ProjectCreationError = Class.new(StandardError) ProjectCreationError = Class.new(StandardError)
TimeoutError = Class.new(StandardError)
ProjectMovedError = Class.new(NotFoundError) ProjectMovedError = Class.new(NotFoundError)
ERROR_MESSAGES = { ERROR_MESSAGES = {
...@@ -26,11 +27,18 @@ module Gitlab ...@@ -26,11 +27,18 @@ module Gitlab
cannot_push_to_read_only: "You can't push code to a read-only GitLab instance." cannot_push_to_read_only: "You can't push code to a read-only GitLab instance."
}.freeze }.freeze
INTERNAL_TIMEOUT = 50.seconds.freeze
LOG_HEADER = <<~MESSAGE
Push operation timed out
Timing information for debugging purposes:
MESSAGE
DOWNLOAD_COMMANDS = %w{git-upload-pack git-upload-archive}.freeze DOWNLOAD_COMMANDS = %w{git-upload-pack git-upload-archive}.freeze
PUSH_COMMANDS = %w{git-receive-pack}.freeze PUSH_COMMANDS = %w{git-receive-pack}.freeze
ALL_COMMANDS = DOWNLOAD_COMMANDS + PUSH_COMMANDS ALL_COMMANDS = DOWNLOAD_COMMANDS + PUSH_COMMANDS
attr_reader :actor, :project, :protocol, :authentication_abilities, :namespace_path, :project_path, :redirected_path, :auth_result_type, :changes attr_reader :actor, :project, :protocol, :authentication_abilities, :namespace_path, :project_path, :redirected_path, :auth_result_type, :changes, :logger
def initialize(actor, project, protocol, authentication_abilities:, namespace_path: nil, project_path: nil, redirected_path: nil, auth_result_type: nil) def initialize(actor, project, protocol, authentication_abilities:, namespace_path: nil, project_path: nil, redirected_path: nil, auth_result_type: nil)
@actor = actor @actor = actor
...@@ -44,6 +52,7 @@ module Gitlab ...@@ -44,6 +52,7 @@ module Gitlab
end end
def check(cmd, changes) def check(cmd, changes)
@logger = Checks::TimedLogger.new(timeout: INTERNAL_TIMEOUT, header: LOG_HEADER)
@changes = changes @changes = changes
check_protocol! check_protocol!
...@@ -269,14 +278,19 @@ module Gitlab ...@@ -269,14 +278,19 @@ module Gitlab
end end
def check_single_change_access(change, skip_lfs_integrity_check: false) def check_single_change_access(change, skip_lfs_integrity_check: false)
Checks::ChangeAccess.new( change_access = Checks::ChangeAccess.new(
change, change,
user_access: user_access, user_access: user_access,
project: project, project: project,
skip_authorization: deploy_key?, skip_authorization: deploy_key?,
skip_lfs_integrity_check: skip_lfs_integrity_check, skip_lfs_integrity_check: skip_lfs_integrity_check,
protocol: protocol protocol: protocol,
).exec logger: logger
)
change_access.exec
rescue Checks::TimedLogger::TimeoutError
raise TimeoutError, logger.full_message
end end
def deploy_key def deploy_key
......
...@@ -72,7 +72,7 @@ module Gitlab ...@@ -72,7 +72,7 @@ module Gitlab
GitalyClient::BlobsStitcher.new(response) GitalyClient::BlobsStitcher.new(response)
end end
def get_new_lfs_pointers(revision, limit, not_in) def get_new_lfs_pointers(revision, limit, not_in, dynamic_timeout = nil)
request = Gitaly::GetNewLFSPointersRequest.new( request = Gitaly::GetNewLFSPointersRequest.new(
repository: @gitaly_repo, repository: @gitaly_repo,
revision: encode_binary(revision), revision: encode_binary(revision),
...@@ -85,7 +85,20 @@ module Gitlab ...@@ -85,7 +85,20 @@ module Gitlab
request.not_in_refs += not_in request.not_in_refs += not_in
end end
response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_new_lfs_pointers, request, timeout: GitalyClient.medium_timeout) timeout =
if dynamic_timeout
[dynamic_timeout, GitalyClient.medium_timeout].min
else
GitalyClient.medium_timeout
end
response = GitalyClient.call(
@gitaly_repo.storage_name,
:blob_service,
:get_new_lfs_pointers,
request,
timeout: timeout
)
map_lfs_pointers(response) map_lfs_pointers(response)
end end
......
...@@ -10,13 +10,16 @@ describe Gitlab::Checks::ChangeAccess do ...@@ -10,13 +10,16 @@ describe Gitlab::Checks::ChangeAccess do
let(:ref) { 'refs/heads/master' } let(:ref) { 'refs/heads/master' }
let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } } let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } }
let(:protocol) { 'ssh' } let(:protocol) { 'ssh' }
let(:timeout) { Gitlab::GitAccess::INTERNAL_TIMEOUT }
let(:logger) { Gitlab::Checks::TimedLogger.new(timeout: timeout) }
subject(:change_access) do subject(:change_access) do
described_class.new( described_class.new(
changes, changes,
project: project, project: project,
user_access: user_access, user_access: user_access,
protocol: protocol protocol: protocol,
logger: logger
) )
end end
...@@ -30,6 +33,19 @@ describe Gitlab::Checks::ChangeAccess do ...@@ -30,6 +33,19 @@ describe Gitlab::Checks::ChangeAccess do
end end
end end
context 'when time limit was reached' do
it 'raises a TimeoutError' do
logger = Gitlab::Checks::TimedLogger.new(start_time: timeout.ago, timeout: timeout)
access = described_class.new(changes,
project: project,
user_access: user_access,
protocol: protocol,
logger: logger)
expect { access.exec }.to raise_error(Gitlab::Checks::TimedLogger::TimeoutError)
end
end
context 'when the user is not allowed to push to the repo' do context 'when the user is not allowed to push to the repo' do
it 'raises an error' do it 'raises an error' do
expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false)
......
...@@ -3,6 +3,7 @@ require 'spec_helper' ...@@ -3,6 +3,7 @@ require 'spec_helper'
describe Gitlab::Checks::LfsIntegrity do describe Gitlab::Checks::LfsIntegrity do
include ProjectForksHelper include ProjectForksHelper
let!(:time_left) { 50 }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:repository) { project.repository } let(:repository) { project.repository }
let(:newrev) do let(:newrev) do
...@@ -15,7 +16,7 @@ describe Gitlab::Checks::LfsIntegrity do ...@@ -15,7 +16,7 @@ describe Gitlab::Checks::LfsIntegrity do
operations.commit_tree('8856a329dd38ca86dfb9ce5aa58a16d88cc119bd', "New LFS objects") operations.commit_tree('8856a329dd38ca86dfb9ce5aa58a16d88cc119bd', "New LFS objects")
end end
subject { described_class.new(project, newrev) } subject { described_class.new(project, newrev, time_left) }
describe '#objects_missing?' do describe '#objects_missing?' do
let(:blob_object) { repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') } let(:blob_object) { repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') }
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Checks::TimedLogger do
let!(:timeout) { 50.seconds }
let!(:start) { Time.now }
let!(:ref) { "bar" }
let!(:logger) { described_class.new(start_time: start, timeout: timeout) }
let!(:log_messages) do
{
foo: "Foo message..."
}
end
before do
logger.append_message("Checking ref: #{ref}")
end
describe '#log_timed' do
it 'logs message' do
Timecop.freeze(start + 30.seconds) do
logger.log_timed(log_messages[:foo], start) { bar_check }
end
expect(logger.full_message).to eq("Checking ref: bar\nFoo message... (30000.0ms)")
end
context 'when time limit was reached' do
it 'cancels action' do
Timecop.freeze(start + 50.seconds) do
expect do
logger.log_timed(log_messages[:foo], start) do
bar_check
end
end.to raise_error(described_class::TimeoutError)
end
expect(logger.full_message).to eq("Checking ref: bar\nFoo message... (cancelled)")
end
it 'cancels action with time elapsed if work was performed' do
Timecop.freeze(start + 30.seconds) do
expect do
logger.log_timed(log_messages[:foo], start) do
grpc_check
end
end.to raise_error(described_class::TimeoutError)
expect(logger.full_message).to eq("Checking ref: bar\nFoo message... (cancelled after 30000.0ms)")
end
end
end
end
def bar_check
2 + 2
end
def grpc_check
raise GRPC::DeadlineExceeded
end
end
...@@ -15,5 +15,9 @@ describe Gitlab::Git::LfsChanges do ...@@ -15,5 +15,9 @@ describe Gitlab::Git::LfsChanges do
it 'limits new_objects using object_limit' do it 'limits new_objects using object_limit' do
expect(subject.new_pointers(object_limit: 1)).to eq([]) expect(subject.new_pointers(object_limit: 1)).to eq([])
end end
it 'times out if given a small dynamic timeout' do
expect { subject.new_pointers(dynamic_timeout: 0.001) }.to raise_error(GRPC::DeadlineExceeded)
end
end end
end end
...@@ -934,6 +934,16 @@ describe Gitlab::GitAccess do ...@@ -934,6 +934,16 @@ describe Gitlab::GitAccess do
# There is still an N+1 query with protected branches # There is still an N+1 query with protected branches
expect { access.check('git-receive-pack', changes) }.not_to exceed_query_limit(control_count).with_threshold(1) expect { access.check('git-receive-pack', changes) }.not_to exceed_query_limit(control_count).with_threshold(1)
end end
it 'raises TimeoutError when #check_single_change_access raises a timeout error' do
message = "Push operation timed out\n\nTiming information for debugging purposes:\nRunning checks for ref: wow"
expect_next_instance_of(Gitlab::Checks::ChangeAccess) do |check|
expect(check).to receive(:exec).and_raise(Gitlab::Checks::TimedLogger::TimeoutError)
end
expect { access.check('git-receive-pack', changes) }.to raise_error(described_class::TimeoutError, message)
end
end end
end end
......
...@@ -494,6 +494,24 @@ describe API::Internal do ...@@ -494,6 +494,24 @@ describe API::Internal do
end end
end end
context 'request times out' do
context 'git push' do
it 'responds with a gateway timeout' do
personal_project = create(:project, namespace: user.namespace)
expect_next_instance_of(Gitlab::GitAccess) do |access|
expect(access).to receive(:check).and_raise(Gitlab::GitAccess::TimeoutError, "Foo")
end
push(key, personal_project)
expect(response).to have_gitlab_http_status(503)
expect(json_response['status']).to be_falsey
expect(json_response['message']).to eq("Foo")
expect(user.reload.last_activity_on).to be_nil
end
end
end
context "archived project" do context "archived project" do
before do before do
project.add_developer(user) project.add_developer(user)
......
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