diff --git a/ee/app/services/security/security_orchestration_policies/validate_policy_service.rb b/ee/app/services/security/security_orchestration_policies/validate_policy_service.rb
index 302bdd4b92345dcc6773fbf9f407685346e15d25..bf1d4360e730d21e5c7ff8ad7b449d3146145dc9 100644
--- a/ee/app/services/security/security_orchestration_policies/validate_policy_service.rb
+++ b/ee/app/services/security/security_orchestration_policies/validate_policy_service.rb
@@ -24,20 +24,13 @@ def execute
return error_with_title(s_('SecurityOrchestration|Invalid Compliance Framework ID(s)'), field: :compliance_frameworks) if invalid_compliance_framework_ids?
return error_with_title(s_('SecurityOrchestration|Required approvals exceed eligible approvers.'), title: s_('SecurityOrchestration|Logic error'), field: :approvers_ids) if required_approvals_exceed_eligible_approvers?
-
- if should_validate_cadence? && invalid_cadence?
- return error_with_title(s_('SecurityOrchestration|Cadence is invalid'), field: :cadence)
- end
+ return error_with_title(s_('SecurityOrchestration|Cadence is invalid'), field: :cadence) if invalid_cadence?
success
end
private
- def should_validate_cadence?
- container.present? && Feature.enabled?(:scan_execution_policy_cadence_validation, container)
- end
-
def error_with_title(message, field: DEFAULT_VALIDATION_ERROR_FIELD, title: nil, level: :error)
pass_back = {
details: [message],
diff --git a/ee/app/workers/security/orchestration_policy_rule_schedule_namespace_worker.rb b/ee/app/workers/security/orchestration_policy_rule_schedule_namespace_worker.rb
index 3f4de18236a7dec5ee754e33a8fe4d4dd73fedf3..3ce168d7651279996f843c3f995d58aea8b6b0de 100644
--- a/ee/app/workers/security/orchestration_policy_rule_schedule_namespace_worker.rb
+++ b/ee/app/workers/security/orchestration_policy_rule_schedule_namespace_worker.rb
@@ -19,6 +19,13 @@ def perform(rule_schedule_id)
security_orchestration_policy_configuration = schedule.security_orchestration_policy_configuration
return unless should_run?(security_orchestration_policy_configuration, schedule)
+ namespace = security_orchestration_policy_configuration.namespace
+
+ unless valid_cadence?(schedule.cron)
+ log_invalid_cadence_error(namespace.id, schedule.cron)
+ return
+ end
+
if Feature.enabled?(:batched_scan_execution_scheduled_pipelines, security_orchestration_policy_configuration.namespace)
return perform_in_batches_with_delay(schedule, security_orchestration_policy_configuration)
end
@@ -28,10 +35,6 @@ def perform(rule_schedule_id)
projects_in_batches(security_orchestration_policy_configuration).each do |projects|
bots_by_project_id = security_policy_bot_ids_by_project_ids(projects)
projects.each do |project|
- if Feature.enabled?(:scan_execution_policy_cadence_validation, project) && !valid_cadence?(schedule.cron)
- next log_invalid_cadence_error(project.id, schedule.cron)
- end
-
user_id = bots_by_project_id[project.id]
next prepare_security_policy_bot_user(project) unless user_id
@@ -118,5 +121,12 @@ def projects_in_batches(configuration, batch_size = BATCH_SIZE)
.select(:id)
.find_in_batches(batch_size: batch_size) # rubocop: disable CodeReuse/ActiveRecord -- A custom batch size is needed because querying too many bot users at once is too expensive
end
+
+ def log_invalid_cadence_error(namespace_id, cadence)
+ Gitlab::AppJsonLogger.info(event: 'scheduled_scan_execution_policy_validation',
+ message: 'Invalid cadence',
+ namespace_id: namespace_id,
+ cadence: cadence)
+ end
end
end
diff --git a/ee/app/workers/security/orchestration_policy_rule_schedule_worker.rb b/ee/app/workers/security/orchestration_policy_rule_schedule_worker.rb
index 4b31e3f66a2cd32ac88340015a4dd663dddf33e8..f723b4e0826c65622f44f23fb5e733d6afaad24a 100644
--- a/ee/app/workers/security/orchestration_policy_rule_schedule_worker.rb
+++ b/ee/app/workers/security/orchestration_policy_rule_schedule_worker.rb
@@ -35,7 +35,7 @@ def schedule_rules(schedule)
user = project.security_policy_bot
- if Feature.enabled?(:scan_execution_policy_cadence_validation, project) && !valid_cadence?(schedule.cron)
+ unless valid_cadence?(schedule.cron)
log_invalid_cadence_error(project.id, schedule.cron)
return
end
diff --git a/ee/config/feature_flags/beta/scan_execution_policy_cadence_validation.yml b/ee/config/feature_flags/beta/scan_execution_policy_cadence_validation.yml
deleted file mode 100644
index da9da70b4477e3e7348de3e24ff55bc4f6071504..0000000000000000000000000000000000000000
--- a/ee/config/feature_flags/beta/scan_execution_policy_cadence_validation.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: scan_execution_policy_cadence_validation
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/147576
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/451859
-milestone: '16.11'
-type: beta
-group: group::security policies
-default_enabled: true
diff --git a/ee/spec/services/security/security_orchestration_policies/validate_policy_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/validate_policy_service_spec.rb
index d051442e86196297ad0b602688ee07da7ac6a462..3dca9b4bab1e89334266df0c7ff541f7c217ab51 100644
--- a/ee/spec/services/security/security_orchestration_policies/validate_policy_service_spec.rb
+++ b/ee/spec/services/security/security_orchestration_policies/validate_policy_service_spec.rb
@@ -497,14 +497,6 @@
it { expect(result[:status]).to eq(:error) }
it { expect(result[:details]).to match_array(['Cadence is invalid']) }
-
- context 'when the feature flag scan_execution_policy_cadence_validation is disabled' do
- before do
- stub_feature_flags(scan_execution_policy_cadence_validation: false)
- end
-
- it { expect(result[:status]).to eq(:success) }
- end
end
end
end
@@ -573,18 +565,7 @@
it_behaves_like 'checks if branches are provided in rule'
it_behaves_like 'checks if timezone is valid'
it_behaves_like 'checks if vulnerability_age is valid'
-
- context 'when cadence is valid' do
- let(:cadence) { '0 0 * * *' }
-
- it { expect(result[:status]).to eq(:success) }
- end
-
- context 'when cadence is invalid' do
- let(:cadence) { '* * * * *' }
-
- it { expect(result[:status]).to eq(:success) }
- end
+ it_behaves_like 'checks if cadence is valid'
end
context 'when project is provided' do
diff --git a/ee/spec/workers/security/orchestration_policy_rule_schedule_namespace_worker_spec.rb b/ee/spec/workers/security/orchestration_policy_rule_schedule_namespace_worker_spec.rb
index 8db21773a1c0ecc232f4d0975b189afeb0e18cf5..f6a7aea1e12e33b4548677b4234faf8608cb062a 100644
--- a/ee/spec/workers/security/orchestration_policy_rule_schedule_namespace_worker_spec.rb
+++ b/ee/spec/workers/security/orchestration_policy_rule_schedule_namespace_worker_spec.rb
@@ -158,13 +158,11 @@
end
it 'logs the error' do
- [project_1, project_2].each do |project|
- expect(::Gitlab::AppJsonLogger).to receive(:info).once.with(
- event: 'scheduled_scan_execution_policy_validation',
- message: 'Invalid cadence',
- project_id: project.id,
- cadence: schedule.cron).and_call_original
- end
+ expect(::Gitlab::AppJsonLogger).to receive(:info).once.with(
+ event: 'scheduled_scan_execution_policy_validation',
+ message: 'Invalid cadence',
+ namespace_id: namespace.id,
+ cadence: schedule.cron).and_call_original
worker.perform(schedule_id)
end