diff --git a/ee/lib/ee/gitlab/checks/push_rule_check.rb b/ee/lib/ee/gitlab/checks/push_rule_check.rb
index 10cff97a08a20e33d77c5caf1494418357bf6137..80da4b3cd48b42627012526bc10fabba0a4efea8 100644
--- a/ee/lib/ee/gitlab/checks/push_rule_check.rb
+++ b/ee/lib/ee/gitlab/checks/push_rule_check.rb
@@ -87,6 +87,8 @@ def run_checks_in_parallel!
# @param git_env [Hash] the current git environment
# @raise [Gitlab::GitAccess::ForbiddenError]
def parallelize(git_env)
+ relative_path = ::Gitlab::Git::HookEnv.get_relative_path
+
@threads << Thread.new do
Thread.current.tap do |t|
t.name = "push_rule_check"
@@ -95,7 +97,7 @@ def parallelize(git_env)
end
::Gitlab::SafeRequestStore.ensure_request_store do
- ::Gitlab::Git::HookEnv.set(project.repository.gl_repository, git_env)
+ ::Gitlab::Git::HookEnv.set(project.repository.gl_repository, relative_path, git_env)
yield
end
diff --git a/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb b/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb
index 92b1adf5afc4f1a53cbfe84b02393b2475b0bfad..fced04bde552fb1fb56a9d570d456013c5c96f16 100644
--- a/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb
+++ b/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb
@@ -110,6 +110,7 @@
before do
::Gitlab::Git::HookEnv.set(project.repository.gl_repository,
+ project.repository.raw_repository.relative_path,
"GIT_OBJECT_DIRECTORY_RELATIVE" => "objects",
"GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE" => [])
end
diff --git a/ee/spec/lib/ee/gitlab/git_access_project_spec.rb b/ee/spec/lib/ee/gitlab/git_access_project_spec.rb
index b891688b92f309fd759526eee56ddeb7fa542d6a..47e3a0f2140fcea4aa5959703f8e92ad84c51be8 100644
--- a/ee/spec/lib/ee/gitlab/git_access_project_spec.rb
+++ b/ee/spec/lib/ee/gitlab/git_access_project_spec.rb
@@ -68,12 +68,11 @@
create(:namespace_root_storage_statistics, namespace: namespace)
end
- context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is set' do
+ context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is set', :request_store do
before do
- allow(Gitlab::Git::HookEnv)
- .to receive(:all)
- .with(repository.gl_repository)
- .and_return({ 'GIT_OBJECT_DIRECTORY_RELATIVE' => 'objects' })
+ ::Gitlab::Git::HookEnv.set(project.repository.gl_repository,
+ project.repository.raw_repository.relative_path,
+ 'GIT_OBJECT_DIRECTORY_RELATIVE' => 'objects')
simulate_quarantine_size(repository, object_directory_size)
end
diff --git a/ee/spec/lib/gitlab/git_access_spec.rb b/ee/spec/lib/gitlab/git_access_spec.rb
index 92d8ff184c190559c8e1bf62a22c5fa12c04e0c1..1bb1e41c7a88335df17555317a96c2a0744ca5c8 100644
--- a/ee/spec/lib/gitlab/git_access_spec.rb
+++ b/ee/spec/lib/gitlab/git_access_spec.rb
@@ -429,12 +429,11 @@ def download_ability
end
end
- context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is set' do
+ context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is set', :request_store do
before do
- allow(Gitlab::Git::HookEnv)
- .to receive(:all)
- .with(repository.gl_repository)
- .and_return({ 'GIT_OBJECT_DIRECTORY_RELATIVE' => 'objects' })
+ ::Gitlab::Git::HookEnv.set(project.repository.gl_repository,
+ project.repository.raw_repository.relative_path,
+ "GIT_OBJECT_DIRECTORY_RELATIVE" => "objects")
# Stub the object directory size to "simulate" quarantine size
allow(repository)
diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb
index 6d880affc42f016c8602ae0d525d6b8619ee0185..fd9e4594a28f65a2e037c70bf55f900d3c0ff671 100644
--- a/lib/api/internal/base.rb
+++ b/lib/api/internal/base.rb
@@ -52,13 +52,11 @@ def check_allowed(params)
end
# Stores some Git-specific env thread-safely
- env = parse_env
- Gitlab::Git::HookEnv.set(gl_repository, env) if container
-
+ #
# Snapshot repositories have different relative path than the main repository. For access
# checks that need quarantined objects the relative path in also sent with Gitaly RPCs
# calls as a header.
- populate_relative_path(params[:relative_path])
+ Gitlab::Git::HookEnv.set(gl_repository, params[:relative_path], parse_env) if container
actor.update_last_used_at!
@@ -104,12 +102,6 @@ def check_allowed(params)
end
# rubocop: enable Metrics/AbcSize
- def populate_relative_path(relative_path)
- return unless Gitlab::SafeRequestStore.active?
-
- Gitlab::SafeRequestStore[:gitlab_git_relative_path] = relative_path
- end
-
def validate_actor(actor)
return 'Could not find the given key' unless actor.key
diff --git a/lib/gitlab/git/hook_env.rb b/lib/gitlab/git/hook_env.rb
index 2524d4c4cfb0e6f210e8771a724b97c79d4b0119..6f412dc1d264f5d613fd15f950be8eb7e0f937f1 100644
--- a/lib/gitlab/git/hook_env.rb
+++ b/lib/gitlab/git/hook_env.rb
@@ -19,13 +19,28 @@ class HookEnv
GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE
].freeze
- def self.set(gl_repository, env)
+ # set stores the quarantining variables into request store.
+ #
+ # relative_path is sent from Gitaly to Rails when invoking internal API. In production it points to the
+ # transaction's snapshot repository. Tests should pass the original relative path of the repository as
+ # Gitaly is stubbed out from the invokation loop and doesn't create a transaction snapshot.
+ def self.set(gl_repository, relative_path, env)
return unless Gitlab::SafeRequestStore.active?
raise "missing gl_repository" if gl_repository.blank?
Gitlab::SafeRequestStore[:gitlab_git_env] ||= {}
Gitlab::SafeRequestStore[:gitlab_git_env][gl_repository] = allowlist_git_env(env)
+ Gitlab::SafeRequestStore[:gitlab_git_relative_path] = relative_path
+ end
+
+ # get_relative_path returns the relative path of the repository this hook call is triggered for.
+ # This is the repository's relative path in the transaction's snapshot and is passed back to Gitaly
+ # in quarantined calls.
+ def self.get_relative_path
+ return unless Gitlab::SafeRequestStore.active?
+
+ Gitlab::SafeRequestStore.fetch(:gitlab_git_relative_path)
end
def self.all(gl_repository)
diff --git a/spec/lib/gitlab/checks/changed_blobs_spec.rb b/spec/lib/gitlab/checks/changed_blobs_spec.rb
index e2cb67261047e3a453a912de20a683900257fe84..659c02465ab11cc0cd9d9bfaf18e09ab87d41e42 100644
--- a/spec/lib/gitlab/checks/changed_blobs_spec.rb
+++ b/spec/lib/gitlab/checks/changed_blobs_spec.rb
@@ -51,20 +51,16 @@
end
end
- context 'with quarantine directory' do
- let_it_be(:project) { create(:project, :small_repo) }
+ context 'with quarantine directory', :request_store do
+ let_it_be_with_refind(:project) { create(:project, :small_repo) }
let(:revisions) { [repository.commit.id] }
- let(:git_env) do
- {
- 'GIT_OBJECT_DIRECTORY_RELATIVE' => "objects",
- 'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['/dir/one', '/dir/two']
- }
- end
-
before do
- allow(Gitlab::Git::HookEnv).to receive(:all).with(repository.gl_repository).and_return(git_env)
+ ::Gitlab::Git::HookEnv.set(project.repository.gl_repository,
+ project.repository.raw_repository.relative_path,
+ 'GIT_OBJECT_DIRECTORY_RELATIVE' => 'objects',
+ 'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['/dir/one', '/dir/two'])
end
context 'when the blob does not exist in the repo' do
@@ -78,7 +74,7 @@
end
context 'when the same file with different paths is committed' do
- let_it_be(:commits) do
+ before_all do
project.repository.commit_files(
user,
branch_name: project.repository.root_ref,
diff --git a/spec/lib/gitlab/checks/file_size_check/hook_environment_aware_any_oversized_blobs_spec.rb b/spec/lib/gitlab/checks/file_size_check/hook_environment_aware_any_oversized_blobs_spec.rb
index bea0c02cfb82716eaca6f7f511eed7c4ceeb5ac2..376b339b36a69f4f643becdf6cfbb657b1b9c9fa 100644
--- a/spec/lib/gitlab/checks/file_size_check/hook_environment_aware_any_oversized_blobs_spec.rb
+++ b/spec/lib/gitlab/checks/file_size_check/hook_environment_aware_any_oversized_blobs_spec.rb
@@ -29,16 +29,12 @@
end
context 'with hook env' do
- context 'with hook environment' do
- let(:git_env) do
- {
- 'GIT_OBJECT_DIRECTORY_RELATIVE' => "objects",
- 'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['/dir/one', '/dir/two']
- }
- end
-
+ context 'with hook environment', :request_store do
before do
- allow(Gitlab::Git::HookEnv).to receive(:all).with(repository.gl_repository).and_return(git_env)
+ ::Gitlab::Git::HookEnv.set(project.repository.gl_repository,
+ project.repository.raw_repository.relative_path,
+ 'GIT_OBJECT_DIRECTORY_RELATIVE' => 'objects',
+ 'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['/dir/one', '/dir/two'])
end
it 'returns an emtpy array' do
diff --git a/spec/lib/gitlab/git/hook_env_spec.rb b/spec/lib/gitlab/git/hook_env_spec.rb
index c8f9218916e56222c6b755379255aed26ed75941..8a8d647162c8ba91faa353d169eb9088ac933f92 100644
--- a/spec/lib/gitlab/git/hook_env_spec.rb
+++ b/spec/lib/gitlab/git/hook_env_spec.rb
@@ -3,14 +3,16 @@
require 'spec_helper'
RSpec.describe Gitlab::Git::HookEnv do
+ let(:relative_path) { 'snapshot/relative-path.git' }
let(:gl_repository) { 'project-123' }
describe ".set" do
context 'with RequestStore disabled' do
it 'does not store anything' do
- described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo')
+ described_class.set(gl_repository, relative_path, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo')
expect(described_class.all(gl_repository)).to be_empty
+ expect(described_class.get_relative_path).to be_nil
end
end
@@ -18,6 +20,7 @@
it 'whitelist some `GIT_*` variables and stores them using RequestStore' do
described_class.set(
gl_repository,
+ relative_path,
GIT_OBJECT_DIRECTORY_RELATIVE: 'foo',
GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: 'bar',
GIT_EXEC_PATH: 'baz',
@@ -34,15 +37,16 @@
end
end
- describe ".all" do
- context 'with RequestStore enabled', :request_store do
- before do
- described_class.set(
- gl_repository,
- GIT_OBJECT_DIRECTORY_RELATIVE: 'foo',
- GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: ['bar'])
- end
+ context 'with RequestStore enabled', :request_store do
+ before do
+ described_class.set(
+ gl_repository,
+ relative_path,
+ GIT_OBJECT_DIRECTORY_RELATIVE: 'foo',
+ GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: ['bar'])
+ end
+ describe ".all" do
it 'returns an env hash' do
expect(described_class.all(gl_repository)).to eq({
'GIT_OBJECT_DIRECTORY_RELATIVE' => 'foo',
@@ -50,6 +54,12 @@
})
end
end
+
+ describe ".get_relative_path" do
+ it 'returns the relative path' do
+ expect(described_class.get_relative_path).to eq(relative_path)
+ end
+ end
end
describe ".to_env_hash" do
@@ -70,7 +80,7 @@
with_them do
before do
- described_class.set(gl_repository, key.to_sym => input)
+ described_class.set(gl_repository, relative_path, key.to_sym => input)
end
it 'puts the right value in the hash' do
@@ -86,26 +96,36 @@
describe 'thread-safety' do
context 'with RequestStore enabled', :request_store do
+ let(:other_relative_path) { 'other_relative_path' }
+
before do
allow(RequestStore).to receive(:active?).and_return(true)
- described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo')
+ described_class.set(gl_repository, relative_path, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo')
end
it 'is thread-safe' do
another_thread = Thread.new do
- described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'bar')
+ described_class.set(gl_repository, other_relative_path, GIT_OBJECT_DIRECTORY_RELATIVE: 'bar')
Thread.stop
- described_class.all(gl_repository)[:GIT_OBJECT_DIRECTORY_RELATIVE]
+
+ {
+ relative_path: described_class.get_relative_path,
+ GIT_OBJECT_DIRECTORY_RELATIVE: described_class.all(gl_repository)[:GIT_OBJECT_DIRECTORY_RELATIVE]
+ }
end
# Ensure another_thread runs first
sleep 0.1 until another_thread.stop?
+ expect(described_class.get_relative_path).to eq(relative_path)
expect(described_class.all(gl_repository)[:GIT_OBJECT_DIRECTORY_RELATIVE]).to eq('foo')
another_thread.run
- expect(another_thread.value).to eq('bar')
+ expect(another_thread.value).to eq({
+ relative_path: other_relative_path,
+ GIT_OBJECT_DIRECTORY_RELATIVE: 'bar'
+ })
end
end
end
diff --git a/spec/lib/gitlab/git_access_snippet_spec.rb b/spec/lib/gitlab/git_access_snippet_spec.rb
index 7916481a853f01fee5f72b815337541907ffe959..3a25bf814162ac72ce544aaafae2adb4e67f54af 100644
--- a/spec/lib/gitlab/git_access_snippet_spec.rb
+++ b/spec/lib/gitlab/git_access_snippet_spec.rb
@@ -371,14 +371,13 @@
it_behaves_like 'migration bot does not err'
end
- context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is set' do
+ context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is set', :request_store do
let(:change_size) { 100 }
before do
- allow(Gitlab::Git::HookEnv)
- .to receive(:all)
- .with(repository.gl_repository)
- .and_return({ 'GIT_OBJECT_DIRECTORY_RELATIVE' => 'objects' })
+ ::Gitlab::Git::HookEnv.set(repository.gl_repository,
+ repository.raw_repository.relative_path,
+ 'GIT_OBJECT_DIRECTORY_RELATIVE' => 'objects')
# Stub the object directory size to "simulate" quarantine size
allow(repository).to receive(:object_directory_size).and_return(change_size)
diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb
index 7149e2dc4ef35ef20d85a284f9cd108805a261b4..481366c23f5fce2baf1620e396be0946adac194e 100644
--- a/spec/requests/api/internal/base_spec.rb
+++ b/spec/requests/api/internal/base_spec.rb
@@ -490,6 +490,7 @@ def perform_request(headers: gitlab_shell_internal_api_request_header)
end
context "access granted" do
+ let(:relative_path) { nil }
let(:env) { {} }
around do |example|
@@ -515,7 +516,7 @@ def perform_request(headers: gitlab_shell_internal_api_request_header)
include_context 'with env passed as a JSON'
it 'sets env in RequestStore' do
- expect(Gitlab::Git::HookEnv).to receive(:set).with(gl_repository, env.stringify_keys)
+ expect(Gitlab::Git::HookEnv).to receive(:set).with(gl_repository, relative_path, env.stringify_keys)
subject
@@ -551,7 +552,7 @@ def request_response(request:, call:, method:, metadata:) # rubocop:disable Lint
end
it 'sets env in RequestStore and routes gRPC messages to primary', :request_store do
- expect(Gitlab::Git::HookEnv).to receive(:set).with(gl_repository, env.stringify_keys).and_call_original
+ expect(Gitlab::Git::HookEnv).to receive(:set).with(gl_repository, relative_path, env.stringify_keys).and_call_original
subject
@@ -560,21 +561,9 @@ def request_response(request:, call:, method:, metadata:) # rubocop:disable Lint
end
end
- context 'when Gitaly provides a relative_path argument', :request_store do
- subject { push(key, project, relative_path: relative_path) }
-
- let(:relative_path) { 'relative_path' }
-
- it 'stores relative_path value in RequestStore' do
- allow(Gitlab::SafeRequestStore).to receive(:[]=).and_call_original
- expect(Gitlab::SafeRequestStore).to receive(:[]=).with(:gitlab_git_relative_path, relative_path)
- subject
-
- expect(response).to have_gitlab_http_status(:ok)
- end
- end
-
context "git push with project.wiki" do
+ let(:relative_path) { project.wiki.repository.relative_path }
+
subject { push(key, project.wiki, env: env.to_json) }
it 'responds with success' do
@@ -625,7 +614,9 @@ def request_response(request:, call:, method:, metadata:) # rubocop:disable Lint
# relative_path is sent from Gitaly to Rails when invoking internal API. In production it points to the
# transaction's snapshot repository. As Gitaly is stubbed out from the invocation loop, there is no transaction
# and thus no snapshot repository. Pass the original relative path.
- subject { push(key, personal_snippet, env: env.to_json, changes: snippet_changes, relative_path: "#{personal_snippet.repository.disk_path}.git") }
+ let(:relative_path) { personal_snippet.repository.relative_path }
+
+ subject { push(key, personal_snippet, env: env.to_json, changes: snippet_changes) }
it 'responds with success' do
subject
@@ -664,7 +655,9 @@ def request_response(request:, call:, method:, metadata:) # rubocop:disable Lint
# relative_path is sent from Gitaly to Rails when invoking internal API. In production it points to the
# transaction's snapshot repository. As Gitaly is stubbed out from the invocation loop, there is no transaction
# and thus no snapshot repository. Pass the original relative path.
- subject { push(key, project_snippet, env: env.to_json, changes: snippet_changes, relative_path: "#{project_snippet.repository.disk_path}.git") }
+ let(:relative_path) { project_snippet.repository.relative_path }
+
+ subject { push(key, project_snippet, env: env.to_json, changes: snippet_changes) }
it 'responds with success' do
subject
diff --git a/spec/support/helpers/api_internal_base_helpers.rb b/spec/support/helpers/api_internal_base_helpers.rb
index d3ae1a5c3b242ff64c88da1611d6db701f88ad1f..896c7a5ffddc88015c8644fb1ffb4b185c9969be 100644
--- a/spec/support/helpers/api_internal_base_helpers.rb
+++ b/spec/support/helpers/api_internal_base_helpers.rb
@@ -41,7 +41,7 @@ def pull(key, container, protocol = 'ssh')
)
end
- def push(key, container, protocol = 'ssh', env: nil, changes: nil, relative_path: nil)
+ def push(key, container, protocol = 'ssh', env: nil, changes: nil)
push_with_path(
key,
full_path: full_path_for(container),
@@ -49,7 +49,7 @@ def push(key, container, protocol = 'ssh', env: nil, changes: nil, relative_path
protocol: protocol,
env: env,
changes: changes,
- relative_path: relative_path
+ relative_path: container.repository.relative_path
)
end