Integrate group-level with project-level MR approval settings
This issue and linked pages contain information related to upcoming products, features, and functionality. It is important to note that the information presented is for informational purposes only. Please do not rely on this information for purchasing or planning purposes. As with all projects, the items mentioned in this video and linked pages are subject to change or delay. The development, release, and timing of any products, features, or functionality remain at the sole discretion of GitLab Inc.
Problem to solve
At the moment, group-level MR approval settings do not have any effect on the project-level counterparts.
Proposal
- When top-level group settings restrict permission, the corresponding settings on the project level are enforced and locked.
- When top-level group settings do not restrict permission, the corresponding settings on the project level are unlocked and revert to project default or previous value.
- All new project MR approval settings inherit from the corresponding top-level group ones
- This change must be backward compatible.
Group Value | Project Value |
---|---|
Restrict |
Disabled (and locked ![]() (Group level is SSOT) |
Do not restrict | Revert to project default or previous value (Project level is SSOT) |
Implementation
Current group and project setting map:
Project Setting | Group Setting |
---|---|
disable_overriding_approvers_per_merge_request | allow_overrides_to_approver_list_per_merge_request |
merge_requests_author_approval | allow_author_approval |
merge_requests_disable_committers_approval | allow_committer_approval |
reset_approvals_on_push | retain_approvals_on_push |
require_password_to_approve | require_password_to_approve |
Option 1
- Rejig MR approval settings on
project
model and take into account the top-level group MR approval settings. Note: watch out for flipping logic 🤯.
class Project
def disable_overriding_approvers_per_merge_request
strong_memoize(:disable_overriding_approvers_per_merge_request) do
next super unless @subject.feature_available?(:group_merge_request_approval_settings)
next false unless group.merge_request_approval_setting.allow_overrides_to_approver_list_per_merge_request?
super
end
end
end
- Ensure the projet settings value are locked, both in UI and API (ie. using
ProjectPolicy
)
# EE::ProjectPolicy
with_scope :global
condition(:locked_approvers_rules) do
@subject.feature_available?(:group_merge_request_approval_settings) &&
!group.merge_request_approval_setting.allow_overrides_to_approver_list_per_merge_request?
end
Option 2
- Create a class that encapsulates the compliance rules (ie.
ComplianceManagement::MergeRequestApprovalSettings
)
# Resolve settings using compliance rules
class ComplianceManagement::MergeRequestApprovalSettings::Resolver
def initialize(project)
@project = project
end
def execute
[
allow_overrides_to_approver_list_per_merge_request,
allow_author_approval,
allow_committer_approval
]
end
private
def allow_overrides_to_approver_list_per_merge_request
instance_value = ::Gitlab::CurrentSettings.disable_overriding_approvers_per_merge_request?
group_value = project.group.merge_request_approval_setting.allow_overrides_to_approver_list_per_merge_request?
project_value = project.disable_overriding_approvers_per_merge_request?
ComplianceManagement::MergeRequestApprovalSettings::Setting.new(
name: :allow_overrides_to_approver_list_per_merge_request,
value: !group_value ? false : project_value,
locked: !group_value,
inherited_from: !group_value ? :group : nil
)
end
end
# Value object
class ComplianceManagement::MergeRequestApprovalSettings::Setting
attr_reader :name, :value, :locked, :inherited_from
def initialize(name:, value:, locked:, inherited_from:)
@name = name
@value = value
@locked = locked
@inherited_from = inherited_from
end
end
# Usage in API
class API::ProjectApprovals
get do
setting = ComplianceManagement::MergeRequestApprovalSetting::Resolver.new(project).execute
end
end