Merge branch 'philipcunningham-cleanup-feature-flag-dast-cross-database-350051' into 'master'

Remove dast_sharded_cloned_ci_builds feature flag

See merge request gitlab-org/gitlab!80133

app/services/ci/copy_cross_database_associations_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class CopyCrossDatabaseAssociationsService
+    def execute(old_build, new_build)
+      ServiceResponse.success
+    end
+  end
+end
+
+Ci::CopyCrossDatabaseAssociationsService.prepend_mod_with('Ci::CopyCrossDatabaseAssociationsService')

app/services/ci/retry_build_service.rb

@@ -40,6 +40,8 @@ module Ci
       new_build = clone_build(build)
 
       new_build.run_after_commit do
+        ::Ci::CopyCrossDatabaseAssociationsService.new.execute(build, new_build)
+
         ::Deployments::CreateForBuildService.new.execute(new_build)
 
         ::MergeRequests::AddTodoWhenBuildFailsService
@@ -70,9 +72,7 @@ module Ci
     def check_assignable_runners!(build); end
 
     def clone_build(build)
-      project.builds.new(build_attributes(build)).tap do |new_build|
-        yield(new_build) if block_given?
-      end
+      project.builds.new(build_attributes(build))
     end
 
     def build_attributes(build)

config/feature_flags/development/dast_sharded_cloned_ci_builds.yml

----
-name: dast_sharded_cloned_ci_builds
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78164
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351980
-milestone: '14.8'
-type: development
-group: group::dynamic analysis
-default_enabled: true

ee/app/services/ee/ci/copy_cross_database_associations_service.rb

+# frozen_string_literal: true
+
+module EE
+  module Ci
+    module CopyCrossDatabaseAssociationsService
+      extend ::Gitlab::Utils::Override
+
+      override :execute
+      def execute(old_build, new_build)
+        response = AppSec::Dast::Builds::AssociateService.new(
+          ci_build_id: new_build.id,
+          dast_site_profile_id: old_build.dast_site_profile&.id,
+          dast_scanner_profile_id: old_build.dast_scanner_profile&.id
+        ).execute
+
+        response.tap do
+          new_build.reset.drop! if response.error?
+        end
+      end
+    end
+  end
+end

ee/app/services/ee/ci/retry_build_service.rb

@@ -16,31 +16,12 @@ module EE
 
         override :extra_accessors
         def extra_accessors
-          return %i[secrets].freeze if ::Feature.enabled?(:dast_sharded_cloned_ci_builds, default_enabled: :yaml)
-
-          %i[dast_site_profile dast_scanner_profile secrets].freeze
+          %i[secrets].freeze
         end
       end
 
       private
 
-      override :clone_build
-      def clone_build(build)
-        super do |new_build|
-          if ::Feature.enabled?(:dast_sharded_cloned_ci_builds, default_enabled: :yaml)
-            new_build.run_after_commit do
-              response = AppSec::Dast::Builds::AssociateService.new(
-                ci_build_id: new_build.id,
-                dast_site_profile_id: build.dast_site_profile&.id,
-                dast_scanner_profile_id: build.dast_scanner_profile&.id
-              ).execute
-
-              new_build.reset.drop! if response.error?
-            end
-          end
-        end
-      end
-
       override :check_access!
       def check_access!(build)
         super

ee/spec/services/ci/copy_cross_database_associations_service_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Ci::CopyCrossDatabaseAssociationsService do
+  let_it_be(:project) { create(:project) }
+  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_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+
+  let_it_be(:old_build) do
+    create(:ci_build, pipeline: pipeline).tap do |build|
+      build.update!(dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile)
+    end
+  end
+
+  let(:new_build) { create(:ci_build, pipeline: pipeline) }
+
+  describe '#execute' do
+    subject(:execute) { described_class.new.execute(old_build, new_build) }
+
+    context 'failure' do
+      before do
+        allow_next_instance_of(AppSec::Dast::Builds::AssociateService) do |instance|
+          allow(instance).to receive(:execute).and_return(ServiceResponse.error(message: 'oops'))
+        end
+      end
+
+      it 'returns an error response' do
+        expect(execute).to have_attributes(status: :error)
+      end
+
+      it 'drops the build' do
+        execute
+
+        expect(new_build.reload).to be_failed
+      end
+    end
+
+    context 'success' do
+      it 'returns a success response' do
+        expect(execute).to be_success
+      end
+
+      it 'clones the profile associations', :aggregate_failures do
+        execute
+
+        expect(new_build.dast_site_profile).to eq(dast_site_profile)
+        expect(new_build.dast_scanner_profile).to eq(dast_scanner_profile)
+        expect(new_build).not_to be_failed
+      end
+    end
+  end
+end

ee/spec/services/ci/retry_build_service_spec.rb

@@ -32,18 +32,11 @@ RSpec.describe Ci::RetryBuildService do
           build.update!(dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile)
         end
 
-        context 'failure' do
-          it 'drops the build' do
-            allow_next_instance_of(AppSec::Dast::Builds::AssociateService) do |instance|
-              allow(instance).to receive(:execute).and_return(ServiceResponse.error(message: 'oops'))
-            end
-
-            expect(new_build.reload).to be_failed
-          end
+        it 'clones the profile associations', :aggregate_failures do
+          expect_next_instance_of(Ci::CopyCrossDatabaseAssociationsService) do |service|
+            expect(service).to receive(:execute).with(build, Ci::Build).and_call_original
           end
 
-        context 'success' do
-          it 'clones the profile associations', :aggregate_failures do
           new_build.reload
 
           expect(new_build.dast_site_profile).to eq(dast_site_profile)
@@ -52,20 +45,6 @@ RSpec.describe Ci::RetryBuildService do
         end
       end
 
-        context 'when dast_sharded_cloned_ci_builds is disabled' do
-          before do
-            stub_feature_flags(dast_sharded_cloned_ci_builds: false)
-          end
-
-          it 'clones the profile associations', :aggregate_failures do
-            ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/350051') do
-              expect(new_build.dast_site_profile).to eq(dast_site_profile)
-              expect(new_build.dast_scanner_profile).to eq(dast_scanner_profile)
-            end
-          end
-        end
-      end
-
       context 'when build has secrets' do
         let(:secrets) do
           {

spec/services/ci/copy_cross_database_associations_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::CopyCrossDatabaseAssociationsService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+  let_it_be(:old_build) { create(:ci_build, pipeline: pipeline) }
+  let_it_be(:new_build) { create(:ci_build, pipeline: pipeline) }
+
+  subject(:execute) { described_class.new.execute(old_build, new_build) }
+
+  describe '#execute' do
+    it 'returns a success response' do
+      expect(execute).to be_success
+    end
+  end
+end