Merge branch 'id-required-code-owners-file' into 'master'

Allow to mark codeowners section as optional

See merge request gitlab-org/gitlab!51002

config/feature_flags/development/optional_code_owners_sections.yml

+---
+name: optional_code_owners_sections
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51002
+rollout_issue_url:
+milestone: '13.8'
+type: development
+group: group::source code
+default_enabled: false

ee/app/models/approval_wrapped_code_owner_rule.rb

@@ -16,7 +16,14 @@ class ApprovalWrappedCodeOwnerRule < ApprovalWrappedRule
 
   def branch_requires_code_owner_approval?
     return false unless project.code_owner_approval_required_available?
+    return false if section_optional?
 
     ProtectedBranch.branch_requires_code_owner_approval?(project, merge_request.target_branch)
   end
+
+  def section_optional?
+    return false unless Feature.enabled?(:optional_code_owners_sections, project)
+
+    Gitlab::CodeOwners.optional_section?(project, merge_request.target_branch, section)
+  end
 end

ee/lib/gitlab/code_owners.rb

@@ -22,6 +22,16 @@ module Gitlab
       Loader.new(project, ref, []).code_owners_sections
     end
 
+    # @param project [Project]
+    # @param ref [String]
+    # @param section [String]
+    # Checks whether all entries are optional
+    def self.optional_section?(project, ref, section)
+      return false unless project.feature_available?(:code_owners)
+
+      Loader.new(project, ref, []).optional_section?(section)
+    end
+
     # @param merge_request [MergeRequest]
     # @param merge_request_diff [MergeRequestDiff]
     #   Find code owners entries at a particular MergeRequestDiff.

ee/lib/gitlab/code_owners/entry.rb

@@ -7,15 +7,15 @@ module Gitlab
 
       DEFAULT_SECTION = "codeowners"
 
-      Data = Struct.new(:pattern, :owner_line, :section)
+      Data = Struct.new(:pattern, :owner_line, :section, :optional)
 
       attr_reader :data
       protected :data
 
       delegate :pattern, :hash, :owner_line, :section, to: :data
 
-      def initialize(pattern, owner_line, section = DEFAULT_SECTION)
-        @data = Data.new(pattern, owner_line, section)
+      def initialize(pattern, owner_line, section = DEFAULT_SECTION, optional = false)
+        @data = Data.new(pattern, owner_line, section, optional)
       end
 
       def all_users
@@ -56,6 +56,10 @@ module Gitlab
         @users.merge(matching_users)
       end
 
+      def optional?
+        data.optional
+      end
+
       def ==(other)
         return false unless other.is_a?(self.class)
 

ee/lib/gitlab/code_owners/file.rb

@@ -5,7 +5,7 @@ module Gitlab
     class File
       include ::Gitlab::Utils::StrongMemoize
 
-      SECTION_HEADER_REGEX = /\[(.*?)\]/.freeze
+      SECTION_HEADER_REGEX = /(\^)?\[(.*?)\]/.freeze
 
       def initialize(blob)
         @blob = blob
@@ -35,6 +35,21 @@ module Gitlab
         parsed_data.keys
       end
 
+      # Check whether any of the entries is optional
+      # In cases of the conflicts:
+      #
+      # [Documentation]
+      # *.go @user
+      #
+      # ^[Documentation]
+      # *.rb @user
+      #
+      # The Documentation section is still required
+      def optional_section?(section)
+        entries = parsed_data[section]&.values
+        entries.present? && entries.all?(&:optional?)
+      end
+
       def entry_for_path(path)
         path = "/#{path}" unless path.start_with?('/')
 
@@ -62,6 +77,7 @@ module Gitlab
       def get_parsed_data
         parsed_sectional_data = {}
         canonical_section_name = ::Gitlab::CodeOwners::Entry::DEFAULT_SECTION
+        section_optional = false
 
         parsed_sectional_data[canonical_section_name] = {}
 
@@ -74,16 +90,17 @@ module Gitlab
           #   set up to hold the entries it contains, and proceed to the next
           #   line in the file.
           #
-          if line.match?(SECTION_HEADER_REGEX)
-            parsed_section_name = line[1...-1].strip
-            canonical_section_name = find_section_name(parsed_section_name, parsed_sectional_data)
+          _, optional, name = line.match(SECTION_HEADER_REGEX).to_a
+          if name
+            canonical_section_name = find_section_name(name, parsed_sectional_data)
+            section_optional = optional.present?
 
             parsed_sectional_data[canonical_section_name] ||= {}
 
             next
           end
 
-          extract_entry_and_populate_parsed_data(line, parsed_sectional_data, canonical_section_name)
+          extract_entry_and_populate_parsed_data(line, parsed_sectional_data, canonical_section_name, section_optional)
         end
 
         parsed_sectional_data
@@ -97,12 +114,12 @@ module Gitlab
         section_headers.find { |k| k.casecmp?(section) } || section
       end
 
-      def extract_entry_and_populate_parsed_data(line, parsed, section)
+      def extract_entry_and_populate_parsed_data(line, parsed, section, optional)
         pattern, _separator, owners = line.partition(/(?<!\\)\s+/)
 
         normalized_pattern = normalize_pattern(pattern)
 
-        parsed[section][normalized_pattern] = Entry.new(pattern, owners, section)
+        parsed[section][normalized_pattern] = Entry.new(pattern, owners, section, optional)
       end
 
       def skip?(line)

ee/lib/gitlab/code_owners/loader.rb

@@ -45,6 +45,10 @@ module Gitlab
         code_owners_file&.sections
       end
 
+      def optional_section?(section)
+        code_owners_file&.optional_section?(section)
+      end
+
       private
 
       def load_bare_entries_for_paths

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

@@ -186,6 +186,33 @@ RSpec.describe Gitlab::CodeOwners::File do
     end
   end
 
+  describe '#optional_section?' do
+    let(:file_content) do
+      <<~CONTENT
+      *.rb @ruby-owner
+
+      [Required]
+      *_spec.rb @gl-test
+
+      ^[Optional]
+      *_spec.rb @gl-test
+
+      [Partially optional]
+      *.md @gl-docs
+
+      ^[Partially optional]
+      doc/* @gl-docs
+      CONTENT
+    end
+
+    it 'returns whether a section is optional' do
+      expect(file.optional_section?('Required')).to eq(false)
+      expect(file.optional_section?('Optional')).to eq(true)
+      expect(file.optional_section?('Partially optional')).to eq(false)
+      expect(file.optional_section?('Does not exist')).to eq(false)
+    end
+  end
+
   describe '#entry_for_path' do
     shared_examples_for "returns expected matches" do
       context 'for a path without matches' do

ee/spec/lib/gitlab/code_owners_spec.rb

@@ -71,6 +71,35 @@ RSpec.describe Gitlab::CodeOwners do
     end
   end
 
+  describe '.optional_section?' do
+    subject { described_class.optional_section?(project, branch, 'codeowners') }
+
+    let(:branch) { TestEnv::BRANCH_SHA['with-codeowners'] }
+    let(:codeowner_lookup_ref) { branch }
+
+    context 'when the feature is available' do
+      before do
+        stub_licensed_features(code_owners: true)
+      end
+
+      it 'returns the optionality of the section' do
+        is_expected.to eq(false)
+      end
+    end
+
+    context 'when the feature is not available' do
+      before do
+        stub_licensed_features(code_owners: false)
+      end
+
+      it 'does not call Loader' do
+        expect(Gitlab::CodeOwners::Loader).not_to receive(:new)
+
+        subject
+      end
+    end
+  end
+
   describe ".fast_path_lookup and .slow_path_lookup" do
     let(:codeowner_lookup_ref) { merge_request.target_branch }
     let(:codeowner_content) { 'files/ruby/feature.rb @owner-1' }

ee/spec/models/approval_merge_request_rule_spec.rb

@@ -2,8 +2,9 @@
 
 require 'spec_helper'
 
-RSpec.describe ApprovalMergeRequestRule do
-  let(:merge_request) { create(:merge_request) }
+RSpec.describe ApprovalMergeRequestRule, factory_default: :keep do
+  let_it_be_with_reload(:project) { create_default(:project, :repository) }
+  let_it_be_with_reload(:merge_request) { create_default(:merge_request) }
 
   subject { create(:approval_merge_request_rule, merge_request: merge_request) }
 
@@ -27,7 +28,7 @@ RSpec.describe ApprovalMergeRequestRule do
     end
 
     context 'approval_project_rule is set' do
-      let(:approval_project_rule) { build(:approval_project_rule) }
+      let(:approval_project_rule) { build(:approval_project_rule, project: build(:project)) }
       let(:merge_request_rule) { build(:approval_merge_request_rule, merge_request: merge_request, approval_project_rule: approval_project_rule) }
 
       context 'when project of approval_project_rule and merge request matches' do

ee/spec/models/approval_wrapped_code_owner_rule_spec.rb

@@ -5,16 +5,18 @@ require 'spec_helper'
 RSpec.describe ApprovalWrappedCodeOwnerRule do
   using RSpec::Parameterized::TableSyntax
 
-  let(:merge_request) { create(:merge_request) }
+  let_it_be_with_reload(:merge_request) { create(:merge_request) }
 
   subject { described_class.new(merge_request, rule) }
 
   describe '#approvals_required' do
-    where(:feature_enabled, :approver_count, :expected_required_approvals) do
-      true  | 0 | 0
-      true  | 2 | 1
-      false | 2 | 0
-      false | 0 | 0
+    where(:feature_enabled, :optional_sections_enabled, :optional_section, :approver_count, :expected_required_approvals) do
+      true  | true  | false | 0 | 0
+      true  | true  | false | 2 | 1
+      true  | false | true  | 2 | 1
+      true  | true  | true  | 2 | 0
+      false | true  | false | 2 | 0
+      false | true  | false | 0 | 0
     end
 
     with_them do
@@ -30,6 +32,9 @@ RSpec.describe ApprovalWrappedCodeOwnerRule do
         before do
           allow(subject.project)
             .to receive(:code_owner_approval_required_available?).and_return(true)
+          allow(Gitlab::CodeOwners).to receive(:optional_section?).and_return(optional_section)
+
+          stub_feature_flags(optional_code_owners_sections: optional_sections_enabled)
         end
 
         context "when the project doesn't require code owner approval on all MRs" do