Merge branch 'fix-phase4-specs-v4' into 'master'

Fix/acknowledge various cross-DB modifications

See merge request gitlab-org/gitlab!78098

app/models/clusters/applications/runner.rb

@@ -41,7 +41,9 @@ module Clusters
       end
 
       def prepare_uninstall
-        runner&.update!(active: false)
+        ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/350180') do
+          runner&.update!(active: false)
+        end
       end
 
       def post_uninstall

app/services/ci/destroy_pipeline_service.rb

@@ -12,9 +12,7 @@ module Ci
       # Ci::Pipeline#destroy triggers `use_fast_destroy :job_artifacts` and
       # ci_builds has ON DELETE CASCADE to ci_pipelines. The pipeline, the builds,
       # job and pipeline artifacts all get destroyed here.
-      ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/345664') do
-        pipeline.reset.destroy!
-      end
+      pipeline.reset.destroy!
 
       ServiceResponse.success(message: 'Pipeline not found')
     rescue ActiveRecord::RecordNotFound

ee/app/services/ee/ci/job_artifacts/destroy_batch_service.rb

@@ -10,7 +10,9 @@ module EE
 
         override :destroy_related_records
         def destroy_related_records(artifacts)
-          destroy_security_findings(artifacts)
+          ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/346236') do
+            destroy_security_findings(artifacts)
+          end
         end
 
         override :after_batch_destroy_hook

ee/spec/replicators/geo/terraform_state_version_replicator_spec.rb

@@ -3,7 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe Geo::TerraformStateVersionReplicator do
-  let(:model_record) { build(:terraform_state_version, terraform_state: create(:terraform_state)) }
+  let_it_be(:ci_build) { create(:ci_build) }
+  let_it_be(:terraform_state) { create(:terraform_state, project: ci_build.project) }
+
+  let(:model_record) { build(:terraform_state_version, build: ci_build, terraform_state: terraform_state) }
 
   include_examples 'a blob replicator'
   include_examples 'a verifiable replicator'

spec/factories/clusters/applications/helm.rb

@@ -118,7 +118,6 @@ FactoryBot.define do
     end
 
     factory :clusters_applications_runner, class: 'Clusters::Applications::Runner' do
-      runner factory: %i(ci_runner)
       cluster factory: %i(cluster with_installed_helm provided_by_gcp)
     end
 

spec/lib/gitlab/ci/pipeline/chain/create_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Create do
   let_it_be(:user) { create(:user) }
 
   let(:pipeline) do
-    build(:ci_empty_pipeline, project: project, ref: 'master')
+    build(:ci_empty_pipeline, project: project, ref: 'master', user: user)
   end
 
   let(:command) do
@@ -59,7 +59,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Create do
 
   context 'tags persistence' do
     let(:stage) do
-      build(:ci_stage_entity, pipeline: pipeline)
+      build(:ci_stage_entity, pipeline: pipeline, project: project)
     end
 
     let(:job) do

spec/models/ci/build_trace_chunk_spec.rb

@@ -49,9 +49,9 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git
   end
 
   context 'FastDestroyAll' do
-    let(:parent) { create(:project) }
-    let(:pipeline) { create(:ci_pipeline, project: parent) }
-    let!(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: parent) }
+    let(:project) { create(:project) }
+    let(:pipeline) { create(:ci_pipeline, project: project) }
+    let!(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) }
     let(:subjects) { build.trace_chunks }
 
     describe 'Forbid #destroy and #destroy_all' do
@@ -84,7 +84,13 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git
         expect(external_data_counter).to be > 0
         expect(subjects.count).to be > 0
 
-        expect { parent.destroy! }.not_to raise_error
+        ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/350185') do
+          # This should use to prevent cross-DB modification
+          # but due to https://gitlab.com/gitlab-org/gitlab/-/issues/350185
+          # the build trace chunks are not destroyed by Projects::DestroyService
+          # Change to: expect { Projects::DestroyService.new(project, project.owner).execute }.to eq(true)
+          expect { project.destroy! }.not_to raise_error
+        end
 
         expect(subjects.count).to eq(0)
         expect(external_data_counter).to eq(0)
@@ -830,7 +836,7 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git
 
         expect(described_class.count).to eq(3)
 
-        subject
+        expect(subject).to be_truthy
 
         expect(described_class.count).to eq(0)
 
@@ -852,7 +858,13 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state, :clean_git
 
     context 'when project is destroyed' do
       let(:subject) do
-        project.destroy!
+        ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/350185') do
+          # This should use to prevent cross-DB modification
+          # but due to https://gitlab.com/gitlab-org/gitlab/-/issues/350185
+          # the build trace chunks are not destroyed by Projects::DestroyService
+          # Change to: Projects::DestroyService.new(project, project.owner).execute
+          project.destroy!
+        end
       end
 
       it_behaves_like 'deletes all build_trace_chunk and data in redis'

spec/requests/api/commits_spec.rb

@@ -1056,9 +1056,7 @@ RSpec.describe API::Commits do
 
     shared_examples_for 'ref with pipeline' do
       let!(:pipeline) do
-        project
-          .ci_pipelines
-          .create!(source: :push, ref: 'master', sha: commit.sha, protected: false)
+        create(:ci_empty_pipeline, project: project, status: :created, source: :push, ref: 'master', sha: commit.sha, protected: false)
       end
 
       it 'includes status as "created" and a last_pipeline object' do
@@ -1090,9 +1088,7 @@ RSpec.describe API::Commits do
 
     shared_examples_for 'ref with unaccessible pipeline' do
       let!(:pipeline) do
-        project
-          .ci_pipelines
-          .create!(source: :push, ref: 'master', sha: commit.sha, protected: false)
+        create(:ci_empty_pipeline, project: project, status: :created, source: :push, ref: 'master', sha: commit.sha, protected: false)
       end
 
       it 'does not include last_pipeline' do

spec/services/ci/retry_build_service_spec.rb

@@ -371,6 +371,14 @@ RSpec.describe Ci::RetryBuildService do
       it_behaves_like 'when build with dynamic environment is retried'
 
       context 'when create_deployment_in_separate_transaction feature flag is disabled' do
+        let(:new_build) do
+          travel_to(1.second.from_now) do
+            ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/345668') do
+              service.clone!(build)
+            end
+          end
+        end
+
         before do
           stub_feature_flags(create_deployment_in_separate_transaction: false)
         end

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

-- "./ee/spec/replicators/geo/terraform_state_version_replicator_spec.rb"
-- "./spec/features/issues/issue_detail_spec.rb"
-- "./spec/features/projects/pipelines/pipeline_spec.rb"
-- "./spec/features/signed_commits_spec.rb"
-- "./spec/lib/gitlab/ci/pipeline/chain/create_spec.rb"
-- "./spec/models/ci/build_trace_chunk_spec.rb"
-- "./spec/models/ci/job_artifact_spec.rb"
-- "./spec/models/clusters/applications/runner_spec.rb"
-- "./spec/requests/api/commits_spec.rb"
-- "./spec/services/ci/retry_build_service_spec.rb"
+[]

spec/support/shared_examples/models/update_project_statistics_shared_examples.rb

@@ -115,16 +115,14 @@ RSpec.shared_examples 'UpdateProjectStatistics' do |with_counter_attribute|
             expect(ProjectStatistics)
               .not_to receive(:increment_statistic)
 
-            project.update!(pending_delete: true)
-            project.destroy!
+            expect(Projects::DestroyService.new(project, project.owner).execute).to eq(true)
           end
 
           it 'does not schedule a namespace statistics worker' do
             expect(Namespaces::ScheduleAggregationWorker)
               .not_to receive(:perform_async)
 
-            project.update!(pending_delete: true)
-            project.destroy!
+            expect(Projects::DestroyService.new(project, project.owner).execute).to eq(true)
           end
         end
       end