Merge branch 'philipcunningham-fix-on-demand-dast-sharding-issue-343382' into 'master'

Move Dast::Profile association out of transaction

See merge request gitlab-org/gitlab!73805

config/sidekiq_queues.yml

@@ -35,6 +35,8 @@
   - 1
 - - analytics_usage_trends_counter_job
   - 1
+- - app_sec_dast_scans_consistency
+  - 1
 - - approval_rules_external_approval_rule_payload
   - 1
 - - approve_blocked_pending_approval_users

ee/app/services/app_sec/dast/scans/run_service.rb

@@ -8,13 +8,13 @@ module AppSec
           return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
 
           service = Ci::CreatePipelineService.new(project, current_user, ref: branch)
-
-          response = service.execute(:ondemand_dast_scan, content: ci_configuration) do |pipeline|
-            pipeline.dast_profile = dast_profile
-          end
+          response = service.execute(:ondemand_dast_scan, content: ci_configuration)
 
           pipeline = response.payload
+
           if pipeline.created_successfully?
+            associate_dast_profile(pipeline, dast_profile) if dast_profile
+
             ServiceResponse.success(payload: pipeline)
           else
             ServiceResponse.error(message: pipeline.full_error_messages)
@@ -26,6 +26,12 @@ module AppSec
         def allowed?
           Ability.allowed?(current_user, :create_on_demand_dast_scan, project)
         end
+
+        def associate_dast_profile(pipeline, dast_profile)
+          AppSec::Dast::Scans::ConsistencyWorker.perform_async(pipeline.id, dast_profile.id)
+
+          pipeline.dast_profile = dast_profile # this assignment performs an insert
+        end
       end
     end
   end

ee/app/workers/all_queues.yml

@@ -876,6 +876,15 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: app_sec_dast_scans_consistency
+  :worker_name: AppSec::Dast::Scans::ConsistencyWorker
+  :feature_category: :dynamic_application_security_testing
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: approval_rules_external_approval_rule_payload
   :worker_name: ApprovalRules::ExternalApprovalRulePayloadWorker
   :feature_category: :source_code_management

ee/app/workers/app_sec/dast/scans/consistency_worker.rb

+# frozen_string_literal: true
+
+module AppSec
+  module Dast
+    module Scans
+      class ConsistencyWorker
+        include ApplicationWorker
+
+        data_consistency :always
+
+        deduplicate :until_executed
+        idempotent!
+
+        feature_category :dynamic_application_security_testing
+
+        def perform(ci_pipeline_id, dast_profile_id)
+          ::Dast::ProfilesPipeline.create!(ci_pipeline_id: ci_pipeline_id, dast_profile_id: dast_profile_id)
+        rescue ActiveRecord::RecordNotUnique
+          # assume record is already associated
+        end
+      end
+    end
+  end
+end

ee/spec/services/app_sec/dast/scans/run_service_spec.rb

@@ -57,6 +57,14 @@ RSpec.describe AppSec::Dast::Scans::RunService do
         expect { subject }.to change(Ci::Pipeline, :count).by(1)
       end
 
+      it 'associates the dast profile', :aggregate_failures do
+        worker_class = AppSec::Dast::Scans::ConsistencyWorker
+        allow(worker_class).to receive(:perform_async).and_call_original
+
+        expect(pipeline.dast_profile).to eq(dast_profile)
+        expect(worker_class).to have_received(:perform_async).with(pipeline.id, dast_profile.id)
+      end
+
       it 'sets the pipeline ref to the branch' do
         expect(pipeline.ref).to eq(project.default_branch)
       end

ee/spec/workers/app_sec/dast/scans/consistency_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AppSec::Dast::Scans::ConsistencyWorker do
+  let(:worker) { described_class.new }
+
+  describe '#perform' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+    let_it_be(:profile) { create(:dast_profile, project: project) }
+
+    let(:job_args) { [pipeline.id, profile.id] }
+
+    it 'ensures cross database association is created', :aggregate_failures do
+      expect { worker.perform(*job_args) }.to change { Dast::ProfilesPipeline.count }.by(1)
+
+      expect(Dast::ProfilesPipeline.where(ci_pipeline_id: pipeline.id, dast_profile_id: profile.id)).to exist
+    end
+
+    it_behaves_like 'an idempotent worker'
+  end
+end

spec/support/database/cross-database-modification-allowlist.yml

 - "./ee/spec/controllers/projects/settings/access_tokens_controller_spec.rb"
-- "./ee/spec/graphql/mutations/dast/profiles/create_spec.rb"
-- "./ee/spec/graphql/mutations/dast/profiles/run_spec.rb"
-- "./ee/spec/graphql/mutations/dast/profiles/update_spec.rb"
-- "./ee/spec/graphql/mutations/dast_on_demand_scans/create_spec.rb"
 - "./ee/spec/lib/gitlab/ci/templates/Jobs/dast_default_branch_gitlab_ci_yaml_spec.rb"
 - "./ee/spec/mailers/notify_spec.rb"
 - "./ee/spec/models/ci/bridge_spec.rb"