Commit 3456b013 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'parallel-push-check-fix' into 'master'

Pass environment to check threads

See merge request gitlab-org/gitlab!46363
parents 9d760370 f16a4b22
...@@ -41,18 +41,23 @@ module EE ...@@ -41,18 +41,23 @@ module EE
check_file_size! check_file_size!
end end
# Run the checks in separate threads for performance benefits # Run the checks in separate threads for performance benefits.
#
# The git hook environment is currently set in the current thread
# in lib/api/internal/base.rb. This needs to be passed into the
# child threads we spawn here.
# #
# @return [Nil] returns nil unless an error is raised # @return [Nil] returns nil unless an error is raised
# @raise [Gitlab::GitAccess::ForbiddenError] if any check fails # @raise [Gitlab::GitAccess::ForbiddenError] if any check fails
def run_checks_in_parallel! def run_checks_in_parallel!
git_env = ::Gitlab::Git::HookEnv.all(project.repository.gl_repository)
@threads = [] @threads = []
parallelize do parallelize(git_env) do
check_tag_or_branch! check_tag_or_branch!
end end
parallelize do parallelize(git_env) do
check_file_size! check_file_size!
end end
...@@ -73,8 +78,9 @@ module EE ...@@ -73,8 +78,9 @@ module EE
# Runs a block inside a new thread. This thread will # Runs a block inside a new thread. This thread will
# exit immediately upon an exception being raised. # exit immediately upon an exception being raised.
# #
# @param git_env [Hash] the current git environment
# @raise [Gitlab::GitAccess::ForbiddenError] # @raise [Gitlab::GitAccess::ForbiddenError]
def parallelize def parallelize(git_env)
@threads << Thread.new do @threads << Thread.new do
Thread.current.tap do |t| Thread.current.tap do |t|
t.name = "push_rule_check" t.name = "push_rule_check"
...@@ -82,7 +88,11 @@ module EE ...@@ -82,7 +88,11 @@ module EE
t.report_on_exception = false t.report_on_exception = false
end end
::Gitlab::WithRequestStore.with_request_store do
::Gitlab::Git::HookEnv.set(project.repository.gl_repository, git_env)
yield yield
end
ensure # rubocop: disable Layout/RescueEnsureAlignment ensure # rubocop: disable Layout/RescueEnsureAlignment
ActiveRecord::Base.clear_active_connections! ActiveRecord::Base.clear_active_connections!
end end
......
...@@ -59,8 +59,25 @@ RSpec.describe EE::Gitlab::Checks::PushRuleCheck do ...@@ -59,8 +59,25 @@ RSpec.describe EE::Gitlab::Checks::PushRuleCheck do
end end
describe '#validate!' do describe '#validate!' do
context "parallel push checks" do
it_behaves_like "push checks" it_behaves_like "push checks"
before do
::Gitlab::Git::HookEnv.set(project.repository.gl_repository,
"GIT_OBJECT_DIRECTORY_RELATIVE" => "objects",
"GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE" => [])
end
it "sets the git env correctly for all hooks", :request_store do
expect(Gitaly::Repository).to receive(:new)
.with(a_hash_including(git_object_directory: "objects"))
.and_call_original
# This push fails because of the commit message check
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError)
end
end
context ":parallel_push_checks feature is disabled" do context ":parallel_push_checks feature is disabled" do
before do before do
stub_feature_flags(parallel_push_checks: false) stub_feature_flags(parallel_push_checks: false)
......
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