diff --git a/ee/config/feature_flags/gitlab_com_derisk/codeowners_file_exclusions.yml b/ee/config/feature_flags/gitlab_com_derisk/codeowners_file_exclusions.yml new file mode 100644 index 0000000000000000000000000000000000000000..b7307377c4c5cbf49702ed50053f728492a5d192 --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/codeowners_file_exclusions.yml @@ -0,0 +1,9 @@ +--- +name: codeowners_file_exclusions +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/41914 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180162 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/517075 +milestone: '17.9' +group: group::source code +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/lib/gitlab/code_owners/entry.rb b/ee/lib/gitlab/code_owners/entry.rb index 1b7a0ef3c4793202143f15df23ebd86d2aabf7b2..628e38a3e54feb6932753406dc4c80e452e85663 100644 --- a/ee/lib/gitlab/code_owners/entry.rb +++ b/ee/lib/gitlab/code_owners/entry.rb @@ -5,16 +5,20 @@ module CodeOwners class Entry include ::Gitlab::Utils::StrongMemoize - Data = Struct.new(:pattern, :owner_line, :section, :optional, :approvals_required) + Data = Struct.new(:pattern, :owner_line, :section, :optional, :approvals_required, :exclusion) attr_reader :data protected :data - delegate :pattern, :hash, :owner_line, :section, :approvals_required, to: :data + delegate :pattern, :hash, :owner_line, :section, :approvals_required, :exclusion, to: :data - def initialize(pattern, owner_line, section = Section::DEFAULT, optional = false, approvals_required = 0) - @data = Data.new(pattern, owner_line, section, optional, approvals_required) + # rubocop:disable Metrics/ParameterLists -- TODO: Reduce number of parameters in this method + def initialize( + pattern, owner_line, section = Section::DEFAULT, optional = false, approvals_required = 0, + exclusion = false) + @data = Data.new(pattern, owner_line, section, optional, approvals_required, exclusion) end + # rubocop:enable Metrics/ParameterLists def all_users(project) strong_memoize(:all_users) do @@ -64,6 +68,10 @@ def optional? data.optional end + def exclusion? + data.exclusion + end + def ==(other) return false unless other.is_a?(self.class) diff --git a/ee/lib/gitlab/code_owners/file.rb b/ee/lib/gitlab/code_owners/file.rb index a229f5e11ac530733c42ef55bfb3d337ef37336d..7f5b64347c8eea3096cd9bc9ca8675a3e9ac1b82 100644 --- a/ee/lib/gitlab/code_owners/file.rb +++ b/ee/lib/gitlab/code_owners/file.rb @@ -61,11 +61,20 @@ def entries_for_path(path) matches = [] parsed_data.each do |_, section_entries| - matching_pattern = section_entries.keys.reverse.detect do |pattern| - path_matches?(pattern, path) - end + if Feature.enabled?(:codeowners_file_exclusions, @blob.repository.project) + matching_patterns = section_entries.keys.reverse.select { |pattern| path_matches?(pattern, path) } + matching_entries = matching_patterns.map { |pattern| section_entries[pattern] } + + next if matching_entries.any?(&:exclusion?) + + matches << matching_entries.first.dup if matching_entries.any? + else + matching_pattern = section_entries.keys.reverse.detect do |pattern| + path_matches?(pattern, path) + end - matches << section_entries[matching_pattern].dup if matching_pattern + matches << section_entries[matching_pattern].dup if matching_pattern + end end matches @@ -120,6 +129,12 @@ def get_parsed_data def parse_entry(line, parsed, section, line_number) pattern, _separator, entry_owners = line.partition(/(?<!\\)\s+/) + + if Feature.enabled?(:codeowners_file_exclusions, @blob.repository.project) + is_exclusion = pattern.start_with?('!') + pattern = pattern[1..] if is_exclusion + end + normalized_pattern = normalize_pattern(pattern) if entry_owners.present? && ReferenceExtractor.new(entry_owners).references.blank? @@ -130,12 +145,17 @@ def parse_entry(line, parsed, section, line_number) add_error(Error::MISSING_ENTRY_OWNER, line_number) if owners.blank? - parsed[section.name][normalized_pattern] = Entry.new( + entry_args = [ pattern, owners, section.name, section.optional, - section.approvals) + section.approvals + ] + + entry_args << is_exclusion if Feature.enabled?(:codeowners_file_exclusions, @blob.repository.project) + + parsed[section.name][normalized_pattern] = Entry.new(*entry_args) end def skip?(line) diff --git a/ee/spec/lib/gitlab/code_owners/file_spec.rb b/ee/spec/lib/gitlab/code_owners/file_spec.rb index faa0fc88dbf308a9d3a8e1d3937343452b1178e9..fd667b70b83e324512e3e80df7ef7359418f10b2 100644 --- a/ee/spec/lib/gitlab/code_owners/file_spec.rb +++ b/ee/spec/lib/gitlab/code_owners/file_spec.rb @@ -21,6 +21,50 @@ def owner_line(pattern) file.parsed_data["codeowners"][pattern].owner_line end + context 'when handling exclusion patterns' do + let(:file_content) do + <<~CONTENT + * @group-x + !*.rb + + [Ruby] + *.rb @ruby-devs + /config/example.yml @config-yml + !/config/**/*.rb + CONTENT + end + + it 'excludes patterns correctly per section' do + parsed = file.parsed_data + + expect(parsed['codeowners'].keys).to contain_exactly('/**/*', '/**/*.rb') + expect(parsed['codeowners']['/**/*'].owner_line).to eq('@group-x') + expect(parsed['codeowners']['/**/*.rb'].exclusion).to be(true) + + expect(parsed['Ruby'].keys).to contain_exactly('/**/*.rb', '/config/example.yml', '/config/**/*.rb') + expect(parsed['Ruby']['/**/*.rb'].owner_line).to eq('@ruby-devs') + expect(parsed['Ruby']['/config/example.yml'].owner_line).to eq('@config-yml') + expect(parsed['Ruby']['/config/**/*.rb'].exclusion).to be(true) + end + + context 'when "codeowners_file_exclusions" is disabled' do + before do + stub_feature_flags(codeowners_file_exclusions: false) + end + + it 'does not mark excluded entries per section' do + parsed = file.parsed_data + + expect(parsed['codeowners'].keys).to contain_exactly('/**/*', '/**/!*.rb') + expect(parsed['codeowners']['/**/*'].owner_line).to eq('@group-x') + + expect(parsed['Ruby'].keys).to contain_exactly('/**/*.rb', '/config/example.yml', '/**/!/config/**/*.rb') + expect(parsed['Ruby']['/**/*.rb'].owner_line).to eq('@ruby-devs') + expect(parsed['Ruby']['/config/example.yml'].owner_line).to eq('@config-yml') + end + end + end + context "when CODEOWNERS file contains no sections" do it 'parses all the required lines' do expected_patterns = [ @@ -428,6 +472,76 @@ def owner_line(pattern) end end + context 'when handling excluded patterns' do + let(:file_content) do + <<~CONTENT + * @group-x + !*.rb + + [Ruby] + *.rb @ruby-devs + !/config/* + CONTENT + end + + it 'matches non-excluded files to default owner' do + entry = file.entries_for_path('file.txt').first + + expect(entry.owner_line).to eq('@group-x') + end + + it 'matches .rb files to ruby owner except in config' do + ruby_entry = file.entries_for_path('app/models/user.rb').first + config_entry = file.entries_for_path('config/routes.rb') + + expect(ruby_entry.owner_line).to eq('@ruby-devs') + expect(config_entry).to be_empty + end + + it 'does not match excluded patterns' do + entries = file.entries_for_path('config/database.rb') + + expect(entries).to be_empty + end + + context 'with nested exclusions' do + let(:file_content) do + <<~CONTENT + * @group-x + + !/app/temp/* + !/app/*/temp/* + CONTENT + end + + it 'handles nested path exclusions correctly' do + regular_entry = file.entries_for_path('app/models/user.rb').first + temp_entry = file.entries_for_path('app/temp/temp.rb') + nested_temp_entry = file.entries_for_path('app/models/temp/file.rb') + + expect(regular_entry.owner_line).to eq('@group-x') + expect(temp_entry).to be_empty + expect(nested_temp_entry).to be_empty + end + + context 'when "codeowners_file_exclusions" is disabled' do + before do + stub_feature_flags(codeowners_file_exclusions: false) + end + + it 'handles nested path exclusions correctly' do + regular_entry = file.entries_for_path('app/models/user.rb').first + temp_entry = file.entries_for_path('app/temp/temp.rb').first + nested_temp_entry = file.entries_for_path('app/models/temp/file.rb').first + + expect(regular_entry.owner_line).to eq('@group-x') + expect(temp_entry.owner_line).to eq('@group-x') + expect(nested_temp_entry.owner_line).to eq('@group-x') + end + end + end + end + context "when CODEOWNERS file contains no sections" do it_behaves_like "returns expected matches" end