Merge branch...

Merge branch 'philipcunningham-extend-services-to-take-optional-branch-when-running-an-on-demand-scan-322526' into 'master'

Extend DAST scan creation services to take optional branch

See merge request gitlab-org/gitlab!55129

ee/app/services/dast_on_demand_scans/params_create_service.rb

@@ -2,6 +2,8 @@
 
 module DastOnDemandScans
   class ParamsCreateService < BaseContainerService
+    include Gitlab::Utils::StrongMemoize
+
     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?
@@ -23,21 +25,33 @@ module DastOnDemandScans
       ).execute.present?
     end
 
+    def branch
+      strong_memoize(:branch) do
+        params[:branch] || container.default_branch
+      end
+    end
+
     def dast_site
-      @dast_site ||= params[:dast_site_profile]&.dast_site
+      strong_memoize(:dast_site) do
+        params[:dast_site_profile]&.dast_site
+      end
     end
 
     def dast_scanner_profile
-      @dast_scanner_profile ||= params[:dast_scanner_profile]
+      strong_memoize(:dast_scanner_profile) do
+        params[:dast_scanner_profile]
+      end
     end
 
     def url_base
-      @url_base ||= DastSiteValidation.get_normalized_url_base(dast_site&.url)
+      strong_memoize(:url_base) do
+        DastSiteValidation.get_normalized_url_base(dast_site&.url)
+      end
     end
 
     def default_config
       {
-        branch: container.default_branch,
+        branch: branch,
         target_url: dast_site&.url
       }
     end

ee/spec/services/dast_on_demand_scans/create_service_spec.rb

@@ -3,32 +3,29 @@
 require 'spec_helper'
 
 RSpec.describe DastOnDemandScans::CreateService do
-  let(:project) { create(:project, :repository) }
-  let(:user) { create(:user) }
-  let(:dast_site_profile) { create(:dast_site_profile, project: project) }
-  let(:dast_scanner_profile) { create(:dast_scanner_profile, project: project) }
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
+  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 do
     described_class.new(
       container: project,
       current_user: user,
-      params: {
-        dast_site_profile: dast_site_profile,
-        dast_scanner_profile: dast_scanner_profile
-      }
+      params: params
     ).execute
   end
 
   describe 'execute' do
     context 'when on demand scan licensed feature is not available' do
       context 'when the user cannot run an on demand scan' do
-        it 'communicates failure' do
+        it 'communicates failure', :aggregate_failures do
           stub_licensed_features(security_on_demand_scans: false)
 
-          aggregate_failures do
-            expect(subject.status).to eq(:error)
-            expect(subject.message).to eq('Insufficient permissions')
-          end
+          expect(subject.status).to eq(:error)
+          expect(subject.message).to eq('Insufficient permissions')
         end
       end
     end
@@ -47,14 +44,12 @@ RSpec.describe DastOnDemandScans::CreateService do
           expect(subject.status).to eq(:success)
         end
 
-        it 'returns a pipeline and pipeline_url' do
-          aggregate_failures do
-            expect(subject.payload[:pipeline]).to be_a(Ci::Pipeline)
-            expect(subject.payload[:pipeline_url]).to be_a(String)
-          end
+        it 'returns a pipeline and pipeline_url', :aggregate_failures do
+          expect(subject.payload[:pipeline]).to be_a(Ci::Pipeline)
+          expect(subject.payload[:pipeline_url]).to be_a(String)
         end
 
-        it 'delegates pipeline creation to Ci::RunDastScanService' do
+        it 'delegates pipeline creation to Ci::RunDastScanService', :aggregate_failures do
           expected_params = {
             branch: 'master',
             full_scan_enabled: false,
@@ -68,14 +63,31 @@ RSpec.describe DastOnDemandScans::CreateService do
           service = double(Ci::RunDastScanService)
           response = ServiceResponse.error(message: 'Stubbed response')
 
-          aggregate_failures do
-            expect(Ci::RunDastScanService).to receive(:new).and_return(service)
-            expect(service).to receive(:execute).with(expected_params).and_return(response)
-          end
+          expect(Ci::RunDastScanService).to receive(:new).and_return(service)
+          expect(service).to receive(:execute).with(expected_params).and_return(response)
 
           subject
         end
 
+        context 'when a branch is specified' do
+          context 'when the branch does not exist' do
+            let(:params) { { dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile, branch: 'other-branch' } }
+
+            it 'responds with error message', :aggregate_failures do
+              expect(subject).not_to be_success
+              expect(subject.message).to eq('Reference not found')
+            end
+          end
+
+          context 'when the branch exists' do
+            let(:params) { { dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile, branch: 'orphaned-branch' } }
+
+            it 'communicates success' do
+              expect(subject.status).to eq(:success)
+            end
+          end
+        end
+
         context 'when dast_scanner_profile is nil' do
           let(:dast_scanner_profile) { nil }
 
@@ -87,11 +99,9 @@ RSpec.describe DastOnDemandScans::CreateService do
         context 'when target is not validated and an active scan is requested' do
           let(:dast_scanner_profile) { create(:dast_scanner_profile, project: project, scan_type: 'active') }
 
-          it 'communicates failure' do
-            aggregate_failures do
-              expect(subject.status).to eq(:error)
-              expect(subject.message).to eq('Cannot run active scan against unvalidated target')
-            end
+          it 'communicates failure', :aggregate_failures do
+            expect(subject.status).to eq(:error)
+            expect(subject.message).to eq('Cannot run active scan against unvalidated target')
           end
         end
       end

ee/spec/services/dast_on_demand_scans/params_create_service_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe DastOnDemandScans::ParamsCreateService do
   subject { described_class.new(container: project, params: params).execute }
 
   describe 'execute' do
-    context 'when dast_site_profile is not provided' do
+    context 'when the 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
@@ -21,8 +21,20 @@ RSpec.describe DastOnDemandScans::ParamsCreateService do
       end
     end
 
-    context 'when dast_site_profile is provided' do
-      context 'and when dast_scanner_profile is not provided' do
+    context 'when the dast_site_profile is provided' do
+      context 'when the branch is provided' do
+        let(:params) { { dast_site_profile: dast_site_profile, branch: 'other-branch' } }
+
+        context 'when the branch exists' do
+          it 'includes the branch in the prepared params' do
+            project.repository.create_branch(params[:branch])
+
+            expect(subject.payload[:branch]).to eq(params[:branch])
+          end
+        end
+      end
+
+      context 'when the 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
@@ -33,12 +45,12 @@ RSpec.describe DastOnDemandScans::ParamsCreateService do
         end
       end
 
-      context 'and when dast_scanner_profile is provided' do
+      context 'when the 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',
+            branch: project.default_branch,
             full_scan_enabled: false,
             show_debug_messages: false,
             spider_timeout: nil,
@@ -48,7 +60,7 @@ RSpec.describe DastOnDemandScans::ParamsCreateService do
           )
         end
 
-        context 'but target is not validated and an active scan is requested' do
+        context 'when the 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 } }