Approval policy <-> protected branch matching behavior
Why are we doing this work
Merge request approval policies contain a design inconsistency that can lead to unexpected outcomes. An approval policy's branches property accepts not only literal strings, but these strings are interpreted as being wildcarded patterns, e.g. foo*. An approval policy's branches/branch_type properties serve two purposes:
- determining which merge requests should be affected by the policy
- determining which protected branches should be affected by the policy's
approval_settings, since some of these settings (block_group_branch_modification,prevent_pushing_and_force_pushing) should take effect depending on the policy'sbranches/branch_type.
The inconsistency lies in an approval policy's branches having to treat Git branches and protected branches equally. But what sets both apart is that a protected branch's name is not a literal, unlike a Git branch, because protected branches can be wild-carded, too.
This poses the unsolvable problem of matching two patterns against each other. For example, your policy may specify branches: [foo*] and there's a wildcarded protected branch *bar. Should the policy affect the protected branch? This is the logical inconsistency we have to work around.
Current implementation
In order to answer the question whether the policy branches: [foo*] matches the protected branch *bar, we currently employ the following two step algorithm:
- Identify all Git branches matched by the policy's
branches: [foo*] - Identify all protected branches that match at least one of the Git branches matched in (1)
However this algorithm can result in unintuitive outcomes. Consider the following scenario where the policy author had no intention of pattern matching:
- Policy:
branches: ['feature/auth'] - Git branches:
feature/auth,feature/payments - Protected branches:
feature/*
Walking through the two steps:
- Git branches matched by
branches: ['feature/auth']:feature/auth - Protected branches matching
feature/auth:feature/*
If the policy sets one of the approval_settings related to protected branches, the policy blocks mutation of the protected branch feature/*, which likely contradicts the policy author's intention.
Proposals
- Add a policy property to change this behaviour, so that protected branch names are matched as literals.
- Or add a property that disables wildcard matching for
branches
References
See for example:
- https://gitlab.com/gitlab-org/gitlab/-/blob/fcd506acd15de2b01fec5c71d4eda9dc49824262/ee/app/services/security/security_orchestration_policies/protected_branches_deletion_check_service.rb
- https://gitlab.com/gitlab-org/gitlab/-/blob/fcd506acd15de2b01fec5c71d4eda9dc49824262/ee/app/services/security/security_orchestration_policies/policy_branches_service.rb