Modify approval_rules when security policy is changed

Sashi Kumar Kumaresanrequested to merge
sk/464034-create-rules into master

What does this MR do and why?

This MR updates Security::SecurityOrchestrationPolicies::SyncProjectService to create/update/delete project approval rules and MR approval rules when a policy is created/updated/deleted. Currently, the approval rules are created through Security::ProcessScanResultPolicyWorker by deleting and re-creating approval rules for all policies even when a single policy is updated. This change updates the behaviour by processing only the changed policy and updating the approval rules instead of deleting and recreating them.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Query Plans

sync_project_approval_rules_for_approval_policy_rules 

SELECT
    "approval_project_rules".* 
FROM
    "approval_project_rules" 
WHERE
    "approval_project_rules"."project_id" = 63046519
    AND "approval_project_rules"."rule_type" = 2 
    AND "approval_project_rules"."approval_policy_rule_id" IN (
        SELECT
            "approval_policy_rules"."id" 
        FROM
            "approval_policy_rules" 
        WHERE
            "approval_policy_rules"."security_policy_id" = 16334
    )

Query Plan

 Nested Loop  (cost=0.71..7.89 rows=1 width=206) (actual time=4.898..5.416 rows=2 loops=1)
   Buffers: shared hit=9 read=4
   I/O Timings: read=5.245 write=0.000
   ->  Index Scan using index_approval_project_rules_on_project_id on public.approval_project_rules  (cost=0.43..4.57 rows=1 width=206) (actual time=4.823..5.327 rows=2 loops=1)
         Index Cond: (approval_project_rules.project_id = 63046519)
         Filter: (approval_project_rules.rule_type = 2)
         Rows Removed by Filter: 0
         Buffers: shared hit=1 read=4
         I/O Timings: read=5.245 write=0.000
   ->  Index Only Scan using idx_approval_policy_rules_security_policy_id_id on public.approval_policy_rules  (cost=0.29..3.30 rows=1 width=8) (actual time=0.040..0.040 rows=2 loops=2)
         Index Cond: (approval_policy_rules.security_policy_id = 16334)
         Heap Fetches: 0
         Buffers: shared hit=8
         I/O Timings: read=0.000 write=0.000
Settings: jit = 'off', seq_page_cost = '4', random_page_cost = '1.5', effective_cache_size = '472585MB', work_mem = '100MB'

Time: 7.864 ms
  - planning: 2.384 ms
  - execution: 5.480 ms
    - I/O read: 5.245 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 9 (~72.00 KiB) from the buffer pool
  - reads: 4 (~32.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Delete approval_project_rules for project

DELETE 
FROM
    "approval_project_rules" 
WHERE
    "approval_project_rules"."security_orchestration_policy_configuration_id" = 1029627 
    AND "approval_project_rules"."project_id" = 63046519
    AND "approval_project_rules"."id" >= 54483806 
    AND "approval_project_rules"."approval_policy_rule_id" IN (
        SELECT
            "approval_policy_rules"."id" 
        FROM
            "approval_policy_rules" 
        WHERE
            "approval_policy_rules"."security_policy_id" = 16334
    )

Query Plan

 ModifyTable on public.approval_project_rules  (cost=0.71..6.76 rows=0 width=0) (actual time=1115.533..1115.535 rows=0 loops=1)
   Buffers: shared hit=565 read=2079 dirtied=154
   WAL: records=275 fpi=153 bytes=1240837
   I/O Timings: read=1096.045 write=0.000
   ->  Nested Loop  (cost=0.71..6.76 rows=1 width=12) (actual time=88.503..1114.583 rows=2 loops=1)
         Buffers: shared hit=550 read=2075 dirtied=151
         WAL: records=273 fpi=151 bytes=1224447
         I/O Timings: read=1095.318 write=0.000
         ->  Index Scan using idx_approval_policy_rules_security_policy_id_id on public.approval_policy_rules  (cost=0.29..3.30 rows=1 width=14) (actual time=0.600..0.604 rows=2 loops=1)
               Index Cond: (approval_policy_rules.security_policy_id = 16334)
               Buffers: shared hit=5 read=1
               I/O Timings: read=0.542 write=0.000
         ->  Index Scan using idx_approval_project_rules_on_config_id_and_policy_rule_id on public.approval_project_rules  (cost=0.43..3.45 rows=1 width=14) (actual time=88.845..556.982 rows=1 loops=2)
               Index Cond: ((approval_project_rules.security_orchestration_policy_configuration_id = 1029627) AND (approval_project_rules.approval_policy_rule_id = approval_policy_rules.id))
               Filter: ((approval_project_rules.id >= 54483806) AND (approval_project_rules.project_id = 63046519))
               Rows Removed by Filter: 1894
               Buffers: shared hit=545 read=2074 dirtied=151
               WAL: records=273 fpi=151 bytes=1224447
               I/O Timings: read=1094.776 write=0.000
Trigger RI_ConstraintTrigger_a_32683 for constraint fk_rails_64e8ed3c7e: time=13.072 calls=2
Trigger RI_ConstraintTrigger_a_32688 for constraint fk_rails_65203aa786: time=13.091 calls=2
Trigger RI_ConstraintTrigger_a_33547 for constraint fk_rails_9071e863d1: time=11.055 calls=2
Trigger RI_ConstraintTrigger_a_34129 for constraint fk_rails_b9e9394efb: time=15.366 calls=2
Settings: random_page_cost = '1.5', effective_cache_size = '472585MB', work_mem = '100MB', jit = 'off', seq_page_cost = '4'

Time: 1.171 s
  - planning: 2.604 ms
  - execution: 1.168 s
    - I/O read: 1.096 s
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 565 (~4.40 MiB) from the buffer pool
  - reads: 2079 (~16.20 MiB) from the OS file cache, including disk I/O
  - dirtied: 154 (~1.20 MiB)
  - writes: 0

Delete approval_merge_request_rules for project

DELETE 
FROM
    "approval_merge_request_rules" 
WHERE
    "approval_merge_request_rules"."id" IN (
        SELECT
            "approval_merge_request_rules"."id" 
        FROM
            "approval_merge_request_rules" 
        INNER JOIN
            "merge_requests" 
                ON "merge_requests"."id" = "approval_merge_request_rules"."merge_request_id" 
        WHERE
            "approval_merge_request_rules"."security_orchestration_policy_configuration_id" = 1029627 
            AND "approval_merge_request_rules"."id" >= 197798797
            AND "approval_merge_request_rules"."id" < 199454293
            AND "merge_requests"."state_id" != 3 
            AND "merge_requests"."target_project_id" = 45830447
            AND "approval_merge_request_rules"."approval_policy_rule_id" IN (
                SELECT
                    "approval_policy_rules"."id" 
                FROM
                    "approval_policy_rules" 
                WHERE
                    "approval_policy_rules"."security_policy_id" = 16334
            )
        )

Query Plan

 ModifyTable on public.approval_merge_request_rules  (cost=12.37..15.40 rows=0 width=0) (actual time=3064.759..3064.763 rows=0 loops=1)
   Buffers: shared hit=4739 read=4505 dirtied=1183
   WAL: records=2044 fpi=1182 bytes=9456470
   I/O Timings: read=2996.405 write=0.000
   ->  Nested Loop  (cost=12.37..15.40 rows=1 width=24) (actual time=3062.557..3062.579 rows=2 loops=1)
         Buffers: shared hit=4722 read=4503 dirtied=1180
         WAL: records=2042 fpi=1180 bytes=9440152
         I/O Timings: read=2994.475 write=0.000
         ->  HashAggregate  (cost=11.80..11.81 rows=1 width=26) (actual time=3059.891..3059.898 rows=2 loops=1)
               Group Key: approval_merge_request_rules_1.id
               Buffers: shared hit=4714 read=4501 dirtied=1180
               WAL: records=2042 fpi=1180 bytes=9440152
               I/O Timings: read=2991.878 write=0.000
               ->  Nested Loop  (cost=1.42..11.80 rows=1 width=26) (actual time=217.063..3059.873 rows=2 loops=1)
                     Buffers: shared hit=4714 read=4501 dirtied=1180
                     WAL: records=2042 fpi=1180 bytes=9440152
                     I/O Timings: read=2991.878 write=0.000
                     ->  Nested Loop  (cost=0.85..8.15 rows=1 width=24) (actual time=6.207..2148.987 rows=988 loops=1)
                           Buffers: shared hit=951 read=3621 dirtied=1172
                           WAL: records=2034 fpi=1172 bytes=9384856
                           I/O Timings: read=2096.600 write=0.000
                           ->  Index Scan using idx_approval_policy_rules_security_policy_id_id on public.approval_policy_rules  (cost=0.29..3.30 rows=1 width=14) (actual time=0.037..0.042 rows=2 loops=1)
                                 Index Cond: (approval_policy_rules.security_policy_id = 16334)
                                 Buffers: shared hit=6
                                 I/O Timings: read=0.000 write=0.000
                           ->  Index Scan using idx_approval_mr_rules_on_config_id_and_policy_rule_id on public.approval_merge_request_rules approval_merge_request_rules_1  (cost=0.57..4.84 rows=1 width=26) (actual time=4.233..1074.217 rows=494 loops=2)
                                 Index Cond: ((approval_merge_request_rules_1.security_orchestration_policy_configuration_id = 1029627) AND (approval_merge_request_rules_1.approval_policy_rule_id = approval_policy_rules.id))
                                 Filter: ((approval_merge_request_rules_1.id >= 197798797) AND (approval_merge_request_rules_1.id < 199454293))
                                 Rows Removed by Filter: 4472
                                 Buffers: shared hit=945 read=3621 dirtied=1172
                                 WAL: records=2034 fpi=1172 bytes=9384856
                                 I/O Timings: read=2096.600 write=0.000
                     ->  Index Scan using idx_merge_requests_on_unmerged_state_id on public.merge_requests  (cost=0.56..3.57 rows=1 width=10) (actual time=0.920..0.920 rows=0 loops=988)
                           Index Cond: (merge_requests.id = approval_merge_request_rules_1.merge_request_id)
                           Filter: ((merge_requests.state_id <> 3) AND (merge_requests.target_project_id = 45830447))
                           Rows Removed by Filter: 1
                           Buffers: shared hit=3763 read=880 dirtied=8
                           WAL: records=8 fpi=8 bytes=55296
                           I/O Timings: read=895.277 write=0.000
         ->  Index Scan using approval_merge_request_rules_pkey on public.approval_merge_request_rules  (cost=0.57..3.58 rows=1 width=14) (actual time=1.336..1.336 rows=1 loops=2)
               Index Cond: (approval_merge_request_rules.id = approval_merge_request_rules_1.id)
               Buffers: shared hit=8 read=2
               I/O Timings: read=2.597 write=0.000
Trigger RI_ConstraintTrigger_a_32478 for constraint fk_rails_5b2ecf6139: time=13.582 calls=2
Trigger RI_ConstraintTrigger_a_32698 for constraint fk_rails_6577725edb: time=16.205 calls=2
Trigger RI_ConstraintTrigger_a_33261 for constraint fk_rails_80e6801803: time=16.076 calls=2
Trigger RI_ConstraintTrigger_a_34876 for constraint fk_rails_e605a04f76: time=11.613 calls=2
Settings: random_page_cost = '1.5', effective_cache_size = '472585MB', work_mem = '100MB', jit = 'off', seq_page_cost = '4'

Time: 3.128 s
  - planning: 5.575 ms
  - execution: 3.122 s
    - I/O read: 2.996 s
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 4739 (~37.00 MiB) from the buffer pool
  - reads: 4505 (~35.20 MiB) from the OS file cache, including disk I/O
  - dirtied: 1183 (~9.20 MiB)
  - writes: 0

Delete scan_result_policy_violations for project

DELETE 
FROM
    "scan_result_policy_violations" 
WHERE
    "scan_result_policy_violations"."project_id" = 11160640
    AND "scan_result_policy_violations"."id" >= 35278026
    AND "scan_result_policy_violations"."id" < 35623546
    AND "scan_result_policy_violations"."approval_policy_rule_id" IN (
        SELECT
            "approval_policy_rules"."id" 
        FROM
            "approval_policy_rules" 
        WHERE
            "approval_policy_rules"."security_policy_id" = 16334
    )

Query Plan

 ModifyTable on public.scan_result_policy_violations  (cost=30.22..34.77 rows=0 width=0) (actual time=114.195..114.197 rows=0 loops=1)
   Buffers: shared hit=129 read=224 dirtied=51
   WAL: records=97 fpi=50 bytes=326228
   I/O Timings: read=25.122 write=0.000
   ->  Nested Loop  (cost=30.22..34.77 rows=1 width=12) (actual time=40.850..114.044 rows=48 loops=1)
         Buffers: shared hit=65 read=224 dirtied=35
         WAL: records=49 fpi=35 bytes=277269
         I/O Timings: read=25.122 write=0.000
         ->  Index Scan using idx_approval_policy_rules_security_policy_id_id on public.approval_policy_rules  (cost=0.29..3.30 rows=1 width=14) (actual time=0.029..0.038 rows=2 loops=1)
               Index Cond: (approval_policy_rules.security_policy_id = 16334)
               Buffers: shared hit=6
               I/O Timings: read=0.000 write=0.000
         ->  Bitmap Heap Scan on public.scan_result_policy_violations  (cost=29.94..31.46 rows=1 width=14) (actual time=32.752..56.975 rows=24 loops=2)
               Filter: ((scan_result_policy_violations.id >= 35278026) AND (scan_result_policy_violations.id < 35623546))
               Rows Removed by Filter: 119
               Buffers: shared hit=59 read=224 dirtied=35
               WAL: records=49 fpi=35 bytes=277269
               I/O Timings: read=25.122 write=0.000
               ->  BitmapAnd  (cost=29.94..29.94 rows=1 width=0) (actual time=13.107..13.108 rows=0 loops=2)
                     Buffers: shared hit=10 read=50
                     I/O Timings: read=22.461 write=0.000
                     ->  Bitmap Index Scan using index_scan_result_policy_violations_on_approval_policy_rule_id  (cost=0.00..2.09 rows=22 width=0) (actual time=10.916..10.916 rows=13701 loops=2)
                           Index Cond: (scan_result_policy_violations.approval_policy_rule_id = approval_policy_rules.id)
                           Buffers: shared hit=4 read=44
                           I/O Timings: read=19.059 write=0.000
                     ->  Bitmap Index Scan using index_scan_result_policy_violations_on_project_id  (cost=0.00..27.49 rows=2009 width=0) (actual time=1.865..1.865 rows=1996 loops=2)
                           Index Cond: (scan_result_policy_violations.project_id = 11160640)
                           Buffers: shared hit=6 read=6
                           I/O Timings: read=3.403 write=0.000
Settings: jit = 'off', seq_page_cost = '4', effective_cache_size = '472585MB', random_page_cost = '1.5', work_mem = '100MB'

Time: 115.833 ms
  - planning: 1.488 ms
  - execution: 114.345 ms
    - I/O read: 25.122 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 129 (~1.00 MiB) from the buffer pool
  - reads: 224 (~1.80 MiB) from the OS file cache, including disk I/O
  - dirtied: 51 (~408.00 KiB)
  - writes: 0

Delete software_license_policies for project

DELETE 
FROM
    "software_license_policies" 
WHERE
    "software_license_policies"."project_id" = 46820979
    AND "software_license_policies"."id" >= 36643731
    AND "software_license_policies"."approval_policy_rule_id" IN (
        SELECT
            "approval_policy_rules"."id" 
        FROM
            "approval_policy_rules" 
        WHERE
            "approval_policy_rules"."security_policy_id" = 13803
    )

Query Plan

 ModifyTable on public.software_license_policies  (cost=0.71..6.67 rows=0 width=0) (actual time=0.009..0.010 rows=0 loops=1)
   Buffers: shared hit=3
   I/O Timings: read=0.000 write=0.000
   ->  Nested Loop  (cost=0.71..6.67 rows=1 width=12) (actual time=0.008..0.009 rows=0 loops=1)
         Buffers: shared hit=3
         I/O Timings: read=0.000 write=0.000
         ->  Index Scan using software_license_policies_pkey on public.software_license_policies  (cost=0.42..3.35 rows=1 width=14) (actual time=0.007..0.008 rows=0 loops=1)
               Index Cond: (software_license_policies.id >= 36643731)
               Filter: (software_license_policies.project_id = 46820979)
               Rows Removed by Filter: 0
               Buffers: shared hit=3
               I/O Timings: read=0.000 write=0.000
         ->  Index Scan using idx_approval_policy_rules_security_policy_id_id on public.approval_policy_rules  (cost=0.29..3.30 rows=1 width=14) (actual time=0.000..0.000 rows=0 loops=0)
               Index Cond: (approval_policy_rules.security_policy_id = 13803)
               I/O Timings: read=0.000 write=0.000
Settings: effective_cache_size = '472585MB', random_page_cost = '1.5', work_mem = '100MB', jit = 'off', seq_page_cost = '4'

Time: 1.679 ms
  - planning: 1.587 ms
  - execution: 0.092 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 3 (~24.00 KiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

How to set up and validate locally

  • Create a group and create multiple projects in the group
  • Create 3 MR approval policy from Secure->Policies
Policy 1 
type: approval_policy
name: 'Security Scan '
description: ''
enabled: true
policy_scope:
  projects:
    excluding: []
rules:
  - type: scan_finding
    scanners: []
    vulnerabilities_allowed: 0
    severity_levels: []
    vulnerability_states: []
    branch_type: protected
actions:
  - type: require_approval
    approvals_required: 1
    role_approvers:
      - developer
  - type: send_bot_message
    enabled: true
approval_settings:
  block_branch_modification: false
  prevent_pushing_and_force_pushing: false
  prevent_approval_by_author: false
  prevent_approval_by_commit_author: false
  remove_approvals_with_new_commit: false
  require_password_to_approve: false
fallback_behavior:
  fail: closed
Policy 2 
type: approval_policy
name: License Scan
description: ''
enabled: true
policy_scope:
  projects:
    excluding: []
rules:
  - type: license_finding
    match_on_inclusion_license: false
    license_types:
      - 3D Slicer License v1.0
      - AMD newlib License
      - AML glslang variant License
    license_states:
      - newly_detected
    branch_type: protected
actions:
  - type: require_approval
    approvals_required: 1
    role_approvers:
      - developer
  - type: send_bot_message
    enabled: true
approval_settings:
  block_branch_modification: false
  prevent_pushing_and_force_pushing: false
  prevent_approval_by_author: false
  prevent_approval_by_commit_author: false
  remove_approvals_with_new_commit: false
  require_password_to_approve: false
fallback_behavior:
  fail: closed
Policy 3 
type: approval_policy
name: 'Any merge_request '
description: ''
enabled: true
policy_scope:
  projects:
    excluding: []
rules:
  - type: any_merge_request
    branch_type: protected
    commits: any
actions:
  - type: send_bot_message
    enabled: true
approval_settings:
  block_branch_modification: true
  prevent_pushing_and_force_pushing: true
  prevent_approval_by_author: true
  prevent_approval_by_commit_author: true
  remove_approvals_with_new_commit: false
  require_password_to_approve: false
fallback_behavior:
  fail: open
  • Create multiple MRs in each of the projects by updating README.md and verify that the approvals are required from 2 policy
  • Update each policy, merge the MR and verify that the approval rules are updated by:
policy = Security::Policy.find(<id>)
project = Project.find(<id>)
project.approval_rules.where(approval_policy_rule_id: policy.approval_policy_rules.select(:id))
ApprovalMergeRequestRule.where(approval_policy_rule_id: policy.approval_policy_rules.select(:id))
Edited by Sashi Kumar Kumaresan

Merge request reports