Investigate backend design for group level merge request approval rules
Topic to Evaluate
This issue is to document backend design for group-level rules to support the delivery of &4367
Tasks to Evaluate
-
Figure out the data model (referring to this doc for inspiration) -
Investigate application interactions (peek at existing fabrication of ApprovalProjectRules
andApprovalMergeRequestRules
)
🐘 Data Model
To store group approval rules, we need to create two new entities and some associative tables
ApprovalGroupRule
GroupProtectedBranch
erDiagram
Namespace ||--o{ ApprovalGroupRule : ""
ApprovalGroupRule }o--o{ User : ""
ApprovalGroupRule }o--o{ Group : ""
ApprovalGroupRule }o--o{ GroupProtectedBranch : ""
To provide the ability to override and revert to group approval rule within project approval rule, we need to create a link table between group-level and project level approval rules
ApprovalProjectRuleSources
erDiagram
MergeRequest ||--o{ ApprovalMergeRequestRule : ""
ApprovalMergeRequestRule ||--o{ ApprovalMergeRequestRuleSources : ""
ApprovalProjectRule ||--o{ ApprovalMergeRequestRuleSources : ""
Project ||--o{ ApprovalProjectRule : ""
ApprovalProjectRule ||--o{ ApprovalProjectRuleSources : ""
ApprovalGroupRule ||--o{ ApprovalProjectRuleSources : ""
Namespace ||--o{ ApprovalGroupRule : ""
🔀 Process Flows
Currently, MR approval rules can be defined at the project and MR level. We are introducing group MR approval rules and would like to explore how this new domain model interacts with the existing architecture. To create an MR approval rule, the following pieces of information have to be provided:
- Name
- Number of approvals
- Approvers (users or groups)
- Target branch
Manage group MR approval rules
Requirements
- User can manage approval rules via the UI (Groups > General Settings) or API.
- User can't update the group approval rules when
Prevent users from modifying MR approval rules.
is enabled on the instance-level
Implementation
- A new Vue app interacts with a new internal RESTful API -
GET/POST/PUT/DELETE groups/:id/approval_rules
(similar toee/lib/api/project_approval_rules.rb
) - Add a new API class
API::GroupApprovalRules
- Extend
ApprovalRules::CreateService
to supportgroup
target (currently supporting project and MR) - Extend
ApprovalRules::UpdateService
to supportgroup
target (currently supporting project and MR) - Create a new service class for destroying
ApprovalRules::GroupRuleDestroyService
- Filter eligible users (allows all users that are members of the group - see
group.direct_and_indirect_users
) - Filter groups (allows all public or visible groups to the user - see
group.public_or_visible_to_user
) - Filter protected branches (get all project ids via
Project.for_group_and_its_subgroups
and query protected branches using those ids) -🎱 is this query performant? - Ensure the instance-level setting
Prevent users from modifying MR approval rules.
is taken into account. If it is enabled,- Prohibit creating and updating group MR approval rules via the API
- Disable creating and updating group MR approval rules via the UI
graph LR
API::GroupApprovalRules --> ApprovalRules::CreateService --> ApprovalRules::Updater
API::GroupApprovalRules --> ApprovalRules::UpdateService --> ApprovalRules::Updater
ApprovalRules::Updater --> ApprovalGroupRule
API::GroupApprovalRules --> ApprovalRules::GroupRuleDestroyService
ApprovalRules::GroupRuleDestroyService --> ApprovalGroupRule
❓
Questions - Why do we have an internal and public API for the project and MR approval rules (ie.
approval_settings
andapproval_rules
)?➡ Private API is to be deprecated. - Why do we have separate destroy service classes but the same for creating and updating service classes?
View the approval rules on the project settings page
Requirements
- User can see inherited group approval rules
- Might need to filter applicable rules base on project branch names, groups and users. For example, a group rule applied to
develop
protected branch should not viewable and applicable on a project that does not havedevelop
protected branch.
Implementation
- Extend
ApprovalState
to support group (ie.user_defined_rules
should account for group approval rules)
graph LR
API::ProjectApprovalRules --> Project --> ApprovalState
ApprovalState --> ApprovalProjectRule
ApprovalState --> ApprovalGroupRule
View the approval rules on the MR page
Requirements
- User can view project MR approval rules when creating a merge request, and have options to override them if allowed. To maintain a similar experience, we also want to surface group MR approval rules in this view.
- User can also modify group approval rules (new MR approval rules are created in the backend). The permission to modify approval rules are controlled by the
Allow overrides to approval lists per merge request (MR)
setting. - User should be able to see which rules they have overridden
- User can reset to the default project approval rules. These rules should be the same as how it is displayed on the project settings page, ie. including inherited group approval rules.
- User can see suggested approvers. We need to make sure it is accounted for approvers defined in group approval rules.
Implementation
- Extend
ApprovalState
to support group (ie.user_defined_rules
should account for group approval rules)
graph LR
API::MergeRequestApprovalRules --> MergeRequest --> ApprovalState
ApprovalState --> ApprovalMergeRequestRule
ApprovalState --> ApprovalProjectRule
ApprovalState --> ApprovalGroupRule
Code references
Approve a merge request
User can only approve merge request if they are on the list of approvers according to the approval rules. We should account for group approval rules in this MergeRquests::ApprovalService
as well.
Implementation
- Extend
ApprovalState
to support group (ie.user_defined_rules
should account for group approval rules)
graph LR
API::MergeRequestApprovals --> MergeRequests::ApprovalService
MergeRequests::ApprovalService --> MergeRequest
MergeRequest --> ApprovalState
ApprovalState --> ApprovalMergeRequestRule
ApprovalState --> ApprovalProjectRule
ApprovalState --> ApprovalGroupRule
Code references
Distribution of project approval rules (100k sample)
gitlabhq_production=> select per_project_count, count(*) from (select project_id, count(*) as per_project_count from approval_project_rules group by project_id limit 100000) AS count group by per_project_count order by per_project_count;
per_project_count | count
-------------------+-------
1 | 91479
2 | 7426
3 | 857
4 | 135
5 | 55
6 | 6
7 | 10
8 | 15
9 | 14
10 | 2
11 | 1
(11 rows)
Risks and Implementation Considerations
- Integrating group-level approval rules should not worsen performance on critical flows (ie. creating and updating merge requests)
- How to effectively deal with a deeply nested group (
max ~ 20
). Should we just use the root namespace? - Efficiently search across descendant groups and users when creating rules
- Efficiently search across descendant branches when creating rules
- How to handle rule conflicts (e.g. rules that contain approvers span across subgroups)
Related issues
- Rearchitecting approval rules (#296786)