Commit fa1ddf5c authored by Patrick Steinhardt's avatar Patrick Steinhardt

gitaly_client: Fix force-routing to primary with empty hook env

With commit edab619a (gitaly: Fix access checks with transactions and
quarantine environments, 2021-02-05), we started injecting a flag into
Gitaly requests to force-route to the primary Gitaly node in case a hook
environment is set in order to not break access to quarantined objects.
Turns out that this change breaks read distribution though as now all
requests are force-routed to the primary.

The cause of this is trivial enough: the SafeRequestStore returns an
empty hash if it wasn't set up to contain anything. Given that the
checks whether a HookEnv was set up only checked whether there was
something in the SafeRequestStore, they thus always thought that
requests were running in the context of a HookEnv.

Fix the issue by checking that the returned value is non-empty.
parent 85090818
---
title: Fix force-routing to Gitaly primary with empty hook env
merge_request: 54317
author:
type: fixed
...@@ -248,6 +248,8 @@ module Gitlab ...@@ -248,6 +248,8 @@ module Gitlab
return {} unless Gitlab::SafeRequestStore[:gitlab_git_env] return {} unless Gitlab::SafeRequestStore[:gitlab_git_env]
return {} if Gitlab::SafeRequestStore[:gitlab_git_env].empty?
{ 'gitaly-route-repository-accessor-policy' => 'primary-only' } { 'gitaly-route-repository-accessor-policy' => 'primary-only' }
end end
private_class_method :route_to_primary private_class_method :route_to_primary
......
...@@ -311,9 +311,21 @@ RSpec.describe Gitlab::GitalyClient do ...@@ -311,9 +311,21 @@ RSpec.describe Gitlab::GitalyClient do
end end
end end
context 'when RequestStore is enabled with git_env', :request_store do context 'when RequestStore is enabled with empty git_env', :request_store do
before do before do
Gitlab::SafeRequestStore[:gitlab_git_env] = true Gitlab::SafeRequestStore[:gitlab_git_env] = {}
end
it 'disables force-routing to primary' do
expect(described_class.request_kwargs('default', timeout: 1)[:metadata][policy]).to be_nil
end
end
context 'when RequestStore is enabled with populated git_env', :request_store do
before do
Gitlab::SafeRequestStore[:gitlab_git_env] = {
"GIT_OBJECT_DIRECTORY_RELATIVE" => "foo/bar"
}
end end
it 'enables force-routing to primary' do it 'enables force-routing to primary' 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