Merge branch 'sk/342025-migrate-nil-location' into 'master'

Add migration to fix nil location value in vulnerability_occurrences

See merge request gitlab-org/gitlab!72788

db/post_migrate/20211102103127_add_temp_index_to_vulnerability_occurrences.rb

+# frozen_string_literal: true
+
+class AddTempIndexToVulnerabilityOccurrences < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'vulnerability_occurrences_location_temp_index'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :vulnerability_occurrences, :id, where: 'location IS NULL', name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :vulnerability_occurrences, name: INDEX_NAME
+  end
+end

db/post_migrate/20211102114802_update_vulnerability_occurrences_location.rb

+# frozen_string_literal: true
+
+class UpdateVulnerabilityOccurrencesLocation < Gitlab::Database::Migration[1.0]
+  BATCH_SIZE = 20_000
+  DELAY_INTERVAL = 3.minutes
+  MIGRATION_NAME = 'UpdateVulnerabilityOccurrencesLocation'
+
+  disable_ddl_transaction!
+
+  def up
+    relation = Gitlab::BackgroundMigration::UpdateVulnerabilityOccurrencesLocation::Occurrence.where(location: nil)
+    queue_background_migration_jobs_by_range_at_intervals(relation,
+                                                          MIGRATION_NAME,
+                                                          DELAY_INTERVAL,
+                                                          batch_size: BATCH_SIZE,
+                                                          track_jobs: true)
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20211102103127

+450028c90cb92f5ce3f8239eb56364b83ed025839aaf305b7ceb4fda077681b1
\ No newline at end of file

db/schema_migrations/20211102114802

+a3f9fcac354cccfdfc42b8f5baab651cb65ca60e4474ce937ab25b552bfe483c
\ No newline at end of file

db/structure.sql

@@ -27795,6 +27795,8 @@ CREATE UNIQUE INDEX vulnerability_feedback_unique_idx ON vulnerability_feedback
 
 CREATE UNIQUE INDEX vulnerability_occurrence_pipelines_on_unique_keys ON vulnerability_occurrence_pipelines USING btree (occurrence_id, pipeline_id);
 
+CREATE INDEX vulnerability_occurrences_location_temp_index ON vulnerability_occurrences USING btree (id) WHERE (location IS NULL);
+
 CREATE UNIQUE INDEX work_item_types_namespace_id_and_name_unique ON work_item_types USING btree (namespace_id, btrim(lower(name)));
 
 ALTER INDEX analytics_cycle_analytics_issue_stage_events_pkey ATTACH PARTITION gitlab_partitions_static.analytics_cycle_analytics_issue_stage_events_00_pkey;

ee/lib/ee/gitlab/background_migration/update_vulnerability_occurrences_location.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module BackgroundMigration
+      module UpdateVulnerabilityOccurrencesLocation
+        LIST_SEPARATOR = ', '
+
+        class Occurrence < ActiveRecord::Base
+          include ::EachBatch
+
+          self.table_name = 'vulnerability_occurrences'
+        end
+
+        def perform(start_id, end_id)
+          Occurrence.select(:id, :raw_metadata).where(id: start_id..end_id, location: nil).each_batch(of: 500) do |occurrences|
+            location_values = parse_location(occurrences)
+
+            next if location_values.empty?
+
+            batch_update_location(location_values)
+            sleep(0.1.seconds)
+          end
+        end
+
+        def parse_location(occurrences)
+          location_values = []
+          occurrences.each do |occurrence|
+            begin
+              raw_metadata = ::Gitlab::Json.parse(occurrence.raw_metadata)
+            rescue JSON::ParserError
+              next
+            end
+
+            next if raw_metadata.empty?
+
+            if raw_metadata['location']
+              location_id_pair = list_of([occurrence.id, "'#{raw_metadata['location'].to_json}'::jsonb"])
+              location_values << "(#{location_id_pair})"
+            end
+          end
+
+          location_values
+        end
+
+        def batch_update_location(location_values)
+          Occurrence.connection.exec_update <<~SQL
+            WITH cte(cte_id, cte_location) AS #{::Gitlab::Database::AsWithMaterialized.materialized_if_supported} (VALUES #{list_of(location_values)})
+            UPDATE vulnerability_occurrences
+            SET
+              location = cte_location
+            FROM
+              cte
+            WHERE
+              cte_id = id
+          SQL
+        end
+
+        def list_of(list)
+          list.join(LIST_SEPARATOR)
+        end
+      end
+    end
+  end
+end

ee/spec/lib/ee/gitlab/background_migration/update_vulnerability_occurrences_location_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::UpdateVulnerabilityOccurrencesLocation, schema: 20211102114802 do
+  let_it_be(:namespaces) { table(:namespaces) }
+  let_it_be(:group) { namespaces.create!(name: 'foo', path: 'foo') }
+  let_it_be(:projects) { table(:projects) }
+  let_it_be(:findings) { table(:vulnerability_occurrences) }
+  let_it_be(:scanners) { table(:vulnerability_scanners) }
+  let_it_be(:identifiers) { table(:vulnerability_identifiers) }
+
+  let_it_be(:project) { projects.create!(id: 123, namespace_id: group.id, name: 'gitlab', path: 'gitlab') }
+
+  let_it_be(:scanner) do
+    scanners.create!(id: 6, project_id: project.id, external_id: 'trivy', name: 'Security Scanner')
+  end
+
+  let_it_be(:identifier) do
+    identifiers.create!(id: 123,
+                        project_id: 123,
+                        fingerprint: 'd432c2ad2953e8bd587a3a43b3ce309b5b0154c123',
+                        external_type: 'SECURITY_ID',
+                        external_id: 'SECURITY_0',
+                        name: 'SECURITY_IDENTIFIER 0')
+  end
+
+  let_it_be(:location) do
+    { "file" => "maven/src/main/java/com/gitlab/security_products/tests/App.java", "start_line" => 29, "end_line" => 29, "class" => "com.gitlab.security_products.tests.App", "method" => "insecureCypher" }
+  end
+
+  let_it_be(:raw_metadata) do
+    { "description" => "The cipher does not provide data integrity update 1",
+      "message" => "The cipher does not provide data integrity",
+      "cve" => "818bf5dacb291e15d9e6dc3c5ac32178:CIPHER",
+      "solution" => "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
+      "location" => location,
+      "links" => [{ "name" => "Cipher does not check for integrity first?", "url" => "https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first" }],
+      "assets" => [{ "type" => "postman", "name" => "Test Postman Collection", "url" => "http://localhost/test.collection" }],
+      "evidence" =>
+        { "summary" => "Credit card detected",
+          "request" => { "headers" => [{ "name" => "Accept", "value" => "*/*" }], "method" => "GET", "url" => "http://goat:8080/WebGoat/logout", "body" => nil },
+          "response" => { "headers" => [{ "name" => "Content-Length", "value" => "0" }], "reason_phrase" => "OK", "status_code" => 200, "body" => nil },
+          "source" => { "id" => "assert:Response Body Analysis", "name" => "Response Body Analysis", "url" => "htpp://hostname/documentation" },
+          "supporting_messages" =>
+            [{ "name" => "Origional", "request" => { "headers" => [{ "name" => "Accept", "value" => "*/*" }], "method" => "GET", "url" => "http://goat:8080/WebGoat/logout", "body" => "" } },
+             { "name" => "Recorded",
+               "request" => { "headers" => [{ "name" => "Accept", "value" => "*/*" }], "method" => "GET", "url" => "http://goat:8080/WebGoat/logout", "body" => "" },
+               "response" => { "headers" => [{ "name" => "Content-Length", "value" => "0" }], "reason_phrase" => "OK", "status_code" => 200, "body" => "" } }] } }
+  end
+
+  shared_examples 'does not update location' do
+    it 'does not update location' do
+      expect(findings.where(location: nil).count).to eq(2)
+
+      described_class.new.perform(vul1.id, vul2.id)
+
+      expect(findings.where(location: nil).count).to eq(2)
+      expect(vul1.reload.read_attribute(:location)).to be_nil
+      expect(vul2.reload.read_attribute(:location)).to be_nil
+    end
+  end
+
+  context 'when raw_metadata is a valid json' do
+    let_it_be(:vul1) { findings.create!(finding_params) }
+    let_it_be(:vul2) { findings.create!(finding_params) }
+    let_it_be(:vul3) { findings.create!(finding_params.merge(location: { "cluster_id" => "1" })) }
+
+    it 'updates location' do
+      expect(findings.where(location: nil).count).to eq(2)
+
+      described_class.new.perform(vul1.id, vul2.id)
+
+      expect(findings.where(location: nil).count).to eq(0)
+      expect(vul1.reload.location).to eq(raw_metadata['location'])
+      expect(vul2.reload.location).to eq(raw_metadata['location'])
+      expect(vul3.reload.location).to eq({ "cluster_id" => "1" })
+    end
+  end
+
+  context 'when raw_metadata is invalid json' do
+    let_it_be(:raw_metadata) { 'invalid' }
+    let_it_be(:vul1) { findings.create!(finding_params) }
+    let_it_be(:vul2) { findings.create!(finding_params) }
+
+    it_behaves_like 'does not update location'
+  end
+
+  context 'when raw_metadata is empty' do
+    let_it_be(:raw_metadata) { '' }
+    let_it_be(:vul1) { findings.create!(finding_params) }
+    let_it_be(:vul2) { findings.create!(finding_params) }
+
+    it_behaves_like 'does not update location'
+  end
+
+  def finding_params
+    uuid = SecureRandom.uuid
+
+    {
+      severity: 0,
+      confidence: 5,
+      report_type: 2,
+      project_id: 123,
+      scanner_id: 6,
+      primary_identifier_id: 123,
+      location: nil,
+      project_fingerprint: SecureRandom.hex(20),
+      location_fingerprint: Digest::SHA1.hexdigest(SecureRandom.hex(10)),
+      uuid: uuid,
+      name: "Vulnerability Finding #{uuid}",
+      metadata_version: '1.3',
+      raw_metadata: raw_metadata.to_json
+    }
+  end
+end

ee/spec/migrations/update_vulnerability_occurrences_location_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe UpdateVulnerabilityOccurrencesLocation, :migration do
+  let(:migration_name) { 'UpdateVulnerabilityOccurrencesLocation' }
+
+  let_it_be(:namespaces) { table(:namespaces) }
+  let_it_be(:group) { namespaces.create!(name: 'foo', path: 'foo') }
+  let_it_be(:projects) { table(:projects) }
+  let_it_be(:findings) { table(:vulnerability_occurrences) }
+  let_it_be(:scanners) { table(:vulnerability_scanners) }
+  let_it_be(:identifiers) { table(:vulnerability_identifiers) }
+
+  let_it_be(:project) { projects.create!(id: 123, namespace_id: group.id, name: 'gitlab', path: 'gitlab') }
+
+  let_it_be(:scanner) do
+    scanners.create!(id: 6, project_id: project.id, external_id: 'trivy', name: 'Security Scanner')
+  end
+
+  let_it_be(:identifier) do
+    identifiers.create!(id: 123,
+                        project_id: 123,
+                        fingerprint: 'd432c2ad2953e8bd587a3a43b3ce309b5b0154c123',
+                        external_type: 'SECURITY_ID',
+                        external_id: 'SECURITY_0',
+                        name: 'SECURITY_IDENTIFIER 0')
+  end
+
+  let_it_be(:location) do
+    { "file" => "maven/src/main/java/com/gitlab/security_products/tests/App.java", "start_line" => 29, "end_line" => 29, "class" => "com.gitlab.security_products.tests.App", "method" => "insecureCypher" }
+  end
+
+  let_it_be(:vul1) { findings.create!(finding_params) }
+  let_it_be(:vul2) { findings.create!(finding_params) }
+  let_it_be(:vul3) { findings.create!(finding_params(location)) }
+
+  it 'correctly schedules background migrations' do
+    Sidekiq::Testing.fake! do
+      freeze_time do
+        migrate!
+
+        expect(BackgroundMigrationWorker.jobs.size).to eq(1)
+        expect(migration_name).to be_scheduled_migration_with_multiple_args(vul1.id, vul2.id)
+      end
+    end
+  end
+
+  private
+
+  def finding_params(location = nil)
+    uuid = SecureRandom.uuid
+
+    {
+      severity: 0,
+      confidence: 5,
+      report_type: 2,
+      project_id: 123,
+      scanner_id: 6,
+      primary_identifier_id: 123,
+      location: location,
+      project_fingerprint: SecureRandom.hex(20),
+      location_fingerprint: Digest::SHA1.hexdigest(SecureRandom.hex(10)),
+      uuid: uuid,
+      name: "Vulnerability Finding #{uuid}",
+      metadata_version: '1.3',
+      raw_metadata: { location: location }.to_json
+    }
+  end
+end

lib/gitlab/background_migration/update_vulnerability_occurrences_location.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # rubocop: disable Style/Documentation
+    class UpdateVulnerabilityOccurrencesLocation
+      def perform(start_id, stop_id)
+      end
+    end
+    # rubocop: enable Style/Documentation
+  end
+end
+
+Gitlab::BackgroundMigration::UpdateVulnerabilityOccurrencesLocation.prepend_mod_with('Gitlab::BackgroundMigration::UpdateVulnerabilityOccurrencesLocation')