Commit dc522c88 authored by Philip Cunningham's avatar Philip Cunningham Committed by Thong Kuah

Remove dast_sharded_cloned_ci_builds feature flag

- Removes feature flag
- Clean up feature flag references
- Extracts database association service

Changelog: changed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80133
EE: true
parent c457e547
# 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')
......@@ -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)
......
---
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
# 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
......@@ -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
......
# 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
......@@ -32,37 +32,16 @@ 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
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
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)
expect(new_build.dast_scanner_profile).to eq(dast_scanner_profile)
expect(new_build).not_to be_failed
end
end
new_build.reload
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
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
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment