Fix force-routing to Gitaly primary with empty hook env
What does this MR do?
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.
/cc @grantyoung Closes gitaly#3457 (closed)
Screenshots (strongly suggested)
First bump is without my fix, second bump is with my fix:
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec -
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team