From c89a6b4548e42ed446099c992c5359bfe34d1de2 Mon Sep 17 00:00:00 2001 From: Marc Shaw <mshaw@gitlab.com> Date: Thu, 15 Oct 2020 17:19:40 +0200 Subject: [PATCH] Remove push_rules_supersede_code_owners feature flag Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/262019 --- .../development/push_rules_supersede_code_owners.yml | 7 ------- doc/user/project/code_owners.md | 5 +++-- doc/user/project/protected_branches.md | 2 ++ ...push_rules_supersede_code_owners_feature_flag.yml | 5 +++++ ee/lib/ee/gitlab/checks/diff_check.rb | 3 +-- ee/spec/lib/gitlab/checks/diff_check_spec.rb | 12 ------------ 6 files changed, 11 insertions(+), 23 deletions(-) delete mode 100644 config/feature_flags/development/push_rules_supersede_code_owners.yml create mode 100644 ee/changelogs/unreleased/remove_push_rules_supersede_code_owners_feature_flag.yml diff --git a/config/feature_flags/development/push_rules_supersede_code_owners.yml b/config/feature_flags/development/push_rules_supersede_code_owners.yml deleted file mode 100644 index d185d19522d81..0000000000000 --- a/config/feature_flags/development/push_rules_supersede_code_owners.yml +++ /dev/null @@ -1,7 +0,0 @@ ---- -name: push_rules_supersede_code_owners -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44126 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/262019 -type: development -group: group::source code -default_enabled: false diff --git a/doc/user/project/code_owners.md b/doc/user/project/code_owners.md index 7c71d18e5be64..4ae3d5ec03232 100644 --- a/doc/user/project/code_owners.md +++ b/doc/user/project/code_owners.md @@ -75,7 +75,6 @@ be used for merge request approvals: - As [merge request eligible approvers](merge_requests/merge_request_approvals.md#code-owners-as-eligible-approvers). - As required approvers for [protected branches](protected_branches.md#protected-branches-approval-by-code-owners). **(PREMIUM)** -NOTE: **Note:** Developer or higher [permissions](../permissions.md) are required in order to approve a merge request. @@ -93,12 +92,14 @@ to specify the actual owners and granular permissions. Using Code Owners in conjunction with [Protected Branches](protected_branches.md#protected-branches-approval-by-code-owners) will prevent any user who is not specified in the `CODEOWNERS` file from pushing -changes for the specified files/paths, even if their role is included in the +changes for the specified files/paths, except those included in the **Allowed to push** column. This allows for a more inclusive push strategy, as administrators don't have to restrict developers from pushing directly to the protected branch, but can restrict pushing to certain files where a review by Code Owners is required. +[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/35097) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.5, users and groups who are allowed to push to protected branches do not require a merge request to merge their feature branches. Thus, they can skip merge request approval rules, Code Owners included. + ## The syntax of Code Owners files Files can be specified using the same kind of patterns you would use diff --git a/doc/user/project/protected_branches.md b/doc/user/project/protected_branches.md index 09757bc73ebdb..7265fd330e33d 100644 --- a/doc/user/project/protected_branches.md +++ b/doc/user/project/protected_branches.md @@ -185,6 +185,8 @@ When enabled, all merge requests targeting these branches will require approval by a Code Owner per matched rule before they can be merged. Additionally, direct pushes to the protected branch are denied if a rule is matched. +[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/35097) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.5, users and groups who are allowed to push to protected branches do not require a merge request to merge their feature branches. Thus, they can skip merge request approval rules. + ## Running pipelines on protected branches The permission to merge or push to protected branches is used to define if a user can diff --git a/ee/changelogs/unreleased/remove_push_rules_supersede_code_owners_feature_flag.yml b/ee/changelogs/unreleased/remove_push_rules_supersede_code_owners_feature_flag.yml new file mode 100644 index 0000000000000..d6f3b8e96174c --- /dev/null +++ b/ee/changelogs/unreleased/remove_push_rules_supersede_code_owners_feature_flag.yml @@ -0,0 +1,5 @@ +--- +title: Allow 'allowed_to_push' to supersede code owner protected branch +merge_request: 45323 +author: +type: changed diff --git a/ee/lib/ee/gitlab/checks/diff_check.rb b/ee/lib/ee/gitlab/checks/diff_check.rb index 8ce8d2e373275..97abe32d05b44 100644 --- a/ee/lib/ee/gitlab/checks/diff_check.rb +++ b/ee/lib/ee/gitlab/checks/diff_check.rb @@ -20,8 +20,7 @@ def path_validations end def validate_code_owners? - return false if updated_from_web? - return false if ::Feature.enabled?(:push_rules_supersede_code_owners, project) && user_access.can_push_to_branch?(branch_name) + return false if updated_from_web? || user_access.can_push_to_branch?(branch_name) project.branch_requires_code_owner_approval?(branch_name) end diff --git a/ee/spec/lib/gitlab/checks/diff_check_spec.rb b/ee/spec/lib/gitlab/checks/diff_check_spec.rb index 2040b726055b9..486a714ad79d2 100644 --- a/ee/spec/lib/gitlab/checks/diff_check_spec.rb +++ b/ee/spec/lib/gitlab/checks/diff_check_spec.rb @@ -60,18 +60,6 @@ it 'returns false' do expect(validate_code_owners).to eq(false) end - - context 'when push_rules_supersede_code_owners is disabled' do - before do - stub_feature_flags(push_rules_supersede_code_owners: false) - end - - it 'returns branch_requires_code_owner_approval?' do - expect(project).to receive(:branch_requires_code_owner_approval?).and_return(true) - - expect(validate_code_owners).to eq(true) - end - end end end -- GitLab