Merge branch '216345-extend-project-group-membership-checks-to-group-ancestors' into 'master'

Add project group and ancestors to group list

See merge request gitlab-org/gitlab!31152

changelogs/unreleased/216345-extend-project-group-membership-checks-to-group-ancestors.yml

+---
+title: Consider project group and group ancestors when processing CODEOWNERS entries
+merge_request: 31152
+author:
+type: added

ee/lib/gitlab/code_owners/groups_loader.rb

@@ -23,8 +23,30 @@ module Gitlab
         return Group.none if extractor.names.empty?
 
         groups = project.invited_groups.where_full_path_in(extractor.names)
+
+        if Feature.enabled?(:codeowners_match_ancestor_groups, default_enabled: true)
+          group_list = groups.with_route.with_users.to_a
+
+          if project.group
+            # If the project.group's ancestor group(s) are listed as owners, add
+            #   them to group_list
+            #
+            if applicable_ancestors(extractor.names).any?
+              group_list.concat(applicable_ancestors(extractor.names))
+            end
+          end
+
+          group_list.uniq
+        else
           groups.with_route.with_users
         end
       end
+
+      def applicable_ancestors(extractor_names)
+        ancestor_groups = project.group.self_and_ancestors.with_route.with_users
+
+        ancestor_groups.select { |group| extractor_names.include?(group.full_path) }
+      end
+    end
   end
 end

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

@@ -103,6 +103,71 @@ describe Gitlab::Checks::DiffCheck do
             expect(validation_result).to be_nil
           end
         end
+
+        context "when the codeowner entity is a group" do
+          let(:group) { create(:group) }
+          let(:project) { create(:project, :repository, namespace: group) }
+          let(:codeowner_content) do
+            <<~CODEOWNERS
+            *.rb @#{code_owner.username}
+            docs/CODEOWNERS @#{group.full_path}
+            *.js.coffee @#{group.full_path}
+            CODEOWNERS
+          end
+
+          before do
+            stub_feature_flags(sectional_codeowners: false)
+          end
+
+          shared_examples_for "returns nil" do
+            it "returns nil" do
+              expect(validation_result).to be_nil
+            end
+          end
+
+          shared_examples_for "returns codeowners validation message" do
+            it "returns an error message" do
+              expect(validation_result).to include("Pushes to protected branches")
+            end
+          end
+
+          context "and the project is under a subgroup" do
+            let(:subgroup) { create(:group, parent: group) }
+            let(:project) { create(:project, :repository, namespace: subgroup) }
+
+            context "and the user is part of the codeowning-group" do
+              before do
+                group.add_developer(user)
+              end
+
+              it_behaves_like "returns nil"
+            end
+
+            context "and the user is not part of the codeowning-group" do
+              it_behaves_like "returns codeowners validation message"
+            end
+
+            context "with :codeowners_match_ancestor_groups disabled" do
+              before do
+                stub_feature_flags(codeowners_match_ancestor_groups: false)
+              end
+
+              it_behaves_like "returns codeowners validation message"
+            end
+          end
+
+          context "and the user is part of the codeowning-group" do
+            before do
+              group.add_developer(user)
+            end
+
+            it_behaves_like "returns nil"
+          end
+
+          context "and the user is not part of the codeowning-group" do
+            it_behaves_like "returns codeowners validation message"
+          end
+        end
       end
 
       context "the MR doesn't contain a matching file path" do

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

@@ -58,6 +58,18 @@ describe Gitlab::CodeOwners::GroupsLoader do
       end
     end
 
+    context "input matches project.group" do
+      let(:group) { create(:group) }
+      let(:project) { create(:project, :repository, namespace: group) }
+      let(:text) { "@#{project.group.full_path}" }
+
+      it "returns the project's group" do
+        load_groups
+
+        expect(entry).to have_received(:add_matching_groups_from).with([group])
+      end
+    end
+
     context 'input as array of strings' do
       let(:text) { super().lines }
 

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

@@ -71,7 +71,7 @@ describe Gitlab::CodeOwners::Loader do
         expect(loader.entries).to contain_exactly(expected_entry, other_entry)
       end
 
-      it 'only performs 3 query for users and groups' do
+      it 'performs 6 query for users and groups' do
         test_group = create(:group, path: 'test-group')
         test_group.add_developer(create(:user))
 
@@ -80,9 +80,12 @@ describe Gitlab::CodeOwners::Loader do
 
         project.invited_groups << [test_group, another_group]
 
-        # One query for users, one for the emails to later divide them across the entries
-        # one for groups with joined routes and users
-        expect { loader.entries }.not_to exceed_query_limit(3)
+        # - 1 query for users
+        # - 1 for the emails to later divide them across the entries
+        # - 2 for groups with joined routes and users
+        # - 2 for loading the users for the parent group(s)
+        #
+        expect { loader.entries }.not_to exceed_query_limit(6)
       end
     end