Requeue backfill security policies background migration

This change requeues the BackfillSecurityPolicies migration
as it there were failures due to incorrect constraint
in description column.

EE: true
Changelog: changed

db/docs/batched_background_migrations/backfill_security_policies.yml

@@ -7,6 +7,6 @@ description: >-
 feature_category: security_policy_management
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/163945
 milestone: '17.6'
-queued_migration_version: 20241110174441
+queued_migration_version: 20241118124301
 finalize_after: '2024-12-22'
 finalized_by: # version of the migration that finalized this BBM

db/post_migrate/20241110174441_queue_backfill_security_policies.rb

@@ -3,25 +3,13 @@
 class QueueBackfillSecurityPolicies < Gitlab::Database::Migration[2.2]
   milestone '17.6'
 
-  restrict_gitlab_migration gitlab_schema: :gitlab_main
-
   MIGRATION = "BackfillSecurityPolicies"
-  DELAY_INTERVAL = 2.minutes
-  BATCH_SIZE = 100
-  SUB_BATCH_SIZE = 10
 
   def up
-    queue_batched_background_migration(
-      MIGRATION,
-      :security_orchestration_policy_configurations,
-      :id,
-      job_interval: DELAY_INTERVAL,
-      batch_size: BATCH_SIZE,
-      sub_batch_size: SUB_BATCH_SIZE
-    )
+    # no-op
   end
 
   def down
-    delete_batched_background_migration(MIGRATION, :security_orchestration_policy_configurations, :id, [])
+    # no-op
   end
 end

db/post_migrate/20241118124301_requeue_backfill_security_policies.rb

+# frozen_string_literal: true
+
+class RequeueBackfillSecurityPolicies < Gitlab::Database::Migration[2.2]
+  milestone '17.7'
+
+  restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+  MIGRATION = "BackfillSecurityPolicies"
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 100
+  SUB_BATCH_SIZE = 10
+
+  def up
+    delete_batched_background_migration(MIGRATION, :security_orchestration_policy_configurations, :id, [])
+
+    queue_batched_background_migration(
+      MIGRATION,
+      :security_orchestration_policy_configurations,
+      :id,
+      job_interval: DELAY_INTERVAL,
+      batch_size: BATCH_SIZE,
+      sub_batch_size: SUB_BATCH_SIZE
+    )
+  end
+
+  def down
+    delete_batched_background_migration(MIGRATION, :security_orchestration_policy_configurations, :id, [])
+  end
+end

db/schema_migrations/20241118124301

+e09162bb0ca18824b8ded0e27141cc195cee524a01eca146c9e568b554f983c9
\ No newline at end of file

ee/lib/ee/gitlab/background_migration/backfill_security_policies.rb

@@ -274,6 +274,7 @@ def all_projects
               Project.id_in(project_id)
             end
           end
+          strong_memoize_attr :all_projects
 
           def approval_policies
             SCAN_RESULT_POLICY_TYPES.flat_map do |type|
@@ -471,7 +472,6 @@ def applicable_for_group?(policy_scope)
         def perform
           each_sub_batch do |sub_batch|
             SecurityOrchestrationPolicyConfiguration.id_in(sub_batch).each do |policy_configuration|
-              next if policy_configuration.security_policies.any?
               next unless policy_configuration.policy_configuration_valid?
 
               create_policies(policy_configuration)
@@ -482,26 +482,35 @@ def perform
         private
 
         def create_policies(policy_configuration)
-          policy_configuration.approval_policies.each_with_index do |policy_hash, index|
-            security_policy = SecurityPolicy.create_policy(policy_configuration, :approval_policy, policy_hash, index)
-            link_policy_to_project(policy_configuration, security_policy)
-          end
+          approval_policies = policy_configuration.approval_policies
+          scan_execution_policies = policy_configuration.scan_execution_policies
+          pipeline_execution_policies = policy_configuration.pipeline_execution_policies
+          vulnerability_management_policies = policy_configuration.vulnerability_management_policies
+
+          db_policies = policy_configuration.security_policies
+          yaml_policies_count = approval_policies.count + scan_execution_policies.count +
+            pipeline_execution_policies.count + vulnerability_management_policies.count
+
+          # policies already persisted in database
+          return if db_policies.count == yaml_policies_count
+
+          create_policies_by_type(db_policies, policy_configuration, approval_policies, :approval_policy)
+          create_policies_by_type(db_policies, policy_configuration, scan_execution_policies, :scan_execution_policy)
+          create_policies_by_type(db_policies, policy_configuration, pipeline_execution_policies,
+            :pipeline_execution_policy)
+          create_policies_by_type(db_policies, policy_configuration, vulnerability_management_policies,
+            :vulnerability_management_policy)
+        end
 
-          policy_configuration.scan_execution_policies.each_with_index do |policy_hash, index|
-            security_policy = SecurityPolicy.create_policy(policy_configuration, :scan_execution_policy, policy_hash,
-              index)
-            link_policy_to_project(policy_configuration, security_policy)
-          end
+        def create_policies_by_type(db_policies, policy_configuration, yaml_policies, policy_type)
+          db_policies_by_index = db_policies
+            .select { |policy| policy.type == policy_type.to_s }
+            .group_by(&:policy_index)
 
-          policy_configuration.pipeline_execution_policies.each_with_index do |policy_hash, index|
-            security_policy = SecurityPolicy.create_policy(policy_configuration, :pipeline_execution_policy,
-              policy_hash, index)
-            link_policy_to_project(policy_configuration, security_policy)
-          end
+          yaml_policies.each_with_index do |policy_hash, index|
+            next if db_policies_by_index[index]
 
-          policy_configuration.vulnerability_management_policies.each_with_index do |policy_hash, index|
-            security_policy = SecurityPolicy.create_policy(policy_configuration, :vulnerability_management_policy,
-              policy_hash, index)
+            security_policy = SecurityPolicy.create_policy(policy_configuration, policy_type, policy_hash, index)
             link_policy_to_project(policy_configuration, security_policy)
           end
         end
@@ -519,6 +528,8 @@ def link_policy_to_project(policy_configuration, security_policy)
               { approval_policy_rule_id: policy_rule.id, project_id: project.id }
             end
 
+            next if attrs.empty?
+
             ApprovalPolicyRuleProjectLink.insert_all(attrs, unique_by: [:approval_policy_rule_id, :project_id])
           end
         end

ee/spec/lib/ee/gitlab/background_migration/backfill_security_policies_spec.rb

@@ -561,6 +561,47 @@
     end
   end
 
+  context 'when policies already exists in database' do
+    let(:policies) do
+      {
+        vulnerability_management_policy: [vulnerability_management_policy],
+        pipeline_execution_policy: [pipeline_execution_policy],
+        scan_result_policy: [scan_result_policy],
+        scan_execution_policy: [scan_execution_policy]
+      }
+    end
+
+    before do
+      create_policy(:approval_policy, scan_result_policy, 0)
+      create_policy(:scan_execution_policy, scan_execution_policy, 0)
+      create_policy(:pipeline_execution_policy, pipeline_execution_policy, 0)
+    end
+
+    context 'when some policies are persisted' do
+      it 'creates missing records', :aggregate_failures do
+        expect { perform_migration }.to change { security_policies.count }.by(1)
+          .and change { vulnerability_management_policy_rules.count }.by(1)
+          .and change { security_policy_project_links.count }.by(1)
+          .and not_change { approval_policy_rules.count }
+          .and not_change { scan_execution_policy_rules.count }
+          .and not_change { approval_policy_rule_project_links.count }
+      end
+    end
+
+    context 'when all policies are persisted' do
+      before do
+        create_policy(:vulnerability_management_policy, vulnerability_management_policy, 0)
+      end
+
+      it 'does not create any records', :aggregate_failures do
+        expect { perform_migration }.to not_change { security_policies.count }
+          .and not_change { vulnerability_management_policy_rules.count }
+          .and not_change { security_policy_project_links.count }
+          .and not_change { approval_policy_rule_project_links.count }
+      end
+    end
+  end
+
   def create_project(name, group)
     project_namespace = namespaces.create!(
       name: name,
@@ -576,4 +617,22 @@ def create_project(name, group)
       archived: true
     )
   end
+
+  def create_policy(policy_type, policy_hash, policy_index)
+    security_policies.create!(
+      {
+        type: described_class::SecurityPolicy.types[policy_type],
+        policy_index: policy_index,
+        name: policy_hash[:name],
+        description: policy_hash[:description],
+        enabled: policy_hash[:enabled],
+        metadata: policy_hash.fetch(:metadata, {}),
+        scope: policy_hash.fetch(:policy_scope, {}),
+        content: policy_hash.slice(*described_class::SecurityPolicy::POLICY_CONTENT_FIELDS[policy_type]),
+        checksum: Digest::SHA256.hexdigest(policy_hash.to_json),
+        security_orchestration_policy_configuration_id: security_policy_config.id,
+        security_policy_management_project_id: security_policy_config.security_policy_management_project_id
+      }
+    )
+  end
 end

spec/migrations/20241110174441_queue_backfill_security_policies_spec.rb

@@ -6,21 +6,14 @@
 RSpec.describe QueueBackfillSecurityPolicies, feature_category: :security_policy_management do
   let!(:batched_migration) { described_class::MIGRATION }
 
-  it 'schedules a new batched migration' do
+  it 'does not schedule a new batched migration' do
     reversible_migration do |migration|
       migration.before -> {
         expect(batched_migration).not_to have_scheduled_batched_migration
       }
 
       migration.after -> {
-        expect(batched_migration).to have_scheduled_batched_migration(
-          table_name: :security_orchestration_policy_configurations,
-          column_name: :id,
-          interval: described_class::DELAY_INTERVAL,
-          batch_size: described_class::BATCH_SIZE,
-          sub_batch_size: described_class::SUB_BATCH_SIZE,
-          gitlab_schema: :gitlab_main
-        )
+        expect(batched_migration).not_to have_scheduled_batched_migration
       }
     end
   end

spec/migrations/20241118124301_requeue_backfill_security_policies_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe RequeueBackfillSecurityPolicies, feature_category: :security_policy_management do
+  let!(:batched_migration) { described_class::MIGRATION }
+
+  it 'schedules a new batched migration' do
+    reversible_migration do |migration|
+      migration.before -> {
+        expect(batched_migration).not_to have_scheduled_batched_migration
+      }
+
+      migration.after -> {
+        expect(batched_migration).to have_scheduled_batched_migration(
+          table_name: :security_orchestration_policy_configurations,
+          column_name: :id,
+          interval: described_class::DELAY_INTERVAL,
+          batch_size: described_class::BATCH_SIZE,
+          sub_batch_size: described_class::SUB_BATCH_SIZE,
+          gitlab_schema: :gitlab_main
+        )
+      }
+    end
+  end
+end