Merge branch '277134_populate_finding_uuid_values_for_vulnerability_feedback' into 'master'

Populate `finding_uuid` attribute for vulnerability_feedback

See merge request gitlab-org/gitlab!49807

changelogs/unreleased/277134_populate_finding_uuid_values_for_vulnerability_feedback.yml

+---
+title: Populate `finding_uuid` attribute for the existing `vulnerability_feedback`
+  records
+merge_request: 49807
+author:
+type: added

db/post_migrate/20201211090634_schedule_populate_finding_uuid_for_vulnerability_feedback.rb

+# frozen_string_literal: true
+
+class SchedulePopulateFindingUuidForVulnerabilityFeedback < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  MIGRATION_CLASS = 'PopulateFindingUuidForVulnerabilityFeedback'
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 1000
+
+  disable_ddl_transaction!
+
+  def up
+    queue_background_migration_jobs_by_range_at_intervals(
+      Gitlab::BackgroundMigration::PopulateFindingUuidForVulnerabilityFeedback::VulnerabilityFeedback,
+      MIGRATION_CLASS,
+      DELAY_INTERVAL,
+      batch_size: BATCH_SIZE
+    )
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20201211090634

+5117b71950bec3c6c746eaf4851c111a335bf2280075ddc2c73b60a30e23d907
\ No newline at end of file

lib/gitlab/background_migration/populate_finding_uuid_for_vulnerability_feedback.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # This class populates the `finding_uuid` attribute for
+    # the existing `vulnerability_feedback` records.
+    class PopulateFindingUuidForVulnerabilityFeedback
+      REPORT_TYPES = {
+        sast: 0,
+        dependency_scanning: 1,
+        container_scanning: 2,
+        dast: 3,
+        secret_detection: 4,
+        coverage_fuzzing: 5,
+        api_fuzzing: 6
+      }.freeze
+
+      class VulnerabilityFeedback < ActiveRecord::Base # rubocop:disable Style/Documentation
+        include EachBatch
+
+        self.table_name = 'vulnerability_feedback'
+
+        enum category: REPORT_TYPES
+
+        scope :in_range, -> (start, stop) { where(id: start..stop) }
+        scope :without_uuid, -> { where(finding_uuid: nil) }
+
+        def self.load_vulnerability_findings
+          all.to_a.tap { |collection| collection.each(&:vulnerability_finding) }
+        end
+
+        def set_finding_uuid
+          return unless vulnerability_finding.present? && vulnerability_finding.primary_identifier.present?
+
+          update_column(:finding_uuid, calculated_uuid)
+        rescue StandardError => error
+          Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error)
+        end
+
+        def vulnerability_finding
+          BatchLoader.for(finding_key).batch(replace_methods: false) do |finding_keys, loader|
+            project_ids = finding_keys.map { |key| key[:project_id] }
+            categories = finding_keys.map { |key| key[:category] }
+            fingerprints = finding_keys.map { |key| key[:project_fingerprint] }
+
+            findings = Finding.with_primary_identifier.where(
+              project_id: project_ids.uniq,
+              report_type: categories.uniq,
+              project_fingerprint: fingerprints.uniq
+            ).to_a
+
+            finding_keys.each do |finding_key|
+              loader.call(
+                finding_key,
+                findings.find { |f| finding_key == f.finding_key }
+              )
+            end
+          end
+        end
+
+        private
+
+        def calculated_uuid
+          Gitlab::UUID.v5(uuid_components)
+        end
+
+        def uuid_components
+          [
+            category,
+            vulnerability_finding.primary_identifier.fingerprint,
+            vulnerability_finding.location_fingerprint,
+            project_id
+          ].join('-')
+        end
+
+        def finding_key
+          {
+            project_id: project_id,
+            category: category,
+            project_fingerprint: project_fingerprint
+          }
+        end
+      end
+
+      class Finding < ActiveRecord::Base # rubocop:disable Style/Documentation
+        include ShaAttribute
+
+        self.table_name = 'vulnerability_occurrences'
+
+        sha_attribute :project_fingerprint
+        sha_attribute :location_fingerprint
+
+        belongs_to :primary_identifier, class_name: 'Gitlab::BackgroundMigration::PopulateFindingUuidForVulnerabilityFeedback::Identifier'
+
+        enum report_type: REPORT_TYPES
+
+        scope :with_primary_identifier, -> { includes(:primary_identifier) }
+
+        def finding_key
+          {
+            project_id: project_id,
+            category: report_type,
+            project_fingerprint: project_fingerprint
+          }
+        end
+      end
+
+      class Identifier < ActiveRecord::Base # rubocop:disable Style/Documentation
+        self.table_name = 'vulnerability_identifiers'
+      end
+
+      def perform(*range)
+        feedback = VulnerabilityFeedback.without_uuid.in_range(*range).load_vulnerability_findings
+        feedback.each(&:set_finding_uuid)
+
+        log_info(feedback.count)
+      end
+
+      def log_info(feedback_count)
+        ::Gitlab::BackgroundMigration::Logger.info(
+          migrator: self.class.name,
+          message: '`finding_uuid` attributes has been set',
+          count: feedback_count
+        )
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/populate_finding_uuid_for_vulnerability_feedback_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::PopulateFindingUuidForVulnerabilityFeedback, schema: 20201211090634 do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:users) { table(:users) }
+  let(:scanners) { table(:vulnerability_scanners) }
+  let(:identifiers) { table(:vulnerability_identifiers) }
+  let(:findings) { table(:vulnerability_occurrences) }
+  let(:vulnerability_feedback) { table(:vulnerability_feedback) }
+
+  let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
+  let(:project) { projects.create!(namespace_id: namespace.id, name: 'foo') }
+  let(:user) { users.create!(username: 'john.doe', projects_limit: 5) }
+  let(:scanner) { scanners.create!(project_id: project.id, external_id: 'foo', name: 'bar') }
+  let(:identifier) { identifiers.create!(project_id: project.id, fingerprint: 'foo', external_type: 'bar', external_id: 'zoo', name: 'baz') }
+  let(:sast_report) { 0 }
+  let(:dependency_scanning_report) { 1 }
+  let(:dast_report) { 3 }
+  let(:secret_detection_report) { 4 }
+  let(:project_fingerprint) { Digest::SHA1.hexdigest(SecureRandom.uuid) }
+  let(:location_fingerprint_1) { Digest::SHA1.hexdigest(SecureRandom.uuid) }
+  let(:location_fingerprint_2) { Digest::SHA1.hexdigest(SecureRandom.uuid) }
+  let(:location_fingerprint_3) { Digest::SHA1.hexdigest(SecureRandom.uuid) }
+  let(:finding_1) { finding_creator.call(sast_report, location_fingerprint_1) }
+  let(:finding_2) { finding_creator.call(dast_report, location_fingerprint_2) }
+  let(:finding_3) { finding_creator.call(secret_detection_report, location_fingerprint_3) }
+  let(:uuid_1_components) { ['sast', identifier.fingerprint, location_fingerprint_1, project.id].join('-') }
+  let(:uuid_2_components) { ['dast', identifier.fingerprint, location_fingerprint_2, project.id].join('-') }
+  let(:uuid_3_components) { ['secret_detection', identifier.fingerprint, location_fingerprint_3, project.id].join('-') }
+  let(:expected_uuid_1) { Gitlab::UUID.v5(uuid_1_components) }
+  let(:expected_uuid_2) { Gitlab::UUID.v5(uuid_2_components) }
+  let(:expected_uuid_3) { Gitlab::UUID.v5(uuid_3_components) }
+  let(:finding_creator) do
+    -> (report_type, location_fingerprint) do
+      findings.create!(
+        project_id: project.id,
+        primary_identifier_id: identifier.id,
+        scanner_id: scanner.id,
+        report_type: report_type,
+        uuid: SecureRandom.uuid,
+        name: 'Foo',
+        location_fingerprint: Gitlab::Database::ShaAttribute.serialize(location_fingerprint),
+        project_fingerprint: Gitlab::Database::ShaAttribute.serialize(project_fingerprint),
+        metadata_version: '1',
+        severity: 0,
+        confidence: 5,
+        raw_metadata: '{}'
+      )
+    end
+  end
+
+  let(:feedback_creator) do
+    -> (category, project_fingerprint) do
+      vulnerability_feedback.create!(
+        project_id: project.id,
+        author_id: user.id,
+        feedback_type: 0,
+        category: category,
+        project_fingerprint: project_fingerprint
+      )
+    end
+  end
+
+  let!(:feedback_1) { feedback_creator.call(finding_1.report_type, project_fingerprint) }
+  let!(:feedback_2) { feedback_creator.call(finding_2.report_type, project_fingerprint) }
+  let!(:feedback_3) { feedback_creator.call(finding_3.report_type, project_fingerprint) }
+  let!(:feedback_4) { feedback_creator.call(finding_1.report_type, 'foo') }
+  let!(:feedback_5) { feedback_creator.call(dependency_scanning_report, project_fingerprint) }
+
+  subject(:populate_finding_uuids) { described_class.new.perform(feedback_1.id, feedback_5.id) }
+
+  before do
+    allow(Gitlab::BackgroundMigration::Logger).to receive(:info)
+  end
+
+  describe '#perform' do
+    it 'updates the `finding_uuid` attributes of the feedback records' do
+      expect { populate_finding_uuids }.to change { feedback_1.reload.finding_uuid }.from(nil).to(expected_uuid_1)
+                                       .and change { feedback_2.reload.finding_uuid }.from(nil).to(expected_uuid_2)
+                                       .and change { feedback_3.reload.finding_uuid }.from(nil).to(expected_uuid_3)
+                                       .and not_change { feedback_4.reload.finding_uuid }
+                                       .and not_change { feedback_5.reload.finding_uuid }
+
+      expect(Gitlab::BackgroundMigration::Logger).to have_received(:info).once
+    end
+
+    it 'preloads the finding and identifier records to prevent N+1 queries' do
+      # Load feedback records(1), load findings(2), load identifiers(3) and finally update feedback records one by one(6)
+      expect { populate_finding_uuids }.not_to exceed_query_limit(6)
+    end
+
+    context 'when setting the `finding_uuid` attribute of a feedback record fails' do
+      let(:expected_error) { RuntimeError.new }
+
+      before do
+        allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
+
+        allow_next_found_instance_of(described_class::VulnerabilityFeedback) do |feedback|
+          allow(feedback).to receive(:update_column).and_raise(expected_error)
+        end
+      end
+
+      it 'captures the errors and does not crash entirely' do
+        expect { populate_finding_uuids }.not_to raise_error
+
+        expect(Gitlab::ErrorTracking).to have_received(:track_and_raise_for_dev_exception).with(expected_error).exactly(3).times
+      end
+    end
+  end
+end

spec/migrations/schedule_populate_finding_uuid_for_vulnerability_feedback_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe SchedulePopulateFindingUuidForVulnerabilityFeedback do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:users) { table(:users) }
+  let(:vulnerability_feedback) { table(:vulnerability_feedback) }
+
+  let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
+  let(:project) { projects.create!(namespace_id: namespace.id, name: 'foo') }
+  let(:user) { users.create!(username: 'john.doe', projects_limit: 1) }
+
+  let(:common_feedback_params) { { feedback_type: 0, category: 0, project_id: project.id, author_id: user.id } }
+  let!(:feedback_1) { vulnerability_feedback.create!(**common_feedback_params, project_fingerprint: 'foo') }
+  let!(:feedback_2) { vulnerability_feedback.create!(**common_feedback_params, project_fingerprint: 'bar') }
+  let!(:feedback_3) { vulnerability_feedback.create!(**common_feedback_params, project_fingerprint: 'zoo', finding_uuid: SecureRandom.uuid) }
+
+  around do |example|
+    freeze_time { Sidekiq::Testing.fake! { example.run } }
+  end
+
+  before do
+    stub_const("#{described_class.name}::BATCH_SIZE", 1)
+  end
+
+  it 'schedules the background jobs', :aggregate_failures do
+    migrate!
+
+    expect(BackgroundMigrationWorker.jobs.size).to be(3)
+    expect(described_class::MIGRATION_CLASS).to be_scheduled_delayed_migration(2.minutes, feedback_1.id, feedback_1.id)
+    expect(described_class::MIGRATION_CLASS).to be_scheduled_delayed_migration(4.minutes, feedback_2.id, feedback_2.id)
+    expect(described_class::MIGRATION_CLASS).to be_scheduled_delayed_migration(6.minutes, feedback_3.id, feedback_3.id)
+  end
+end