diff --git a/db/docs/batched_background_migrations/set_project_vulnerability_count.yml b/db/docs/batched_background_migrations/set_project_vulnerability_count.yml deleted file mode 100644 index f70d00f313b55467a632643c4c1bc9229c030801..0000000000000000000000000000000000000000 --- a/db/docs/batched_background_migrations/set_project_vulnerability_count.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -migration_job_name: SetProjectVulnerabilityCount -description: Sets the number of vulnerabilities a project has in the vulnerbaility_count column in the project_statistics table -feature_category: vulnerability_management -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/166471 -milestone: '17.5' -queued_migration_version: 20240918122045 -# Replace with the approximate date you think it's best to ensure the completion of this BBM. -finalized_by: # version of the migration that finalized this BBM diff --git a/db/docs/batched_background_migrations/set_total_number_of_vulnerabilities_for_existing_projects.yml b/db/docs/batched_background_migrations/set_total_number_of_vulnerabilities_for_existing_projects.yml new file mode 100644 index 0000000000000000000000000000000000000000..45add42a53e958969ccdcc28d7fc1272ba68c824 --- /dev/null +++ b/db/docs/batched_background_migrations/set_total_number_of_vulnerabilities_for_existing_projects.yml @@ -0,0 +1,8 @@ +--- +migration_job_name: SetTotalNumberOfVulnerabilitiesForExistingProjects +description: Sets the `vulnerability_count` column of `project_security_statistics` table. +feature_category: vulnerability_management +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167812 +milestone: '17.5' +queued_migration_version: 20241001115912 +finalized_by: # version of the migration that finalized this BBM diff --git a/db/post_migrate/20240918122045_reschedule_setting_project_vulnerability_count.rb b/db/post_migrate/20240918122045_reschedule_setting_project_vulnerability_count.rb index 1a0818b775288f579d3fed64f63f16d2a4f80bdb..6bde872265f4e30afaed84096781e6d2c106b58f 100644 --- a/db/post_migrate/20240918122045_reschedule_setting_project_vulnerability_count.rb +++ b/db/post_migrate/20240918122045_reschedule_setting_project_vulnerability_count.rb @@ -3,27 +3,7 @@ class RescheduleSettingProjectVulnerabilityCount < Gitlab::Database::Migration[2.2] milestone '17.5' - restrict_gitlab_migration gitlab_schema: :gitlab_main - - MIGRATION = "SetProjectVulnerabilityCount" - DELAY_INTERVAL = 2.minutes - BATCH_SIZE = 1000 - SUB_BATCH_SIZE = 100 - - def up - delete_batched_background_migration(MIGRATION, :project_settings, :project_id, []) - - queue_batched_background_migration( - MIGRATION, - :project_settings, - :project_id, - job_interval: DELAY_INTERVAL, - batch_size: BATCH_SIZE, - sub_batch_size: SUB_BATCH_SIZE - ) - end - - def down - delete_batched_background_migration(MIGRATION, :project_settings, :project_id, []) + def change + # no-op end end diff --git a/db/post_migrate/20241001115912_queue_set_total_number_of_vulnerabilities_for_existing_projects.rb b/db/post_migrate/20241001115912_queue_set_total_number_of_vulnerabilities_for_existing_projects.rb new file mode 100644 index 0000000000000000000000000000000000000000..3f54866fba98c71f3e324193c63e4652df1569c6 --- /dev/null +++ b/db/post_migrate/20241001115912_queue_set_total_number_of_vulnerabilities_for_existing_projects.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class QueueSetTotalNumberOfVulnerabilitiesForExistingProjects < Gitlab::Database::Migration[2.2] + milestone '17.5' + + restrict_gitlab_migration gitlab_schema: :gitlab_sec + + MIGRATION = "SetTotalNumberOfVulnerabilitiesForExistingProjects" + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 1000 + SUB_BATCH_SIZE = 100 + + def up + delete_batched_background_migration('SetProjectVulnerabilityCount', :project_settings, :project_id, []) + + queue_batched_background_migration( + MIGRATION, + :vulnerability_reads, + :project_id, + batch_class_name: 'LooseIndexScanBatchingStrategy', + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :vulnerability_reads, :project_id, []) + end +end diff --git a/db/schema_migrations/20241001115912 b/db/schema_migrations/20241001115912 new file mode 100644 index 0000000000000000000000000000000000000000..8a404945ac17f78e5b40716276ae7663309a84db --- /dev/null +++ b/db/schema_migrations/20241001115912 @@ -0,0 +1 @@ +7f37b4779c85f0290bc5c27104934e74cf9afe968906cdebfd02f5d915336c67 \ No newline at end of file diff --git a/ee/lib/ee/gitlab/background_migration/set_project_vulnerability_count.rb b/ee/lib/ee/gitlab/background_migration/set_project_vulnerability_count.rb deleted file mode 100644 index 465078e353f415a6546dcd52e1fe70e8fda183ad..0000000000000000000000000000000000000000 --- a/ee/lib/ee/gitlab/background_migration/set_project_vulnerability_count.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -module EE - module Gitlab - module BackgroundMigration - module SetProjectVulnerabilityCount - extend ActiveSupport::Concern - extend ::Gitlab::Utils::Override - - prepended do - scope_to ->(relation) { relation.where("has_vulnerabilities IS TRUE") } - operation_name :set_project_vulnerability_count - feature_category :vulnerability_management - end - - class ProjectStatistics < ApplicationRecord - self.table_name = 'project_statistics' - end - - class VulnerabilityRead < ApplicationRecord - include EachBatch - - self.table_name = 'vulnerability_reads' - self.primary_key = :vulnerability_id - end - - override :perform - - def perform - each_sub_batch do |sub_batch| - sub_batch.each do |project_settings| - project_statistics = ProjectStatistics.find_by(project_id: project_settings.project_id) - latest_vulnerability_id = nil - - ProjectStatistics.transaction do - project_statistics.lock! - - latest_vulnerability_id = VulnerabilityRead.where(project_id: project_settings.project_id).last&.id - - project_statistics.vulnerability_count = 0 - project_statistics.save! - end - - next unless latest_vulnerability_id - - total_count = 0 - - VulnerabilityRead.where('project_id = ? AND vulnerability_id <= ?', - project_settings.project_id, latest_vulnerability_id).each_batch do |batch| - total_count += batch.count - end - - ProjectStatistics.connection.exec_query(update_query, 'UPDATE', [total_count, project_statistics.id]) - end - end - end - - def update_query - <<~SQL - UPDATE - project_statistics - SET - vulnerability_count = vulnerability_count + $1 - WHERE - id = $2 - SQL - end - end - end - end -end diff --git a/ee/lib/ee/gitlab/background_migration/set_total_number_of_vulnerabilities_for_existing_projects.rb b/ee/lib/ee/gitlab/background_migration/set_total_number_of_vulnerabilities_for_existing_projects.rb new file mode 100644 index 0000000000000000000000000000000000000000..0298508bf249eb197fbfa1333b1841237f75549a --- /dev/null +++ b/ee/lib/ee/gitlab/background_migration/set_total_number_of_vulnerabilities_for_existing_projects.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +module EE + module Gitlab + module BackgroundMigration + module SetTotalNumberOfVulnerabilitiesForExistingProjects + extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override + + prepended do + operation_name :set_vulnerability_count + end + + PROJECT_STATISTICS_UPDATE_QUERY = <<~SQL + UPDATE + project_security_statistics + SET + vulnerability_count = vulnerability_count + $1 + WHERE + project_id = $2 + SQL + + class ProjectSecurityStatistics < ::Gitlab::Database::SecApplicationRecord; end + + class VulnerabilityRead < ::Gitlab::Database::SecApplicationRecord + include EachBatch + + self.primary_key = :vulnerability_id + end + + override :perform + def perform + distinct_each_batch do |sub_batch| + project_ids = sub_batch.pluck(:project_id) + + ensure_statistics(project_ids) + + project_ids.each { |project_id| update_project_statistics(project_id) } + end + end + + private + + def ensure_statistics(project_ids) + project_ids.map { |project_id| { project_id: project_id } } + .then { |attributes| ProjectSecurityStatistics.upsert_all(attributes) } + end + + def update_project_statistics(project_id) + latest_vulnerability_id = reset_statistics_and_get_latest_vulnerability_id(project_id) + total_count = calculate_number_of_vulnerabilities(project_id, latest_vulnerability_id) + + persist_statistics(project_id, total_count) + end + + def reset_statistics_and_get_latest_vulnerability_id(project_id) + latest_vulnerability_id = nil + + ProjectSecurityStatistics.transaction do + statistics = ProjectSecurityStatistics.lock.find(project_id) + + latest_vulnerability_id = VulnerabilityRead.where(project_id: project_id).last.vulnerability_id + + statistics.vulnerability_count = 0 + statistics.save! + end + + latest_vulnerability_id + end + + def calculate_number_of_vulnerabilities(project_id, latest_vulnerability_id) + total_count = 0 + + VulnerabilityRead.where('project_id = ? AND vulnerability_id <= ?', + project_id, latest_vulnerability_id).each_batch do |batch| + total_count += batch.count + end + + total_count + end + + def persist_statistics(project_id, total_count) + bind_params = [total_count, project_id] + + ProjectSecurityStatistics.connection.exec_query(PROJECT_STATISTICS_UPDATE_QUERY, 'UPDATE', bind_params) + end + end + end + end +end diff --git a/ee/spec/lib/ee/gitlab/background_migration/set_project_vulnerability_count_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/set_project_vulnerability_count_spec.rb deleted file mode 100644 index df0f85c6875f4be92f5e10fafb98fea11fb78e7c..0000000000000000000000000000000000000000 --- a/ee/spec/lib/ee/gitlab/background_migration/set_project_vulnerability_count_spec.rb +++ /dev/null @@ -1,152 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::SetProjectVulnerabilityCount, feature_category: :vulnerability_management do - let!(:user) { table(:users).create!(email: 'author@example.com', username: 'author', projects_limit: 10) } - let(:scanners) { table(:vulnerability_scanners) } - let(:namespaces) { table(:namespaces) } - let(:projects) { table(:projects) } - let(:project_settings) { table(:project_settings) } - let(:project_statistics) { table(:project_statistics) } - let(:vulnerabilities) { table(:vulnerabilities) } - let(:vulnerability_reads) { table(:vulnerability_reads) } - let(:vulnerability_occurrences) { table(:vulnerability_occurrences) } - let(:vulnerability_identifiers) { table(:vulnerability_identifiers) } - - let!(:namespace) { namespaces.create!(name: 'Test Namespace', path: 'namespace-path') } - let!(:project_namespace_1) { namespaces.create!(name: 'Project Namespace 1', path: 'project_1') } - let!(:project_namespace_2) { namespaces.create!(name: 'Project Namespace 2', path: 'project_2') } - let!(:project_namespace_3) { namespaces.create!(name: 'Project Namespace 3', path: 'project_3') } - - let!(:project_1) do - projects.create!( - name: "proj1", - path: "proj1", - namespace_id: namespace.id, - project_namespace_id: project_namespace_1.id - ) - end - - let!(:project_2) do - projects.create!( - name: "proj2", - path: "proj2", - namespace_id: namespace.id, - project_namespace_id: project_namespace_2.id - ) - end - - let!(:project_3) do - projects.create!( - name: "proj3", - path: "proj3", - namespace_id: namespace.id, - project_namespace_id: project_namespace_3.id - ) - end - - let!(:project_statistics_1) do - project_statistics.create!(project_id: project_1.id, namespace_id: namespace.id, vulnerability_count: 0) - end - - let!(:project_statistics_2) do - project_statistics.create!(project_id: project_2.id, namespace_id: namespace.id, vulnerability_count: 0) - end - - let!(:project_statistics_3) do - project_statistics.create!(project_id: project_3.id, namespace_id: namespace.id, vulnerability_count: 0) - end - - let!(:project_settings_1) { project_settings.create!(project_id: project_1.id, has_vulnerabilities: true) } - let!(:project_settings_2) { project_settings.create!(project_id: project_2.id, has_vulnerabilities: true) } - let!(:project_settings_3) { project_settings.create!(project_id: project_3.id, has_vulnerabilities: false) } - - let(:migration_instance) do - described_class.new( - start_id: project_settings.minimum(:project_id), - end_id: project_settings.maximum(:project_id), - batch_table: :project_settings, - batch_column: :project_id, - sub_batch_size: 100, - pause_ms: 0, - connection: project_settings.connection - ) - end - - subject(:perform_migration) { migration_instance.perform } - - before do - scanner_project_1 = scanners.create!( - project_id: project_1.id, - external_id: "external-id-1", - name: "Scanner 1" - ) - scanner_project_2 = scanners.create!( - project_id: project_2.id, - external_id: "external-id-2", - name: "Scanner 2" - ) - create_vulnerability_read(project_1.id, scanner_project_1.id) - create_vulnerability_read(project_1.id, scanner_project_1.id) - create_vulnerability_read(project_1.id, scanner_project_1.id) - create_vulnerability_read(project_2.id, scanner_project_2.id) - create_vulnerability_read(project_2.id, scanner_project_2.id) - end - - it 'sets the vulnerability_count column' do - expect do - perform_migration - project_statistics_1.reload - project_statistics_2.reload - project_statistics_3.reload - end.to change { project_statistics_1.vulnerability_count }.from(0).to(3) - .and change { project_statistics_2.vulnerability_count }.from(0).to(2) - .and not_change { project_statistics_3.vulnerability_count }.from(0) - end - - def create_vulnerability_read(project_id, scanner_id) - identifier = vulnerability_identifiers.create!( - project_id: project_id, - fingerprint: SecureRandom.bytes(20), - external_type: "", - external_id: "", - name: "" - ) - - finding = vulnerability_occurrences.create!( - project_id: project_id, - severity: 1, - report_type: 1, - scanner_id: scanner_id, - primary_identifier_id: identifier.id, - project_fingerprint: "", - location_fingerprint: "", - name: "name", - metadata_version: "sast:1.0", - raw_metadata: "", - uuid: SecureRandom.uuid - ) - - attrs = { - project_id: project_id, - author_id: user.id, - title: 'title', - severity: 1, - report_type: 1, - finding_id: finding.id - } - - vulnerability = vulnerabilities.create!(attrs) - - vulnerability_reads.create!( - vulnerability_id: vulnerability.id, - project_id: project_id, - scanner_id: scanner_id, - report_type: vulnerability.report_type, - severity: vulnerability.severity, - state: 1, - uuid: SecureRandom.uuid - ) - end -end diff --git a/ee/spec/lib/ee/gitlab/background_migration/set_total_number_of_vulnerabilities_for_existing_projects_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/set_total_number_of_vulnerabilities_for_existing_projects_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b36098dfee60fee7cae2197ea75c5436932d163a --- /dev/null +++ b/ee/spec/lib/ee/gitlab/background_migration/set_total_number_of_vulnerabilities_for_existing_projects_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::SetTotalNumberOfVulnerabilitiesForExistingProjects, feature_category: :vulnerability_management do + let(:users) { table(:users) } + let(:scanners) { table(:vulnerability_scanners) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:security_statistics) { table(:project_security_statistics) } + let(:vulnerabilities) { table(:vulnerabilities) } + let(:vulnerability_reads) { table(:vulnerability_reads) } + let(:vulnerability_occurrences) { table(:vulnerability_occurrences) } + let(:vulnerability_identifiers) { table(:vulnerability_identifiers) } + + let(:user) { users.create!(email: 'john@doe', username: 'john_doe', projects_limit: 10) } + let(:namespace) { namespaces.create!(name: 'Test', path: 'test') } + let(:project_namespace_1) { namespaces.create!(name: 'Project Namespace 1', path: 'project_1') } + let(:project_namespace_2) { namespaces.create!(name: 'Project Namespace 2', path: 'project_2') } + let(:project_namespace_3) { namespaces.create!(name: 'Project Namespace 3', path: 'project_3') } + + let(:project_1) do + projects.create!( + name: 'project_1', + path: 'project_1', + namespace_id: namespace.id, + project_namespace_id: project_namespace_1.id + ) + end + + let(:project_2) do + projects.create!( + name: 'project_2', + path: 'project_2', + namespace_id: namespace.id, + project_namespace_id: project_namespace_2.id + ) + end + + let(:project_3) do + projects.create!( + name: 'project_3', + path: 'project_3', + namespace_id: namespace.id, + project_namespace_id: project_namespace_3.id + ) + end + + let!(:project_statistics_1) { security_statistics.create!(project_id: project_1.id, vulnerability_count: 5) } + let!(:project_statistics_2) { security_statistics.create!(project_id: project_2.id, vulnerability_count: 0) } + let(:project_statistics_3_relation) { security_statistics.where(project_id: project_3.id) } + + let(:migration) do + described_class.new( + start_id: vulnerability_reads.minimum(:project_id), + end_id: vulnerability_reads.maximum(:project_id), + batch_table: :vulnerability_reads, + batch_column: :project_id, + sub_batch_size: 100, + pause_ms: 0, + connection: vulnerability_reads.connection + ) + end + + before do + create_vulnerability_read(project_1.id) + create_vulnerability_read(project_2.id) + create_vulnerability_read(project_3.id) + end + + it 'updates vulnerability_count for vulnerable projects' do + expect { migration.perform }.to change { project_statistics_1.reload.vulnerability_count }.to(1) + .and change { project_statistics_2.reload.vulnerability_count }.to(1) + .and change { project_statistics_3_relation.pick(:vulnerability_count) }.to(1) + end + + def create_vulnerability_read(project_id) + scanner = scanners.create!( + project_id: project_id, + external_id: 'external-id', + name: 'Scanner' + ) + + identifier = vulnerability_identifiers.create!( + project_id: project_id, + fingerprint: SecureRandom.bytes(20), + external_type: '', + external_id: '', + name: '' + ) + + finding = vulnerability_occurrences.create!( + project_id: project_id, + severity: 1, + report_type: 1, + scanner_id: scanner.id, + primary_identifier_id: identifier.id, + project_fingerprint: '', + location_fingerprint: '', + name: 'name', + metadata_version: '15.0', + raw_metadata: '', + uuid: SecureRandom.uuid + ) + + attrs = { + project_id: project_id, + author_id: user.id, + title: 'title', + severity: 1, + report_type: 1, + finding_id: finding.id + } + + vulnerability = vulnerabilities.create!(attrs) + + vulnerability_reads.create!( + vulnerability_id: vulnerability.id, + project_id: project_id, + scanner_id: scanner.id, + report_type: vulnerability.report_type, + severity: vulnerability.severity, + state: 1, + uuid: SecureRandom.uuid + ) + end +end diff --git a/lib/gitlab/background_migration/set_project_vulnerability_count.rb b/lib/gitlab/background_migration/set_project_vulnerability_count.rb deleted file mode 100644 index acab4266edee112d094d7b9c5816ec7a266d92d9..0000000000000000000000000000000000000000 --- a/lib/gitlab/background_migration/set_project_vulnerability_count.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module BackgroundMigration - # Background migration to set the vulnerability count in the project_statistics table. - class SetProjectVulnerabilityCount < BatchedMigrationJob - feature_category :vulnerability_management - - def perform; end - end - end -end -Gitlab::BackgroundMigration::SetProjectVulnerabilityCount.prepend_mod diff --git a/lib/gitlab/background_migration/set_total_number_of_vulnerabilities_for_existing_projects.rb b/lib/gitlab/background_migration/set_total_number_of_vulnerabilities_for_existing_projects.rb new file mode 100644 index 0000000000000000000000000000000000000000..ae54045c66141142c3af8a2a83228e60b4c29601 --- /dev/null +++ b/lib/gitlab/background_migration/set_total_number_of_vulnerabilities_for_existing_projects.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Sets the `vulnerability_count` column of `project_security_statistics` table. + class SetTotalNumberOfVulnerabilitiesForExistingProjects < BatchedMigrationJob + feature_category :vulnerability_management + + def perform; end + end + end +end + +Gitlab::BackgroundMigration::SetTotalNumberOfVulnerabilitiesForExistingProjects.prepend_mod diff --git a/spec/migrations/20240918122045_reschedule_setting_project_vulnerability_count_spec.rb b/spec/migrations/20241001115912_queue_set_total_number_of_vulnerabilities_for_existing_projects_spec.rb similarity index 71% rename from spec/migrations/20240918122045_reschedule_setting_project_vulnerability_count_spec.rb rename to spec/migrations/20241001115912_queue_set_total_number_of_vulnerabilities_for_existing_projects_spec.rb index 46d5b88f06b66b93e5d3bd5503a712e7fa10c3f2..3871efb598b048556b0d1de9ac59adf830fe8c04 100644 --- a/spec/migrations/20240918122045_reschedule_setting_project_vulnerability_count_spec.rb +++ b/spec/migrations/20241001115912_queue_set_total_number_of_vulnerabilities_for_existing_projects_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require_migration! -RSpec.describe RescheduleSettingProjectVulnerabilityCount, feature_category: :vulnerability_management do +RSpec.describe QueueSetTotalNumberOfVulnerabilitiesForExistingProjects, feature_category: :vulnerability_management do let!(:batched_migration) { described_class::MIGRATION } it 'schedules a new batched migration' do @@ -14,8 +14,10 @@ migration.after -> { expect(batched_migration).to have_scheduled_batched_migration( - table_name: :project_settings, + gitlab_schema: :gitlab_sec, + table_name: :vulnerability_reads, column_name: :project_id, + batch_class_name: 'LooseIndexScanBatchingStrategy', interval: described_class::DELAY_INTERVAL, batch_size: described_class::BATCH_SIZE, sub_batch_size: described_class::SUB_BATCH_SIZE