From 19274fac01c60b4d3b31308e2cc41d43b677fde0 Mon Sep 17 00:00:00 2001 From: Gregory Havenga <11164960-ghavenga@users.noreply.gitlab.com> Date: Thu, 20 Jul 2023 18:08:49 +0000 Subject: [PATCH] Backfill missing vulnerability dismissal information Changelog: fixed --- ...issing_vulnerability_dismissal_details.yml | 6 + ...missing_vulnerability_dismissal_details.rb | 27 ++++ db/schema_migrations/20230712145557 | 1 + ...missing_vulnerability_dismissal_details.rb | 74 ++++++++++ ...ng_vulnerability_dismissal_details_spec.rb | 134 ++++++++++++++++++ ...missing_vulnerability_dismissal_details.rb | 17 +++ ...ng_vulnerability_dismissal_details_spec.rb | 26 ++++ 7 files changed, 285 insertions(+) create mode 100644 db/docs/batched_background_migrations/backfill_missing_vulnerability_dismissal_details.yml create mode 100644 db/post_migrate/20230712145557_queue_backfill_missing_vulnerability_dismissal_details.rb create mode 100644 db/schema_migrations/20230712145557 create mode 100644 ee/lib/ee/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details.rb create mode 100644 ee/spec/lib/ee/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details_spec.rb create mode 100644 lib/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details.rb create mode 100644 spec/migrations/20230712145557_queue_backfill_missing_vulnerability_dismissal_details_spec.rb diff --git a/db/docs/batched_background_migrations/backfill_missing_vulnerability_dismissal_details.yml b/db/docs/batched_background_migrations/backfill_missing_vulnerability_dismissal_details.yml new file mode 100644 index 0000000000000..f84a6ad84ad12 --- /dev/null +++ b/db/docs/batched_background_migrations/backfill_missing_vulnerability_dismissal_details.yml @@ -0,0 +1,6 @@ +--- +migration_job_name: BackfillMissingVulnerabilityDismissalDetails +description: Backfill missing vulnerability dimissal information as a result of https://gitlab.com/gitlab-org/gitlab/-/issues/412983 +feature_category: vulnerability_management +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126253 +milestone: 16.2 diff --git a/db/post_migrate/20230712145557_queue_backfill_missing_vulnerability_dismissal_details.rb b/db/post_migrate/20230712145557_queue_backfill_missing_vulnerability_dismissal_details.rb new file mode 100644 index 0000000000000..b3e54111562e2 --- /dev/null +++ b/db/post_migrate/20230712145557_queue_backfill_missing_vulnerability_dismissal_details.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class QueueBackfillMissingVulnerabilityDismissalDetails < Gitlab::Database::Migration[2.1] + MIGRATION = "BackfillMissingVulnerabilityDismissalDetails" + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 500 + SUB_BATCH_SIZE = 100 + + disable_ddl_transaction! + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + queue_batched_background_migration( + MIGRATION, + :vulnerabilities, + :id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :vulnerabilities, :id, []) + end +end diff --git a/db/schema_migrations/20230712145557 b/db/schema_migrations/20230712145557 new file mode 100644 index 0000000000000..56fd5d3dc70ec --- /dev/null +++ b/db/schema_migrations/20230712145557 @@ -0,0 +1 @@ +7e3effd7d0b7485193bac1226dd7544dee71381c6b5c1323500f4c4c5b2da690 \ No newline at end of file diff --git a/ee/lib/ee/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details.rb b/ee/lib/ee/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details.rb new file mode 100644 index 0000000000000..c8f18513f9b91 --- /dev/null +++ b/ee/lib/ee/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module EE + module Gitlab + module BackgroundMigration + # rubocop: disable Style/Documentation + module BackfillMissingVulnerabilityDismissalDetails + extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override + + DISMISSED_STATE = 2 + + prepended do + scope_to ->(relation) { relation.where(state: DISMISSED_STATE, dismissed_at: nil, dismissed_by_id: nil) } + operation_name :backfill_missing_vulnerability_dismissal_details + end + + class StateTransition < ::ApplicationRecord + self.table_name = 'vulnerability_state_transitions' + end + + class Vulnerability < ::ApplicationRecord + self.table_name = 'vulnerabilities' + end + + override :perform + def perform + each_sub_batch do |sub_batch| + state_transitions = StateTransition.where(vulnerability_id: sub_batch.select(:id), + to_state: DISMISSED_STATE).where( + <<~SQL + NOT EXISTS (SELECT 1 FROM "vulnerability_state_transitions" AS "vst" + WHERE "vst"."created_at" > "vulnerability_state_transitions"."created_at" + AND "vst"."to_state" = #{DISMISSED_STATE} + AND "vst"."vulnerability_id" = "vulnerability_state_transitions"."vulnerability_id") + SQL + ) + + sub_batch.each do |vulnerability| + vst = state_transitions.find { |transition| transition.vulnerability_id == vulnerability.id } + + if vst.nil? + log_missing_transition(vulnerability) + next + end + + vulnerability.update!(dismissed_at: vst.created_at, dismissed_by_id: vst.author_id) + log_success(vulnerability) + end + end + end + # rubocop: enable Style/Documentation + + private + + def log_success(vulnerability) + ::Gitlab::BackgroundMigration::Logger.info( + migrator: self.class.name, + message: 'Vulnerability dismissal information restored.', + vulnerability_id: vulnerability.id + ) + end + + def log_missing_transition(vulnerability) + ::Gitlab::BackgroundMigration::Logger.warn( + migrator: self.class.name, + message: 'Invalid dismissed vulnerability lacks a state transition to restore from.', + vulnerability_id: vulnerability.id + ) + end + end + end + end +end diff --git a/ee/spec/lib/ee/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details_spec.rb new file mode 100644 index 0000000000000..4d9d7cee82efc --- /dev/null +++ b/ee/spec/lib/ee/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details_spec.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillMissingVulnerabilityDismissalDetails, schema: 20230712145557, feature_category: :vulnerability_management do # rubocop:disable Layout/LineLength + let(:projects_table) { table(:projects) } + let(:namespaces_table) { table(:namespaces) } + let(:users_table) { table(:users) } + let(:vulnerabilities_table) { table(:vulnerabilities) } + let(:vsts_table) { table(:vulnerability_state_transitions) } + + let!(:user) { users_table.create!(username: 'john_doe', email: 'johndoe@gitlab.com', projects_limit: 10) } + let!(:user2) { users_table.create!(username: 'jane_doe', email: 'janedoe@gitlab.com', projects_limit: 10) } + let!(:namespace) { namespaces_table.create!(name: 'namespace', path: 'namespace-path-1') } + let!(:project) do + projects_table + .create!( + name: 'project1', + path: 'path1', + namespace_id: namespace.id, + project_namespace_id: namespace.id, + visibility_level: 0 + ) + end + + subject(:perform_migration) do + described_class.new(start_id: vulnerabilities_table.minimum(:id), + end_id: vulnerabilities_table.maximum(:id), + batch_table: :vulnerabilities, + batch_column: :id, + sub_batch_size: 2, + pause_ms: 0, + connection: vulnerabilities_table.connection) + .perform + end + + context 'when the vulnerability is not correctly dismissed' do + let!(:mangled_dismissed_vulnerability) { create_vulnerability(dismissed_at: nil, dismissed_by_id: nil) } + let!(:mangled_dismissed_vulnerability_non_dismissal_vst) do + create_vst(vulnerability_id: mangled_dismissed_vulnerability.id, to_state: 3) + end + + let!(:mangled_dismissed_vulnerability_older_dismissal_vst) do + create_vst( + vulnerability_id: mangled_dismissed_vulnerability.id, + to_state: 2, + created_at: Time.zone.now - 1.day + ) + end + + let!(:mangled_dismissed_vulnerability_dismissal_vst) do + create_vst(vulnerability_id: mangled_dismissed_vulnerability.id, author_id: user.id) + end + + let!(:incorrect_attributes) { mangled_dismissed_vulnerability.attributes } + + let!(:correct_attributes) do + mangled_dismissed_vulnerability.attributes.merge({ + "dismissed_at" => mangled_dismissed_vulnerability_dismissal_vst.created_at, + "dismissed_by_id" => mangled_dismissed_vulnerability_dismissal_vst.author_id + }) + end + + it 'applies the applicable state transition information to malformed state transitions', :freeze_time do + expect { perform_migration }.to change { + mangled_dismissed_vulnerability.reload.attributes + }.to(correct_attributes) + end + + it 'logs a succesful outcome' do + expect(::Gitlab::BackgroundMigration::Logger).to receive(:info).with( + migrator: "Gitlab::BackgroundMigration::BackfillMissingVulnerabilityDismissalDetails", + message: 'Vulnerability dismissal information restored.', + vulnerability_id: mangled_dismissed_vulnerability.id + ) + + perform_migration + end + + context 'and does not have a corresponding vst' do + let!(:mangled_dismissed_vulnerability_with_no_vst) do + create_vulnerability(dismissed_at: nil, dismissed_by_id: nil) + end + + it 'does not modify the vulnerability' do + expect { perform_migration }.not_to change { mangled_dismissed_vulnerability_with_no_vst.reload.attributes } + end + + it 'logs an appropriate warning' do + expect(::Gitlab::BackgroundMigration::Logger).to receive(:warn).with( + migrator: "Gitlab::BackgroundMigration::BackfillMissingVulnerabilityDismissalDetails", + message: 'Invalid dismissed vulnerability lacks a state transition to restore from.', + vulnerability_id: mangled_dismissed_vulnerability_with_no_vst.id + ) + + perform_migration + end + end + end + + context 'when the vulnerability is correctly dismissed' do + let!(:correctly_dismissed_vulnerability) { create_vulnerability } + let!(:correct_dismissed_vulnerability_vst) { create_vst(vulnerability_id: correctly_dismissed_vulnerability.id) } + + it 'does not modify correct vulnerabilities' do + expect { perform_migration }.not_to change { correctly_dismissed_vulnerability.reload.attributes } + end + end + + private + + def create_vulnerability(params = {}) + vulnerabilities_table.create!({ + project_id: project.id, + author_id: user.id, + title: 'test', + severity: 1, + confidence: 1, + report_type: 1, + state: 2, + dismissed_at: Time.zone.now, + dismissed_by_id: user.id, + detected_at: Time.zone.now + }.merge(params)) + end + + def create_vst(params = {}) + vsts_table.create!({ + author_id: user2.id, + from_state: 1, + to_state: 2 + }.merge(params)) + end +end diff --git a/lib/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details.rb b/lib/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details.rb new file mode 100644 index 0000000000000..8399f53b724bc --- /dev/null +++ b/lib/gitlab/background_migration/backfill_missing_vulnerability_dismissal_details.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # rubocop: disable Style/Documentation + class BackfillMissingVulnerabilityDismissalDetails < BatchedMigrationJob + feature_category :vulnerability_management + + def perform + # no-op. The logic is defined in EE module. + end + end + # rubocop: enable Style/Documentation + end +end + +::Gitlab::BackgroundMigration::BackfillMissingVulnerabilityDismissalDetails.prepend_mod diff --git a/spec/migrations/20230712145557_queue_backfill_missing_vulnerability_dismissal_details_spec.rb b/spec/migrations/20230712145557_queue_backfill_missing_vulnerability_dismissal_details_spec.rb new file mode 100644 index 0000000000000..6595164da417d --- /dev/null +++ b/spec/migrations/20230712145557_queue_backfill_missing_vulnerability_dismissal_details_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueueBackfillMissingVulnerabilityDismissalDetails, feature_category: :vulnerability_management do + let!(:batched_migration) { described_class::MIGRATION } + + it 'schedules a new batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).to have_scheduled_batched_migration( + table_name: :vulnerabilities, + column_name: :id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + } + end + end +end -- GitLab