Merge branch '351524_implement_retentation_period_for_security_findings' into 'master'

Add security scan status to GraphQL API

See merge request gitlab-org/gitlab!81305

app/graphql/types/base_enum.rb

@@ -38,7 +38,7 @@ module Types
         description(enum_mod.description) if use_description
 
         enum_mod.definition.each do |key, content|
-          value(key.to_s.upcase, **content)
+          value(key.to_s.upcase, value: key.to_s, description: content[:description])
         end
       end
       # rubocop: enable Graphql/Descriptions

doc/api/graphql/reference/index.md

@@ -15271,6 +15271,7 @@ Represents the security scan information.
 | ---- | ---- | ----------- |
 | <a id="scanerrors"></a>`errors` | [`[String!]!`](#string) | List of errors. |
 | <a id="scanname"></a>`name` | [`String!`](#string) | Name of the scan. |
+| <a id="scanstatus"></a>`status` | [`ScanStatus!`](#scanstatus) | Indicates the status of the scan. |
 | <a id="scanwarnings"></a>`warnings` | [`[String!]!`](#string) | List of warnings. |
 
 ### `ScanExecutionPolicy`
@@ -18142,6 +18143,20 @@ Size of UI component in SAST configuration page.
 | <a id="sastuicomponentsizemedium"></a>`MEDIUM` | Size of UI component in SAST configuration page is medium. |
 | <a id="sastuicomponentsizesmall"></a>`SMALL` | Size of UI component in SAST configuration page is small. |
 
+### `ScanStatus`
+
+The status of the security scan.
+
+| Value | Description |
+| ----- | ----------- |
+| <a id="scanstatuscreated"></a>`CREATED` | The scan has been created. |
+| <a id="scanstatusjob_failed"></a>`JOB_FAILED` | The related CI build failed. |
+| <a id="scanstatuspreparation_failed"></a>`PREPARATION_FAILED` | Report couldn't be prepared. |
+| <a id="scanstatuspreparing"></a>`PREPARING` | Preparing the report for the scan. |
+| <a id="scanstatuspurged"></a>`PURGED` | Report for the scan has been removed from the database. |
+| <a id="scanstatusreport_error"></a>`REPORT_ERROR` | The report artifact provided by the CI build couldn't be parsed. |
+| <a id="scanstatussucceeded"></a>`SUCCEEDED` | The report has been successfully prepared. |
+
 ### `SecurityReportTypeEnum`
 
 | Value | Description |

ee/app/enums/security/scan_status_enum.rb

+# frozen_string_literal: true
+
+module Security
+  module ScanStatusEnum
+    extend DeclarativeEnum
+
+    key :status
+    name 'ScanStatus'
+    description 'The status of the security scan'
+
+    define do
+      created value: 0, description: N_('The scan has been created.')
+      succeeded value: 1, description: N_('The report has been successfully prepared.')
+      job_failed value: 2, description: N_('The related CI build failed.')
+      report_error value: 3, description: N_('The report artifact provided by the CI build couldn\'t be parsed.')
+      preparing value: 4, description: N_('Preparing the report for the scan.')
+      preparation_failed value: 5, description: N_('Report couldn\'t be prepared.')
+      purged value: 6, description: N_('Report for the scan has been removed from the database.')
+    end
+  end
+end

ee/app/graphql/types/scan_status_enum.rb

+# frozen_string_literal: true
+
+module Types
+  class ScanStatusEnum < BaseEnum
+    declarative_enum ::Security::ScanStatusEnum
+  end
+end

ee/app/graphql/types/scan_type.rb

@@ -13,5 +13,6 @@ module Types
     field :errors, [GraphQL::Types::String], null: false, description: 'List of errors.'
     field :name, GraphQL::Types::String, null: false, description: 'Name of the scan.'
     field :warnings, [GraphQL::Types::String], null: false, description: 'List of warnings.'
+    field :status, Types::ScanStatusEnum, null: false, description: 'Indicates the status of the scan.'
   end
 end

ee/app/models/security/scan.rb

@@ -27,7 +27,7 @@ module Security
       cluster_image_scanning: 8
     }
 
-    enum status: { created: 0, succeeded: 1, failed: 2 }
+    declarative_enum Security::ScanStatusEnum
 
     scope :by_scan_types, -> (scan_types) { where(scan_type: sanitize_scan_types(scan_types)) }
     scope :distinct_scan_types, -> { select(:scan_type).distinct.pluck(:scan_type) }

ee/app/services/security/store_scan_service.rb

@@ -24,9 +24,11 @@ module Security
       override_finding_uuids! if override_uuids?
       set_security_scan_non_latest! if job.retried?
 
-      return deduplicate if security_scan.has_errors? || !security_scan.latest? || !security_scan.succeeded?
+      return deduplicate if !security_scan.latest? || security_scan.report_error? || security_scan.job_failed?
 
       store_findings
+
+      deduplicate_findings?
     end
 
     private
@@ -47,10 +49,16 @@ module Security
       @security_scan ||= Security::Scan.safe_find_or_create_by!(build: job, scan_type: artifact.file_type) do |scan|
         scan.processing_errors = security_report.errors.map(&:stringify_keys) if security_report.errored?
         scan.processing_warnings = security_report.warnings.map(&:stringify_keys)
-        scan.status = job.success? ? :succeeded : :failed
+        scan.status = initial_scan_status
       end
     end
 
+    def initial_scan_status
+      return :report_error if security_report.errored?
+
+      job.success? ? :preparing : :job_failed
+    end
+
     def store_findings
       StoreFindingsMetadataService.execute(security_scan, security_report, register_finding_keys).then do |result|
         # If `StoreFindingsMetadataService` returns error, it means the findings
@@ -58,7 +66,11 @@ module Security
         update_deduplicated_findings if result[:status] == :error && deduplicate_findings?
       end
 
-      deduplicate_findings?
+      security_scan.succeeded!
+    rescue StandardError => error
+      security_scan.preparation_failed!
+
+      Gitlab::ErrorTracking.track_exception(error)
     end
 
     def set_security_scan_non_latest!

ee/spec/graphql/types/scan_type_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe GitlabSchema.types['Scan'] do
   include GraphqlHelpers
 
-  let(:fields) { %i(name errors warnings) }
+  let(:fields) { %i(name errors warnings status) }
 
   it { expect(described_class).to have_graphql_fields(fields) }
   it { expect(described_class).to require_graphql_authorizations(:read_scan) }
@@ -48,5 +48,11 @@ RSpec.describe GitlabSchema.types['Scan'] do
 
       it { is_expected.to match_array(['[foo] bar']) }
     end
+
+    describe 'status' do
+      let(:field_name) { :status }
+
+      it { is_expected.to eq('created') }
+    end
   end
 end

ee/spec/models/security/scan_spec.rb

@@ -196,7 +196,7 @@ RSpec.describe Security::Scan do
   describe '.latest_successful' do
     let!(:first_successful_scan) { create(:security_scan, latest: false, status: :succeeded) }
     let!(:second_successful_scan) { create(:security_scan, latest: true, status: :succeeded) }
-    let!(:failed_scan) { create(:security_scan, latest: true, status: :failed) }
+    let!(:failed_scan) { create(:security_scan, latest: true, status: :job_failed) }
 
     subject { described_class.latest_successful }
 

ee/spec/services/security/store_scan_service_spec.rb

@@ -102,6 +102,10 @@ RSpec.describe Security::StoreScanService do
         expect(store_scan).to be(false)
         expect(Security::StoreFindingsMetadataService).not_to have_received(:execute)
       end
+
+      it 'sets the status of the scan as `report_error`' do
+        expect { store_scan }.to change { Security::Scan.report_error.count }.by(1)
+      end
     end
 
     context 'when the report is produced by a failed job' do
@@ -109,13 +113,28 @@ RSpec.describe Security::StoreScanService do
         artifact.job.update!(status: :failed)
       end
 
-      it 'does not call the `Security::StoreFindingsMetadataService` and sets the security scan as failed' do
-        expect { store_scan }.to change { Security::Scan.failed.count }.by(1)
+      it 'does not call the `Security::StoreFindingsMetadataService` and sets the security scan as `job_failed`' do
+        expect { store_scan }.to change { Security::Scan.job_failed.count }.by(1)
 
         expect(Security::StoreFindingsMetadataService).not_to have_received(:execute)
       end
     end
 
+    context 'when storing the findings raises an error' do
+      let(:error) { RuntimeError.new }
+
+      before do
+        allow(Security::StoreFindingsMetadataService).to receive(:execute).and_raise(error)
+        allow(Gitlab::ErrorTracking).to receive(:track_exception)
+      end
+
+      it 'marks the security scan as `preparation_failed` and tracks the error' do
+        expect { store_scan }.to change { Security::Scan.preparation_failed.count }.by(1)
+
+        expect(Gitlab::ErrorTracking).to have_received(:track_exception).with(error)
+      end
+    end
+
     context 'when the report is produced by a retried job' do
       before do
         artifact.job.update!(retried: true)

locale/gitlab.pot

@@ -27737,6 +27737,9 @@ msgstr ""
 msgid "Preferences|When you type in a description or comment box, selected text is surrounded by the corresponding character after typing one of the following characters: %{supported_characters}."
 msgstr ""
 
+msgid "Preparing the report for the scan."
+msgstr ""
+
 msgid "Prev"
 msgstr ""
 
@@ -30792,6 +30795,12 @@ msgstr ""
 msgid "Report abuse to admin"
 msgstr ""
 
+msgid "Report couldn't be prepared."
+msgstr ""
+
+msgid "Report for the scan has been removed from the database."
+msgstr ""
+
 msgid "Reported %{timeAgo} by %{reportedBy}"
 msgstr ""
 
@@ -36825,6 +36834,9 @@ msgstr ""
 msgid "The regular expression used to find test coverage output in the job log. For example, use %{regex} for Simplecov (Ruby). Leave blank to disable."
 msgstr ""
 
+msgid "The related CI build failed."
+msgstr ""
+
 msgid "The remote mirror URL is invalid."
 msgstr ""
 
@@ -36834,6 +36846,12 @@ msgstr ""
 msgid "The remote repository is being updated..."
 msgstr ""
 
+msgid "The report artifact provided by the CI build couldn't be parsed."
+msgstr ""
+
+msgid "The report has been successfully prepared."
+msgstr ""
+
 msgid "The repository can be committed to, and issues, comments and other entities can be created."
 msgstr ""
 
@@ -36855,6 +36873,9 @@ msgstr ""
 msgid "The same shared runner executes code from multiple projects, unless you configure autoscaling with %{link} set to 1 (which it is on GitLab.com)."
 msgstr ""
 
+msgid "The scan has been created."
+msgstr ""
+
 msgid "The snippet can be accessed without any authentication."
 msgstr ""
 

spec/graphql/types/base_enum_spec.rb

@@ -102,9 +102,9 @@ RSpec.describe Types::BaseEnum do
       it 'sets the values defined by the declarative enum' do
         set_declarative_enum
 
-        expect(enum_type.values.keys).to eq(['FOO'])
-        expect(enum_type.values.values.map(&:description)).to eq(['description of foo'])
-        expect(enum_type.values.values.map(&:value)).to eq([0])
+        expect(enum_type.values.keys).to contain_exactly('FOO')
+        expect(enum_type.values.values.map(&:description)).to contain_exactly('description of foo')
+        expect(enum_type.values.values.map(&:value)).to contain_exactly('foo')
       end
     end
   end