Merge branch '262112_populate_missing_dismissal_information_for_vulnerabilities' into 'master'

Populate missing dismissal information for vulnerabilities

See merge request gitlab-org/gitlab!46370

changelogs/unreleased/262112_populate_missing_dismissal_information_for_vulnerabilities.yml

+---
+title: Populate missing `dismissed_at` and `dismissed_by_id` attributes of vulnerabilities
+merge_request: 46370
+author:
+type: fixed

db/migrate/20201028160831_add_temporary_index_to_vulnerabilities_table.rb

+# frozen_string_literal: true
+
+class AddTemporaryIndexToVulnerabilitiesTable < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'temporary_index_vulnerabilities_on_id'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :vulnerabilities, :id, where: "state = 2 AND (dismissed_at IS NULL OR dismissed_by_id IS NULL)", name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :vulnerabilities, INDEX_NAME
+  end
+end

db/post_migrate/20201028160832_schedule_populate_missing_dismissal_information_for_vulnerabilities.rb

+# frozen_string_literal: true
+
+class SchedulePopulateMissingDismissalInformationForVulnerabilities < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 1_000
+  DELAY_INTERVAL = 3.minutes.to_i
+  MIGRATION_CLASS = 'PopulateMissingVulnerabilityDismissalInformation'
+
+  disable_ddl_transaction!
+
+  def up
+    ::Gitlab::BackgroundMigration::PopulateMissingVulnerabilityDismissalInformation::Vulnerability.broken.each_batch(of: BATCH_SIZE) do |batch, index|
+      vulnerability_ids = batch.pluck(:id)
+      migrate_in(index * DELAY_INTERVAL, MIGRATION_CLASS, vulnerability_ids)
+    end
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20201028160831

+4b0c70d8cd2648149011adab4f302922483436406f361c3037f26efb12b19042
\ No newline at end of file

db/schema_migrations/20201028160832

+9ea8e8f1234d6291ea00e725d380bfe33d804853b90da1221be8781b3dd9bb77
\ No newline at end of file

db/structure.sql

@@ -22200,6 +22200,8 @@ CREATE UNIQUE INDEX snippet_user_mentions_on_snippet_id_index ON snippet_user_me
 
 CREATE UNIQUE INDEX taggings_idx ON taggings USING btree (tag_id, taggable_id, taggable_type, context, tagger_id, tagger_type);
 
+CREATE INDEX temporary_index_vulnerabilities_on_id ON vulnerabilities USING btree (id) WHERE ((state = 2) AND ((dismissed_at IS NULL) OR (dismissed_by_id IS NULL)));
+
 CREATE UNIQUE INDEX term_agreements_unique_index ON term_agreements USING btree (user_id, term_id);
 
 CREATE INDEX terraform_state_versions_verification_checksum_partial ON terraform_state_versions USING btree (verification_checksum) WHERE (verification_checksum IS NOT NULL);

lib/gitlab/background_migration/populate_missing_vulnerability_dismissal_information.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # This class populates missing dismissal information for
+    # vulnerability entries.
+    class PopulateMissingVulnerabilityDismissalInformation
+      class Vulnerability < ActiveRecord::Base # rubocop:disable Style/Documentation
+        include EachBatch
+
+        self.table_name = 'vulnerabilities'
+
+        has_one :finding, class_name: '::Gitlab::BackgroundMigration::PopulateMissingVulnerabilityDismissalInformation::Finding'
+
+        scope :broken, -> { where('state = 2 AND (dismissed_at IS NULL OR dismissed_by_id IS NULL)') }
+
+        def copy_dismissal_information
+          return unless finding&.dismissal_feedback
+
+          update_columns(
+            dismissed_at: finding.dismissal_feedback.created_at,
+            dismissed_by_id: finding.dismissal_feedback.author_id
+          )
+        end
+      end
+
+      class Finding < ActiveRecord::Base # rubocop:disable Style/Documentation
+        include ShaAttribute
+
+        self.table_name = 'vulnerability_occurrences'
+
+        sha_attribute :project_fingerprint
+
+        def dismissal_feedback
+          Feedback.dismissal.where(category: report_type, project_fingerprint: project_fingerprint, project_id: project_id).first
+        end
+      end
+
+      class Feedback < ActiveRecord::Base # rubocop:disable Style/Documentation
+        DISMISSAL_TYPE = 0
+
+        self.table_name = 'vulnerability_feedback'
+
+        scope :dismissal, -> { where(feedback_type: DISMISSAL_TYPE) }
+      end
+
+      def perform(*vulnerability_ids)
+        Vulnerability.includes(:finding).where(id: vulnerability_ids).each { |vulnerability| populate_for(vulnerability) }
+
+        log_info(vulnerability_ids)
+      end
+
+      private
+
+      def populate_for(vulnerability)
+        log_warning(vulnerability) unless vulnerability.copy_dismissal_information
+      rescue StandardError => error
+        log_error(error, vulnerability)
+      end
+
+      def log_info(vulnerability_ids)
+        ::Gitlab::BackgroundMigration::Logger.info(
+          migrator: self.class.name,
+          message: 'Dismissal information has been copied',
+          count: vulnerability_ids.length
+        )
+      end
+
+      def log_warning(vulnerability)
+        ::Gitlab::BackgroundMigration::Logger.warn(
+          migrator: self.class.name,
+          message: 'Could not update vulnerability!',
+          vulnerability_id: vulnerability.id
+        )
+      end
+
+      def log_error(error, vulnerability)
+        ::Gitlab::BackgroundMigration::Logger.error(
+          migrator: self.class.name,
+          message: error.message,
+          vulnerability_id: vulnerability.id
+        )
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/populate_missing_vulnerability_dismissal_information_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::PopulateMissingVulnerabilityDismissalInformation, schema: 20201028160832 do
+  let(:users) { table(:users) }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:vulnerabilities) { table(:vulnerabilities) }
+  let(:findings) { table(:vulnerability_occurrences) }
+  let(:scanners) { table(:vulnerability_scanners) }
+  let(:identifiers) { table(:vulnerability_identifiers) }
+  let(:feedback) { table(:vulnerability_feedback) }
+
+  let(:user) { users.create!(name: 'test', email: 'test@example.com', projects_limit: 5) }
+  let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
+  let(:project) { projects.create!(namespace_id: namespace.id, name: 'foo') }
+  let(:vulnerability_1) { vulnerabilities.create!(title: 'title', state: 2, severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id) }
+  let(:vulnerability_2) { vulnerabilities.create!(title: 'title', state: 2, severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id) }
+  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: 'identifier') }
+
+  before do
+    feedback.create!(feedback_type: 0,
+                     category: 'sast',
+                     project_fingerprint: '418291a26024a1445b23fe64de9380cdcdfd1fa8',
+                     project_id: project.id,
+                     author_id: user.id,
+                     created_at: Time.current)
+
+    findings.create!(name: 'Finding',
+                     report_type: 'sast',
+                     project_fingerprint: Gitlab::Database::ShaAttribute.new.serialize('418291a26024a1445b23fe64de9380cdcdfd1fa8'),
+                     location_fingerprint: 'bar',
+                     severity: 1,
+                     confidence: 1,
+                     metadata_version: 1,
+                     raw_metadata: '',
+                     uuid: SecureRandom.uuid,
+                     project_id: project.id,
+                     vulnerability_id: vulnerability_1.id,
+                     scanner_id: scanner.id,
+                     primary_identifier_id: identifier.id)
+
+    allow(::Gitlab::BackgroundMigration::Logger).to receive_messages(info: true, warn: true, error: true)
+  end
+
+  describe '#perform' do
+    it 'updates the missing dismissal information of the vulnerability' do
+      expect { subject.perform(vulnerability_1.id, vulnerability_2.id) }.to change { vulnerability_1.reload.dismissed_at }.from(nil)
+                                                                        .and change { vulnerability_1.reload.dismissed_by_id }.from(nil).to(user.id)
+    end
+
+    it 'writes log messages' do
+      subject.perform(vulnerability_1.id, vulnerability_2.id)
+
+      expect(::Gitlab::BackgroundMigration::Logger).to have_received(:info).with(migrator: described_class.name,
+                                                                                 message: 'Dismissal information has been copied',
+                                                                                 count: 2)
+      expect(::Gitlab::BackgroundMigration::Logger).to have_received(:warn).with(migrator: described_class.name,
+                                                                                 message: 'Could not update vulnerability!',
+                                                                                 vulnerability_id: vulnerability_2.id)
+    end
+  end
+end

spec/migrations/schedule_populate_missing_dismissal_information_for_vulnerabilities_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe SchedulePopulateMissingDismissalInformationForVulnerabilities do
+  let(:users) { table(:users) }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:vulnerabilities) { table(:vulnerabilities) }
+  let(:user) { users.create!(name: 'test', email: 'test@example.com', projects_limit: 5) }
+  let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
+  let(:project) { projects.create!(namespace_id: namespace.id, name: 'foo') }
+
+  let!(:vulnerability_1) { vulnerabilities.create!(title: 'title', state: 2, severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id) }
+  let!(:vulnerability_2) { vulnerabilities.create!(title: 'title', state: 2, severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id, dismissed_at: Time.now) }
+  let!(:vulnerability_3) { vulnerabilities.create!(title: 'title', state: 2, severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id, dismissed_by_id: user.id) }
+  let!(:vulnerability_4) { vulnerabilities.create!(title: 'title', state: 2, severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id, dismissed_at: Time.now, dismissed_by_id: user.id) }
+  let!(:vulnerability_5) { vulnerabilities.create!(title: 'title', state: 1, severity: 0, confidence: 5, report_type: 2, project_id: project.id, author_id: user.id) }
+
+  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(3.minutes, vulnerability_1.id)
+    expect(described_class::MIGRATION_CLASS).to be_scheduled_delayed_migration(6.minutes, vulnerability_2.id)
+    expect(described_class::MIGRATION_CLASS).to be_scheduled_delayed_migration(9.minutes, vulnerability_3.id)
+  end
+end