From 4ff82387edfac3c37dd04194af1abd54830e47d4 Mon Sep 17 00:00:00 2001
From: can eldem <eldemcan@gmail.com>
Date: Mon, 7 Sep 2020 12:54:22 +0100
Subject: [PATCH] Add sha field for existing background migration
Make old migration no op
---
...te_location_fingerprint_for_cs_findings.rb | 13 +++-----
...te_location_fingerprint_for_cs_findings.rb | 33 +++++++++++++++++++
db/schema_migrations/20200907123723 | 1 +
.../add-sha-for-background-migration.yml | 5 +++
...te_location_fingerprint_for_cs_findings.rb | 9 ++++-
...cation_fingerprint_for_cs_findings_spec.rb | 4 +--
...ation_fingerprint_for_cs_findings_spec.rb} | 12 +++----
7 files changed, 59 insertions(+), 18 deletions(-)
create mode 100644 db/post_migrate/20200907123723_fix_update_location_fingerprint_for_cs_findings.rb
create mode 100644 db/schema_migrations/20200907123723
create mode 100644 ee/changelogs/unreleased/add-sha-for-background-migration.yml
rename ee/spec/migrations/{20200824140259_update_location_fingerprint_for_cs_findings_spec.rb => 20200907123723_fix_update_location_fingerprint_for_cs_findings_spec.rb} (83%)
diff --git a/db/post_migrate/20200824140259_update_location_fingerprint_for_cs_findings.rb b/db/post_migrate/20200824140259_update_location_fingerprint_for_cs_findings.rb
index 7ef4d5040b289..d261e713b83df 100644
--- a/db/post_migrate/20200824140259_update_location_fingerprint_for_cs_findings.rb
+++ b/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
diff --git a/db/post_migrate/20200907123723_fix_update_location_fingerprint_for_cs_findings.rb b/db/post_migrate/20200907123723_fix_update_location_fingerprint_for_cs_findings.rb
new file mode 100644
index 0000000000000..064ee64f3afbc
--- /dev/null
+++ b/db/post_migrate/20200907123723_fix_update_location_fingerprint_for_cs_findings.rb
@@ -0,0 +1,33 @@
+# 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
diff --git a/db/schema_migrations/20200907123723 b/db/schema_migrations/20200907123723
new file mode 100644
index 0000000000000..1a6288ee6c995
--- /dev/null
+++ b/db/schema_migrations/20200907123723
@@ -0,0 +1 @@
+74f6adf29b9293537d653b993ab0e2f31af2ab5a8581ca631e8a87fc589ca284
\ No newline at end of file
diff --git a/ee/changelogs/unreleased/add-sha-for-background-migration.yml b/ee/changelogs/unreleased/add-sha-for-background-migration.yml
new file mode 100644
index 0000000000000..9aedd29204654
--- /dev/null
+++ b/ee/changelogs/unreleased/add-sha-for-background-migration.yml
@@ -0,0 +1,5 @@
+---
+title: Fix Update location fingerprint for existing CS vulnerabilities
+merge_request: 41671
+author:
+type: fixed
diff --git a/ee/lib/ee/gitlab/background_migration/update_location_fingerprint_for_cs_findings.rb b/ee/lib/ee/gitlab/background_migration/update_location_fingerprint_for_cs_findings.rb
index b7de7bbad32f9..53a06bcc51a40 100644
--- a/ee/lib/ee/gitlab/background_migration/update_location_fingerprint_for_cs_findings.rb
+++ b/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
diff --git a/ee/spec/lib/ee/gitlab/background_migration/update_location_fingerprint_for_cs_findings_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/update_location_fingerprint_for_cs_findings_spec.rb
index ec967e64760c5..acf9890cde13f 100644
--- a/ee/spec/lib/ee/gitlab/background_migration/update_location_fingerprint_for_cs_findings_spec.rb
+++ b/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,
diff --git a/ee/spec/migrations/20200824140259_update_location_fingerprint_for_cs_findings_spec.rb b/ee/spec/migrations/20200907123723_fix_update_location_fingerprint_for_cs_findings_spec.rb
similarity index 83%
rename from ee/spec/migrations/20200824140259_update_location_fingerprint_for_cs_findings_spec.rb
rename to ee/spec/migrations/20200907123723_fix_update_location_fingerprint_for_cs_findings_spec.rb
index efb01d60217dc..bee4ed806915d 100644
--- a/ee/spec/migrations/20200824140259_update_location_fingerprint_for_cs_findings_spec.rb
+++ b/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,
--
GitLab