Merge branch '10395-require-code-owner-approval-on-pushes' into 'master'

Add "Require code owner approval" setting to protected branches

See merge request gitlab-org/gitlab-ee!14900

app/models/merge_request.rb

@@ -176,6 +176,7 @@ class MergeRequest < ApplicationRecord
   scope :from_project, ->(project) { where(source_project_id: project.id) }
   scope :merged, -> { with_state(:merged) }
   scope :closed_and_merged, -> { with_states(:closed, :merged) }
+  scope :open_and_closed, -> { with_states(:opened, :closed) }
   scope :from_source_branches, ->(branches) { where(source_branch: branches) }
   scope :by_commit_sha, ->(sha) do
     where('EXISTS (?)', MergeRequestDiff.select(1).where('merge_requests.latest_merge_request_diff_id = merge_request_diffs.id').by_commit_sha(sha)).reorder(nil)
@@ -189,6 +190,11 @@ class MergeRequest < ApplicationRecord
             target_project: [:route, { namespace: :route }],
             source_project: [:route, { namespace: :route }])
   }
+  scope :by_target_branch_wildcard, ->(wildcard_branch_name) do
+    where("target_branch LIKE ?", ApplicationRecord.sanitize_sql_like(wildcard_branch_name).tr('*', '%'))
+  end
+  scope :by_target_branch, ->(branch_name) { where(target_branch: branch_name) }
+  scope :preload_source_project, -> { preload(:source_project) }
 
   after_save :keep_around_commit
 

app/models/protected_branch.rb

@@ -3,6 +3,9 @@
 class ProtectedBranch < ApplicationRecord
   include ProtectedRef
 
+  scope :requiring_code_owner_approval,
+        -> { where(code_owner_approval_required: true) }
+
   protected_ref_access_levels :merge, :push
 
   def self.protected_ref_accessible_to?(ref, user, project:, action:, protected_refs: nil)

app/services/protected_branches/create_service.rb

@@ -5,7 +5,8 @@ module ProtectedBranches
     def execute(skip_authorization: false)
       raise Gitlab::Access::AccessDeniedError unless skip_authorization || authorized?
 
-      protected_branch.save
+      save_protected_branch
+
       protected_branch
     end
 
@@ -15,8 +16,14 @@ module ProtectedBranches
 
     private
 
+    def save_protected_branch
+      protected_branch.save
+    end
+
     def protected_branch
       @protected_branch ||= project.protected_branches.new(params)
     end
   end
 end
+
+ProtectedBranches::CreateService.prepend_if_ee('EE::ProtectedBranches::CreateService')

db/migrate/20190729180447_add_merge_requests_require_code_owner_approval_to_protected_branches.rb

+# frozen_string_literal: true
+
+class AddMergeRequestsRequireCodeOwnerApprovalToProtectedBranches < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(
+      :protected_branches,
+      :code_owner_approval_required,
+      :boolean,
+      default: false
+    )
+
+    add_concurrent_index(
+      :protected_branches,
+      [:project_id, :code_owner_approval_required],
+      name: "code_owner_approval_required",
+      where: "code_owner_approval_required = #{Gitlab::Database.true_value}")
+  end
+
+  def down
+    remove_concurrent_index(:protected_branches, name: "code_owner_approval_required")
+
+    remove_column(:protected_branches, :code_owner_approval_required)
+  end
+end

db/schema.rb

@@ -2932,6 +2932,8 @@ ActiveRecord::Schema.define(version: 2019_09_04_173203) do
     t.string "name", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.boolean "code_owner_approval_required", default: false, null: false
+    t.index ["project_id", "code_owner_approval_required"], name: "code_owner_approval_required", where: "(code_owner_approval_required = true)"
     t.index ["project_id"], name: "index_protected_branches_on_project_id"
   end
 

ee/app/models/approval_wrapped_rule.rb

@@ -92,9 +92,16 @@ class ApprovalWrappedRule
 
   def code_owner_approvals_required
     strong_memoize(:code_owner_approvals_required) do
-      next 0 unless project.merge_requests_require_code_owner_approval?
+      next 0 unless merge_requests_require_code_owner_approval?
 
       approvers.any? ? REQUIRED_APPROVALS_PER_CODE_OWNER_RULE : 0
     end
   end
+
+  def merge_requests_require_code_owner_approval?
+    return false unless project.code_owner_approval_required_available?
+
+    project.merge_requests_require_code_owner_approval? ||
+      project.branch_requires_code_owner_approval?(merge_request.target_branch)
+  end
 end

ee/app/models/concerns/ee/protected_branch.rb

@@ -8,6 +8,11 @@ module EE
       protected_ref_access_levels :unprotect
     end
 
+    def code_owner_approval_required
+      super && project.code_owner_approval_required_available?
+    end
+    alias_method :code_owner_approval_required?, :code_owner_approval_required
+
     def can_unprotect?(user)
       return true if unprotect_access_levels.empty?
 

ee/app/models/ee/project.rb

@@ -379,6 +379,12 @@ module EE
       super && code_owner_approval_required_available?
     end
 
+    def branch_requires_code_owner_approval?(branch_name)
+      return false unless code_owner_approval_required_available?
+
+      protected_branches.requiring_code_owner_approval.matching(branch_name).any?
+    end
+
     def require_password_to_approve
       super && password_authentication_enabled_for_web?
     end

ee/app/services/ee/protected_branches/api_service.rb

@@ -9,6 +9,15 @@ module EE
       def protected_branch_params
         super.tap do |hash|
           hash[:unprotect_access_levels_attributes] = ::ProtectedBranches::AccessLevelParams.new(:unprotect, params).access_levels
+          hash[:code_owner_approval_required] = code_owner_approval_required?
+        end
+      end
+
+      def code_owner_approval_required?
+        if project.code_owner_approval_required_available?
+          params[:code_owner_approval_required] || false
+        else
+          false
         end
       end
     end

ee/app/services/ee/protected_branches/create_service.rb

+# frozen_string_literal: true
+
+module EE
+  module ProtectedBranches
+    module CreateService
+      extend ::Gitlab::Utils::Override
+      include ::Gitlab::Utils::StrongMemoize
+
+      private
+
+      override :save_protected_branch
+      def save_protected_branch
+        super
+
+        sync_code_owner_approval_rules if project.feature_available?(:code_owners)
+
+        protected_branch
+      end
+
+      def sync_code_owner_approval_rules
+        merge_requests_for_protected_branch.each do |merge_request|
+          ::MergeRequests::SyncCodeOwnerApprovalRules.new(merge_request).execute
+        end
+      end
+
+      def merge_requests_for_protected_branch
+        strong_memoize(:protected_branch_merge_requests) do
+          if protected_branch.wildcard?
+            merge_requests_for_wildcard_branch
+          else
+            merge_requests_for_branch
+          end
+        end
+      end
+
+      def merge_requests_for_wildcard_branch
+        project.merge_requests
+          .open_and_closed
+          .by_target_branch_wildcard(protected_branch.name)
+          .preload_source_project
+          .select(&:source_project)
+      end
+
+      def merge_requests_for_branch
+        project.merge_requests
+          .open_and_closed
+          .by_target_branch(protected_branch.name)
+          .preload_source_project
+          .select(&:source_project)
+      end
+    end
+  end
+end

ee/changelogs/unreleased/10395-require-code-owner-approval-on-pushes.yml

+---
+title: Add ability to block API pushes to protected branches when contents match CODEOWNERS rule
+merge_request: 14900
+author:
+type: changed

ee/lib/ee/api/entities.rb

@@ -111,6 +111,7 @@ module EE
 
         prepended do
           expose :unprotect_access_levels, using: ::API::Entities::ProtectedRefAccess
+          expose :code_owner_approval_required
         end
       end
 

ee/lib/ee/api/helpers/protected_branches_helpers.rb

@@ -29,6 +29,8 @@ module EE
               optional :user_id, type: Integer
               optional :group_id, type: Integer
             end
+
+            optional :code_owner_approval_required, type: Grape::API::Boolean, default: false, desc: 'Prevent pushes to this branch if it matches an item in CODEOWNERS'
           end
         end
       end

ee/lib/ee/gitlab/checks/diff_check.rb

@@ -9,6 +9,35 @@ module EE
 
         private
 
+        def path_validations
+          validations = [super].flatten
+
+          if !updated_from_web? && project.branch_requires_code_owner_approval?(branch_name)
+            validations << validate_code_owners
+          end
+
+          validations
+        end
+
+        def validate_code_owners
+          lambda do |paths|
+            loader = ::Gitlab::CodeOwners::Loader.new(project, branch_name, paths)
+
+            assemble_error_msg_for_codeowner_matches(loader) if loader.entries.any?
+          end
+        end
+
+        def assemble_error_msg_for_codeowner_matches(loader)
+          matched_rules = loader.entries.collect { |e| "- #{e.pattern}" }
+          code_owner_path = project.repository.code_owners_blob(ref: branch_name).path || "CODEOWNERS"
+
+          "Pushes to protected branches that contain changes to files that\n" \
+            "match patterns defined in `#{code_owner_path}` are disabled for\n" \
+            "this project. Please submit these changes via a merge request.\n\n" \
+            "The following pattern(s) from `#{code_owner_path}` were matched:\n" \
+            "#{matched_rules.join('\n')}\n"
+        end
+
         def validate_path_locks?
           strong_memoize(:validate_path_locks) do
             project.feature_available?(:file_locks) &&

ee/lib/gitlab/code_owners/file.rb

@@ -25,6 +25,10 @@ module Gitlab
         parsed_data[matching_pattern].dup if matching_pattern
       end
 
+      def path
+        @blob&.path
+      end
+
       private
 
       def data

ee/lib/gitlab/code_owners/loader.rb

@@ -37,6 +37,10 @@ module Gitlab
         code_owners_file.empty?
       end
 
+      def code_owners_path
+        code_owners_file&.path
+      end
+
       private
 
       def load_bare_entries_for_paths

ee/spec/lib/gitlab/checks/diff_check_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::Checks::DiffCheck do
+  include FakeBlobHelpers
+
   include_context 'push rules checks context'
 
   describe '#validate!' do
@@ -16,6 +18,88 @@ describe Gitlab::Checks::DiffCheck do
       end
     end
 
+    describe "#validate_code_owners" do
+      let!(:code_owner) { create(:user, username: "owner-1") }
+      let(:project) { create(:project, :repository) }
+      let(:codeowner_content) { "*.rb @#{code_owner.username}\ndocs/CODEOWNERS @owner-1" }
+      let(:codeowner_blob) { fake_blob(path: "CODEOWNERS", data: codeowner_content) }
+      let(:codeowner_blob_ref) { fake_blob(path: "CODEOWNERS", data: codeowner_content) }
+      let(:codeowner_lookup_ref) { merge_request.target_branch }
+      let(:merge_request) do
+        build(
+          :merge_request,
+          source_project: project,
+          source_branch: 'feature',
+          target_project: project,
+          target_branch: 'master'
+        )
+      end
+
+      before do
+        project.add_developer(code_owner)
+        allow(project.repository).to receive(:code_owners_blob)
+          .with(ref: codeowner_lookup_ref)
+          .and_return(codeowner_blob)
+      end
+
+      context "the MR contains a matching file path" do
+        it "return an error message" do
+          expect(subject.send(:validate_code_owners)
+            .call(["docs/CODEOWNERS", "README"])).not_to be_nil
+        end
+      end
+
+      context "the MR doesn't contain a matching file path" do
+        it "doesn't raise an exception" do
+          expect(subject.send(:validate_code_owners)
+            .call(["docs/SAFE_FILE_NAME", "README"])).to be_nil
+        end
+      end
+    end
+
+    describe "#path_validations" do
+      include_context 'change access checks context'
+
+      context "when the feature isn't enabled on the project" do
+        before do
+          expect(project).to receive(:branch_requires_code_owner_approval?)
+            .once.and_return(false)
+        end
+
+        it "returns an empty array" do
+          expect(subject.send(:path_validations)).to eq([])
+        end
+      end
+
+      context "when the feature is enabled on the project" do
+        context "updated_from_web? == false" do
+          before do
+            expect(subject).to receive(:updated_from_web?).and_return(false)
+            expect(project).to receive(:branch_requires_code_owner_approval?)
+              .once.and_return(true)
+          end
+
+          it "returns an array of Proc(s)" do
+            validations = subject.send(:path_validations)
+
+            expect(validations.any?).to be_truthy
+            expect(validations.any? { |v| !v.is_a? Proc }).to be_falsy
+          end
+        end
+
+        context "updated_from_web? == true" do
+          before do
+            expect(subject).to receive(:updated_from_web?).and_return(true)
+            expect(project).not_to receive(:branch_requires_code_owner_approval?)
+          end
+
+          it "returns an empty array" do
+            expect(subject.send(:path_validations)).to eq([])
+          end
+        end
+      end
+    end
+
     context 'file name rules' do
       # Notice that the commit used creates a file named 'README'
       context 'file name regex check' do

ee/spec/lib/gitlab/code_owners/file_spec.rb

@@ -58,6 +58,22 @@ describe Gitlab::CodeOwners::File do
     end
   end
 
+  describe "#path" do
+    context "when the blob exists" do
+      it "returns the path to the file" do
+        expect(subject.path).to eq(blob.path)
+      end
+    end
+
+    context "when the blob is nil" do
+      let(:blob) { nil }
+
+      it "returns nil" do
+        expect(subject.path).to be_nil
+      end
+    end
+  end
+
   describe '#entry_for_path' do
     context 'for a path without matches' do
       let(:file_content) do

ee/spec/lib/gitlab/code_owners/loader_spec.rb

@@ -126,6 +126,22 @@ describe Gitlab::CodeOwners::Loader do
     end
   end
 
+  describe "#code_owners_path" do
+    context "when the file exists" do
+      it "returns the path to the code_owners file" do
+        expect(loader.code_owners_path).to eq("CODEOWNERS")
+      end
+    end
+
+    context "when the file does not exist" do
+      let(:codeowner_blob) { nil }
+
+      it "returns nil" do
+        expect(loader.code_owners_path).to be_nil
+      end
+    end
+  end
+
   describe '#empty_code_owners?' do
     context 'when file does not exist' do
       let(:codeowner_blob) { nil }

ee/spec/lib/gitlab/git_access_spec.rb

@@ -358,16 +358,19 @@ describe Gitlab::GitAccess do
       merge_into_protected_branch
     end
 
+    let(:start_sha) { '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' }
+    let(:end_sha)   { '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' }
+
     let(:changes) do
       { any: Gitlab::GitAccess::ANY,
-        push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow",
-        push_master: '6f6d7e7ed 570e7b2ab refs/heads/master',
-        push_protected_branch: '6f6d7e7ed 570e7b2ab refs/heads/feature',
-        push_remove_protected_branch: "570e7b2ab #{Gitlab::Git::BLANK_SHA} "\
-                                      'refs/heads/feature',
-        push_tag: '6f6d7e7ed 570e7b2ab refs/tags/v1.0.0',
-        push_new_tag: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/tags/v7.8.9",
-        push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'],
+        push_new_branch: "#{Gitlab::Git::BLANK_SHA} #{end_sha} refs/heads/wow",
+        push_master: "#{start_sha} #{end_sha} refs/heads/master",
+        push_protected_branch: "#{start_sha} #{end_sha} refs/heads/feature",
+        push_remove_protected_branch: "#{end_sha} #{Gitlab::Git::BLANK_SHA} "\
+                                      "refs/heads/feature",
+        push_tag: "#{start_sha} #{end_sha} refs/tags/v1.0.0",
+        push_new_tag: "#{Gitlab::Git::BLANK_SHA} #{end_sha} refs/tags/v7.8.9",
+        push_all: ["#{start_sha} #{end_sha} refs/heads/master", "#{start_sha} #{end_sha} refs/heads/feature"],
         merge_into_protected_branch: "0b4bc9a #{merge_into_protected_branch} refs/heads/feature" }
     end
 

ee/spec/models/approval_wrapped_rule_spec.rb

@@ -188,10 +188,44 @@ describe ApprovalWrappedRule do
                  users: create_list(:user, approver_count))
         end
 
-        it 'returns the correct number of approvals' do
-          allow(subject.project).to receive(:merge_requests_require_code_owner_approval?).and_return(feature_enabled)
+        let(:branch) { subject.project.repository.branches.find { |b| b.name == merge_request.target_branch } }
+
+        context "when project.code_owner_approval_required_available? is true" do
+          before do
+            allow(subject.project)
+              .to receive(:code_owner_approval_required_available?).and_return(true)
+          end
+
+          context "when no protected_branches require code owner approval" do
+            it 'returns the correct number of approvals' do
+              allow(subject.project)
+                .to receive(:merge_requests_require_code_owner_approval?).and_return(feature_enabled)
+              allow(subject.project)
+                .to receive(:branch_requires_code_owner_approval?).with(branch.name).and_return(false)
+
+              expect(subject.approvals_required).to eq(expected_required_approvals)
+            end
+          end
+
+          context "when the project doesn't require code owner approval on all MRs" do
+            it 'returns the expected number of approvals for protected_branches that do require approval' do
+              allow(subject.project)
+                .to receive(:merge_requests_require_code_owner_approval?).and_return(false)
+              allow(subject.project)
+                .to receive(:branch_requires_code_owner_approval?).with(branch.name).and_return(feature_enabled)
+
+              expect(subject.approvals_required).to eq(expected_required_approvals)
+            end
+          end
+        end
+
+        context "when project.code_owner_approval_required_available? is falsy" do
+          it "returns nil" do
+            allow(subject.project)
+              .to receive(:code_owner_approval_required_available?).and_return(false)
 
-          expect(subject.approvals_required).to eq(expected_required_approvals)
+            expect(subject.approvals_required).to eq(0)
+          end
         end
       end
     end

ee/spec/models/ee/protected_branch_spec.rb

@@ -118,6 +118,32 @@ describe ProtectedBranch do
   it_behaves_like 'uniqueness validation', ProtectedBranch::MergeAccessLevel
   it_behaves_like 'uniqueness validation', ProtectedBranch::PushAccessLevel
 
+  describe "#code_owner_approval_required" do
+    context "when the attr code_owner_approval_required is true" do
+      let(:subject_branch) { create(:protected_branch, code_owner_approval_required: true) }
+
+      it "returns true" do
+        expect(subject_branch.project)
+          .to receive(:code_owner_approval_required_available?).once.and_return(true)
+        expect(subject_branch.code_owner_approval_required).to be_truthy
+      end
+
+      it "returns false when the project doesn't require approvals" do
+        expect(subject_branch.project)
+          .to receive(:code_owner_approval_required_available?).once.and_return(false)
+        expect(subject_branch.code_owner_approval_required).to be_falsy
+      end
+    end
+
+    context "when the attr code_owner_approval_required is false" do
+      let(:subject_branch) { create(:protected_branch, code_owner_approval_required: false) }
+
+      it "returns false" do
+        expect(subject_branch.code_owner_approval_required).to be_falsy
+      end
+    end
+  end
+
   describe '#can_unprotect?' do
     let(:admin) { create(:user, :admin) }
     let(:maintainer) do

ee/spec/models/project_spec.rb

@@ -921,6 +921,47 @@ describe Project do
     end
   end
 
+  describe '#branch_requires_code_owner_approval?' do
+    let(:protected_branch) { create(:protected_branch, code_owner_approval_required: false) }
+    let(:protected_branch_needing_approval) { create(:protected_branch, code_owner_approval_required: true) }
+
+    context "when feature is enabled" do
+      before do
+        stub_licensed_features(code_owner_approval_required: true)
+      end
+
+      it 'returns true when code owner approval is required' do
+        project = protected_branch_needing_approval.project
+
+        expect(project.branch_requires_code_owner_approval?(protected_branch_needing_approval.name)).to eq(true)
+      end
+
+      it 'returns false when code owner approval is not required' do
+        project = protected_branch.project
+
+        expect(project.branch_requires_code_owner_approval?(protected_branch.name)).to eq(false)
+      end
+    end
+
+    context "when feature is not enabled" do
+      before do
+        stub_licensed_features(code_owner_approval_required: false)
+      end
+
+      it 'returns true when code owner approval is required' do
+        project = protected_branch_needing_approval.project
+
+        expect(project.branch_requires_code_owner_approval?(protected_branch_needing_approval.name)).to eq(false)
+      end
+
+      it 'returns false when code owner approval is not required' do
+        project = protected_branch.project
+
+        expect(project.branch_requires_code_owner_approval?(protected_branch.name)).to eq(false)
+      end
+    end
+  end
+
   shared_examples 'project with disabled services' do
     it 'has some disabled services' do
       stub_const('License::ANY_PLAN_FEATURES', [])

ee/spec/requests/api/protected_branches_spec.rb

@@ -113,6 +113,55 @@ describe API::ProtectedBranches do
         expect(json_response['unprotect_access_levels'][0]['access_level']).to eq(Gitlab::Access::ADMIN)
       end
 
+      context "code_owner_approval_required" do
+        context "when feature is enabled" do
+          before do
+            stub_licensed_features(code_owner_approval_required: true)
+          end
+
+          it "sets :code_owner_approval_required to true when the param is true" do
+            expect(project.protected_branches.find_by_name(branch_name)).to be_nil
+
+            post post_endpoint, params: { name: branch_name, code_owner_approval_required: true }
+
+            expect(response).to have_gitlab_http_status(201)
+            expect(json_response["code_owner_approval_required"]).to eq(true)
+
+            new_branch = project.protected_branches.find_by_name(branch_name)
+            expect(new_branch.code_owner_approval_required).to be_truthy
+            expect(new_branch[:code_owner_approval_required]).to be_truthy
+          end
+
+          it "sets :code_owner_approval_required to false when the param is false" do
+            expect(project.protected_branches.find_by_name(branch_name)).to be_nil
+
+            post post_endpoint, params: { name: branch_name, code_owner_approval_required: false }
+
+            expect(response).to have_gitlab_http_status(201)
+            expect(json_response["code_owner_approval_required"]).to eq(false)
+
+            new_branch = project.protected_branches.find_by_name(branch_name)
+            expect(new_branch.code_owner_approval_required).to be_falsy
+            expect(new_branch[:code_owner_approval_required]).to be_falsy
+          end
+        end
+
+        context "when feature is not enabled" do
+          it "sets :code_owner_approval_required to false when the param is false" do
+            expect(project.protected_branches.find_by_name(branch_name)).to be_nil
+
+            post post_endpoint, params: { name: branch_name, code_owner_approval_required: true }
+
+            expect(response).to have_gitlab_http_status(201)
+            expect(json_response["code_owner_approval_required"]).to eq(false)
+
+            new_branch = project.protected_branches.find_by_name(branch_name)
+            expect(new_branch.code_owner_approval_required).to be_falsy
+            expect(new_branch[:code_owner_approval_required]).to be_falsy
+          end
+        end
+      end
+
       context 'with granular access' do
         let(:invited_group) do
           create(:project_group_link, project: project).group

ee/spec/services/ee/protected_branches/create_service_spec.rb

+# frozen_string_literal: true
+
+require "spec_helper"
+
+describe ProtectedBranches::CreateService do
+  include ProjectForksHelper
+
+  let(:source_project) { create(:project) }
+  let(:target_project) { fork_project(source_project, user, repository: true) }
+
+  let(:user) { source_project.owner }
+
+  let(:params) do
+    {
+      name: "feature",
+      merge_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }],
+      push_access_levels_attributes: [{ access_level: Gitlab::Access::MAINTAINER }]
+    }
+  end
+
+  describe "#execute" do
+    subject(:service) { described_class.new(target_project, user, params) }
+
+    before do
+      target_project.add_user(user, :developer)
+    end
+
+    context "when there are open merge requests" do
+      let!(:merge_request) do
+        create(:merge_request,
+          source_project: source_project,
+          target_project: target_project,
+          discussion_locked: false
+        )
+      end
+
+      it "calls MergeRequest::SyncCodeOwnerApprovalRules to update open MRs" do
+        expect(::MergeRequests::SyncCodeOwnerApprovalRules).to receive(:new).with(merge_request).and_call_original
+        expect { service.execute }.to change(ProtectedBranch, :count).by(1)
+      end
+
+      context "when the branch is a wildcard" do
+        %w(*ture *eatur* feat*).each do |wildcard|
+          before do
+            params[:name] = wildcard
+          end
+
+          it "calls MergeRequest::SyncCodeOwnerApprovalRules to update open MRs for #{wildcard}" do
+            expect(::MergeRequests::SyncCodeOwnerApprovalRules).to receive(:new).with(merge_request).and_call_original
+            expect { service.execute }.to change(ProtectedBranch, :count).by(1)
+          end
+        end
+      end
+    end
+  end
+end

lib/api/protected_branches.rb

@@ -59,6 +59,7 @@ module API
       # rubocop: disable CodeReuse/ActiveRecord
       post ':id/protected_branches' do
         protected_branch = user_project.protected_branches.find_by(name: params[:name])
+
         if protected_branch
           conflict!("Protected branch '#{params[:name]}' already exists")
         end

spec/lib/gitlab/checks/diff_check_spec.rb

@@ -20,9 +20,8 @@ describe Gitlab::Checks::DiffCheck do
         allow(project).to receive(:lfs_enabled?).and_return(false)
       end
 
-      it 'skips the validation' do
-        expect(subject).not_to receive(:validate_diff)
-        expect(subject).not_to receive(:validate_file_paths)
+      it 'does not invoke :lfs_file_locks_validation' do
+        expect(subject).not_to receive(:lfs_file_locks_validation)
 
         subject.validate!
       end

spec/lib/gitlab/git_access_spec.rb

@@ -943,7 +943,7 @@ describe Gitlab::GitAccess do
         changes = ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature']
 
         # There is still an N+1 query with protected branches
-        expect { access.check('git-receive-pack', changes) }.not_to exceed_query_limit(control_count).with_threshold(1)
+        expect { access.check('git-receive-pack', changes) }.not_to exceed_query_limit(control_count).with_threshold(2)
       end
 
       it 'raises TimeoutError when #check_single_change_access raises a timeout error' do

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -467,6 +467,7 @@ ProtectedBranch:
 - name
 - created_at
 - updated_at
+- code_owner_approval_required
 ProtectedTag:
 - id
 - project_id