Merge branch...

Merge branch '330882-dependency-scanning-vulnerability-reports-a-lot-of-duplicate-links-index' into 'master'

Unique Index for Vulnerability LInks

See merge request gitlab-org/gitlab!67993

db/migrate/20210811193033_add_unique_index_to_vulnerability_finding_links.rb

+# frozen_string_literal: true
+
+class AddUniqueIndexToVulnerabilityFindingLinks < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  NAME_URL_INDEX_NAME = 'finding_link_name_url_idx'
+  URL_INDEX_NAME = 'finding_link_url_idx'
+
+  def up
+    add_concurrent_index :vulnerability_finding_links, [:vulnerability_occurrence_id, :name, :url], unique: true, name: NAME_URL_INDEX_NAME
+    add_concurrent_index :vulnerability_finding_links, [:vulnerability_occurrence_id, :url], unique: true, where: 'name is null', name: URL_INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index :vulnerability_finding_links, [:vulnerability_occurrence_id, :name, :url], name: NAME_URL_INDEX_NAME
+    remove_concurrent_index :vulnerability_finding_links, [:vulnerability_occurrence_id, :url], name: URL_INDEX_NAME
+  end
+end

db/schema_migrations/20210811193033

+a7e259fa72dfdfa40137d278499d6b63b84f939f46936c0f4ed289ed152d9356
\ No newline at end of file

db/structure.sql

@@ -24949,6 +24949,10 @@ CREATE UNIQUE INDEX epic_user_mentions_on_epic_id_index ON epic_user_mentions US
 
 CREATE UNIQUE INDEX finding_evidences_on_unique_vulnerability_occurrence_id ON vulnerability_finding_evidences USING btree (vulnerability_occurrence_id);
 
+CREATE UNIQUE INDEX finding_link_name_url_idx ON vulnerability_finding_links USING btree (vulnerability_occurrence_id, name, url);
+
+CREATE UNIQUE INDEX finding_link_url_idx ON vulnerability_finding_links USING btree (vulnerability_occurrence_id, url) WHERE (name IS NULL);
+
 CREATE INDEX finding_links_on_vulnerability_occurrence_id ON vulnerability_finding_links USING btree (vulnerability_occurrence_id);
 
 CREATE UNIQUE INDEX i_ci_job_token_project_scope_links_on_source_and_target_project ON ci_job_token_project_scope_links USING btree (source_project_id, target_project_id);

ee/spec/services/security/store_report_service_spec.rb

@@ -45,7 +45,7 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
 
       context 'for different security reports' do
         where(:case_name, :trait, :scanners, :identifiers, :findings, :finding_identifiers, :finding_pipelines, :remediations, :signatures, :finding_links, :finding_flags) do
-          'with SAST report'                | :sast                            | 1 | 6  | 5 | 7  | 5  | 0 | 2 | 0 | 0
+          'with SAST report'                | :sast                            | 1 | 6  | 5 | 7  | 5  | 0 | 2 | 2 | 0
           'with exceeding identifiers'      | :with_exceeding_identifiers      | 1 | 20 | 1 | 20 | 1  | 0 | 0 | 0 | 0
           'with Dependency Scanning report' | :dependency_scanning_remediation | 1 | 3  | 2 | 3  | 2  | 1 | 0 | 6 | 0
           'with Container Scanning report'  | :container_scanning              | 1 | 8  | 8 | 8  | 8  | 0 | 0 | 8 | 0
@@ -323,6 +323,9 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
                algorithm_type: existing_finding.signatures.first.algorithm_type,
                signature_sha: existing_finding.signatures.first.signature_sha)
 
+        create(:finding_link,
+               finding: created_finding)
+
         created_finding
       end
 
@@ -397,6 +400,10 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
         expect { subject }.to change { Vulnerabilities::Identifier.count }.by(4)
       end
 
+      it 'inserts only new links and reuse existing ones' do
+        expect { subject }.to change { Vulnerabilities::FindingLink.count }.by(2)
+      end
+
       it 'inserts only new findings and reuse existing ones' do
         expect { subject }.to change { Vulnerabilities::Finding.count }.by(3)
       end

spec/fixtures/security_reports/master/gl-sast-report.json

@@ -26,6 +26,16 @@
           "value": "PREDICTABLE_RANDOM",
           "url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM"
         }
+      ],
+      "links": [
+        {
+          "name": "Link1",
+          "url": "https://www.url1.com"
+        },
+        {
+          "name": "Link2",
+          "url": "https://www.url2.com"
+        }
       ]
     },
     {

spec/lib/gitlab/background_migration/remove_vulnerability_finding_links_spec.rb

@@ -41,8 +41,8 @@ RSpec.describe Gitlab::BackgroundMigration::RemoveVulnerabilityFindingLinks, :mi
   # vulnerability finding links
   let!(:links) do
     {
-      findings.first => Array.new(5) { |id| finding_links.create!(vulnerability_occurrence_id: findings.first.id, name: "Link Name 1", url: "link_url1.example") },
-      findings.second => Array.new(5) { |id| finding_links.create!(vulnerability_occurrence_id: findings.second.id, name: "Link Name 2", url: "link_url2.example") }
+      findings.first => Array.new(5) { |id| finding_links.create!(vulnerability_occurrence_id: findings.first.id, name: "Link Name 1", url: "link_url1_#{id}.example") },
+      findings.second => Array.new(5) { |id| finding_links.create!(vulnerability_occurrence_id: findings.second.id, name: "Link Name 2", url: "link_url2_#{id}.example") }
     }
   end