diff --git a/ee/app/services/app_sec/dast/site_validations/runner_service.rb b/ee/app/services/app_sec/dast/site_validations/runner_service.rb index 63357939b780abb6d68c6fc82387a7e5b6b934a2..60e1ef575e801b824a78cefe499b7856d77107cf 100644 --- a/ee/app/services/app_sec/dast/site_validations/runner_service.rb +++ b/ee/app/services/app_sec/dast/site_validations/runner_service.rb @@ -7,6 +7,10 @@ class RunnerService < BaseProjectService def execute return ServiceResponse.error(message: _('Insufficient permissions')) unless allowed? + unless available_runners_exist? + return ServiceResponse.error(message: _('No suitable runners available for DAST validation')) + end + service = Ci::CreatePipelineService.new(project, current_user, ref: project.default_branch_or_main) result = service.execute(:ondemand_dast_validation, content: ci_configuration.to_yaml, variables_attributes: dast_site_validation_variables) @@ -30,7 +34,47 @@ def dast_site_validation end def ci_configuration - { 'include' => [{ 'template' => 'Security/DAST-Runner-Validation.gitlab-ci.yml' }] } + ci_config = { + 'include' => [{ 'template' => 'Security/DAST-Runner-Validation.gitlab-ci.yml' }] + } + + ci_config['validation'] = { 'tags' => [dast_validation_tag] } if tagged_runners_available? + + ci_config + end + + def dast_validation_tag + 'dast-validation-runner' + end + + def available_runners_exist? + tagged_runners_available? || untagged_runners_available? + end + + def runners_available?(tagged: false) + params = { + project: project, + status: 'active' + } + + if tagged + params[:tag_name] = dast_validation_tag + else + params[:run_untagged] = true + end + + ::Ci::RunnersFinder.new( + current_user: current_user, + params: params + ).execute.exists? + end + + strong_memoize_attr def tagged_runners_available? + runners_available?(tagged: true) + end + + strong_memoize_attr def untagged_runners_available? + runners_available?(tagged: false) end def dast_site_validation_variables diff --git a/ee/spec/graphql/mutations/dast_site_validations/create_spec.rb b/ee/spec/graphql/mutations/dast_site_validations/create_spec.rb index 855388eeef6f15236df69e979934b8e2ebd95fed..6628283dc9f7b0219f3709f9c046ca089adf2552 100644 --- a/ee/spec/graphql/mutations/dast_site_validations/create_spec.rb +++ b/ee/spec/graphql/mutations/dast_site_validations/create_spec.rb @@ -18,6 +18,18 @@ before do project.update!(ci_pipeline_variables_minimum_override_role: :developer) stub_licensed_features(security_on_demand_scans: true) + + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(current_user, :read_admin_cicd).and_return(true) + allow(Ability).to receive(:allowed?).with(current_user, :read_runners, project).and_return(true) + + allow_next_instance_of(AppSec::Dast::SiteValidations::RunnerService) do |instance| + allow(instance).to receive_messages( + available_runners_exists?: true, + tagged_runners_available?: false, + untagged_runners_available?: true + ) + end end specify { expect(described_class).to require_graphql_authorizations(:create_on_demand_dast_scan) } diff --git a/ee/spec/requests/api/graphql/mutations/dast_site_validations/create_spec.rb b/ee/spec/requests/api/graphql/mutations/dast_site_validations/create_spec.rb index b56b8c546d4e8b344cfa7bb20d0c1211e9ccd10f..ba3956a5754d9715c6ca115ffee9ffdfa1ac54c7 100644 --- a/ee/spec/requests/api/graphql/mutations/dast_site_validations/create_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/dast_site_validations/create_spec.rb @@ -28,6 +28,18 @@ it_behaves_like 'an on-demand scan mutation when user can run an on-demand scan' do before do project.update!(ci_pipeline_variables_minimum_override_role: :developer) + + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(current_user, :read_admin_cicd).and_return(true) + allow(Ability).to receive(:allowed?).with(current_user, :read_runners, project).and_return(true) + + allow_next_instance_of(AppSec::Dast::SiteValidations::RunnerService) do |instance| + allow(instance).to receive_messages( + available_runners_exists?: true, + tagged_runners_available?: false, + untagged_runners_available?: true + ) + end end it 'returns the dast_site_validation id' do diff --git a/ee/spec/services/app_sec/dast/site_validations/find_or_create_service_spec.rb b/ee/spec/services/app_sec/dast/site_validations/find_or_create_service_spec.rb index 26642f0c2232d2ec08e8269c7bb9cdc1abc47c4f..c264acbc268dfb87f863135d2bc00ce19cee4e30 100644 --- a/ee/spec/services/app_sec/dast/site_validations/find_or_create_service_spec.rb +++ b/ee/spec/services/app_sec/dast/site_validations/find_or_create_service_spec.rb @@ -14,6 +14,10 @@ before do project.update!(ci_pipeline_variables_minimum_override_role: :developer) + # Grant admin permissions to avoid access denied errors in Ci::RunnersFinder + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(developer, :read_admin_cicd).and_return(true) + allow(Ability).to receive(:allowed?).with(developer, :read_runners, project).and_return(true) end describe 'execute', :clean_gitlab_redis_shared_state do @@ -31,6 +35,14 @@ context 'when the licensed feature is available' do before do stub_licensed_features(security_on_demand_scans: true) + # Mock runner availability for successful execution + allow_next_instance_of(AppSec::Dast::SiteValidations::RunnerService) do |instance| + allow(instance).to receive_messages( + available_runners_exists?: true, + tagged_runners_available?: false, + untagged_runners_available?: true + ) + end end it 'communicates success' do diff --git a/ee/spec/services/app_sec/dast/site_validations/runner_service_spec.rb b/ee/spec/services/app_sec/dast/site_validations/runner_service_spec.rb index cd4779032a955541b872307019c674e89b3f8a08..ba632ce505a62f3798435005cd25f343d4e7fe0b 100644 --- a/ee/spec/services/app_sec/dast/site_validations/runner_service_spec.rb +++ b/ee/spec/services/app_sec/dast/site_validations/runner_service_spec.rb @@ -7,6 +7,7 @@ let_it_be(:developer) { create(:user, developer_of: project) } let_it_be(:dast_site_token) { create(:dast_site_token, project: project) } let_it_be(:dast_site_validation) { create(:dast_site_validation, dast_site_token: dast_site_token) } + let_it_be(:default_runner) { create(:ci_runner, :project, projects: [project], run_untagged: true) } subject do described_class.new(project: project, current_user: developer, params: { dast_site_validation: dast_site_validation }).execute @@ -14,6 +15,10 @@ before do project.update!(ci_pipeline_variables_minimum_override_role: :developer) + # Grant admin permissions to avoid access denied errors in Ci::RunnersFinder + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(developer, :read_admin_cicd).and_return(true) + allow(Ability).to receive(:allowed?).with(developer, :read_runners, project).and_return(true) end describe 'execute' do @@ -94,11 +99,86 @@ end end + context 'when no suitable runners are available' do + before do + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:available_runners_exist?).and_return(false) + end + end + + it 'returns an error' do + expect(subject).to have_attributes( + status: :error, + message: 'No suitable runners available for DAST validation' + ) + end + end + + context 'when tagged runners are available' do + let(:ci_tag) { create(:ci_tag, name: 'dast-validation-runner') } + let(:tagged_runner) { create(:ci_runner, :project, projects: [project]) } + + before do + # Create the runner-tag association (avoid duplicates) + unless tagged_runner.taggings.joins(:tag).where(tags: { name: ci_tag.name }).exists? + create(:ci_runner_tagging, runner: tagged_runner, tag: ci_tag) + end + end + + it 'creates pipeline with tagged configuration' do + expect { subject }.to change { Ci::Pipeline.count }.by(1) + + pipeline = Ci::Pipeline.last + expect(pipeline.source).to eq('ondemand_dast_validation') + end + + it 'sets tags in ci_configuration' do + service = described_class.new(project: project, current_user: developer, params: { dast_site_validation: dast_site_validation }) + config = service.send(:ci_configuration) + expect(config['validation']['tags']).to eq(['dast-validation-runner']) + end + end + + context 'when only untagged runners are available' do + it 'creates pipeline without tags configuration' do + expect { subject }.to change { Ci::Pipeline.count }.by(1) + + pipeline = Ci::Pipeline.last + expect(pipeline.source).to eq('ondemand_dast_validation') + end + + it 'does not set validation entry in ci_configuration' do + service = described_class.new(project: project, current_user: developer, params: { dast_site_validation: dast_site_validation }) + config = service.send(:ci_configuration) + expect(config).not_to have_key('validation') + end + end + + context 'when both tagged and untagged runners are available' do + let(:ci_tag_2) { create(:ci_tag, name: 'dast-validation-runner') } + let(:tagged_runner_2) { create(:ci_runner, :project, projects: [project]) } + + before do + # Create the runner-tag association (avoid duplicates) + unless tagged_runner_2.taggings.joins(:tag).where(tags: { name: ci_tag_2.name }).exists? + create(:ci_runner_tagging, runner: tagged_runner_2, tag: ci_tag_2) + end + end + + it 'prioritizes tagged runners and sets tags in configuration' do + service = described_class.new(project: project, current_user: developer, params: { dast_site_validation: dast_site_validation }) + config = service.send(:ci_configuration) + + expect(config['validation']['tags']).to eq(['dast-validation-runner']) + end + end + context 'when pipeline creation fails' do before do - allow_next_instance_of(Ci::Pipeline) do |instance| - allow(instance).to receive(:created_successfully?).and_return(false) - allow(instance).to receive(:full_error_messages).and_return('error message') + # Mock successful Ci::CreatePipelineService result but failed pipeline + allow_next_instance_of(Ci::CreatePipelineService) do |instance| + mock_result = instance_double(ServiceResponse, success?: false) + allow(instance).to receive(:execute).and_return(mock_result) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 131e7ea8b4423dd1ee75a72838afedffe140c201..02d26aef9a3399dc7a730faea5c4b915d2eb6ab4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -42573,6 +42573,9 @@ msgstr "" msgid "No suggestions found" msgstr "" +msgid "No suitable runners available for DAST validation" +msgstr "" + msgid "No template" msgstr ""