ActiveRecord::RecordInvalid: Validation failed: Name has already been taken
https://sentry.gitlab.net/gitlab/gitlabcom/issues/4090964/?referrer=gitlab_plugin
ActiveRecord::RecordInvalid: Validation failed: Name has already been taken
lib/gitlab/database/load_balancing/connection_proxy.rb:120:in `public_send'
connection.public_send(...)
lib/gitlab/database/load_balancing/connection_proxy.rb:120:in `block in write_using_load_balancer'
connection.public_send(...)
lib/gitlab/database/load_balancing/load_balancer.rb:127:in `block in read_write'
yield connection
lib/gitlab/database/load_balancing/load_balancer.rb:205:in `retry_with_backoff'
return yield attempt # Yield the current attempt count
lib/gitlab/database/load_balancing/load_balancer.rb:116:in `read_write'
retry_with_backoff(attempts: attempts) do |attempt|
...
(175 additional frame(s) were not displayed)
Validation failed: Name has already been taken
The exception occurs in Security::ProcessScanResultPolicyWorker
when the worker is executed for a same project with different security_orchestration_policy_configuration_id
. The worker performs these steps in a transaction:
- Delete approval rules for project associated to security_orchestration_policy_configuration
- Create approval rules for project associated to security_orchestration_policy_configuration
- Syncs newly created project approval rules to mr approval rules through
Security::SecurityOrchestrationPolicies::SyncOpenedMergeRequestsService
But step 3 performs the synchronisation at the whole project level which would lead to recreation of mr approval rules that already exist. This is also the reason why SyncOpenedMergeRequestsService
is inside the transaction.
graph TD
A[Security::SyncScanPoliciesWorker] -->|Project A, Config A| B(Security::ProcessScanResultPolicyWorker)
G[Security::SyncScanPoliciesWorker] -->|Project A, Config A| H(Security::ProcessScanResultPolicyWorker)
G[Security::SyncScanPoliciesWorker] -->|Project A, Config B| C(Security::ProcessScanResultPolicyWorker)
B --> D[Delete approval rules for config A]
B --> I[Deduplicated]
H --> I[Deduplicated]
C --> E[Delete approval rules for config B]
D --> F[Sync project approval rules for project A]
E --> F[Sync project approval rules for project A]
Even though ProcessScanResultPolicyWorker
has de-duplication enabled, for different configuration_id, it will not be deduplicated.
Implementation Plan
-
Update Security::SecurityOrchestrationPolicies::SyncOpenedMergeRequestsService
to perform synchronisation at project and configuration level -
Move Security::SecurityOrchestrationPolicies::SyncOpenedMergeRequestsService
out of transaction -
Move Security::SecurityOrchestrationPolicies::SyncOpenMergeRequestsHeadPipelineService
toSyncOpenedMergeRequestsService
as we don't have it inside the transaction, this also avoid iterating over the opened merge request twice.
graph TD
A[Security::SyncScanPoliciesWorker] -->|Project A, Config A| B(Security::ProcessScanResultPolicyWorker)
G[Security::SyncScanPoliciesWorker] -->|Project A, Config A| H(Security::ProcessScanResultPolicyWorker)
G[Security::SyncScanPoliciesWorker] -->|Project A, Config B| C(Security::ProcessScanResultPolicyWorker)
B --> D[Delete approval rules for config A]
B --> I[Deduplicated]
H --> I[Deduplicated]
C --> E[Delete approval rules for config B]
D --> F[Sync project approval rules for project A, Config A]
E --> J[Sync project approval rules for project A, config B]
Edited by Sashi Kumar Kumaresan