Skip to content

Update policy approval rules instead of policy rebuild

Martin Čavoj requested to merge 432859-update-rules-on-authorization-changes into master

What does this MR do and why?

This MR adds a new event for added authorizations to perform more optimized approval rules updates after users are given access to a project that has security policies approval rules.

Depends on Remove user from approval rules when removing a... (!138691 - merged) so that access removals are also reflected in approval rules.

Screenshots or screen recordings

Removing and re-adding a policy approver in a group:

CleanShot_2023-12-07_at_17.32.10

Database

configuration.approval_project_rules.for_project(project.id).for_policy_index(policy_indexes).select(*APPROVAL_RULES_ATTRIBUTES)

Query:

SELECT "approval_project_rules"."id", "approval_project_rules"."orchestration_policy_idx" FROM "approval_project_rules" WHERE "approval_project_rules"."security_orchestration_policy_configuration_id" = 4249178 AND "approval_project_rules"."project_id" = 52293885 AND "approval_project_rules"."orchestration_policy_idx" = 0;

Plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/24810/commands/78820

configuration.approval_merge_request_rules.for_unmerged_merge_requests.for_merge_request_project(project.id).for_policy_index(policy_indexes).select(*APPROVAL_RULES_ATTRIBUTES)

Query:

SELECT "approval_merge_request_rules"."id", "approval_merge_request_rules"."orchestration_policy_idx" 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" = 4249178 AND "merge_requests"."state_id" != 3 AND "merge_requests"."target_project_id" = 52293885 AND "approval_merge_request_rules"."orchestration_policy_idx" = 0;

Plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/24810/commands/78831

project.team.users.id_in(user_ids).select(:id, :username)

Query:

SELECT "users"."id", "users"."username" FROM "users" INNER JOIN "project_authorizations" ON "users"."id" = "project_authorizations"."user_id" WHERE "project_authorizations"."project_id" = 50532553 AND "users"."id" = 13904527

Plan: https://console.postgres.ai/gitlab/gitlab-production-main/sessions/24616/commands/78286

ApprovalProjectRulesUser.insert_all(project_rule_param_batch)

Query:

INSERT INTO "approval_project_rules_users" ("approval_project_rule_id","user_id") VALUES (31158144, 13904527), (31158478, 13904527) ON CONFLICT  DO NOTHING RETURNING "id"

Plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/24783/commands/78735

ApprovalMergeRequestRulesUser.insert_all(merge_request_rule_param_batch)

Query:

INSERT INTO "approval_merge_request_rules_users" ("approval_merge_request_rule_id","user_id") VALUES (131381346, 13904527), (131381345, 13904527) ON CONFLICT  DO NOTHING RETURNING "id"

Plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/24783/commands/78738

How to set up and validate locally

  1. In rails console enable the feature flag

    Feature.enable(:add_policy_approvers_to_rules)

  2. Create a group

  3. In the group, create a project

  4. In the group, go to Policies and create a new scan result policy. Example YAML:

    type: scan_result_policy
name: Security
description: ''
enabled: true
policy_scope:
  compliance_frameworks: []
rules:
  - type: scan_finding
    scanners: []
    vulnerabilities_allowed: 0
    severity_levels: []
    vulnerability_states: []
    branch_type: protected
actions:
  - type: require_approval
    approvals_required: 1
    user_approvers_ids:
      - 1
      - 4 # User to test with

  5. In the project, create an MR (e.g. changing a README)

  6. Notice the required approvals

  7. Add the user referenced in the policy as a group member with a developer role

  8. Verify that the user is added as an approver in the MR to the correct rules (rules where they are referenced as approvers)

MR acceptance checklist

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

Related to #432859 (closed)

Edited by Martin Čavoj

Merge request reports