diff --git a/db/migrate/20210722151951_add_columns_to_security_scans.rb b/db/migrate/20210722151951_add_columns_to_security_scans.rb new file mode 100644 index 0000000000000000000000000000000000000000..341cef057ce9014e734e468da205df5cd5c44352 --- /dev/null +++ b/db/migrate/20210722151951_add_columns_to_security_scans.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddColumnsToSecurityScans < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + def up + with_lock_retries do + add_column :security_scans, :project_id, :bigint + add_column :security_scans, :pipeline_id, :bigint + end + end + + def down + with_lock_retries do + remove_column :security_scans, :project_id, :bigint + remove_column :security_scans, :pipeline_id, :bigint + end + end +end diff --git a/db/migrate/20210728174349_add_fk_to_security_scans_columns.rb b/db/migrate/20210728174349_add_fk_to_security_scans_columns.rb new file mode 100644 index 0000000000000000000000000000000000000000..418097b92e5ad1093a244b00124d3ea3e04eea6b --- /dev/null +++ b/db/migrate/20210728174349_add_fk_to_security_scans_columns.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddFkToSecurityScansColumns < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + def up + add_concurrent_index :security_scans, :project_id + add_concurrent_foreign_key :security_scans, :projects, column: :project_id, on_delete: :cascade + + add_concurrent_index :security_scans, :pipeline_id + end + + def down + remove_foreign_key :security_scans, column: :project_id + remove_concurrent_index_by_name :security_scans, name: 'index_security_scans_on_project_id' + + remove_concurrent_index_by_name :security_scans, name: 'index_security_scans_on_pipeline_id' + end +end diff --git a/db/post_migrate/20210811214811_schedule_copy_ci_builds_columns_to_security_scans.rb b/db/post_migrate/20210811214811_schedule_copy_ci_builds_columns_to_security_scans.rb new file mode 100644 index 0000000000000000000000000000000000000000..dc90c202c410af18cbb44671794494f311afdaff --- /dev/null +++ b/db/post_migrate/20210811214811_schedule_copy_ci_builds_columns_to_security_scans.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class ScheduleCopyCiBuildsColumnsToSecurityScans < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + INTERVAL = 2.minutes.to_i + BATCH_SIZE = 5_000 + MIGRATION = 'CopyCiBuildsColumnsToSecurityScans' + + disable_ddl_transaction! + + class SecurityScan < ActiveRecord::Base + include EachBatch + + self.table_name = 'security_scans' + end + + def up + SecurityScan.reset_column_information + + queue_background_migration_jobs_by_range_at_intervals( + SecurityScan, + MIGRATION, + INTERVAL, + batch_size: BATCH_SIZE, + track_jobs: true + ) + end + + def down + # noop + end +end diff --git a/db/schema_migrations/20210722151951 b/db/schema_migrations/20210722151951 new file mode 100644 index 0000000000000000000000000000000000000000..a5e6a8c0963f0011ae20797eb9bc64c38c293c51 --- /dev/null +++ b/db/schema_migrations/20210722151951 @@ -0,0 +1 @@ +7289fb2a65c1210a352991fae7fac0c8e1129a33c166d0dad6f2aed98cb672a6 \ No newline at end of file diff --git a/db/schema_migrations/20210728174349 b/db/schema_migrations/20210728174349 new file mode 100644 index 0000000000000000000000000000000000000000..59035edce302f67ecc68cd12765030dac5c5b653 --- /dev/null +++ b/db/schema_migrations/20210728174349 @@ -0,0 +1 @@ +3a56c903333f13e9e3d39e5b65a3b70fdcfbf967cdac8bff348dfb71c0fde520 \ No newline at end of file diff --git a/db/schema_migrations/20210811214811 b/db/schema_migrations/20210811214811 new file mode 100644 index 0000000000000000000000000000000000000000..b34641b6b443038e84008ec16f1101185263ffc4 --- /dev/null +++ b/db/schema_migrations/20210811214811 @@ -0,0 +1 @@ +9e66aa8fc5e2a32ce0857f7ef77e906424bdf86c49643dfc71ed1a2e353b2095 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 870277556bb34f4c2e16783525a214e993a25b5a..8955a08a6ba47212f3dabcfcaf08cd8802ed76d4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18133,7 +18133,9 @@ CREATE TABLE security_scans ( updated_at timestamp with time zone NOT NULL, build_id bigint NOT NULL, scan_type smallint NOT NULL, - info jsonb DEFAULT '{}'::jsonb NOT NULL + info jsonb DEFAULT '{}'::jsonb NOT NULL, + project_id bigint, + pipeline_id bigint ); CREATE SEQUENCE security_scans_id_seq @@ -25160,6 +25162,10 @@ CREATE INDEX index_security_scans_on_created_at ON security_scans USING btree (c CREATE INDEX index_security_scans_on_date_created_at_and_id ON security_scans USING btree (date(timezone('UTC'::text, created_at)), id); +CREATE INDEX index_security_scans_on_pipeline_id ON security_scans USING btree (pipeline_id); + +CREATE INDEX index_security_scans_on_project_id ON security_scans USING btree (project_id); + CREATE INDEX index_self_managed_prometheus_alert_events_on_environment_id ON self_managed_prometheus_alert_events USING btree (environment_id); CREATE INDEX index_sent_notifications_on_noteable_type_noteable_id ON sent_notifications USING btree (noteable_id) WHERE ((noteable_type)::text = 'Issue'::text); @@ -26691,6 +26697,9 @@ ALTER TABLE ONLY label_links ALTER TABLE ONLY project_group_links ADD CONSTRAINT fk_daa8cee94c FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY security_scans + ADD CONSTRAINT fk_dbc89265b9 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY epics ADD CONSTRAINT fk_dccd3f98fc FOREIGN KEY (assignee_id) REFERENCES users(id) ON DELETE SET NULL; diff --git a/ee/app/models/security/scan.rb b/ee/app/models/security/scan.rb index 23dd80689f1794b79f9341a4de694c6975c63828..e15d5ef6e8ba396b62ec5d1c2a2ae0f417e86833 100644 --- a/ee/app/models/security/scan.rb +++ b/ee/app/models/security/scan.rb @@ -42,8 +42,17 @@ module Security delegate :project, :name, to: :build + before_save :ensure_project_id_pipeline_id + def has_errors? info&.fetch('errors', []).present? end + + private + + def ensure_project_id_pipeline_id + self.project_id ||= build.project_id + self.pipeline_id ||= build.commit_id + end end end diff --git a/ee/spec/models/security/scan_spec.rb b/ee/spec/models/security/scan_spec.rb index 9d7743a4a5f6261e2087ef29960f3d22d432d79e..68ba281be14d777ab74953a3f3ccffd89b5e9d24 100644 --- a/ee/spec/models/security/scan_spec.rb +++ b/ee/spec/models/security/scan_spec.rb @@ -108,4 +108,14 @@ RSpec.describe Security::Scan do end it_behaves_like 'having unique enum values' + + it 'sets `project_id` and `pipeline_id` before save' do + scan = create(:security_scan) + scan.update_columns(project_id: nil, pipeline_id: nil) + + scan.save! + + expect(scan.project_id).to eq(scan.build.project_id) + expect(scan.pipeline_id).to eq(scan.build.commit_id) + end end diff --git a/lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans.rb b/lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans.rb new file mode 100644 index 0000000000000000000000000000000000000000..59ce06d3e8fae48e53e43ee56941eb2c6b8a5074 --- /dev/null +++ b/lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + class CopyCiBuildsColumnsToSecurityScans + extend ::Gitlab::Utils::Override + + UPDATE_BATCH_SIZE = 500 + + def perform(start_id, stop_id) + (start_id..stop_id).step(UPDATE_BATCH_SIZE).each do |offset| + batch_start = offset + batch_stop = offset + UPDATE_BATCH_SIZE - 1 + + ActiveRecord::Base.connection.execute <<~SQL + UPDATE + security_scans + SET + project_id = ci_builds.project_id, + pipeline_id = ci_builds.commit_id + FROM ci_builds + WHERE ci_builds.type='Ci::Build' + AND ci_builds.id=security_scans.build_id + AND security_scans.id BETWEEN #{Integer(batch_start)} AND #{Integer(batch_stop)} + SQL + end + + mark_job_as_succeeded(start_id, stop_id) + rescue StandardError => error + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error) + end + end + + private + + def mark_job_as_succeeded(*arguments) + Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded( + 'CopyCiBuildsColumnsToSecurityScans', + arguments + ) + end + end +end diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 18f2f7b54c478d4ac901515384e74f486086fa35..7e4b8c5388510b1beea86baf3866330be8a9f6b4 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -86,7 +86,8 @@ RSpec.describe 'Database schema' do users: %w[color_scheme_id created_by_id theme_id email_opted_in_source_id], users_star_projects: %w[user_id], vulnerability_identifiers: %w[external_id], - vulnerability_scanners: %w[external_id] + vulnerability_scanners: %w[external_id], + security_scans: %w[pipeline_id] # foreign key is not added as ci_pipeline table will be moved into different db soon }.with_indifferent_access.freeze context 'for table' do diff --git a/spec/lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans_spec.rb b/spec/lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..db822f36c21fd4174045e2fe48c6dd31bf6262f1 --- /dev/null +++ b/spec/lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::CopyCiBuildsColumnsToSecurityScans, schema: 20210728174349 do + let(:migration) { described_class.new } + + let_it_be(:namespaces) { table(:namespaces) } + let_it_be(:projects) { table(:projects) } + let_it_be(:ci_pipelines) { table(:ci_pipelines) } + let_it_be(:ci_builds) { table(:ci_builds) } + let_it_be(:security_scans) { table(:security_scans) } + + let!(:namespace) { namespaces.create!(name: 'namespace', path: 'namespace') } + let!(:project1) { projects.create!(namespace_id: namespace.id) } + let!(:project2) { projects.create!(namespace_id: namespace.id) } + let!(:pipeline1) { ci_pipelines.create!(status: "success")} + let!(:pipeline2) { ci_pipelines.create!(status: "success")} + + let!(:build1) { ci_builds.create!(commit_id: pipeline1.id, type: 'Ci::Build', project_id: project1.id) } + let!(:build2) { ci_builds.create!(commit_id: pipeline2.id, type: 'Ci::Build', project_id: project2.id) } + let!(:build3) { ci_builds.create!(commit_id: pipeline1.id, type: 'Ci::Build', project_id: project1.id) } + + let!(:scan1) { security_scans.create!(build_id: build1.id, scan_type: 1) } + let!(:scan2) { security_scans.create!(build_id: build2.id, scan_type: 1) } + let!(:scan3) { security_scans.create!(build_id: build3.id, scan_type: 1) } + + subject { migration.perform(scan1.id, scan2.id) } + + before do + stub_const("#{described_class}::UPDATE_BATCH_SIZE", 2) + end + + it 'copies `project_id`, `commit_id` from `ci_builds` to `security_scans`', :aggregate_failures do + expect(migration).to receive(:mark_job_as_succeeded).with(scan1.id, scan2.id) + + subject + + scan1.reload + expect(scan1.project_id).to eq(project1.id) + expect(scan1.pipeline_id).to eq(pipeline1.id) + + scan2.reload + expect(scan2.project_id).to eq(project2.id) + expect(scan2.pipeline_id).to eq(pipeline2.id) + + scan3.reload + expect(scan3.project_id).to be_nil + expect(scan3.pipeline_id).to be_nil + end +end diff --git a/spec/migrations/schedule_copy_ci_builds_columns_to_security_scans_spec.rb b/spec/migrations/schedule_copy_ci_builds_columns_to_security_scans_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5ebd875389208ca42d18f81d6166dcac4918f890 --- /dev/null +++ b/spec/migrations/schedule_copy_ci_builds_columns_to_security_scans_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe ScheduleCopyCiBuildsColumnsToSecurityScans do + let_it_be(:namespaces) { table(:namespaces) } + let_it_be(:projects) { table(:projects) } + let_it_be(:ci_pipelines) { table(:ci_pipelines) } + let_it_be(:ci_builds) { table(:ci_builds) } + let_it_be(:security_scans) { table(:security_scans) } + + let!(:namespace) { namespaces.create!(name: 'namespace', path: 'namespace') } + let!(:project) { projects.create!(namespace_id: namespace.id) } + let!(:pipeline) { ci_pipelines.create!(status: "success")} + + let!(:build1) { ci_builds.create!(commit_id: pipeline.id, type: 'Ci::Build', project_id: project.id) } + let!(:build2) { ci_builds.create!(commit_id: pipeline.id, type: 'Ci::Build', project_id: project.id) } + let!(:build3) { ci_builds.create!(commit_id: pipeline.id, type: 'Ci::Build', project_id: project.id) } + + let!(:scan1) { security_scans.create!(build_id: build1.id, scan_type: 1) } + let!(:scan2) { security_scans.create!(build_id: build2.id, scan_type: 1) } + let!(:scan3) { security_scans.create!(build_id: build3.id, scan_type: 1) } + + before do + stub_const("#{described_class}::BATCH_SIZE", 2) + allow_next_instance_of(Gitlab::BackgroundMigration::CopyCiBuildsColumnsToSecurityScans) do |instance| + allow(instance).to receive(:mark_job_as_succeeded) + end + end + + around do |example| + freeze_time { Sidekiq::Testing.fake! { example.run } } + end + + it 'schedules background migrations', :aggregate_failures do + migrate! + + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, scan1.id, scan2.id) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, scan3.id, scan3.id) + end +end