Merge branch 'pedropombeiro/337690/1-rest-api-fix-type-scopes' into 'master'

REST: Fix scope of /groups/:id/runners?type endpoint

See merge request gitlab-org/gitlab!79447

app/models/ci/runner.rb

@@ -162,6 +162,23 @@ module Ci
       )
     end
 
+    scope :group_or_instance_wide, -> (group) do
+      group_and_ancestor_runners =
+        if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, group, default_enabled: :yaml)
+          belonging_to_group_and_ancestors(group.id)
+        else
+          legacy_belonging_to_group(group.id, include_ancestors: true)
+        end
+
+      from_union(
+        [
+          group_and_ancestor_runners,
+          instance_type
+        ],
+        remove_duplicates: false
+      )
+    end
+
     scope :assignable_for, ->(project) do
       # FIXME: That `to_sql` is needed to workaround a weird Rails bug.
       #        Without that, placeholders would miss one and couldn't match.

doc/api/runners.md

@@ -448,8 +448,7 @@ Example response:
 
 ## List project's runners
 
-List all runners (specific and shared) available in the project. Shared runners
-are listed if at least one shared runner is defined.
+List all runners available in the project, including from ancestor groups and any shared runners.
 
 ```plaintext
 GET /projects/:id/runners
@@ -567,8 +566,7 @@ curl --request DELETE --header "PRIVATE-TOKEN: <your_access_token>" "https://git
 
 ## List group's runners
 
-List all runners (specific and shared) available in the group as well it's ancestor groups.
-Shared runners are listed if at least one shared runner is defined.
+List all runners available in the group as well as its ancestor groups, including any shared runners.
 
 ```plaintext
 GET /groups/:id/runners

lib/api/ci/runners.rb

@@ -232,12 +232,7 @@ module API
           use :pagination
         end
         get ':id/runners' do
-          runners = if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, user_group, default_enabled: :yaml)
-                      ::Ci::Runner.belonging_to_group_and_ancestors(user_group.id)
-                    else
-                      ::Ci::Runner.legacy_belonging_to_group(user_group.id, include_ancestors: true)
-                    end
-
+          runners = ::Ci::Runner.group_or_instance_wide(user_group)
           runners = apply_filter(runners, params)
 
           present paginate(runners), with: Entities::Ci::Runner

spec/models/ci/runner_spec.rb

@@ -265,22 +265,37 @@ RSpec.describe Ci::Runner do
     it_behaves_like '.belonging_to_parent_group_of_project'
   end
 
-  describe '.owned_or_instance_wide' do
-    it 'returns a globally shared, a project specific and a group specific runner' do
+  context 'with existing system wide, group and project runners' do
     # group specific
-      group = create(:group)
-      project = create(:project, group: group)
-      group_runner = create(:ci_runner, :group, groups: [group])
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, group: group) }
+    let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group]) }
 
     # project specific
-      project_runner = create(:ci_runner, :project, projects: [project])
+    let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project]) }
 
     # globally shared
-      shared_runner = create(:ci_runner, :instance)
+    let_it_be(:shared_runner) { create(:ci_runner, :instance) }
 
-      expect(described_class.owned_or_instance_wide(project.id)).to contain_exactly(
-        group_runner, project_runner, shared_runner
-      )
+    describe '.owned_or_instance_wide' do
+      subject { described_class.owned_or_instance_wide(project.id) }
+
+      it 'returns a globally shared, a project specific and a group specific runner' do
+        is_expected.to contain_exactly(group_runner, project_runner, shared_runner)
+      end
+    end
+
+    describe '.group_or_instance_wide' do
+      subject { described_class.group_or_instance_wide(group) }
+
+      before do
+        # Ensure the project runner is instantiated
+        project_runner
+      end
+
+      it 'returns a globally shared and a group specific runner' do
+        is_expected.to contain_exactly(group_runner, shared_runner)
+      end
     end
   end
 

spec/requests/api/ci/runners_spec.rb

@@ -1026,7 +1026,8 @@ RSpec.describe API::Ci::Runners do
         get api("/groups/#{group.id}/runners", user)
 
         expect(json_response).to match_array([
-          a_hash_including('description' => 'Group runner A', 'active' => true, 'paused' => false)
+          a_hash_including('description' => 'Group runner A', 'active' => true, 'paused' => false),
+          a_hash_including('description' => 'Shared runner', 'active' => true, 'paused' => false)
         ])
       end
 
@@ -1039,6 +1040,14 @@ RSpec.describe API::Ci::Runners do
           ])
         end
 
+        it 'returns instance runners when instance_type is specified' do
+          get api("/groups/#{group.id}/runners?type=instance_type", user)
+
+          expect(json_response).to match_array([
+            a_hash_including('description' => 'Shared runner')
+          ])
+        end
+
         it 'returns empty result when type does not match' do
           get api("/groups/#{group.id}/runners?type=project_type", user)