Merge branch...

Merge branch '280589-set-vulnerabilities-as-dismissed-when-they-have-dismissed-feedback' into 'master'

Set vulnerability as dismissed when there is dismissal feedback

See merge request gitlab-org/gitlab!48795

changelogs/unreleased/280589-set-vulnerabilities-as-dismissed-when-they-have-dismissed-feedback.yml

+---
+title: Set vulnerability as dismissed when there is dismissal feedback
+merge_request: 48795
+author:
+type: added

db/post_migrate/20201130103926_schedule_populate_dismissed_state_for_vulnerabilities.rb

+# frozen_string_literal: true
+
+class SchedulePopulateDismissedStateForVulnerabilities < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  TMP_INDEX_NAME = 'tmp_index_on_vulnerabilities_non_dismissed'
+
+  DOWNTIME = false
+  BATCH_SIZE = 1_000
+  VULNERABILITY_BATCH_SIZE = 5_000
+  DELAY_INTERVAL = 3.minutes.to_i
+  MIGRATION_CLASS = 'PopulateDismissedStateForVulnerabilities'
+
+  VULNERABILITY_JOIN_CONDITION = 'JOIN "vulnerability_occurrences" ON "vulnerability_occurrences"."vulnerability_id" = "vulnerabilities"."id"'
+  FEEDBACK_WHERE_CONDITION = <<~SQL
+    EXISTS (SELECT 1 FROM vulnerability_feedback
+      WHERE "vulnerability_occurrences"."project_id" = "vulnerability_feedback"."project_id"
+      AND "vulnerability_occurrences"."report_type" = "vulnerability_feedback"."category"
+      AND ENCODE("vulnerability_occurrences"."project_fingerprint", 'hex') = "vulnerability_feedback"."project_fingerprint"
+      AND "vulnerability_feedback"."feedback_type" = 0
+    )
+  SQL
+
+  class Vulnerability < ActiveRecord::Base # rubocop:disable Style/Documentation
+    include EachBatch
+
+    self.table_name = 'vulnerabilities'
+  end
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(:vulnerabilities, :id, where: 'state <> 2', name: TMP_INDEX_NAME)
+
+    batch = []
+    index = 1
+
+    Vulnerability.where('state <> 2').each_batch(of: VULNERABILITY_BATCH_SIZE) do |relation|
+      ids = relation
+        .joins(VULNERABILITY_JOIN_CONDITION)
+        .where(FEEDBACK_WHERE_CONDITION)
+        .pluck('vulnerabilities.id')
+
+      ids.each do |id|
+        batch << id
+
+        if batch.size == BATCH_SIZE
+          migrate_in(index * DELAY_INTERVAL, MIGRATION_CLASS, batch)
+          index += 1
+
+          batch.clear
+        end
+      end
+    end
+
+    migrate_in(index * DELAY_INTERVAL, MIGRATION_CLASS, batch) unless batch.empty?
+  end
+
+  def down
+    remove_concurrent_index_by_name(:vulnerabilities, TMP_INDEX_NAME)
+  end
+end

db/schema_migrations/20201130103926

+27cd7e7cd01175c157e6aa666b2263bf29210277d5acd997a0619cee67870345
\ No newline at end of file

db/structure.sql

@@ -22876,6 +22876,8 @@ CREATE INDEX tmp_build_stage_position_index ON ci_builds USING btree (stage_id,
 
 CREATE INDEX tmp_index_for_email_unconfirmation_migration ON emails USING btree (id) WHERE (confirmed_at IS NOT NULL);
 
+CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING btree (id) WHERE (state <> 2);
+
 CREATE UNIQUE INDEX unique_merge_request_metrics_by_merge_request_id ON merge_request_metrics USING btree (merge_request_id);
 
 CREATE UNIQUE INDEX vulnerability_feedback_unique_idx ON vulnerability_feedback USING btree (project_id, category, feedback_type, project_fingerprint);

ee/spec/migrations/schedule_populate_dismissed_state_for_vulnerabilities_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20201130103926_schedule_populate_dismissed_state_for_vulnerabilities.rb')
+
+RSpec.describe SchedulePopulateDismissedStateForVulnerabilities do
+  let(:users) { table(:users) }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:vulnerabilities) { table(:vulnerabilities) }
+  let(:findings) { table(:vulnerability_occurrences) }
+  let(:feedback) { table(:vulnerability_feedback) }
+  let(:identifiers) { table(:vulnerability_identifiers) }
+  let(:scanners) { table(:vulnerability_scanners) }
+
+  let!(:namespace) { namespaces.create!(name: "foo", path: "bar") }
+  let!(:user) { users.create!(name: 'John Doe', email: 'test@example.com', projects_limit: 5) }
+  let!(:project) { projects.create!(namespace_id: namespace.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') }
+  let!(:vulnerability_params) do
+    {
+      project_id: project.id,
+      author_id: user.id,
+      title: 'Vulnerability',
+      severity: 5,
+      confidence: 5,
+      report_type: 5
+    }
+  end
+
+  let!(:identifier_1) { identifiers.create!(project_id: project.id, fingerprint: 'foo1', external_type: 'bar', external_id: 'zoo', name: 'identifier') }
+  let!(:identifier_2) { identifiers.create!(project_id: project.id, fingerprint: 'foo2', external_type: 'bar', external_id: 'zoo', name: 'identifier') }
+  let!(:identifier_3) { identifiers.create!(project_id: project.id, fingerprint: 'foo3', external_type: 'bar', external_id: 'zoo', name: 'identifier') }
+
+  let!(:feedback_1) { feedback.create!(feedback_type: 0, category: 'sast', project_fingerprint: '418291a26024a1445b23fe64de9380cdcdfd1fa8', project_id: project.id, author_id: user.id) }
+  let!(:feedback_2) { feedback.create!(feedback_type: 0, category: 'dast', project_fingerprint: 'a98c8aed53514eddba2976b942162bf3418291a2', project_id: project.id, author_id: user.id) }
+  let!(:feedback_3) { feedback.create!(feedback_type: 0, category: 'dast', project_fingerprint: 'a98c8aed53514eddba2976b942162bf3418291a3', project_id: project.id, author_id: user.id) }
+
+  let!(:vulnerability_1) { vulnerabilities.create!(vulnerability_params.merge(state: 1)) }
+  let!(:vulnerability_2) { vulnerabilities.create!(vulnerability_params.merge(state: 3)) }
+  let!(:vulnerability_3) { vulnerabilities.create!(vulnerability_params.merge(state: 2)) }
+
+  let!(:finding_1) do
+    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_1.id)
+  end
+
+  let!(:finding_2) do
+    findings.create!(name: 'Finding',
+      report_type: 'dast',
+      project_fingerprint: Gitlab::Database::ShaAttribute.new.serialize('a98c8aed53514eddba2976b942162bf3418291a2'),
+      location_fingerprint: 'bar',
+      severity: 1,
+      confidence: 1,
+      metadata_version: 1,
+      raw_metadata: '',
+      uuid: SecureRandom.uuid,
+      project_id: project.id,
+      vulnerability_id: vulnerability_2.id,
+      scanner_id: scanner.id,
+      primary_identifier_id: identifier_2.id)
+  end
+
+  let!(:finding_3) do
+    findings.create!(name: 'Finding',
+      report_type: 'dast',
+      project_fingerprint: Gitlab::Database::ShaAttribute.new.serialize('a98c8aed53514eddba2976b942162bf3418291a3'),
+      location_fingerprint: 'bar',
+      severity: 1,
+      confidence: 1,
+      metadata_version: 1,
+      raw_metadata: '',
+      uuid: SecureRandom.uuid,
+      project_id: project.id,
+      vulnerability_id: vulnerability_3.id,
+      scanner_id: scanner.id,
+      primary_identifier_id: identifier_3.id)
+  end
+
+  it 'correctly schedules background migrations invalid vulnerabilities only', :aggregate_failures do
+    stub_const("#{described_class.name}::BATCH_SIZE", 1)
+
+    Sidekiq::Testing.fake! do
+      freeze_time do
+        migrate!
+
+        expect(described_class::MIGRATION_CLASS)
+          .to be_scheduled_delayed_migration(3.minutes, 1)
+
+        expect(described_class::MIGRATION_CLASS)
+          .to be_scheduled_delayed_migration(6.minutes, 2)
+
+        expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+      end
+    end
+  end
+end

lib/gitlab/background_migration/populate_dismissed_state_for_vulnerabilities.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # This class updates vulnerabilities entities with state dismissed
+    class PopulateDismissedStateForVulnerabilities
+      class Vulnerability < ActiveRecord::Base # rubocop:disable Style/Documentation
+        self.table_name = 'vulnerabilities'
+      end
+
+      def perform(*vulnerability_ids)
+        Vulnerability.where(id: vulnerability_ids).update_all(state: 2)
+        PopulateMissingVulnerabilityDismissalInformation.new.perform(*vulnerability_ids)
+      end
+    end
+  end
+end

lib/gitlab/background_migration/populate_missing_vulnerability_dismissal_information.rb

@@ -26,13 +26,16 @@ module Gitlab
 
       class Finding < ActiveRecord::Base # rubocop:disable Style/Documentation
         include ShaAttribute
+        include ::Gitlab::Utils::StrongMemoize
 
         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
+          strong_memoize(:dismissal_feedback) do
+            Feedback.dismissal.where(category: report_type, project_fingerprint: project_fingerprint, project_id: project_id).first
+          end
         end
       end
 

spec/lib/gitlab/background_migration/populate_dismissed_state_for_vulnerabilities_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Gitlab::BackgroundMigration::PopulateDismissedStateForVulnerabilities, schema: 2020_11_30_103926 do
+  let(:users) { table(:users) }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:vulnerabilities) { table(:vulnerabilities) }
+
+  let!(:namespace) { namespaces.create!(name: "foo", path: "bar") }
+  let!(:user) { users.create!(name: 'John Doe', email: 'test@example.com', projects_limit: 5) }
+  let!(:project) { projects.create!(namespace_id: namespace.id) }
+  let!(:vulnerability_params) do
+    {
+      project_id: project.id,
+      author_id: user.id,
+      title: 'Vulnerability',
+      severity: 5,
+      confidence: 5,
+      report_type: 5
+    }
+  end
+
+  let!(:vulnerability_1) { vulnerabilities.create!(vulnerability_params.merge(state: 1)) }
+  let!(:vulnerability_2) { vulnerabilities.create!(vulnerability_params.merge(state: 3)) }
+
+  describe '#perform' do
+    it 'changes state of vulnerability to dismissed' do
+      subject.perform(vulnerability_1.id, vulnerability_2.id)
+
+      expect(vulnerability_1.reload.state).to eq(2)
+      expect(vulnerability_2.reload.state).to eq(2)
+    end
+
+    it 'populates missing dismissal information' do
+      expect_next_instance_of(::Gitlab::BackgroundMigration::PopulateMissingVulnerabilityDismissalInformation) do |migration|
+        expect(migration).to receive(:perform).with(vulnerability_1.id, vulnerability_2.id)
+      end
+
+      subject.perform(vulnerability_1.id, vulnerability_2.id)
+    end
+  end
+end