Extract logic to prepare DAST job variables into separate service

This change extracts logic responsible for preparing service params and
job variables into separate services so they can be used in different
context.

ee/app/services/ci/dast_scan_ci_configuration_service.rb

+# frozen_string_literal: true
+
+module Ci
+  module DastScanCiConfigurationService
+    ENV_MAPPING = {
+      spider_timeout: 'DAST_SPIDER_MINS',
+      target_timeout: 'DAST_TARGET_AVAILABILITY_TIMEOUT',
+      target_url: 'DAST_WEBSITE',
+      use_ajax_spider: 'DAST_USE_AJAX_SPIDER',
+      show_debug_messages: 'DAST_DEBUG',
+      full_scan_enabled: 'DAST_FULL_SCAN_ENABLED'
+    }.freeze
+
+    def self.execute(args)
+      variables = args.slice(*ENV_MAPPING.keys).compact.to_h do |key, val|
+        [ENV_MAPPING[key], to_env_value(val)]
+      end
+
+      {
+        'stages' => ['dast'],
+        'include' => [{ 'template' => 'DAST-On-Demand-Scan.gitlab-ci.yml' }],
+        'variables' => variables
+      }.to_yaml
+    end
+
+    def self.bool?(value)
+      !!value == value
+    end
+    private_class_method :bool?
+
+    def self.to_env_value(value)
+      bool?(value) ? value.to_s : value
+    end
+    private_class_method :to_env_value
+  end
+end

ee/app/services/ci/run_dast_scan_service.rb

@@ -2,28 +2,6 @@
 
 module Ci
   class RunDastScanService < BaseService
-    ENV_MAPPING = {
-      spider_timeout: 'DAST_SPIDER_MINS',
-      target_timeout: 'DAST_TARGET_AVAILABILITY_TIMEOUT',
-      target_url: 'DAST_WEBSITE',
-      use_ajax_spider: 'DAST_USE_AJAX_SPIDER',
-      show_debug_messages: 'DAST_DEBUG',
-      full_scan_enabled: 'DAST_FULL_SCAN_ENABLED'
-    }.freeze
-
-    def self.ci_template
-      @ci_template ||= YAML.safe_load(ci_template_raw)
-    end
-
-    def self.ci_template_raw
-      <<~YAML
-        stages:
-          - dast
-        include:
-          - template: DAST-On-Demand-Scan.gitlab-ci.yml
-      YAML
-    end
-
     def execute(branch:, **args)
       return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
 
@@ -44,16 +22,7 @@ module Ci
     end
 
     def ci_yaml(args)
-      variables = args.each_with_object({}) do |(key, val), hash|
-        next if val.nil? || !ENV_MAPPING[key]
-
-        hash[ENV_MAPPING[key]] = !!val == val ? val.to_s : val
-        hash
-      end
-
-      self.class.ci_template.deep_merge(
-        'variables' => variables
-      ).to_yaml
+      Ci::DastScanCiConfigurationService.execute(args)
     end
   end
 end

ee/app/services/dast_on_demand_scans/create_service.rb

@@ -4,7 +4,6 @@ module DastOnDemandScans
   class CreateService < BaseContainerService
     def execute
       return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
-      return ServiceResponse.error(message: 'Cannot run active scan against unvalidated target') unless active_scan_allowed?
 
       create_pipeline
     rescue KeyError => err
@@ -17,49 +16,6 @@ module DastOnDemandScans
       container.feature_available?(:security_on_demand_scans)
     end
 
-    def active_scan_allowed?
-      return true unless dast_scanner_profile&.full_scan_enabled?
-
-      dast_site_validation = DastSiteValidationsFinder.new(
-        project_id: container.id,
-        state: :passed,
-        url_base: url_base
-      ).execute.first
-
-      dast_site_validation.present?
-    end
-
-    def dast_site
-      @dast_site ||= params.fetch(:dast_site_profile).dast_site
-    end
-
-    def dast_scanner_profile
-      @dast_scanner_profile ||= params[:dast_scanner_profile]
-    end
-
-    def url_base
-      @url_base ||= DastSiteValidation.get_normalized_url_base(dast_site.url)
-    end
-
-    def default_config
-      {
-        branch: container.default_branch,
-        target_url: dast_site.url
-      }
-    end
-
-    def scanner_profile_config
-      return {} unless dast_scanner_profile
-
-      {
-        spider_timeout: dast_scanner_profile.spider_timeout,
-        target_timeout: dast_scanner_profile.target_timeout,
-        full_scan_enabled: dast_scanner_profile.full_scan_enabled?,
-        use_ajax_spider: dast_scanner_profile.use_ajax_spider,
-        show_debug_messages: dast_scanner_profile.show_debug_messages
-      }
-    end
-
     def success_response(pipeline)
       pipeline_url = Rails.application.routes.url_helpers.project_pipeline_url(
         container,
@@ -75,8 +31,11 @@ module DastOnDemandScans
     end
 
     def create_pipeline
-      params = default_config.merge(scanner_profile_config)
-      result = ::Ci::RunDastScanService.new(container, current_user).execute(**params)
+      params_result = DastOnDemandScans::ParamsCreateService.new(container: container, current_user: current_user, params: params).execute
+
+      return params_result unless params_result.success?
+
+      result = ::Ci::RunDastScanService.new(container, current_user).execute(**params_result.payload)
 
       return success_response(result.payload) if result.success?
 

ee/app/services/dast_on_demand_scans/params_create_service.rb

+# frozen_string_literal: true
+
+module DastOnDemandScans
+  class ParamsCreateService < BaseContainerService
+    def execute
+      return ServiceResponse.error(message: 'Site Profile was not provided') unless dast_site.present?
+      return ServiceResponse.error(message: 'Cannot run active scan against unvalidated target') unless active_scan_allowed?
+
+      ServiceResponse.success(
+        payload: default_config.merge(scanner_profile_config)
+      )
+    end
+
+    private
+
+    def active_scan_allowed?
+      return true unless dast_scanner_profile&.full_scan_enabled?
+
+      DastSiteValidationsFinder.new(
+        project_id: container.id,
+        state: :passed,
+        url_base: url_base
+      ).execute.present?
+    end
+
+    def dast_site
+      @dast_site ||= params[:dast_site_profile]&.dast_site
+    end
+
+    def dast_scanner_profile
+      @dast_scanner_profile ||= params[:dast_scanner_profile]
+    end
+
+    def url_base
+      @url_base ||= DastSiteValidation.get_normalized_url_base(dast_site&.url)
+    end
+
+    def default_config
+      {
+        branch: container.default_branch,
+        target_url: dast_site&.url
+      }
+    end
+
+    def scanner_profile_config
+      return {} unless dast_scanner_profile
+
+      {
+        spider_timeout: dast_scanner_profile.spider_timeout,
+        target_timeout: dast_scanner_profile.target_timeout,
+        full_scan_enabled: dast_scanner_profile.full_scan_enabled?,
+        use_ajax_spider: dast_scanner_profile.use_ajax_spider,
+        show_debug_messages: dast_scanner_profile.show_debug_messages
+      }
+    end
+  end
+end

ee/spec/services/ci/dast_scan_ci_configuration_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::DastScanCiConfigurationService do
+  describe '.execute' do
+    subject(:yaml_configuration) { described_class.execute(params) }
+
+    context 'when all variables are provided' do
+      let(:params) do
+        {
+          spider_timeout: 1000,
+          target_timeout: 100,
+          target_url: 'https://gitlab.local',
+          use_ajax_spider: true,
+          show_debug_messages: true,
+          full_scan_enabled: true
+        }
+      end
+
+      let(:expected_yaml_configuration) do
+        <<~YAML
+        ---
+        stages:
+        - dast
+        include:
+        - template: DAST-On-Demand-Scan.gitlab-ci.yml
+        variables:
+          DAST_SPIDER_MINS: 1000
+          DAST_TARGET_AVAILABILITY_TIMEOUT: 100
+          DAST_WEBSITE: https://gitlab.local
+          DAST_USE_AJAX_SPIDER: 'true'
+          DAST_DEBUG: 'true'
+          DAST_FULL_SCAN_ENABLED: 'true'
+        YAML
+      end
+
+      it 'return YAML configuration of the On-Demand DAST scan' do
+        expect(yaml_configuration).to eq(expected_yaml_configuration)
+      end
+    end
+
+    context 'when additional variables are provided' do
+      let(:params) do
+        {
+          target_url: 'https://gitlab.local',
+          use_ajax_spider: false,
+          show_debug_messages: nil,
+          full_scan_enabled: nil,
+          additional_argument: true,
+          additional_list: ['item a']
+        }
+      end
+
+      let(:expected_yaml_configuration) do
+        <<~YAML
+        ---
+        stages:
+        - dast
+        include:
+        - template: DAST-On-Demand-Scan.gitlab-ci.yml
+        variables:
+          DAST_WEBSITE: https://gitlab.local
+          DAST_USE_AJAX_SPIDER: 'false'
+        YAML
+      end
+
+      it 'return YAML configuration of the On-Demand DAST scan' do
+        expect(yaml_configuration).to eq(expected_yaml_configuration)
+      end
+    end
+
+    context 'when no variable is provided' do
+      let(:params) { {} }
+
+      let(:expected_yaml_configuration) do
+        <<~YAML
+        ---
+        stages:
+        - dast
+        include:
+        - template: DAST-On-Demand-Scan.gitlab-ci.yml
+        variables: {}
+        YAML
+      end
+
+      it 'return YAML configuration of the On-Demand DAST scan' do
+        expect(yaml_configuration).to eq(expected_yaml_configuration)
+      end
+    end
+  end
+end

ee/spec/services/ci/run_dast_scan_service_spec.rb

@@ -15,16 +15,6 @@ RSpec.describe Ci::RunDastScanService do
     stub_licensed_features(security_on_demand_scans: true)
   end
 
-  describe '.ci_template' do
-    it 'builds a hash' do
-      expect(described_class.ci_template).to be_a(Hash)
-    end
-
-    it 'has only one stage' do
-      expect(described_class.ci_template['stages']).to eq(['dast'])
-    end
-  end
-
   describe '#execute' do
     subject { described_class.new(project, user).execute(branch: branch, target_url: target_url, spider_timeout: 42, target_timeout: 21, use_ajax_spider: use_ajax_spider, show_debug_messages: show_debug_messages, full_scan_enabled: full_scan_enabled) }
 
@@ -140,7 +130,7 @@ RSpec.describe Ci::RunDastScanService do
             'public' => true
           }
         ]
-        expect(build.yaml_variables).to eq(expected_variables)
+        expect(build.yaml_variables).to contain_exactly(*expected_variables)
       end
 
       it 'enqueues a build' do

ee/spec/services/dast_on_demand_scans/params_create_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DastOnDemandScans::ParamsCreateService do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:dast_site_profile) { create(:dast_site_profile, project: project) }
+  let_it_be(:dast_scanner_profile) { create(:dast_scanner_profile, project: project) }
+
+  let(:params) { { dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile } }
+
+  subject { described_class.new(container: project, params: params).execute }
+
+  describe 'execute' do
+    context 'when dast_site_profile is not provided' do
+      let(:params) { { dast_site_profile: nil, dast_scanner_profile: dast_scanner_profile } }
+
+      it 'responds with error message', :aggregate_failures do
+        expect(subject).not_to be_success
+        expect(subject.message).to eq('Site Profile was not provided')
+      end
+    end
+
+    context 'when dast_site_profile is provided' do
+      context 'and when dast_scanner_profile is not provided' do
+        let(:params) { { dast_site_profile: dast_site_profile, dast_scanner_profile: nil } }
+
+        it 'returns prepared scanner params in the payload' do
+          expect(subject.payload).to eq(
+            branch: 'master',
+            target_url: dast_site_profile.dast_site.url
+          )
+        end
+      end
+
+      context 'and when dast_scanner_profile is provided' do
+        let(:params) { { dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile } }
+
+        it 'returns prepared scanner params in the payload' do
+          expect(subject.payload).to eq(
+            branch: 'master',
+            full_scan_enabled: false,
+            show_debug_messages: false,
+            spider_timeout: nil,
+            target_timeout: nil,
+            target_url: dast_site_profile.dast_site.url,
+            use_ajax_spider: false
+          )
+        end
+
+        context 'but target is not validated and an active scan is requested' do
+          let_it_be(:active_dast_scanner_profile) { create(:dast_scanner_profile, project: project, scan_type: 'active') }
+
+          let(:params) { { dast_site_profile: dast_site_profile, dast_scanner_profile: active_dast_scanner_profile } }
+
+          it 'responds with error message', :aggregate_failures do
+            expect(subject).not_to be_success
+            expect(subject.message).to eq('Cannot run active scan against unvalidated target')
+          end
+        end
+      end
+    end
+  end
+end