Fix duplicate name validation in ProcessScanResultPolicyWorker
What does this MR do and why?
Addresses #404415 (closed)
Follow-up MR to remove dead code: !117467 (merged)
Before
This MR fixes an exception 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.
After
This MR fixes this by doing these things:
- 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]
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.