diff --git a/ee/app/services/ee/merge_requests/after_create_service.rb b/ee/app/services/ee/merge_requests/after_create_service.rb
index b2f90fe1f5f7ed365f8620eea4e92fbc95dbd5db..908062da0bf8997c3ac97996fd5012e70b3a28a1 100644
--- a/ee/app/services/ee/merge_requests/after_create_service.rb
+++ b/ee/app/services/ee/merge_requests/after_create_service.rb
@@ -35,8 +35,7 @@ def schedule_sync_for(merge_request)
::Security::ScanResultPolicies::SyncAnyMergeRequestApprovalRulesWorker.perform_async(merge_request.id)
end
- if ::Feature.enabled?(:security_policies_sync_preexisting_state, project, type: :gitlab_com_derisk) &&
- merge_request.approval_rules.scan_finding.any?
+ if merge_request.approval_rules.scan_finding.any?
::Security::ScanResultPolicies::SyncPreexistingStatesApprovalRulesWorker.perform_async(merge_request.id)
end
diff --git a/ee/app/services/ee/merge_requests/refresh_service.rb b/ee/app/services/ee/merge_requests/refresh_service.rb
index d9f44112b42201544649ce3ae7e5ca639bfd5e11..f7b154767304b04d184c9eb70b9e5e3b9f972868 100644
--- a/ee/app/services/ee/merge_requests/refresh_service.rb
+++ b/ee/app/services/ee/merge_requests/refresh_service.rb
@@ -74,8 +74,6 @@ def sync_any_merge_request_approval_rules
end
def sync_preexiting_states_approval_rules
- return if ::Feature.disabled?(:security_policies_sync_preexisting_state, project, type: :gitlab_com_derisk)
-
merge_requests_for_source_branch.each do |merge_request|
if merge_request.approval_rules.scan_finding.any?
::Security::ScanResultPolicies::SyncPreexistingStatesApprovalRulesWorker.perform_async(merge_request.id)
diff --git a/ee/app/services/security/scan_result_policies/update_approvals_service.rb b/ee/app/services/security/scan_result_policies/update_approvals_service.rb
index 7639b724d661ac3fbc059906d2d98b16eff3466a..a38a5e23d910e509ca2d3456539db4c6c513924a 100644
--- a/ee/app/services/security/scan_result_policies/update_approvals_service.rb
+++ b/ee/app/services/security/scan_result_policies/update_approvals_service.rb
@@ -21,20 +21,18 @@ def execute
all_scan_finding_rules = merge_request.approval_rules.scan_finding
- approval_rules = if security_policies_sync_preexisting_state_enabled?
- all_scan_finding_rules.select { |rule| include_newly_detected?(rule) }
- else
- all_scan_finding_rules
- end
+ approval_rules_with_newly_detected_states = all_scan_finding_rules.select do |rule|
+ include_newly_detected?(rule)
+ end
- return if approval_rules.empty?
+ return if approval_rules_with_newly_detected_states.empty?
log_update_approval_rule('Evaluating MR approval rules from scan result policies',
pipeline_ids: related_pipeline_ids,
target_pipeline_ids: related_target_pipeline_ids
)
- violated_rules, unviolated_rules = partition_rules(approval_rules)
+ violated_rules, unviolated_rules = partition_rules(approval_rules_with_newly_detected_states)
update_required_approvals(violated_rules, unviolated_rules)
violations.add(violated_rules.pluck(:scan_result_policy_id), unviolated_rules.pluck(:scan_result_policy_id)) # rubocop:disable CodeReuse/ActiveRecord
@@ -96,11 +94,7 @@ def log_update_approval_rule(message, **attributes)
def violates_approval_rule?(approval_rule)
target_pipeline_uuids = target_pipeline_findings_uuids(approval_rule)
-
- return true if findings_count_violated?(approval_rule, target_pipeline_uuids)
- return true if preexisting_findings_count_violated?(approval_rule, target_pipeline_uuids)
-
- false
+ findings_count_violated?(approval_rule, target_pipeline_uuids)
end
def missing_scans(approval_rule)
@@ -153,15 +147,6 @@ def findings_count_violated?(approval_rule, target_pipeline_uuids)
end
end
- def preexisting_findings_count_violated?(approval_rule, target_pipeline_uuids)
- return false if security_policies_sync_preexisting_state_enabled?
- return false if target_pipeline_uuids.empty? || include_newly_detected?(approval_rule)
-
- vulnerabilities_count = vulnerabilities_count_for_uuids(target_pipeline_uuids, approval_rule)
-
- vulnerabilities_count[:exceeded_allowed_count]
- end
-
def related_pipeline_sources
Enums::Ci::Pipeline.ci_and_security_orchestration_sources.values
end
@@ -220,10 +205,6 @@ def vulnerabilities_count_for_uuids(uuids, approval_rule)
vulnerability_age: approval_rule.scan_result_policy_read&.vulnerability_age
).execute
end
-
- def security_policies_sync_preexisting_state_enabled?
- Feature.enabled?(:security_policies_sync_preexisting_state, merge_request.project, type: :gitlab_com_derisk)
- end
end
end
end
diff --git a/ee/app/services/security/security_orchestration_policies/sync_opened_merge_requests_service.rb b/ee/app/services/security/security_orchestration_policies/sync_opened_merge_requests_service.rb
index 2bd7e5fa548e482285c483b17bf3db6f945035bd..2f3d6d843d79b1bdcb0031e297a71cba2b5d2d5d 100644
--- a/ee/app/services/security/security_orchestration_policies/sync_opened_merge_requests_service.rb
+++ b/ee/app/services/security/security_orchestration_policies/sync_opened_merge_requests_service.rb
@@ -36,9 +36,6 @@ def sync_any_merge_request_approval_rules(merge_request)
end
def sync_preexisting_state_approval_rules(merge_request)
- return if ::Feature.disabled?(:security_policies_sync_preexisting_state, merge_request.project,
- type: :gitlab_com_derisk)
-
return unless merge_request.approval_rules.scan_finding.any?
::Security::ScanResultPolicies::SyncPreexistingStatesApprovalRulesWorker.perform_async(merge_request.id)
diff --git a/ee/app/workers/security/scan_result_policies/sync_preexisting_states_approval_rules_worker.rb b/ee/app/workers/security/scan_result_policies/sync_preexisting_states_approval_rules_worker.rb
index b9c0347313296a12c3fa21aef818a5b65aa133a1..6c90a4c2354c3020cac9069b258a8279e5db3d75 100644
--- a/ee/app/workers/security/scan_result_policies/sync_preexisting_states_approval_rules_worker.rb
+++ b/ee/app/workers/security/scan_result_policies/sync_preexisting_states_approval_rules_worker.rb
@@ -15,9 +15,6 @@ def perform(merge_request_id)
merge_request = MergeRequest.find_by_id(merge_request_id)
return unless merge_request
- return if Feature.disabled?(:security_policies_sync_preexisting_state, merge_request.project,
- type: :gitlab_com_derisk)
-
Security::ScanResultPolicies::SyncPreexistingStatesApprovalRulesService.new(merge_request).execute
end
end
diff --git a/ee/config/feature_flags/gitlab_com_derisk/security_policies_sync_preexisting_state.yml b/ee/config/feature_flags/gitlab_com_derisk/security_policies_sync_preexisting_state.yml
deleted file mode 100644
index 5b6f0ce5b221e2b1c6a3b0d2b7e3d16aa20e3aa0..0000000000000000000000000000000000000000
--- a/ee/config/feature_flags/gitlab_com_derisk/security_policies_sync_preexisting_state.yml
+++ /dev/null
@@ -1,9 +0,0 @@
----
-name: security_policies_sync_preexisting_state
-feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/425482
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141095
-rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/17361
-milestone: '16.9'
-group: group::security policies
-type: gitlab_com_derisk
-default_enabled: false
diff --git a/ee/spec/services/ee/merge_requests/refresh_service_spec.rb b/ee/spec/services/ee/merge_requests/refresh_service_spec.rb
index c346494f3d7f365cd59f6104142ddf13c3079c28..1ec4596318b9335fe98cefed861c78c7abf00fb1 100644
--- a/ee/spec/services/ee/merge_requests/refresh_service_spec.rb
+++ b/ee/spec/services/ee/merge_requests/refresh_service_spec.rb
@@ -231,18 +231,6 @@
subject
end
- context 'when security_policies_sync_preexisting_state is disabled' do
- before do
- stub_feature_flags(security_policies_sync_preexisting_state: false)
- end
-
- it 'does not enqueue SyncPreexistingStatesApprovalRulesWorker' do
- expect(Security::ScanResultPolicies::SyncPreexistingStatesApprovalRulesWorker).not_to receive(:perform_async)
-
- subject
- end
- end
-
context 'without scan_finding rule' do
let!(:scan_finding_rule) { nil }
diff --git a/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb b/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb
index b1fd897fed5c1c496273a288d352b9760def3a21..19fe8976df4ef9ee7dc639175c7d0d452475c93a 100644
--- a/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb
+++ b/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb
@@ -463,15 +463,6 @@
let(:vulnerability_states) { [] }
it_behaves_like 'sets approvals_required to 0'
-
- context 'when security_policies_sync_preexisting_state is disabled' do
- before do
- stub_feature_flags(security_policies_sync_preexisting_state: false)
- end
-
- it_behaves_like 'sets approvals_required to 0'
- it_behaves_like 'triggers policy bot comment', :scan_finding, false
- end
end
context 'when vulnerability_states has new_needs_triage' do
@@ -501,15 +492,6 @@
context 'when vulnerabilities count exceeds the allowed limit' do
it_behaves_like 'does not update approvals_required'
it_behaves_like 'does not trigger policy bot comment'
-
- context 'when security_policies_sync_preexisting_state is disabled' do
- before do
- stub_feature_flags(security_policies_sync_preexisting_state: false)
- end
-
- it_behaves_like 'does not update approvals_required'
- it_behaves_like 'triggers policy bot comment', :scan_finding, true
- end
end
context 'when vulnerabilities count does not exceed the allowed limit' do
@@ -517,15 +499,6 @@
it_behaves_like 'does not update approvals_required'
it_behaves_like 'does not trigger policy bot comment'
-
- context 'when security_policies_sync_preexisting_state is disabled' do
- before do
- stub_feature_flags(security_policies_sync_preexisting_state: false)
- end
-
- it_behaves_like 'sets approvals_required to 0'
- it_behaves_like 'triggers policy bot comment', :scan_finding, false
- end
end
end
@@ -536,15 +509,6 @@
it_behaves_like 'does not update approvals_required'
it_behaves_like 'triggers policy bot comment', :scan_finding, true
-
- context 'when security_policies_sync_preexisting_state is disabled' do
- before do
- stub_feature_flags(security_policies_sync_preexisting_state: false)
- end
-
- it_behaves_like 'does not update approvals_required'
- it_behaves_like 'triggers policy bot comment', :scan_finding, true
- end
end
end
diff --git a/ee/spec/services/security/security_orchestration_policies/sync_opened_merge_requests_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/sync_opened_merge_requests_service_spec.rb
index f3d1b5567ac59ad8395473f8a7050d5fefb7655b..2c6f4ae00d27b2e46b941afaa4a7f580173e20e4 100644
--- a/ee/spec/services/security/security_orchestration_policies/sync_opened_merge_requests_service_spec.rb
+++ b/ee/spec/services/security/security_orchestration_policies/sync_opened_merge_requests_service_spec.rb
@@ -123,20 +123,6 @@
)
end
- context 'when security_policies_sync_preexisting_state is disabled' do
- before do
- stub_feature_flags(security_policies_sync_preexisting_state: false)
- end
-
- it 'does not enqueue SyncPreexistingStatesApprovalRulesWorker' do
- expect(::Security::ScanResultPolicies::SyncPreexistingStatesApprovalRulesWorker).not_to(
- receive(:perform_async)
- )
-
- subject
- end
- end
-
it "enqueues SyncPreexistingStatesApprovalRulesWorker with opened merge requests" do
expect(::Security::ScanResultPolicies::SyncPreexistingStatesApprovalRulesWorker).to(
receive(:perform_async).with(opened_merge_request.id)
diff --git a/ee/spec/workers/security/scan_result_policies/sync_preexisting_states_approval_rules_worker_spec.rb b/ee/spec/workers/security/scan_result_policies/sync_preexisting_states_approval_rules_worker_spec.rb
index ccfb121a66a3a99f8af945e88b5ec90c43252a81..ee3fad305172c1de1c3a0b47b7b3978ee517cd66 100644
--- a/ee/spec/workers/security/scan_result_policies/sync_preexisting_states_approval_rules_worker_spec.rb
+++ b/ee/spec/workers/security/scan_result_policies/sync_preexisting_states_approval_rules_worker_spec.rb
@@ -19,18 +19,6 @@
run_worker
end
- context 'when security_policies_sync_preexisting_state is disabled' do
- before do
- stub_feature_flags(security_policies_sync_preexisting_state: false)
- end
-
- it 'does not call SyncPreexistingStatesApprovalRulesService' do
- expect(Security::ScanResultPolicies::SyncPreexistingStatesApprovalRulesService).not_to receive(:new)
-
- run_worker
- end
- end
-
context 'when merge_request does not exist' do
let(:merge_request_id) { non_existing_record_id }