Replace cron job CreateOrchestrationPolicyWorker with a PostReceive hook to catch direct updates to policy.yml

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Work on this issue
• Close this issue

Why are we doing this work

We could simplify the sync of outdated policies when policy.yml is modified directly and not via MR. Currently, there are two entry points to the policy updates:

• Changes to policy.yml in a Security Policy Project via MR, where the sync of policies is triggered via PostMergeService
• Changes to policy.yml directly without MR where the changes are picked up by a cron job CreateOrchestrationPolicyWorker. This cron job compared a timestamp on the policy configuration with the timestamp of a last update in the SPP repository and runs every 10 minutes.

Downsides to using a cron job:

• The updates via direct policy.yml modifications are delayed by up to 10 minutes
• The cron job is doing unnecessary work
• Any updates to the SPP repository trigger Security::SyncScanPoliciesWorker where we invalidate the policy cache, even if there's no change to policy.yml (for example, if README.md or any other file in the repo changes). This may be causing additional unnecessary work.

We implemented a comparison with the DB policies to avoid unnecessary sync of all project policies to reduce some of the extra work, but we could improve that.

We can add a hook in PostReceive and trigger the sync when we detect a change in policy.yml in a SPP project and remove the cron job which checks with_outdated_configurations. This would have the following advantages:

• Reduce unnecessary work by removing the cron job that runs every 10 minutes
• More targeted sync only when policy.yml is changed, as opposed to anything in the SPP repository
• Faster sync of policies when policy.yml is modified directly

Relevant links

Non-functional requirements

• Documentation:
• Feature flag: changes should be added behind feature flag just in case
• Performance:
• Testing:

Implementation plan

• Cron job
  • Change the cron job to run only once per day (to be extra safe we're not missing any updates)
  • We could add logging to check that it doesn't perform anything (as it shouldn't, only maybe in a race condition scenario, in which case the difference between the two timestamps should be very small).
  • Create a follow-up issue to evaluate the logs and check that the cron job is not needed, and remove it.
• Trigger the policy sync via BranchPushService

Rough idea:

diff --git a/ee/app/services/ee/git/branch_push_service.rb b/ee/app/services/ee/git/branch_push_service.rb
index 4f5a8e6339d4..5df2dbaa98e8 100644
--- a/ee/app/services/ee/git/branch_push_service.rb
+++ b/ee/app/services/ee/git/branch_push_service.rb
@@ -12,12 +12,28 @@ def execute
         enqueue_update_external_pull_requests
         enqueue_product_analytics_event_metrics
         enqueue_repository_xray
+        enqueue_policies_sync
 
         super
       end
 
       private
 
+      def enqueue_policies_sync
+        return unless project.licensed_feature_available?(:security_orchestration_policies)
+        return unless default_branch?
+        return unless ::Security::OrchestrationPolicyConfiguration.policy_management_project?(project.id)
+
+        # Move the following to a worker and trigger it
+        modified_paths = ::Gitlab::Git::Push.new(project, oldrev, newrev, ref).modified_paths
+        return unless modified_paths.include?(::Security::OrchestrationPolicyConfiguration::POLICY_PATH)
+
+        configurations = ::Security::OrchestrationPolicyConfiguration.for_management_project(project)
+        configurations.each do |configuration|
+          Security::SyncScanPoliciesWorker.perform_async(configuration.id)
+        end
+      end
+
       def enqueue_product_analytics_event_metrics
         return unless project.product_analytics_enabled?
         return unless default_branch?

Verification steps