Merge branch '343332_update_vulnerability_statistics_after_ingestion' into 'master'

Update vulnerability statistics on report ingestion

See merge request gitlab-org/gitlab!75268

ee/app/models/vulnerabilities/statistic.rb

@@ -39,6 +39,28 @@ module Vulnerabilities
         end
       end
 
+      def letter_grade_sql_for(target_values, excluded_values)
+        <<~SQL
+          SELECT (
+            CASE
+            WHEN TARGET.critical + EXCLUDED.critical > 0 THEN
+              #{Vulnerabilities::Statistic.letter_grades[:f]}
+            WHEN TARGET.high + TARGET.unknown + EXCLUDED.high + EXCLUDED.unknown > 0 THEN
+              #{Vulnerabilities::Statistic.letter_grades[:d]}
+            WHEN TARGET.medium + EXCLUDED.medium > 0 THEN
+              #{Vulnerabilities::Statistic.letter_grades[:c]}
+            WHEN TARGET.low + EXCLUDED.low > 0 THEN
+              #{Vulnerabilities::Statistic.letter_grades[:b]}
+            ELSE
+              #{Vulnerabilities::Statistic.letter_grades[:a]}
+            END
+          ) as letter_grade
+          FROM
+            (values #{target_values}) as TARGET (critical, unknown, high, medium, low),
+            (values #{excluded_values}) as EXCLUDED (critical, unknown, high, medium, low)
+        SQL
+      end
+
       def set_latest_pipeline_with(pipeline)
         upsert_sql = upsert_latest_pipeline_id_sql(pipeline)
 

ee/app/services/security/ingestion/finding_map.rb

@@ -13,7 +13,7 @@ module Security
       attr_reader :security_finding, :report_finding
       attr_accessor :finding_id, :vulnerability_id, :new_record, :identifier_ids
 
-      delegate :uuid, :scanner_id, to: :security_finding
+      delegate :uuid, :scanner_id, :severity, to: :security_finding
       delegate :scan, to: :security_finding, private: true
       delegate :project, to: :scan, private: true
       delegate :project_fingerprint, to: :report_finding, private: true

ee/app/services/security/ingestion/ingest_report_slice_service.rb

@@ -18,6 +18,7 @@ module Security
         IngestFindingSignatures
         IngestVulnerabilityFlags
         IngestIssueLinks
+        IngestVulnerabilityStatistics
         IngestRemediations
       ].freeze
 

ee/app/services/security/ingestion/tasks/ingest_vulnerability_statistics.rb

+# frozen_string_literal: true
+
+module Security
+  module Ingestion
+    module Tasks
+      # Ingests the data for the `Vulnerabilities::Statistic` model.
+      #
+      # This ingestion task is neither `BulkInsertable` nor `BulkUpdatable` like
+      # the rest of the ingestion tasks.
+      # The data stored in the `vulnerability_statistics` table is incremental
+      # therefore we need a different type of UPSERT query which is not supported
+      # by ActiveRecord yet.
+      class IngestVulnerabilityStatistics < AbstractTask
+        TARGET_VALUES = "(TARGET.critical, TARGET.unknown, TARGET.high, TARGET.medium, TARGET.low)"
+        EXCLUDED_VALUES = "(EXCLUDED.critical, EXCLUDED.unknown, EXCLUDED.high, EXCLUDED.medium, EXCLUDED.low)"
+        UPSERT_SQL_TEMPLATE = <<~SQL
+          INSERT INTO #{Vulnerabilities::Statistic.table_name} AS target (project_id, %{insert_attributes}, letter_grade, created_at, updated_at)
+            VALUES (%{project_id}, %{insert_values}, %{letter_grade}, now(), now())
+          ON CONFLICT (project_id)
+            DO UPDATE SET
+              %{update_values},
+              letter_grade = (#{Vulnerabilities::Statistic.letter_grade_sql_for(TARGET_VALUES, EXCLUDED_VALUES)}),
+              updated_at = now()
+        SQL
+
+        def execute
+          connection.execute(upsert_sql)
+        end
+
+        private
+
+        delegate :project_id, to: :pipeline, private: true
+        delegate :connection, to: ::Vulnerabilities::Statistic, private: true
+        delegate :quote, :quote_column_name, to: :connection, private: true
+
+        def upsert_sql
+          format(
+            UPSERT_SQL_TEMPLATE,
+            project_id: project_id,
+            insert_attributes: insert_attributes,
+            insert_values: insert_values,
+            letter_grade: letter_grade,
+            update_values: update_values
+          )
+        end
+
+        def insert_attributes
+          sql_safe_severity_counts.keys.join(', ')
+        end
+
+        def insert_values
+          sql_safe_severity_counts.values.join(', ')
+        end
+
+        # We have a `before_save` callback in `Vulnerabilities::Statistic` model
+        # but the `before_save` callbacks are not invocated by the `BulkInsertSafe` module
+        # so we have to calculate it here.
+        def letter_grade
+          Vulnerabilities::Statistic.letter_grade_for(severity_counts)
+        end
+
+        def update_values
+          sql_safe_severity_counts.map { |severity, count| "#{severity} = TARGET.#{severity} + #{count}" }
+                                 .join(', ')
+        end
+
+        def sql_safe_severity_counts
+          @sql_safe_severity_counts ||= severity_counts.transform_keys(&method(:quote_column_name))
+                                                       .transform_values(&method(:quote))
+        end
+
+        def severity_counts
+          @severity_counts ||= finding_maps.select(&:new_record)
+                                           .map(&:severity)
+                                           .tally
+        end
+      end
+    end
+  end
+end

ee/spec/models/vulnerabilities/statistic_spec.rb

@@ -47,6 +47,56 @@ RSpec.describe Vulnerabilities::Statistic do
     end
   end
 
+  describe '.letter_grade_sql_for' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:target_critical, :target_unknown, :target_high, :target_medium, :target_low, :excluded_critical, :excluded_unknown, :excluded_high, :excluded_medium, :excluded_low) do
+      0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0
+
+      0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 1
+      0 | 0 | 0 | 0 | 1 | 0 | 0 | 0 | 0 | 0
+      0 | 0 | 0 | 0 | 1 | 0 | 0 | 0 | 0 | 1
+
+      0 | 0 | 0 | 0 | 1 | 0 | 0 | 0 | 1 | 1
+      0 | 0 | 0 | 1 | 1 | 0 | 0 | 0 | 0 | 1
+      0 | 0 | 0 | 1 | 1 | 0 | 0 | 0 | 1 | 1
+
+      0 | 0 | 0 | 1 | 1 | 0 | 0 | 1 | 1 | 1
+      0 | 0 | 1 | 1 | 1 | 0 | 0 | 0 | 1 | 1
+      0 | 0 | 1 | 1 | 1 | 0 | 0 | 1 | 1 | 1
+
+      0 | 0 | 1 | 1 | 1 | 0 | 1 | 1 | 1 | 1
+      0 | 1 | 1 | 1 | 1 | 0 | 0 | 1 | 1 | 1
+      0 | 1 | 1 | 1 | 1 | 0 | 1 | 1 | 1 | 1
+
+      0 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1
+      1 | 1 | 1 | 1 | 1 | 0 | 1 | 1 | 1 | 1
+      1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1
+    end
+
+    with_them do
+      let(:target) { "(#{target_critical}, #{target_unknown}, #{target_high}, #{target_medium}, #{target_low})" }
+      let(:excluded) { "(#{excluded_critical}, #{excluded_unknown}, #{excluded_high}, #{excluded_medium}, #{excluded_low})" }
+      let(:object) do
+        {
+          critical: target_critical + excluded_critical,
+          uknown:   target_unknown + excluded_unknown,
+          high:     target_high + excluded_high,
+          medium:   target_medium + excluded_medium,
+          low:      target_low + excluded_low
+        }.stringify_keys
+      end
+
+      let(:letter_grade_sql) { described_class.letter_grade_sql_for(target, excluded) }
+      let(:letter_grade_on_db) { described_class.connection.execute(letter_grade_sql).first['letter_grade'] }
+      let(:letter_grade_on_app) { described_class.letter_grade_for(object) }
+
+      it 'matches the application layer logic' do
+        expect(letter_grade_on_db).to eq(letter_grade_on_app)
+      end
+    end
+  end
+
   describe '.set_latest_pipeline_with' do
     let_it_be(:pipeline) { create(:ci_pipeline) }
     let_it_be(:project) { pipeline.project }

ee/spec/services/security/ingestion/ingest_report_slice_service_spec.rb

@@ -31,6 +31,7 @@ RSpec.describe Security::Ingestion::IngestReportSliceService do
       expect(Security::Ingestion::Tasks::IngestFindingSignatures).to have_received(:execute).ordered.with(pipeline, finding_maps)
       expect(Security::Ingestion::Tasks::IngestVulnerabilityFlags).to have_received(:execute).ordered.with(pipeline, finding_maps)
       expect(Security::Ingestion::Tasks::IngestIssueLinks).to have_received(:execute).ordered.with(pipeline, finding_maps)
+      expect(Security::Ingestion::Tasks::IngestVulnerabilityStatistics).to have_received(:execute).ordered.with(pipeline, finding_maps)
       expect(Security::Ingestion::Tasks::IngestRemediations).to have_received(:execute).ordered.with(pipeline, finding_maps)
     end
 

ee/spec/services/security/ingestion/tasks/ingest_vulnerability_statistics_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::Ingestion::Tasks::IngestVulnerabilityStatistics do
+  describe '#execute' do
+    let_it_be(:pipeline) { create(:ci_pipeline) }
+    let_it_be(:project) { pipeline.project }
+    let_it_be(:security_finding_1) { create(:security_finding, severity: :critical) }
+    let_it_be(:security_finding_2) { create(:security_finding, severity: :medium) }
+    let_it_be(:security_finding_3) { create(:security_finding, severity: :low) }
+    let_it_be(:finding_map_1) { create(:finding_map, :new_record, security_finding: security_finding_1) }
+    let_it_be(:finding_map_2) { create(:finding_map, :new_record, security_finding: security_finding_2) }
+    let_it_be(:finding_map_3) { create(:finding_map, :with_finding, security_finding: security_finding_3) }
+    let_it_be(:finding_maps) { [finding_map_1, finding_map_2, finding_map_3] }
+
+    subject(:ingest_statistics) { described_class.new(pipeline, finding_maps).execute }
+
+    context 'when there is no statistics record for the project' do
+      it 'creates a new Vulnerabilities::Statistic record' do
+        expect { ingest_statistics }.to change { Vulnerabilities::Statistic.where(project: project).count }.by(1)
+      end
+
+      it 'sets the correct attributes for the recently created record' do
+        ingest_statistics
+
+        expect(project.vulnerability_statistic).to have_attributes(critical: 1, high: 0, unknown: 0, medium: 1, low: 0, letter_grade: 'f')
+      end
+    end
+
+    context 'when there is already a statistics record for the project' do
+      let_it_be(:vulnerability_statistic) { create(:vulnerability_statistic, :grade_c, project: project) }
+
+      it 'does not create a new record and updates the existing one' do
+        expect { ingest_statistics }.to change { vulnerability_statistic.reload.letter_grade }.from('c').to('f')
+                                    .and change { vulnerability_statistic.reload.critical }.from(0).to(1)
+                                    .and change { vulnerability_statistic.reload.medium }.from(1).to(2)
+                                    .and not_change { Vulnerabilities::Statistic.count }
+      end
+    end
+  end
+end