Merge branch '239177_scope_remediations_to_projects' into 'master'

Scope remediations records to projects

See merge request gitlab-org/gitlab!49219

changelogs/unreleased/239177_scope_remediations_to_projects.yml

+---
+title: Add `project_id` column into the `vulnerability_remediations` table to scope
+  the records with projects
+merge_request: 49219
+author:
+type: added

db/migrate/20201204085522_add_project_id_into_vulnerability_remediations.rb

+# frozen_string_literal: true
+
+class AddProjectIdIntoVulnerabilityRemediations < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    connection.execute('DELETE FROM vulnerability_remediations')
+
+    add_column :vulnerability_remediations, :project_id, :bigint, null: false # rubocop:disable Rails/NotNullColumn
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :vulnerability_remediations, :project_id
+    end
+  end
+end

db/migrate/20201204090855_add_compound_index_to_vulnerability_remediations_table.rb

+# frozen_string_literal: true
+
+class AddCompoundIndexToVulnerabilityRemediationsTable < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  NEW_INDEX_NAME = 'index_vulnerability_remediations_on_project_id_and_checksum'
+  OLD_INDEX_NAME = 'index_vulnerability_remediations_on_checksum'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :vulnerability_remediations, [:project_id, :checksum], unique: true, name: NEW_INDEX_NAME
+    add_concurrent_foreign_key :vulnerability_remediations, :projects, column: :project_id
+
+    remove_concurrent_index_by_name :vulnerability_remediations, OLD_INDEX_NAME
+  end
+
+  def down
+    add_concurrent_index :vulnerability_remediations, :checksum, unique: true, name: OLD_INDEX_NAME
+
+    remove_concurrent_index_by_name :vulnerability_remediations, NEW_INDEX_NAME
+
+    with_lock_retries do
+      remove_foreign_key_if_exists :vulnerability_remediations, column: :project_id
+    end
+  end
+end

db/schema_migrations/20201204085522

+a1d8228731066fb6dfe436b4d8d034353421d1f45f3896e963f3c7f15fb09fbc
\ No newline at end of file

db/schema_migrations/20201204090855

+01712e32d95578fe701738529abfa0e051ef68ed646f7a9c7f775f8a8d108578
\ No newline at end of file

db/structure.sql

@@ -17584,6 +17584,7 @@ CREATE TABLE vulnerability_remediations (
     summary text NOT NULL,
     file text NOT NULL,
     checksum bytea NOT NULL,
+    project_id bigint NOT NULL,
     CONSTRAINT check_ac0ccabff3 CHECK ((char_length(summary) <= 200)),
     CONSTRAINT check_fe3325e3ba CHECK ((char_length(file) <= 255))
 );
@@ -22669,7 +22670,7 @@ CREATE UNIQUE INDEX index_vulnerability_occurrences_on_uuid ON vulnerability_occ
 
 CREATE INDEX index_vulnerability_occurrences_on_vulnerability_id ON vulnerability_occurrences USING btree (vulnerability_id);
 
-CREATE UNIQUE INDEX index_vulnerability_remediations_on_checksum ON vulnerability_remediations USING btree (checksum);
+CREATE UNIQUE INDEX index_vulnerability_remediations_on_project_id_and_checksum ON vulnerability_remediations USING btree (project_id, checksum);
 
 CREATE UNIQUE INDEX index_vulnerability_scanners_on_project_id_and_external_id ON vulnerability_scanners USING btree (project_id, external_id);
 
@@ -23698,6 +23699,9 @@ ALTER TABLE ONLY ci_stages
 ALTER TABLE ONLY system_note_metadata
     ADD CONSTRAINT fk_fbd87415c9 FOREIGN KEY (description_version_id) REFERENCES description_versions(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY vulnerability_remediations
+    ADD CONSTRAINT fk_fc61a535a0 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY merge_requests
     ADD CONSTRAINT fk_fd82eae0b9 FOREIGN KEY (head_pipeline_id) REFERENCES ci_pipelines(id) ON DELETE SET NULL;
 

ee/app/models/ee/project.rb

@@ -78,6 +78,7 @@ module EE
       has_many :vulnerability_identifiers, class_name: 'Vulnerabilities::Identifier'
       has_many :vulnerability_scanners, class_name: 'Vulnerabilities::Scanner'
       has_many :vulnerability_exports, class_name: 'Vulnerabilities::Export'
+      has_many :vulnerability_remediations, class_name: 'Vulnerabilities::Remediation', inverse_of: :project
 
       has_many :dast_site_profiles
       has_many :dast_site_tokens

ee/app/models/vulnerabilities/remediation.rb

@@ -9,6 +9,8 @@ module Vulnerabilities
 
     sha_attribute :checksum
 
+    belongs_to :project, optional: false
+
     has_many :finding_remediations, class_name: 'Vulnerabilities::FindingRemediation', inverse_of: :remediation, foreign_key: 'vulnerability_remediation_id'
     has_many :findings, through: :finding_remediations
 

ee/app/services/security/store_report_service.rb

@@ -144,7 +144,7 @@ module Security
     def find_existing_remediations_for(finding)
       checksums = finding.remediations.map(&:checksum)
 
-      Vulnerabilities::Remediation.by_checksum(checksums)
+      @project.vulnerability_remediations.by_checksum(checksums)
     end
 
     def build_new_remediations_for(finding, existing_remediations)
@@ -159,7 +159,7 @@ module Security
     end
 
     def build_vulnerability_remediation(remediation)
-      Vulnerabilities::Remediation.new(summary: remediation.summary, file: remediation.diff_file, checksum: remediation.checksum)
+      @project.vulnerability_remediations.new(summary: remediation.summary, file: remediation.diff_file, checksum: remediation.checksum)
     end
 
     def create_vulnerability_pipeline_object(vulnerability_finding, pipeline)

ee/spec/factories/vulnerabilities/remediations.rb

@@ -2,6 +2,7 @@
 
 FactoryBot.define do
   factory :vulnerabilities_remediation, class: 'Vulnerabilities::Remediation' do
+    project
     summary { 'Remediation Summary' }
     file { Tempfile.new }
 

ee/spec/models/project_spec.rb

@@ -45,6 +45,7 @@ RSpec.describe Project do
     it { is_expected.to have_many(:downstream_project_subscriptions) }
     it { is_expected.to have_many(:downstream_projects) }
     it { is_expected.to have_many(:vulnerability_historical_statistics).class_name('Vulnerabilities::HistoricalStatistic') }
+    it { is_expected.to have_many(:vulnerability_remediations).class_name('Vulnerabilities::Remediation') }
 
     it { is_expected.to have_one(:github_service) }
     it { is_expected.to have_many(:project_aliases) }

ee/spec/models/vulnerabilities/remediation_spec.rb

@@ -3,6 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Vulnerabilities::Remediation do
+  it { is_expected.to belong_to(:project).required }
   it { is_expected.to have_many(:finding_remediations).class_name('Vulnerabilities::FindingRemediation') }
   it { is_expected.to have_many(:findings).through(:finding_remediations) }
 

ee/spec/services/security/store_report_service_spec.rb

@@ -30,6 +30,11 @@ RSpec.describe Security::StoreReportService, '#execute' do
   subject { described_class.new(pipeline, report).execute }
 
   context 'without existing data' do
+    before(:all) do
+      checksum = 'f00bc6261fa512f0960b7fc3bfcce7fb31997cf32b96fa647bed5668b2c77fee'
+      create(:vulnerabilities_remediation, checksum: checksum)
+    end
+
     before do
       project.add_developer(user)
       allow(pipeline).to receive(:user).and_return(user)
@@ -66,7 +71,7 @@ RSpec.describe Security::StoreReportService, '#execute' do
       end
 
       it 'inserts all remediations' do
-        expect { subject }.to change { Vulnerabilities::Remediation.count }.by(remediations)
+        expect { subject }.to change { project.vulnerability_remediations.count }.by(remediations)
       end
 
       it 'inserts all vulnerabilties' do

spec/lib/gitlab/import_export/all_models.yml

@@ -552,6 +552,7 @@ project:
 - build_report_results
 - vulnerability_statistic
 - vulnerability_historical_statistics
+- vulnerability_remediations
 - product_analytics_events
 - pipeline_artifacts
 - terraform_states