Skip to content

Add service for syncing approval rules from security_findings

Sashi Kumar Kumaresan requested to merge sk/387277-sync-approvals-from-db into master

What does this MR do and why?

Addresses #387277 (closed)

Currently we are performing all calculations based on security reports assigned to the pipeline. This means we have to parse data from Security Report JSON files and perform all calculations in Ruby. It is a part of the code that was related to old Vulnerability-Check feature and during the development on Scan Result Policies it was not changed

As more and more customers are using Scan Result Policies and we want to make sure this feature performs well on larger scale we need to change the way we are currently performing this calculations and move them to service that is more performant to do them - to database.

We already have Security::Scan and Security::Finding models and data in database created for all pipelines, so we could reuse it. This MR introduces a new service Security::ScanResultPolicies::SyncFindingsToApprovalRulesService that makes use of security_findings table to query the findings for a pipeline instead of parsing and reading the security reports. The current codepath uses Gitlab::Ci::Reports::Security::Concerns::ScanFinding to calculate if a MR violates approval rules, The service introduced in this MR will be behind sync_approval_rules_from_findings feature flag.

Second MR to introduce the worker: !115825 (merged)

Sequence

Before

sequenceDiagram
    participant Pipeline
    participant StoreScansWorker
    participant SyncReportsToReportApprovalRulesWorker
    participant ApprovalRules
    Pipeline->>StoreScansWorker: transitioned to complete status
    Pipeline->>SyncReportsToReportApprovalRulesWorker: transitioned to complete status
    SyncReportsToReportApprovalRulesWorker->>ApprovalRules: parse findings and update approvals_required

After

sequenceDiagram
    participant Pipeline
    participant StoreScansWorker
    participant SyncReportsToReportApprovalRulesWorker
    participant SyncFindingsToApprovalRulesWorker
    participant ApprovalRules
    Pipeline->>StoreScansWorker: transitioned to complete status
    Pipeline->>SyncReportsToReportApprovalRulesWorker: transitioned to complete status
    StoreScansWorker->>SyncFindingsToApprovalRulesWorker: after inserting security_findings
    SyncFindingsToApprovalRulesWorker->>ApprovalRules: update approvals_required

Query plans

Pipeline scan types

SELECT
  "security_scans"."scan_type"
FROM
  "security_scans"
WHERE
  "security_scans"."pipeline_id" = 782703843;

Plan

 Index Scan using index_security_scans_on_pipeline_id on public.security_scans  (cost=0.56..5.78 rows=6 width=2) (actual time=0.044..0.069 rows=5 loops=1)
   Index Cond: (security_scans.pipeline_id = 782703843)
   Buffers: shared hit=12
   I/O Timings: read=0.000 write=0.000

Findings UUIDs

SELECT
    "security_findings"."uuid"
FROM
    "security_findings"
INNER JOIN
    "security_scans"
ON
    "security_findings"."scan_id" = "security_scans"."id"
WHERE
    "security_scans"."pipeline_id" = 782703843
AND
    "security_findings"."severity" IN (5, 6, 7)     
AND
    "security_scans"."scan_type" IN (1, 2, 3, 4, 5, 6, 7);

Plan

 Nested Loop  (cost=1.13..95606.38 rows=76 width=16) (actual time=167.762..389.736 rows=527 loops=1)
   Buffers: shared hit=216 read=283
   I/O Timings: read=384.158 write=0.000
   ->  Index Scan using index_security_scans_on_pipeline_id on public.security_scans  (cost=0.56..5.81 rows=6 width=8) (actual time=26.031..30.863 rows=5 loops=1)
         Index Cond: (security_scans.pipeline_id = 782703843)
         Filter: (security_scans.scan_type = ANY ('{1,2,3,4,5,6,7}'::integer[]))
         Rows Removed by Filter: 0
         Buffers: shared hit=3 read=9
         I/O Timings: read=30.633 write=0.000
   ->  Append  (cost=0.57..15829.99 rows=10344 width=24) (actual time=29.129..71.700 rows=105 loops=5)
         Buffers: shared hit=213 read=274
         I/O Timings: read=353.524 write=0.000
         ->  Index Scan using security_findings_41_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_41  (cost=0.57..1037.67 rows=778 width=24) (actual time=6.079..6.079 rows=0 loops=5)
               Index Cond: (security_findings_41.scan_id = security_scans.id)
               Filter: (security_findings_41.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=30.243 write=0.000
         ->  Index Scan using security_findings_42_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_42  (cost=0.57..1308.87 rows=753 width=24) (actual time=4.043..4.043 rows=0 loops=5)
               Index Cond: (security_findings_42.scan_id = security_scans.id)
               Filter: (security_findings_42.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=20.047 write=0.000
         ->  Index Scan using security_findings_43_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_43  (cost=0.57..2132.00 rows=775 width=24) (actual time=4.357..4.357 rows=0 loops=5)
               Index Cond: (security_findings_43.scan_id = security_scans.id)
               Filter: (security_findings_43.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=21.581 write=0.000
         ->  Index Scan using security_findings_44_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_44  (cost=0.57..989.02 rows=671 width=24) (actual time=3.720..3.720 rows=0 loops=5)
               Index Cond: (security_findings_44.scan_id = security_scans.id)
               Filter: (security_findings_44.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=18.480 write=0.000
         ->  Index Scan using security_findings_45_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_45  (cost=0.57..1080.69 rows=765 width=24) (actual time=3.654..3.654 rows=0 loops=5)
               Index Cond: (security_findings_45.scan_id = security_scans.id)
               Filter: (security_findings_45.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=18.158 write=0.000
         ->  Index Scan using security_findings_46_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_46  (cost=0.57..1138.58 rows=796 width=24) (actual time=3.343..3.343 rows=0 loops=5)
               Index Cond: (security_findings_46.scan_id = security_scans.id)
               Filter: (security_findings_46.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=16.610 write=0.000
         ->  Index Scan using security_findings_47_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_47  (cost=0.57..1205.57 rows=841 width=24) (actual time=3.901..39.443 rows=105 loops=5)
               Index Cond: (security_findings_47.scan_id = security_scans.id)
               Filter: (security_findings_47.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 1
               Buffers: shared hit=21 read=226
               I/O Timings: read=194.162 write=0.000
         ->  Index Scan using security_findings_48_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_48  (cost=0.57..1052.15 rows=773 width=24) (actual time=1.051..1.051 rows=0 loops=5)
               Index Cond: (security_findings_48.scan_id = security_scans.id)
               Filter: (security_findings_48.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=5.098 write=0.000
         ->  Index Scan using security_findings_49_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_49  (cost=0.57..1174.70 rows=823 width=24) (actual time=1.540..1.540 rows=0 loops=5)
               Index Cond: (security_findings_49.scan_id = security_scans.id)
               Filter: (security_findings_49.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=7.592 write=0.000
         ->  Index Scan using security_findings_50_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_50  (cost=0.57..1213.63 rows=855 width=24) (actual time=1.105..1.105 rows=0 loops=5)
               Index Cond: (security_findings_50.scan_id = security_scans.id)
               Filter: (security_findings_50.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=5.426 write=0.000
         ->  Index Scan using security_findings_51_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_51  (cost=0.57..1191.48 rows=864 width=24) (actual time=1.128..1.128 rows=0 loops=5)
               Index Cond: (security_findings_51.scan_id = security_scans.id)
               Filter: (security_findings_51.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=5.540 write=0.000
         ->  Index Scan using security_findings_52_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_52  (cost=0.57..1304.72 rows=935 width=24) (actual time=1.001..1.001 rows=0 loops=5)
               Index Cond: (security_findings_52.scan_id = security_scans.id)
               Filter: (security_findings_52.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=4.910 write=0.000
         ->  Index Scan using security_findings_53_scan_id_id_idx on gitlab_partitions_dynamic.security_findings_53  (cost=0.56..949.19 rows=715 width=24) (actual time=1.154..1.154 rows=0 loops=5)
               Index Cond: (security_findings_53.scan_id = security_scans.id)
               Filter: (security_findings_53.severity = ANY ('{5,6,7}'::integer[]))
               Rows Removed by Filter: 0
               Buffers: shared hit=16 read=4
               I/O Timings: read=5.676 write=0.000

Vulnerabilities Count

SELECT
    COUNT(*) 
FROM
    "vulnerability_reads" 
WHERE
    "vulnerability_reads"."project_id" = 43831199     
    AND "vulnerability_reads"."uuid" IN (
        '03008860-d99c-5db5-9d7e-696e281a666e', '16da8d31-cc9a-53b3-ba80-ce5c9a2d014c', '29887843-b0c3-51eb-ae32-6541ce126d8c', '7eec5d87-b1a2-530e-b63c-84ea61d2fe3c', 'b7ee0340-1677-5e1f-9d67-bbb39d361671', 'c6a62a41-8ae2-5ce8-b2c1-a9a3d01bd870', 'cb5d498a-c216-5278-85fd-364793805495', 'd09bc1f6-1c75-5016-89b5-3b5e43851117', 'ef7da7cd-095b-56d6-964e-da65da71bb7f', '1ffb3985-a983-5162-af9b-e6dbeb81deeb', '7514bafd-d628-53cb-ad32-85e77844f5d2', '85651ee3-01cd-5b0a-bc23-00c494b45b6a', '04ebf666-c4ef-50b4-b62e-313b4e325d48', '0bf4f16d-8aea-5e4c-b4df-1681baf405e8', '10ae5b79-a964-5bb2-a867-6b3a59236939', '11b31277-fce6-5bb3-874a-1bf3d5c25d3c', '12a53708-2711-5788-8922-5bf2c0de6eaa', '2b2e1d00-1c3c-5185-be54-1c49f69ad166', '2bfa606c-9962-5877-9857-ba6b25f0a914', '2c6944e3-4335-50c9-856c-4b818ee714c2', '2ea4f566-10a8-5d30-95af-ded9b566d801', '3b3c81d4-dd31-5b52-8b57-5fb6e68bfedf', '3fd16cbf-73b2-54d5-8fdc-c6ee775e4498', '41289776-b162-56d8-9054-7db0e7d12146', '46f624da-37f1-500c-a434-b3c65af91fa4', '47174b58-041b-51da-b9f0-99bd9a434f37', '5037e0bd-12d4-523e-91f0-412c93989974', '50d634ab-4ee1-53c5-a4c9-fc05c766be18', '51f1c4c3-23ea-5d98-9a7d-ce279b14f10b', '5b68c2e0-7451-5950-93b7-60ba04235b48', '5c5738c2-193f-5677-a635-ed75c6e5518b', '64fb668c-971f-57c1-8df3-877277e706b7', '6b737a49-7c0f-5391-9185-9a570ab83156', '70104250-aafa-540f-ae36-6ed6d68e3306', '7012f101-97fb-5aa5-ab3e-09b470265864', '79a83d10-0c09-58da-b5d0-9362d2b05837', '82e25ab8-1705-53e2-8fb7-9305e157a84b', '8c07ce7a-ed01-58d4-82f3-981f4954c16e', '9343b6dd-ec32-5cca-bc0b-8bfb2d2bfde1', '97e49d98-a8d4-580a-aaf2-4da860a650c7', '9c701f87-96d3-54d9-a406-aa91d0e61611', 'a2abac6f-c509-5465-8053-aaf5a8213e18', 'a80bc127-b1a5-551c-8126-99235c91c956', 'b334d494-9175-50dd-add3-886df3094986', 'b953f9f0-6dcd-542a-a22f-c8f8b85b1d1c', 'b98df0db-81e8-53ef-84e6-6dd99e05ee3b', 'bcd56f27-2096-59e8-b1ac-7321ddd16aa1', 'c501814e-f79a-5f77-bab5-d1cd70ae3eb7', 'd109ce06-5c79-5118-a48a-4bbb1be24ae3', 'd6a14526-ee08-5d46-ad4a-e51d5d9f6234'
    )     
    AND "vulnerability_reads"."state" IN (
        1, 4, 2, 3
    );

Plan

  Aggregate  (cost=179.63..179.64 rows=1 width=8) (actual time=343.633..343.635 rows=1 loops=1)
   Buffers: shared hit=104 read=146 dirtied=22
   I/O Timings: read=330.881 write=0.000
   ->  Index Scan using index_vulnerability_reads_on_uuid on public.vulnerability_reads  (cost=0.57..179.62 rows=1 width=0) (actual time=10.551..343.488 rows=50 loops=1)
         Index Cond: (vulnerability_reads.uuid = ANY ('{03008860-d99c-5db5-9d7e-696e281a666e,16da8d31-cc9a-53b3-ba80-ce5c9a2d014c,29887843-b0c3-51eb-ae32-6541ce126d8c,7eec5d87-b1a2-530e-b63c-84ea61d2fe3c,b7ee0340-1677-5e1f-9d67-bbb39d361671,c6a62a41-8ae2-5ce8-b2c1-a9a3d01bd870,cb5d498a-c216-5278-85fd-364793805495,d09bc1f6-1c75-5016-89b5-3b5e43851117,ef7da7cd-095b-56d6-964e-da65da71bb7f,1ffb3985-a983-5162-af9b-e6dbeb81deeb,7514bafd-d628-53cb-ad32-85e77844f5d2,85651ee3-01cd-5b0a-bc23-00c494b45b6a,04ebf666-c4ef-50b4-b62e-313b4e325d48,0bf4f16d-8aea-5e4c-b4df-1681baf405e8,10ae5b79-a964-5bb2-a867-6b3a59236939,11b31277-fce6-5bb3-874a-1bf3d5c25d3c,12a53708-2711-5788-8922-5bf2c0de6eaa,2b2e1d00-1c3c-5185-be54-1c49f69ad166,2bfa606c-9962-5877-9857-ba6b25f0a914,2c6944e3-4335-50c9-856c-4b818ee714c2,2ea4f566-10a8-5d30-95af-ded9b566d801,3b3c81d4-dd31-5b52-8b57-5fb6e68bfedf,3fd16cbf-73b2-54d5-8fdc-c6ee775e4498,41289776-b162-56d8-9054-7db0e7d12146,46f624da-37f1-500c-a434-b3c65af91fa4,47174b58-041b-51da-b9f0-99bd9a434f37,5037e0bd-12d4-523e-91f0-412c93989974,50d634ab-4ee1-53c5-a4c9-fc05c766be18,51f1c4c3-23ea-5d98-9a7d-ce279b14f10b,5b68c2e0-7451-5950-93b7-60ba04235b48,5c5738c2-193f-5677-a635-ed75c6e5518b,64fb668c-971f-57c1-8df3-877277e706b7,6b737a49-7c0f-5391-9185-9a570ab83156,70104250-aafa-540f-ae36-6ed6d68e3306,7012f101-97fb-5aa5-ab3e-09b470265864,79a83d10-0c09-58da-b5d0-9362d2b05837,82e25ab8-1705-53e2-8fb7-9305e157a84b,8c07ce7a-ed01-58d4-82f3-981f4954c16e,9343b6dd-ec32-5cca-bc0b-8bfb2d2bfde1,97e49d98-a8d4-580a-aaf2-4da860a650c7,9c701f87-96d3-54d9-a406-aa91d0e61611,a2abac6f-c509-5465-8053-aaf5a8213e18,a80bc127-b1a5-551c-8126-99235c91c956,b334d494-9175-50dd-add3-886df3094986,b953f9f0-6dcd-542a-a22f-c8f8b85b1d1c,b98df0db-81e8-53ef-84e6-6dd99e05ee3b,bcd56f27-2096-59e8-b1ac-7321ddd16aa1,c501814e-f79a-5f77-bab5-d1cd70ae3eb7,d109ce06-5c79-5118-a48a-4bbb1be24ae3,d6a14526-ee08-5d46-ad4a-e51d5d9f6234}'::uuid[]))
         Filter: ((vulnerability_reads.project_id = 43831199) AND (vulnerability_reads.state = ANY ('{1,4,2,3}'::integer[])))
         Rows Removed by Filter: 0
         Buffers: shared hit=104 read=146 dirtied=22
         I/O Timings: read=330.881 write=0.000

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Sashi Kumar Kumaresan

Merge request reports