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:

1. Delete approval rules for project associated to security_orchestration_policy_configuration
2. Create approval rules for project associated to security_orchestration_policy_configuration
3. 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 to SyncOpenedMergeRequestsService 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]