From 3ecd09b1b21fedee47c4be5ba19c9e04482f92d5 Mon Sep 17 00:00:00 2001 From: Marcos Rocha <mrocha@gitlab.com> Date: Tue, 29 Nov 2022 04:46:44 +0000 Subject: [PATCH] Filter runners by project This MR adds the option to filter the runners available for a given project. Changelog: added --- app/finders/ci/runners_finder.rb | 15 +- .../resolvers/ci/project_runners_resolver.rb | 15 ++ app/graphql/types/project_type.rb | 5 + .../on_demand_scans_runner_tags.yml | 8 + doc/api/graphql/reference/index.md | 23 +++ spec/finders/ci/runners_finder_spec.rb | 149 ++++++++++++++++++ .../ci/project_runners_resolver_spec.rb | 86 ++++++++++ .../api/graphql/project/runners_spec.rb | 68 ++++++++ 8 files changed, 368 insertions(+), 1 deletion(-) create mode 100644 app/graphql/resolvers/ci/project_runners_resolver.rb create mode 100644 config/feature_flags/development/on_demand_scans_runner_tags.yml create mode 100644 spec/graphql/resolvers/ci/project_runners_resolver_spec.rb create mode 100644 spec/requests/api/graphql/project/runners_spec.rb diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb index d0d98a5967784..d2e8f5c2a4c40 100644 --- a/app/finders/ci/runners_finder.rb +++ b/app/finders/ci/runners_finder.rb @@ -10,6 +10,7 @@ class RunnersFinder < UnionFinder def initialize(current_user:, params:) @params = params @group = params.delete(:group) + @project = params.delete(:project) @current_user = current_user end @@ -36,7 +37,13 @@ def sort_key private def search! - @group ? group_runners : all_runners + if @project && Feature.enabled?(:on_demand_scans_runner_tags, @project) + project_runners + elsif @group + group_runners + else + all_runners + end @runners = @runners.search(@params[:search]) if @params[:search].present? end @@ -66,6 +73,12 @@ def group_runners end end + def project_runners + raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_project, @project) + + @runners = ::Ci::Runner.owned_or_instance_wide(@project.id) + end + def filter_by_active! @runners = @runners.active(@params[:active]) if @params.include?(:active) end diff --git a/app/graphql/resolvers/ci/project_runners_resolver.rb b/app/graphql/resolvers/ci/project_runners_resolver.rb new file mode 100644 index 0000000000000..378fa73c065c8 --- /dev/null +++ b/app/graphql/resolvers/ci/project_runners_resolver.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Resolvers + module Ci + class ProjectRunnersResolver < RunnersResolver + type Types::Ci::RunnerType.connection_type, null: true + + def parent_param + raise 'Expected project missing' unless parent.is_a?(Project) + + { project: parent } + end + end + end +end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index 0577505244093..5efd76003c3b5 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -540,6 +540,11 @@ class ProjectType < BaseObject description: "Programming languages used in the project.", calls_gitaly: true + field :runners, Types::Ci::RunnerType.connection_type, + null: true, + resolver: ::Resolvers::Ci::ProjectRunnersResolver, + description: "Find runners visible to the current user." + def timelog_categories object.project_namespace.timelog_categories if Feature.enabled?(:timelog_categories) end diff --git a/config/feature_flags/development/on_demand_scans_runner_tags.yml b/config/feature_flags/development/on_demand_scans_runner_tags.yml new file mode 100644 index 0000000000000..e25c6f8e3d1c0 --- /dev/null +++ b/config/feature_flags/development/on_demand_scans_runner_tags.yml @@ -0,0 +1,8 @@ +--- +name: on_demand_scans_runner_tags +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/103634 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/381910 +milestone: '15.7' +type: development +group: group::dynamic analysis +default_enabled: false diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 1b606cc50bfe0..2dfe7216a6ad8 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -17911,6 +17911,29 @@ four standard [pagination arguments](#connection-pagination-arguments): | <a id="projectrequirementssort"></a>`sort` | [`Sort`](#sort) | List requirements by sort order. | | <a id="projectrequirementsstate"></a>`state` | [`RequirementState`](#requirementstate) | Filter requirements by state. | +##### `Project.runners` + +Find runners visible to the current user. + +Returns [`CiRunnerConnection`](#cirunnerconnection). + +This field returns a [connection](#connections). It accepts the +four standard [pagination arguments](#connection-pagination-arguments): +`before: String`, `after: String`, `first: Int`, `last: Int`. + +###### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="projectrunnersactive"></a>`active` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated** in 14.8. This was renamed. Use: `paused`. | +| <a id="projectrunnerspaused"></a>`paused` | [`Boolean`](#boolean) | Filter runners by `paused` (true) or `active` (false) status. | +| <a id="projectrunnerssearch"></a>`search` | [`String`](#string) | Filter by full token or partial text in description field. | +| <a id="projectrunnerssort"></a>`sort` | [`CiRunnerSort`](#cirunnersort) | Sort order of results. | +| <a id="projectrunnersstatus"></a>`status` | [`CiRunnerStatus`](#cirunnerstatus) | Filter runners by status. | +| <a id="projectrunnerstaglist"></a>`tagList` | [`[String!]`](#string) | Filter by tags associated with the runner (comma-separated or array). | +| <a id="projectrunnerstype"></a>`type` | [`CiRunnerType`](#cirunnertype) | Filter runners by type. | +| <a id="projectrunnersupgradestatus"></a>`upgradeStatus` | [`CiRunnerUpgradeStatus`](#cirunnerupgradestatus) | Filter by upgrade status. | + ##### `Project.scanExecutionPolicies` Scan Execution Policies of the project. diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb index 18eecd0f0736b..372e6a3ff7e66 100644 --- a/spec/finders/ci/runners_finder_spec.rb +++ b/spec/finders/ci/runners_finder_spec.rb @@ -473,4 +473,153 @@ def execute end end end + + context 'project' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:other_project) { create(:project) } + + let(:extra_params) { {} } + let(:params) { { project: project }.merge(extra_params).reject { |_, v| v.nil? } } + + describe '#execute' do + subject { described_class.new(current_user: user, params: params).execute } + + context 'with user as project admin' do + before do + project.add_maintainer(user) + end + + context 'with project runners' do + let_it_be(:runner_project) { create(:ci_runner, :project, contacted_at: 7.minutes.ago, projects: [project]) } + + it 'returns runners available to project' do + expect(subject).to match_array([runner_project]) + end + end + + context 'with ancestor group runners' do + let_it_be(:runner_instance) { create(:ci_runner, contacted_at: 13.minutes.ago) } + let_it_be(:runner_group) { create(:ci_runner, :group, contacted_at: 12.minutes.ago, groups: [group]) } + + it 'returns runners available to project' do + expect(subject).to match_array([runner_instance, runner_group]) + end + end + + context 'with allowed shared runners' do + let_it_be(:runner_instance) { create(:ci_runner, :instance, contacted_at: 13.minutes.ago) } + + it 'returns runners available to project' do + expect(subject).to match_array([runner_instance]) + end + end + + context 'with project, ancestor group, and allowed shared runners' do + let_it_be(:runner_project) { create(:ci_runner, :project, contacted_at: 7.minutes.ago, projects: [project]) } + let_it_be(:runner_group) { create(:ci_runner, :group, contacted_at: 12.minutes.ago, groups: [group]) } + let_it_be(:runner_instance) { create(:ci_runner, :instance, contacted_at: 13.minutes.ago) } + + it 'returns runners available to project' do + expect(subject).to match_array([runner_project, runner_group, runner_instance]) + end + end + + context 'filtering' do + let_it_be(:runner_instance_inactive) { create(:ci_runner, :instance, active: false, contacted_at: 13.minutes.ago) } + let_it_be(:runner_instance_active) { create(:ci_runner, :instance, active: true, contacted_at: 13.minutes.ago) } + let_it_be(:runner_project_active) { create(:ci_runner, :project, contacted_at: 5.minutes.ago, active: true, projects: [project]) } + let_it_be(:runner_project_inactive) { create(:ci_runner, :project, contacted_at: 5.minutes.ago, active: false, projects: [project]) } + let_it_be(:runner_other_project_inactive) { create(:ci_runner, :project, contacted_at: 5.minutes.ago, active: false, projects: [other_project]) } + + context 'by search term' do + let_it_be(:runner_project_1) { create(:ci_runner, :project, contacted_at: 5.minutes.ago, description: 'runner_project_search', projects: [project]) } + let_it_be(:runner_project_2) { create(:ci_runner, :project, contacted_at: 5.minutes.ago, description: 'runner_project', projects: [project]) } + let_it_be(:runner_another_project) { create(:ci_runner, :project, contacted_at: 5.minutes.ago, description: 'runner_project_search', projects: [other_project]) } + + let(:extra_params) { { search: 'runner_project_search' } } + + it 'returns the correct runner' do + expect(subject).to match_array([runner_project_1]) + end + end + + context 'by active status' do + let(:extra_params) { { active: false } } + + it 'returns the correct runners' do + expect(subject).to match_array([runner_instance_inactive, runner_project_inactive]) + end + end + + context 'by status' do + let(:extra_params) { { status_status: 'paused' } } + + it 'returns correct runner' do + expect(subject).to match_array([runner_instance_inactive, runner_project_inactive]) + end + end + + context 'by tag_name' do + let_it_be(:runner_project_1) { create(:ci_runner, :project, contacted_at: 3.minutes.ago, tag_list: %w[runner_tag], projects: [project]) } + let_it_be(:runner_project_2) { create(:ci_runner, :project, contacted_at: 3.minutes.ago, tag_list: %w[other_tag], projects: [project]) } + let_it_be(:runner_other_project) { create(:ci_runner, :project, contacted_at: 3.minutes.ago, tag_list: %w[runner_tag], projects: [other_project]) } + + let(:extra_params) { { tag_name: %w[runner_tag] } } + + it 'returns correct runner' do + expect(subject).to match_array([runner_project_1]) + end + end + + context 'by runner type' do + let(:extra_params) { { type_type: 'project_type' } } + + it 'returns correct runners' do + expect(subject).to match_array([runner_project_active, runner_project_inactive]) + end + end + end + end + + context 'with user as project developer' do + let(:user) { create(:user) } + + before do + project.add_developer(user) + end + + it 'returns no runners' do + expect(subject).to be_empty + end + end + + context 'when user is nil' do + let_it_be(:user) { nil } + + it 'returns no runners' do + expect(subject).to be_empty + end + end + + context 'with nil project_full_path' do + let(:project_full_path) { nil } + + it 'returns no runners' do + expect(subject).to be_empty + end + end + + context 'when on_demand_scans_runner_tags feature flag is disabled' do + before do + stub_feature_flags(on_demand_scans_runner_tags: false) + end + + it 'returns no runners' do + expect(subject).to be_empty + end + end + end + end end diff --git a/spec/graphql/resolvers/ci/project_runners_resolver_spec.rb b/spec/graphql/resolvers/ci/project_runners_resolver_spec.rb new file mode 100644 index 0000000000000..5025878cc21da --- /dev/null +++ b/spec/graphql/resolvers/ci/project_runners_resolver_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Resolvers::Ci::ProjectRunnersResolver do + include GraphqlHelpers + + describe '#resolve' do + subject do + resolve(described_class, obj: obj, ctx: { current_user: user }, args: args, + arg_style: :internal) + end + + include_context 'runners resolver setup' + + let(:obj) { project } + let(:args) { {} } + + context 'when user cannot see runners' do + it 'returns no runners' do + expect(subject.items.to_a).to eq([]) + end + end + + context 'with user as project admin' do + before do + project.add_maintainer(user) + end + + let(:available_runners) { [inactive_project_runner, offline_project_runner, group_runner, instance_runner] } + + it 'returns all runners available to the project' do + expect(subject.items.to_a).to match_array(available_runners) + end + end + + context 'with obj set to nil' do + let(:obj) { nil } + + it 'raises an error' do + expect { subject }.to raise_error('Expected project missing') + end + end + + context 'with obj not set to project' do + let(:obj) { build(:group) } + + it 'raises an error' do + expect { subject }.to raise_error('Expected project missing') + end + end + + describe 'Allowed query arguments' do + let(:finder) { instance_double(::Ci::RunnersFinder) } + let(:args) do + { + status: 'active', + type: :group_type, + tag_list: ['active_runner'], + search: 'abc', + sort: :contacted_asc + } + end + + let(:expected_params) do + { + status_status: 'active', + type_type: :group_type, + tag_name: ['active_runner'], + preload: { tag_name: false }, + search: 'abc', + sort: 'contacted_asc', + project: project + } + end + + it 'calls RunnersFinder with expected arguments' do + allow(::Ci::RunnersFinder).to receive(:new).with(current_user: user, + params: expected_params).once.and_return(finder) + allow(finder).to receive(:execute).once.and_return([:execute_return_value]) + + expect(subject.items.to_a).to eq([:execute_return_value]) + end + end + end +end diff --git a/spec/requests/api/graphql/project/runners_spec.rb b/spec/requests/api/graphql/project/runners_spec.rb new file mode 100644 index 0000000000000..da4b5e87b7ddc --- /dev/null +++ b/spec/requests/api/graphql/project/runners_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Project.runners' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group, :public) } + let_it_be(:project) { create(:project, :public, group: group) } + let_it_be(:instance_runner) { create(:ci_runner, :instance) } + let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let_it_be(:other_project) { create(:project, :repository, :public) } + let_it_be(:other_project_runner) { create(:ci_runner, :project, projects: [other_project]) } + + let_it_be(:query) do + %( + query { + project(fullPath: "#{project.full_path}") { + runners { + nodes { + id + } + } + } + } + ) + end + + context 'when the user is a project admin' do + before do + project.add_maintainer(user) + end + + let(:expected_ids) { [project_runner, group_runner, instance_runner].map { |g| g.to_global_id.to_s } } + + it 'returns all runners available to project' do + post_graphql(query, current_user: user) + + expect(graphql_data_at(:project, :runners, :nodes).pluck('id')).to match_array(expected_ids) + end + end + + context 'when the user is a project developer' do + before do + project.add_developer(user) + end + + it 'returns no runners' do + post_graphql(query, current_user: user) + + expect(graphql_data_at(:project, :runners, :nodes)).to be_empty + end + end + + context 'when on_demand_scans_runner_tags feature flag is disabled' do + before do + stub_feature_flags(on_demand_scans_runner_tags: false) + end + + it 'returns no runners' do + post_graphql(query, current_user: user) + + expect(graphql_data_at(:project, :runners, :nodes)).to be_empty + end + end +end -- GitLab