Introduce Security::VulnerabilityUUID helper

This helper will prevent wrong order when calculating UUIDv5 for
Vulnerabilities

app/services/security/vulnerability_uuid.rb

+# frozen_string_literal: true
+
+module Security
+  class VulnerabilityUUID
+    def self.generate(report_type:, primary_identifier_fingerprint:, location_fingerprint:, project_id:)
+      Gitlab::UUID.v5("#{report_type}-#{primary_identifier_fingerprint}-#{location_fingerprint}-#{project_id}")
+    end
+  end
+end

ee/app/models/vulnerabilities/finding.rb

@@ -369,7 +369,16 @@ module Vulnerabilities
     # We will eventually have only UUIDv5 values for the `uuid`
     # attribute of the finding records.
     def uuid_v5
-      Gitlab::UUID.v5?(uuid) ? uuid : Gitlab::UUID.v5(uuid_v5_name)
+      if Gitlab::UUID.v5?(uuid)
+        uuid
+      else
+        ::Security::VulnerabilityUUID.generate(
+          report_type: report_type,
+          primary_identifier_fingerprint: primary_identifier.fingerprint,
+          location_fingerprint: location_fingerprint,
+          project_id: project_id
+        )
+      end
     end
 
     def pipeline_branch
@@ -391,14 +400,5 @@ module Vulnerabilities
         project_fingerprint: project_fingerprint
       }
     end
-
-    def uuid_v5_name
-      [
-        report_type,
-        primary_identifier.fingerprint,
-        location_fingerprint,
-        project_id
-      ].join('-')
-    end
   end
 end

ee/lib/gitlab/ci/parsers/security/common.rb

@@ -180,9 +180,12 @@ module Gitlab
               return
             end
 
-            name = uuid_v5_name_components.values.join('-')
-
-            Gitlab::UUID.v5(name)
+            ::Security::VulnerabilityUUID.generate(
+              report_type: uuid_v5_name_components[:report_type],
+              primary_identifier_fingerprint: uuid_v5_name_components[:primary_identifier_fingerprint],
+              location_fingerprint: uuid_v5_name_components[:location_fingerprint],
+              project_id: uuid_v5_name_components[:project_id]
+            )
           end
         end
       end

ee/spec/factories/ci/reports/security/findings.rb

@@ -32,7 +32,12 @@ FactoryBot.define do
     severity { :high }
     scan factory: :ci_reports_security_scan
     sequence(:uuid) do |n|
-      Gitlab::UUID.v5("#{report_type}-#{identifiers.first&.fingerprint}-#{location.fingerprint}-#{n}")
+      ::Security::VulnerabilityUUID.generate(
+        report_type: report_type,
+        primary_identifier_fingerprint: identifiers.first&.fingerprint,
+        location_fingerprint: location.fingerprint,
+        project_id: n
+      )
     end
 
     skip_create

ee/spec/factories/vulnerabilities/findings.rb

@@ -48,7 +48,12 @@ FactoryBot.define do
     location_fingerprint { SecureRandom.hex(20) }
     report_type { :sast }
     uuid do
-      Gitlab::UUID.v5("#{report_type}-#{primary_identifier.fingerprint}-#{location_fingerprint}-#{project_id}")
+      Security::VulnerabilityUUID.generate(
+        report_type: report_type,
+        primary_identifier_fingerprint: primary_identifier.fingerprint,
+        location_fingerprint: location_fingerprint,
+        project_id: project_id
+      )
     end
     severity { :high }
     confidence { :medium }

ee/spec/lib/gitlab/ci/parsers/security/common_spec.rb

@@ -163,10 +163,24 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
 
     describe 'setting the uuid' do
       let(:finding_uuids) { report.findings.map(&:uuid) }
-      let(:uuid_1_components) { "dependency_scanning-4ff8184cd18485b6e85d5b101e341b12eacd1b3b-33dc9f32c77dde16d39c69d3f78f27ca3114a7c5-#{pipeline.project_id}" }
-      let(:uuid_2_components) { "dependency_scanning-d55f9e66e79882ae63af9fd55cc822ab75307e31-33dc9f32c77dde16d39c69d3f78f27ca3114a7c5-#{pipeline.project_id}" }
-      let(:uuid_1) { Gitlab::UUID.v5(uuid_1_components) }
-      let(:uuid_2) { Gitlab::UUID.v5(uuid_2_components) }
+      let(:uuid_1) do
+        Security::VulnerabilityUUID.generate(
+          report_type: "dependency_scanning",
+          primary_identifier_fingerprint: "4ff8184cd18485b6e85d5b101e341b12eacd1b3b",
+          location_fingerprint: "33dc9f32c77dde16d39c69d3f78f27ca3114a7c5",
+          project_id: pipeline.project_id
+        )
+      end
+
+      let(:uuid_2) do
+        Security::VulnerabilityUUID.generate(
+          report_type: "dependency_scanning",
+          primary_identifier_fingerprint: "d55f9e66e79882ae63af9fd55cc822ab75307e31",
+          location_fingerprint: "33dc9f32c77dde16d39c69d3f78f27ca3114a7c5",
+          project_id: pipeline.project_id
+        )
+      end
+
       let(:expected_uuids) { [uuid_1, uuid_2, nil] }
 
       it 'sets the UUIDv5 for findings', :aggregate_failures do

ee/spec/migrations/remove_duplicated_cs_findings_spec.rb

@@ -5,6 +5,8 @@ require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20200910131218_remove_duplicated_cs_findings.rb')
 
 RSpec.describe RemoveDuplicatedCsFindings, :migration do
+  include MigrationHelpers::VulnerabilitiesFindingsHelper
+
   let(:migration) { 'RemoveDuplicateCsFindings'}
   let(:namespaces) { table(:namespaces) }
   let(:notes) { table(:notes) }
@@ -90,21 +92,16 @@ RSpec.describe RemoveDuplicatedCsFindings, :migration do
   end
 
   def finding_params(primary_identifier_id, project_id)
-    attrs = attributes_for(:vulnerabilities_finding) # rubocop: disable RSpec/FactoriesInMigrationSpecs
-    {
+    attrs = attributes_for_vulnerabilities_finding
+    custom_attrs = {
       severity: 0,
       confidence: 5,
       report_type: 2,
       project_id: project_id,
       scanner_id: 6,
-      primary_identifier_id: primary_identifier_id,
-      project_fingerprint: attrs[:project_fingerprint],
-      location_fingerprint: Digest::SHA1.hexdigest(SecureRandom.hex(10)),
-      uuid: attrs[:uuid],
-      name: attrs[:name],
-      metadata_version: '1.3',
-      raw_metadata: attrs[:raw_metadata]
+      primary_identifier_id: primary_identifier_id
     }
+    attrs.merge(custom_attrs)
   end
 
   def create_identifier(number_of)

ee/spec/services/security/store_report_service_spec.rb

@@ -131,12 +131,15 @@ RSpec.describe Security::StoreReportService, '#execute' do
 
     let!(:vulnerability) { create(:vulnerability, findings: [finding], project: project) }
 
-    let(:uuid_v5_components) do
-      "#{finding.report_type}-#{finding.primary_identifier.fingerprint}-#{finding.location_fingerprint}-#{finding.project_id}"
+    let(:desired_uuid) do
+      Security::VulnerabilityUUID.generate(
+        report_type: finding.report_type,
+        primary_identifier_fingerprint: finding.primary_identifier.fingerprint,
+        location_fingerprint: finding.location_fingerprint,
+        project_id: finding.project_id
+      )
     end
 
-    let(:desired_uuid) { Gitlab::UUID.v5(uuid_v5_components) }
-
     let!(:finding_with_uuidv5) do
       create(:vulnerabilities_finding,
         pipelines: [pipeline],

lib/gitlab/background_migration/populate_finding_uuid_for_vulnerability_feedback.rb

@@ -61,16 +61,12 @@ module Gitlab
         private
 
         def calculated_uuid
-          Gitlab::UUID.v5(uuid_components)
-        end
-
-        def uuid_components
-          [
-            category,
-            vulnerability_finding.primary_identifier.fingerprint,
-            vulnerability_finding.location_fingerprint,
-            project_id
-          ].join('-')
+          ::Security::VulnerabilityUUID.generate(
+            report_type: category,
+            primary_identifier_fingerprint: vulnerability_finding.primary_identifier.fingerprint,
+            location_fingerprint: vulnerability_finding.location_fingerprint,
+            project_id: project_id
+          )
         end
 
         def finding_key

spec/lib/gitlab/background_migration/populate_finding_uuid_for_vulnerability_feedback_spec.rb

@@ -27,12 +27,33 @@ RSpec.describe Gitlab::BackgroundMigration::PopulateFindingUuidForVulnerabilityF
   let(:finding_1) { finding_creator.call(sast_report, location_fingerprint_1) }
   let(:finding_2) { finding_creator.call(dast_report, location_fingerprint_2) }
   let(:finding_3) { finding_creator.call(secret_detection_report, location_fingerprint_3) }
-  let(:uuid_1_components) { ['sast', identifier.fingerprint, location_fingerprint_1, project.id].join('-') }
-  let(:uuid_2_components) { ['dast', identifier.fingerprint, location_fingerprint_2, project.id].join('-') }
-  let(:uuid_3_components) { ['secret_detection', identifier.fingerprint, location_fingerprint_3, project.id].join('-') }
-  let(:expected_uuid_1) { Gitlab::UUID.v5(uuid_1_components) }
-  let(:expected_uuid_2) { Gitlab::UUID.v5(uuid_2_components) }
-  let(:expected_uuid_3) { Gitlab::UUID.v5(uuid_3_components) }
+  let(:expected_uuid_1) do
+    Security::VulnerabilityUUID.generate(
+      report_type: 'sast',
+      primary_identifier_fingerprint: identifier.fingerprint,
+      location_fingerprint: location_fingerprint_1,
+      project_id: project.id
+    )
+  end
+
+  let(:expected_uuid_2) do
+    Security::VulnerabilityUUID.generate(
+      report_type: 'dast',
+      primary_identifier_fingerprint: identifier.fingerprint,
+      location_fingerprint: location_fingerprint_2,
+      project_id: project.id
+    )
+  end
+
+  let(:expected_uuid_3) do
+    Security::VulnerabilityUUID.generate(
+      report_type: 'secret_detection',
+      primary_identifier_fingerprint: identifier.fingerprint,
+      location_fingerprint: location_fingerprint_3,
+      project_id: project.id
+    )
+  end
+
   let(:finding_creator) do
     -> (report_type, location_fingerprint) do
       findings.create!(