Commit 4ff82387 authored by can eldem's avatar can eldem

Add sha field for existing background migration

Make old migration no op
parent 60a89b28
...@@ -12,15 +12,10 @@ class UpdateLocationFingerprintForCsFindings < ActiveRecord::Migration[6.0] ...@@ -12,15 +12,10 @@ class UpdateLocationFingerprintForCsFindings < ActiveRecord::Migration[6.0]
# 815_565 records # 815_565 records
def up def up
return unless Gitlab.ee? # no-op
# There was a bug introduced with this migration for gitlab.com
migration = Gitlab::BackgroundMigration::UpdateLocationFingerprintForCsFindings # We created new migration to mitigate that VERISON=20200907123723
migration_name = migration.to_s.demodulize # and change this one to no-op to prevent running migration twice
relation = migration::Finding.container_scanning
queue_background_migration_jobs_by_range_at_intervals(relation,
migration_name,
INTERVAL,
batch_size: BATCH_SIZE)
end end
def down def down
......
# 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
74f6adf29b9293537d653b993ab0e2f31af2ab5a8581ca631e8a87fc589ca284
\ No newline at end of file
---
title: Fix Update location fingerprint for existing CS vulnerabilities
merge_request: 41671
author:
type: fixed
...@@ -7,6 +7,7 @@ module EE ...@@ -7,6 +7,7 @@ module EE
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
class Finding < ActiveRecord::Base class Finding < ActiveRecord::Base
include ::ShaAttribute
include ::EachBatch include ::EachBatch
self.table_name = 'vulnerability_occurrences' self.table_name = 'vulnerability_occurrences'
...@@ -17,6 +18,8 @@ module EE ...@@ -17,6 +18,8 @@ module EE
enum report_type: REPORT_TYPES enum report_type: REPORT_TYPES
sha_attribute :location_fingerprint
# Copied from Reports::Security::Locations # Copied from Reports::Security::Locations
def calculate_new_fingerprint(image, package_name) def calculate_new_fingerprint(image, package_name)
return if image.nil? || package_name.nil? return if image.nil? || package_name.nil?
...@@ -58,7 +61,11 @@ module EE ...@@ -58,7 +61,11 @@ module EE
next if new_fingerprint.nil? 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 end
end end
......
...@@ -35,13 +35,13 @@ RSpec.describe Gitlab::BackgroundMigration::UpdateLocationFingerprintForCsFindin ...@@ -35,13 +35,13 @@ RSpec.describe Gitlab::BackgroundMigration::UpdateLocationFingerprintForCsFindin
described_class.new.perform(vul1.id, vul3.id) 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) expect(location_fingerprints).to match_array(new_fingerprints)
end end
def create_identifier(number_of) 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, identifiers.create!(id: identifier_id,
project_id: 123, project_id: 123,
fingerprint: 'd432c2ad2953e8bd587a3a43b3ce309b5b0154c' + identifier_id.to_s, fingerprint: 'd432c2ad2953e8bd587a3a43b3ce309b5b0154c' + identifier_id.to_s,
......
...@@ -2,9 +2,9 @@ ...@@ -2,9 +2,9 @@
require 'spec_helper' 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(:namespaces) { table(:namespaces) }
let(:users) { table(:users) } let(:users) { table(:users) }
let(:group) { namespaces.create!(name: 'foo', path: 'foo') } let(:group) { namespaces.create!(name: 'foo', path: 'foo') }
...@@ -43,7 +43,7 @@ RSpec.describe UpdateLocationFingerprintForCsFindings, :migration do ...@@ -43,7 +43,7 @@ RSpec.describe UpdateLocationFingerprintForCsFindings, :migration do
migrate! 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) expect(location_fingerprints).to match_array(new_fingerprints)
end end
...@@ -56,17 +56,17 @@ RSpec.describe UpdateLocationFingerprintForCsFindings, :migration do ...@@ -56,17 +56,17 @@ RSpec.describe UpdateLocationFingerprintForCsFindings, :migration do
findings.create!(finding_params(1)) findings.create!(finding_params(1))
findings.create!(finding_params(2)) 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! 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) expect(after_location_fingerprints).to match_array(before_location_fingerprints)
end end
def create_identifier(number_of) 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, identifiers.create!(id: identifier_id,
project_id: 123, project_id: 123,
fingerprint: 'd432c2ad2953e8bd587a3a43b3ce309b5b0154c' + identifier_id.to_s, fingerprint: 'd432c2ad2953e8bd587a3a43b3ce309b5b0154c' + identifier_id.to_s,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment