Merge branch 'add-sha-for-background-migration' into 'master'

Add sha field for existing background migration

See merge request gitlab-org/gitlab!41671

db/post_migrate/20200824140259_update_location_fingerprint_for_cs_findings.rb

@@ -12,15 +12,10 @@ class UpdateLocationFingerprintForCsFindings < ActiveRecord::Migration[6.0]
 
   # 815_565 records
   def up
-    return unless Gitlab.ee?
-
-    migration = Gitlab::BackgroundMigration::UpdateLocationFingerprintForCsFindings
-    migration_name = migration.to_s.demodulize
-    relation = migration::Finding.container_scanning
-    queue_background_migration_jobs_by_range_at_intervals(relation,
-                                                          migration_name,
-                                                          INTERVAL,
-                                                          batch_size: BATCH_SIZE)
+    # no-op
+    # There was a bug introduced with this migration for gitlab.com
+    # We created new migration to mitigate that VERISON=20200907123723
+    # and change this one to no-op to prevent running migration twice
   end
 
   def down

db/post_migrate/20200907123723_fix_update_location_fingerprint_for_cs_findings.rb

+# frozen_string_literal: true
+
+class FixUpdateLocationFingerprintForCsFindings < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  BATCH_SIZE = 1_000
+  INTERVAL = 2.minutes
+
+  # 815_565 records
+  def up
+    return unless Gitlab.ee?
+
+    migration = Gitlab::BackgroundMigration::UpdateLocationFingerprintForCsFindings
+    migration_name = migration.to_s.demodulize
+
+    Gitlab::BackgroundMigration.steal(migration_name)
+
+    relation = migration::Finding.container_scanning
+    queue_background_migration_jobs_by_range_at_intervals(relation,
+                                                          migration_name,
+                                                          INTERVAL,
+                                                          batch_size: BATCH_SIZE)
+  end
+
+  def down
+    # no-op
+    # intentionally blank
+  end
+end

db/schema_migrations/20200907123723

+74f6adf29b9293537d653b993ab0e2f31af2ab5a8581ca631e8a87fc589ca284
\ No newline at end of file

ee/changelogs/unreleased/add-sha-for-background-migration.yml

+---
+title: Fix Update location fingerprint for existing CS vulnerabilities
+merge_request: 41671
+author:
+type: fixed

ee/lib/ee/gitlab/background_migration/update_location_fingerprint_for_cs_findings.rb

@@ -7,6 +7,7 @@ module UpdateLocationFingerprintForCsFindings
         extend ::Gitlab::Utils::Override
 
         class Finding < ActiveRecord::Base
+          include ::ShaAttribute
           include ::EachBatch
 
           self.table_name = 'vulnerability_occurrences'
@@ -17,6 +18,8 @@ class Finding < ActiveRecord::Base
 
           enum report_type: REPORT_TYPES
 
+          sha_attribute :location_fingerprint
+
           # Copied from Reports::Security::Locations
           def calculate_new_fingerprint(image, package_name)
             return if image.nil? || package_name.nil?
@@ -58,7 +61,11 @@ def perform(start_id, stop_id)
 
                     next if new_fingerprint.nil?
 
-                    finding.update_column(:location_fingerprint, new_fingerprint)
+                    begin
+                      finding.update_column(:location_fingerprint, new_fingerprint)
+                    rescue ActiveRecord::RecordNotUnique
+                      Gitlab::BackgroundMigration::Logger.warn("Duplicate finding found with finding id #{finding.id}")
+                    end
                   end
         end
       end

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

@@ -35,13 +35,13 @@
 
     described_class.new.perform(vul1.id, vul3.id)
 
-    location_fingerprints = findings.select(:location_fingerprint).pluck(:location_fingerprint)
+    location_fingerprints = findings.pluck(:location_fingerprint).flat_map { |x| Gitlab::Database::ShaAttribute.new.deserialize(x) }
 
     expect(location_fingerprints).to match_array(new_fingerprints)
   end
 
   def create_identifier(number_of)
-    (1..number_of).to_a.each do |identifier_id|
+    (1..number_of).each do |identifier_id|
       identifiers.create!(id: identifier_id,
                           project_id: 123,
                           fingerprint: 'd432c2ad2953e8bd587a3a43b3ce309b5b0154c' + identifier_id.to_s,

ee/spec/migrations/20200824140259_update_location_fingerprint_for_cs_findings_spec.rb → ee/spec/migrations/20200907123723_fix_update_location_fingerprint_for_cs_findings_spec.rb

@@ -2,9 +2,9 @@
 
 require 'spec_helper'
 
-require Rails.root.join('db', 'post_migrate', '20200824140259_update_location_fingerprint_for_cs_findings.rb')
+require Rails.root.join('db', 'post_migrate', '20200907123723_fix_update_location_fingerprint_for_cs_findings.rb')
 
-RSpec.describe UpdateLocationFingerprintForCsFindings, :migration do
+RSpec.describe FixUpdateLocationFingerprintForCsFindings, :migration do
   let(:namespaces) { table(:namespaces) }
   let(:users) { table(:users) }
   let(:group) { namespaces.create!(name: 'foo', path: 'foo') }
@@ -43,7 +43,7 @@
 
     migrate!
 
-    location_fingerprints = findings.select(:location_fingerprint).pluck(:location_fingerprint)
+    location_fingerprints = findings.pluck(:location_fingerprint).flat_map { |x| Gitlab::Database::ShaAttribute.new.deserialize(x) }
 
     expect(location_fingerprints).to match_array(new_fingerprints)
   end
@@ -56,17 +56,17 @@
     findings.create!(finding_params(1))
     findings.create!(finding_params(2))
 
-    before_location_fingerprints = findings.select(:location_fingerprint).pluck(:location_fingerprint)
+    before_location_fingerprints = findings.pluck(:location_fingerprint).flat_map { |x| Gitlab::Database::ShaAttribute.new.deserialize(x) }
 
     migrate!
 
-    after_location_fingerprints = findings.select(:location_fingerprint).pluck(:location_fingerprint)
+    after_location_fingerprints = findings.pluck(:location_fingerprint).flat_map { |x| Gitlab::Database::ShaAttribute.new.deserialize(x) }
 
     expect(after_location_fingerprints).to match_array(before_location_fingerprints)
   end
 
   def create_identifier(number_of)
-    (1..number_of).to_a.each do |identifier_id|
+    (1..number_of).each do |identifier_id|
       identifiers.create!(id: identifier_id,
                           project_id: 123,
                           fingerprint: 'd432c2ad2953e8bd587a3a43b3ce309b5b0154c' + identifier_id.to_s,