diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index f3d2ee67258beef27fa248eecb2226f4a997ad70..c55886293a55182128197076dceaa23c884f113e 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -33331,6 +33331,7 @@ Returns [`FindingReportsComparer`](#findingreportscomparer). | Name | Type | Description | | ---- | ---- | ----------- | | `reportType` | [`ComparableSecurityReportType!`](#comparablesecurityreporttype) | Filter vulnerability findings by report type. | +| `scanMode` | [`ScanModeEnum`](#scanmodeenum) | Filter results by scan mode. | ##### `MergeRequest.notes` @@ -48709,6 +48710,16 @@ Values for sbom source types. | `DEPENDENCY_SCANNING` | Source Type: dependency_scanning. | | `NIL_SOURCE` | Enum source nil. | +### `ScanModeEnum` + +Options for filtering by scan mode. + +| Value | Description | +| ----- | ----------- | +| `ALL` | Return results from all scans. | +| `FULL` | Return results from full scans. | +| `PARTIAL` | Return results from partial scans. | + ### `ScanStatus` The status of the security scan. diff --git a/ee/app/controllers/ee/projects/merge_requests_controller.rb b/ee/app/controllers/ee/projects/merge_requests_controller.rb index b3fb97e907a621288e469e4063b6387365036e31..d29bf019cec84241b4b0a2a0bd9d43a748f52ae5 100644 --- a/ee/app/controllers/ee/projects/merge_requests_controller.rb +++ b/ee/app/controllers/ee/projects/merge_requests_controller.rb @@ -95,7 +95,9 @@ def api_fuzzing_reports end def security_reports - report = ::Security::MergeRequestSecurityReportGenerationService.execute(merge_request, params[:type]) + params[:report_type] = params[:type] + comparison_params = params.permit(:report_type, :scan_mode) + report = ::Security::MergeRequestSecurityReportGenerationService.execute(merge_request, comparison_params) reports_response(report, head_pipeline) rescue ::Security::MergeRequestSecurityReportGenerationService::InvalidReportTypeError diff --git a/ee/app/finders/security/findings_finder.rb b/ee/app/finders/security/findings_finder.rb index 7da86d1ece8defff8e9e529fb5ec6bc5d5902336..01fb0986415ca92cc28a23249acce0b8cefc2ed9 100644 --- a/ee/app/finders/security/findings_finder.rb +++ b/ee/app/finders/security/findings_finder.rb @@ -8,10 +8,11 @@ # Arguments: # pipeline - object to filter findings # params: -# severity: Array -# report_type: Array -# scope: String -# limit: Int +# severity: Array +# report_type: Array +# scan_mode: String +# scope: String +# limit: Int module Security class FindingsFinder @@ -65,6 +66,7 @@ def execute .then { |relation| by_scanner_external_ids(relation) } .then { |relation| by_state(relation) } .then { |relation| by_include_dismissed(relation) } + .then { |relation| by_scan_mode(relation) } .limit(limit + 1) from_sql = <<~SQL.squish @@ -138,6 +140,34 @@ def by_report_types(relation) relation.merge(::Security::Scan.by_scan_types(params[:report_type])) end + def by_scan_mode(relation) + scan_mode = params.fetch(:scan_mode, 'all') + + return relation if ::Feature.disabled?(:vulnerability_partial_scans, project) || scan_mode == 'all' + + exists_clause = <<~SQL + EXISTS ( + SELECT + 1 + FROM + "vulnerability_partial_scans" + WHERE + "vulnerability_partial_scans"."scan_id" = "security_scans"."id" + ) + SQL + + # rubocop:disable CodeReuse/ActiveRecord -- Requires lateral join with security scans + case scan_mode + when 'full' + relation.where.not(exists_clause) + when 'partial' + relation.where(exists_clause) + else + relation + end + # rubocop:enable CodeReuse/ActiveRecord + end + def severities if params[:severity] Security::Finding.severities.fetch_values(*params[:severity]) diff --git a/ee/app/graphql/resolvers/security_report/finding_reports_comparer_resolver.rb b/ee/app/graphql/resolvers/security_report/finding_reports_comparer_resolver.rb index 0ae3fb3d665691ac2a50eaf778bdd9c494b84c3b..bb7131fb5603a7458e0cb8852be0986cf0ec318b 100644 --- a/ee/app/graphql/resolvers/security_report/finding_reports_comparer_resolver.rb +++ b/ee/app/graphql/resolvers/security_report/finding_reports_comparer_resolver.rb @@ -14,11 +14,15 @@ class FindingReportsComparerResolver < BaseResolver required: true, description: 'Filter vulnerability findings by report type.' - def resolve(report_type:) + argument :scan_mode, Types::Security::ScanModeEnum, + required: false, + description: 'Filter results by scan mode.' + + def resolve(**params) # Necessary to use project as actor in FF check context[:project] = object.project - ::Security::MergeRequestSecurityReportGenerationService.execute(object, report_type) + ::Security::MergeRequestSecurityReportGenerationService.execute(object, params) end end end diff --git a/ee/app/graphql/types/security/scan_mode_enum.rb b/ee/app/graphql/types/security/scan_mode_enum.rb new file mode 100644 index 0000000000000000000000000000000000000000..83e3f918266cfb1cc1e685128629ec06b6c3e244 --- /dev/null +++ b/ee/app/graphql/types/security/scan_mode_enum.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Types + module Security + class ScanModeEnum < Types::BaseEnum + graphql_name 'ScanModeEnum' + description 'Options for filtering by scan mode.' + + value 'ALL', value: 'all', description: "Return results from all scans." + value 'FULL', value: 'full', description: "Return results from full scans." + value 'PARTIAL', value: 'partial', description: "Return results from partial scans." + end + end +end diff --git a/ee/app/services/ci/compare_security_reports_service.rb b/ee/app/services/ci/compare_security_reports_service.rb index 1965615c0efe15175e89a78dbbbd43b62c08667f..4cbafda355f518d3423ac4a85f4c4d02c00317dd 100644 --- a/ee/app/services/ci/compare_security_reports_service.rb +++ b/ee/app/services/ci/compare_security_reports_service.rb @@ -68,6 +68,7 @@ def get_report(pipeline) params: { report_type: [params[:report_type]], scope: 'all', + scan_mode: params[:scan_mode], limit: Gitlab::Ci::Reports::Security::SecurityFindingsReportsComparer::MAX_FINDINGS_COUNT } ).execute.with_api_scopes diff --git a/ee/app/services/security/merge_request_security_report_generation_service.rb b/ee/app/services/security/merge_request_security_report_generation_service.rb index 6ede9cac4bbb7bd50dcb694b2debe76f17f6cfc5..666993e8c1e78fc5c8b41e585aa32f595f56569e 100644 --- a/ee/app/services/security/merge_request_security_report_generation_service.rb +++ b/ee/app/services/security/merge_request_security_report_generation_service.rb @@ -3,6 +3,7 @@ module Security class MergeRequestSecurityReportGenerationService include Gitlab::Utils::StrongMemoize + include ReactiveCaching DEFAULT_FINDING_STATE = 'detected' ALLOWED_REPORT_TYPES = %w[sast secret_detection container_scanning @@ -10,13 +11,24 @@ class MergeRequestSecurityReportGenerationService InvalidReportTypeError = Class.new(ArgumentError) - def self.execute(merge_request, report_type) - new(merge_request, report_type).execute + self.reactive_cache_work_type = :no_dependency + self.reactive_cache_worker_finder = ->(id, params) { from_cache(id, params) } + + def self.execute(merge_request, params) + new(merge_request, params).execute + end + + def self.from_cache(merge_request_id, params) + merge_request = ::MergeRequest.find_by_id(merge_request_id) + + return unless merge_request + + new(merge_request, params) end - def initialize(merge_request, report_type) + def initialize(merge_request, params) @merge_request = merge_request - @report_type = report_type + @params = params.to_h.symbolize_keys end def execute @@ -28,9 +40,15 @@ def execute report end + delegate :project, :id, to: :merge_request + private - attr_reader :merge_request, :report_type + attr_reader :merge_request, :params + + def report_type + params[:report_type] + end def report_available? report[:status] == :parsed @@ -88,26 +106,30 @@ def fixed_findings strong_memoize_attr def report validate_report_type! - case report_type - when 'sast' - merge_request.compare_sast_reports(nil) - when 'secret_detection' - merge_request.compare_secret_detection_reports(nil) - when 'container_scanning' - merge_request.compare_container_scanning_reports(nil) - when 'dependency_scanning' - merge_request.compare_dependency_scanning_reports(nil) - when 'dast' - merge_request.compare_dast_reports(nil) - when 'coverage_fuzzing' - merge_request.compare_coverage_fuzzing_reports(nil) - when 'api_fuzzing' - merge_request.compare_api_fuzzing_reports(nil) - end + with_reactive_cache(params.stringify_keys) do |data| + latest = Ci::CompareSecurityReportsService.new(project, nil, params).latest?(base_pipeline, head_pipeline, data) + raise InvalidateReactiveCache unless latest + + data + end || { status: :parsing } end def validate_report_type! raise InvalidReportTypeError unless ALLOWED_REPORT_TYPES.include?(report_type) end + + def calculate_reactive_cache(cache_params) + Ci::CompareSecurityReportsService + .new(project, nil, cache_params.symbolize_keys) + .execute(base_pipeline, head_pipeline) + end + + def base_pipeline + merge_request.comparison_base_pipeline(Ci::CompareSecurityReportsService) + end + + def head_pipeline + merge_request.diff_head_pipeline + end end end diff --git a/ee/spec/factories/ci/builds.rb b/ee/spec/factories/ci/builds.rb index e5da1bc0aa59066c941aa67a9a91d622af8ad803..0812746a8522be74829cc5872e88f036f80542b2 100644 --- a/ee/spec/factories/ci/builds.rb +++ b/ee/spec/factories/ci/builds.rb @@ -91,6 +91,12 @@ end end + trait :sast_differential_scan do + after(:build) do |build| + build.job_artifacts << build(:ee_ci_job_artifact, :sast_differential_scan, job: build) + end + end + trait :secret_detection_feature_branch do after(:build) do |build| build.job_artifacts << build(:ee_ci_job_artifact, :secret_detection_feature_branch, job: build) diff --git a/ee/spec/factories/ci/pipelines.rb b/ee/spec/factories/ci/pipelines.rb index 19cf911270afdf057500b739771da21ba9eabb3b..d7092e0b2396d5d214577db07b1848fd56d50746 100644 --- a/ee/spec/factories/ci/pipelines.rb +++ b/ee/spec/factories/ci/pipelines.rb @@ -71,6 +71,14 @@ end end + trait :sast_differential_scan do + status { :success } + + after(:build) do |pipeline, evaluator| + pipeline.builds << build(:ee_ci_build, :sast_differential_scan, pipeline: pipeline, project: pipeline.project) + end + end + trait :with_secret_detection_feature_branch do status { :success } diff --git a/ee/spec/finders/security/findings_finder_spec.rb b/ee/spec/finders/security/findings_finder_spec.rb index e7afc6036d4acdc827d77f631c26b0885db9dec7..c2f6d82afab478d7b93982e96ade2beaf11b60a8 100644 --- a/ee/spec/finders/security/findings_finder_spec.rb +++ b/ee/spec/finders/security/findings_finder_spec.rb @@ -49,6 +49,8 @@ context 'when the pipeline has security findings' do let(:findings) { service_object.execute.to_a } + let(:duplicate_security_finding_uuids) { [Security::Finding.second[:uuid]] } + let(:expected_uuids) { result_uuids - duplicate_security_finding_uuids } before_all do ds_content = File.read(artifact_ds.file.path) @@ -104,11 +106,7 @@ describe '#findings' do context 'when the `security_findings` records have `overridden_uuid`s' do let(:security_findings) { Security::Finding.by_build_ids(build_1) } - let(:security_finding_uuids) { Security::Finding.pluck(:uuid) } - let(:nondeduplicated_security_finding_uuid) { Security::Finding.second[:uuid] } - let(:expected_uuids) do - security_finding_uuids - Array(nondeduplicated_security_finding_uuid) - end + let(:result_uuids) { Security::Finding.pluck(:uuid) } subject { findings.map(&:uuid) } @@ -139,7 +137,7 @@ subject { findings.map(&:uuid) } context 'with the default parameters' do - let(:expected_uuids) { Security::Finding.pluck(:uuid) - [Security::Finding.second[:uuid]] } + let(:result_uuids) { Security::Finding.pluck(:uuid) } it { is_expected.to match_array(expected_uuids) } end @@ -179,9 +177,8 @@ context 'when the `report_types` is provided' do let(:report_types) { :dependency_scanning } - let(:expected_uuids) do - Security::Finding.by_scan(Security::Scan.find_by(scan_type: 'dependency_scanning')).pluck(:uuid) - - [Security::Finding.second[:uuid]] + let(:result_uuids) do + Security::Finding.by_scan(Security::Scan.find_by(scan_type: 'dependency_scanning')).pluck(:uuid) end it { is_expected.to match_array(expected_uuids) } @@ -189,8 +186,7 @@ context 'when the `scope` is provided as `all`' do let(:scope) { 'all' } - - let(:expected_uuids) { Security::Finding.pluck(:uuid) - [Security::Finding.second[:uuid]] } + let(:result_uuids) { Security::Finding.pluck(:uuid) } it { is_expected.to match_array(expected_uuids) } end @@ -220,9 +216,8 @@ let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning, job: retried_build) } let(:report) { create(:ci_reports_security_report, pipeline: pipeline, type: :dependency_scanning) } let(:report_types) { :dependency_scanning } - let(:expected_uuids) do - Security::Finding.by_scan(Security::Scan.find_by(scan_type: 'dependency_scanning')).pluck(:uuid) - - [Security::Finding.second[:uuid]] + let(:result_uuids) do + Security::Finding.by_scan(Security::Scan.find_by(scan_type: 'dependency_scanning')).pluck(:uuid) end before do @@ -290,6 +285,66 @@ it { is_expected.to eq(vulnerability_finding.vulnerability) } end end + + context 'when there is a partial scan' do + let_it_be(:sast_scan) { Security::Scan.find_by(scan_type: 'sast') } + let_it_be(:other_scans) { Security::Scan.id_not_in(sast_scan.id) } + + before_all do + create(:vulnerabilities_partial_scan, scan: sast_scan) + end + + context 'with no params' do + let(:params) { {} } + let(:result_uuids) { Security::Finding.pluck(:uuid) } + + it 'returns both full and partial scan results' do + is_expected.to match_array(expected_uuids) + end + end + + context 'when scan_mode is full' do + let(:params) { { scan_mode: 'full' } } + let(:result_uuids) { other_scans.flat_map(&:findings).pluck(:uuid) } + + it 'returns only full scan results' do + is_expected.to match_array(expected_uuids) + end + + context 'when feature is disabled' do + before do + stub_feature_flags(vulnerability_partial_scans: false) + end + + let(:result_uuids) { Security::Finding.pluck(:uuid) } + + it 'returns both full and partial scan results' do + is_expected.to match_array(expected_uuids) + end + end + end + + context 'when scan_mode is partial' do + let(:params) { { scan_mode: 'partial' } } + let(:result_uuids) { sast_scan.findings.pluck(:uuid) } + + it 'returns only partial scan results' do + is_expected.to match_array(expected_uuids) + end + + context 'when feature is disabled' do + before do + stub_feature_flags(vulnerability_partial_scans: false) + end + + let(:result_uuids) { Security::Finding.pluck(:uuid) } + + it 'returns both full and partial scan results' do + is_expected.to match_array(expected_uuids) + end + end + end + end end end @@ -330,7 +385,7 @@ end context 'with the default parameters' do - let(:expected_uuids) { Security::Finding.pluck(:uuid) - [Security::Finding.second[:uuid]] } + let(:result_uuids) { Security::Finding.pluck(:uuid) } it 'includes child build findings' do expect(finding_uuids).to match_array(expected_uuids) diff --git a/ee/spec/graphql/types/security/scan_mode_enum_spec.rb b/ee/spec/graphql/types/security/scan_mode_enum_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9c90d549e2ad9fcb1d2e3c9e2191952266680d79 --- /dev/null +++ b/ee/spec/graphql/types/security/scan_mode_enum_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['ScanModeEnum'], feature_category: :vulnerability_management do + specify { expect(described_class.graphql_name).to eq('ScanModeEnum') } + + describe 'statuses' do + using RSpec::Parameterized::TableSyntax + + where(:graphql_value, :param_value) do + 'ALL' | 'all' + 'FULL' | 'full' + 'PARTIAL' | 'partial' + end + + with_them do + it 'exposes a status with the correct value' do + expect(described_class.values[graphql_value].value).to eq(param_value) + end + end + end +end diff --git a/ee/spec/requests/api/graphql/merge_requests/finding_reports_comparer_spec.rb b/ee/spec/requests/api/graphql/merge_requests/finding_reports_comparer_spec.rb index f91d00152c9461a4b844a0c58926861c9d80dae6..22c9493fd042cbee3bee7e74a1e939d9d0dd5730 100644 --- a/ee/spec/requests/api/graphql/merge_requests/finding_reports_comparer_spec.rb +++ b/ee/spec/requests/api/graphql/merge_requests/finding_reports_comparer_spec.rb @@ -96,7 +96,7 @@ let(:finding_reports_comparer_fields) do <<~QUERY - findingReportsComparer(reportType: SAST) { + findingReportsComparer(reportType: SAST, scanMode: PARTIAL) { status statusReason report { @@ -193,7 +193,9 @@ subject(:result) { graphql_data_at(:project, :merge_request, :finding_reports_comparer) } before do - allow(::Security::MergeRequestSecurityReportGenerationService).to receive(:execute).and_return(mock_report) + allow(::Security::MergeRequestSecurityReportGenerationService).to receive(:execute) + .with(merge_request, { report_type: 'sast', scan_mode: 'partial' }) + .and_return(mock_report) end context 'when the user is not authorized to read the field' do diff --git a/ee/spec/requests/projects/merge_requests_controller_spec.rb b/ee/spec/requests/projects/merge_requests_controller_spec.rb index 6e57d8a2cb93c81451fd56ab62c73299006cfeea..ddd7540ace7e72af2ef20a9003b4383deda02866 100644 --- a/ee/spec/requests/projects/merge_requests_controller_spec.rb +++ b/ee/spec/requests/projects/merge_requests_controller_spec.rb @@ -138,8 +138,11 @@ def get_index describe 'security_reports' do let_it_be(:merge_request) { create(:merge_request, :with_head_pipeline) } let_it_be(:user) { create(:user) } + let(:params) { { type: :sast } } - subject(:request_report) { get security_reports_project_merge_request_path(project, merge_request, type: :sast, format: :json) } + subject(:request_report) do + get security_reports_project_merge_request_path(project, merge_request, params: params, format: :json) + end before do stub_licensed_features(security_dashboard: true) @@ -194,7 +197,7 @@ def get_index context 'when the given type is valid' do before do allow(::Security::MergeRequestSecurityReportGenerationService) - .to receive(:execute).with(an_instance_of(MergeRequest), 'sast').and_return(report_payload) + .to receive(:execute).with(an_instance_of(MergeRequest), hash_including(report_type: 'sast')).and_return(report_payload) end context 'when comparison is being processed' do diff --git a/ee/spec/services/ci/compare_security_reports_service_spec.rb b/ee/spec/services/ci/compare_security_reports_service_spec.rb index fc309bb8e0ba6b522f19d060fdd1a37ce03b2234..a4e0627e5d4c9483843e16a41bfc9206a104d399 100644 --- a/ee/spec/services/ci/compare_security_reports_service_spec.rb +++ b/ee/spec/services/ci/compare_security_reports_service_spec.rb @@ -21,13 +21,15 @@ with_sast_feature_branch: create(:ee_ci_pipeline, :with_sast_feature_branch, project: project), with_secret_detection_feature_branch: create(:ee_ci_pipeline, :with_secret_detection_feature_branch, project: project), with_corrupted_dependency_scanning_report: create(:ee_ci_pipeline, :with_corrupted_dependency_scanning_report, project: project), - with_corrupted_container_scanning_report: create(:ee_ci_pipeline, :with_corrupted_container_scanning_report, project: project) + with_corrupted_container_scanning_report: create(:ee_ci_pipeline, :with_corrupted_container_scanning_report, project: project), + sast_differential_scan: create(:ee_ci_pipeline, :sast_differential_scan, project: project) } end - let(:service) { described_class.new(project, current_user, report_type: scan_type.to_s) } + let(:params) { { report_type: scan_type.to_s } } + let(:service) { described_class.new(project, current_user, params) } - def create_scan_with_findings(scan_type, pipeline, count = 1) + def create_scan_with_findings(scan_type, pipeline, count = 1, partial: false) scan = create( :security_scan, :latest_successful, @@ -36,6 +38,8 @@ def create_scan_with_findings(scan_type, pipeline, count = 1) scan_type: scan_type ) + create(:vulnerabilities_partial_scan, scan: scan) if partial + create_list( :security_finding, count, @@ -55,6 +59,7 @@ def create_scan_with_findings(scan_type, pipeline, count = 1) create_scan_with_findings('container_scanning', test_pipelines[:with_container_scanning_feature_branch], 8) create_scan_with_findings('dast', test_pipelines[:with_dast_feature_branch], 20) create_scan_with_findings('sast', test_pipelines[:with_sast_feature_branch], 5) + create_scan_with_findings('sast', test_pipelines[:sast_differential_scan], 6, partial: true) create_scan_with_findings('secret_detection', test_pipelines[:with_secret_detection_feature_branch]) end @@ -303,6 +308,26 @@ def create_scan_with_findings(scan_type, pipeline, count = 1) end it_behaves_like 'when a pipeline has scan that is not in the `succeeded` state' + + context 'when scan_mode is partial' do + let(:params) { { report_type: 'sast', scan_mode: 'partial' } } + + it_behaves_like 'when only the head pipeline has a report' do + let_it_be(:head_pipeline) { test_pipelines[:sast_differential_scan] } + let(:num_findings_in_fixture) { 6 } + end + + it_behaves_like 'when base and head pipelines have scanning reports' do + let_it_be(:base_pipeline) { test_pipelines[:with_sast_report] } + let_it_be(:head_pipeline) { test_pipelines[:sast_differential_scan] } + + # Since the sast report on the base pipeline is a full scan and not a partial, + # it should not be considered in the comparison. Partial scans should only run + # on feature branches. + let(:num_fixed_findings) { 0 } + let(:num_added_findings) { 6 } + end + end end context 'with secret detection' do diff --git a/ee/spec/services/security/merge_request_security_report_generation_service_spec.rb b/ee/spec/services/security/merge_request_security_report_generation_service_spec.rb index 117b6bd73729ae8d22287373d0bf7b01bd6c4059..6c51c90d312688601102f1b5304220133e9cf289 100644 --- a/ee/spec/services/security/merge_request_security_report_generation_service_spec.rb +++ b/ee/spec/services/security/merge_request_security_report_generation_service_spec.rb @@ -2,13 +2,29 @@ require 'spec_helper' -RSpec.describe Security::MergeRequestSecurityReportGenerationService, feature_category: :vulnerability_management do +RSpec.describe Security::MergeRequestSecurityReportGenerationService, :use_clean_rails_memory_store_caching, :sidekiq_inline, feature_category: :vulnerability_management do + include ReactiveCachingHelpers + describe '#execute' do let_it_be(:merge_request) { create(:merge_request) } - let(:service_object) { described_class.new(merge_request, report_type) } + let(:params) { { report_type: } } + let(:report_generation_service) { described_class.new(merge_request, params) } + + subject(:report) { report_generation_service.execute } - subject(:report) { service_object.execute } + def stub_report(data) + allow_next_instance_of(Ci::CompareSecurityReportsService, merge_request.project, nil, params) do |instance| + expect(instance).to receive(:latest?) + .with( + merge_request.comparison_base_pipeline(Ci::CompareSecurityReportsService), + merge_request.diff_head_pipeline, + data + ) + .and_return(true) + end + stub_reactive_cache(report_generation_service, data, params.stringify_keys) + end context 'when the given report type is invalid' do let(:report_type) { 'foo' } @@ -63,7 +79,7 @@ end before do - allow(merge_request).to receive(:compare_sast_reports).with(nil).and_return(mock_report) + stub_report(mock_report) end it 'returns the report with overridden severity' do @@ -91,8 +107,6 @@ end context 'when the given report type is valid' do - using RSpec::Parameterized::TableSyntax - let_it_be(:confirmed_finding) { create(:vulnerabilities_finding, :confirmed, severity: :critical) } let_it_be(:dismissed_finding) { create(:vulnerabilities_finding, :dismissed, severity: :medium) } @@ -100,14 +114,16 @@ let(:confirmed_uuid) { confirmed_finding.uuid } let(:dismissed_uuid) { dismissed_finding.uuid } - where(:report_type, :mr_report_method) do - 'sast' | :compare_sast_reports - 'secret_detection' | :compare_secret_detection_reports - 'container_scanning' | :compare_container_scanning_reports - 'dependency_scanning' | :compare_dependency_scanning_reports - 'dast' | :compare_dast_reports - 'coverage_fuzzing' | :compare_coverage_fuzzing_reports - 'api_fuzzing' | :compare_api_fuzzing_reports + where(:report_type) do + %w[ + sast + secret_detection + container_scanning + dependency_scanning + dast + coverage_fuzzing + api_fuzzing + ] end with_them do @@ -147,62 +163,86 @@ } end - before do - allow(merge_request).to receive(mr_report_method).with(nil).and_return(mock_report) - end - - context 'when the report status is `parsing`' do - let(:report_status) { :parsing } + context 'with no data cached' do + it 'queues reactive caching update and returns parsing' do + expect_reactive_cache_update_queued(report_generation_service, params.stringify_keys) - it 'returns the report' do - expect(report).to eq(mock_report) + expect(report).to eq({ status: :parsing }) end end - context 'when the report status is `parsed`' do - let(:report_status) { :parsed } - let(:expected_report) do - { - status: report_status, - data: { - 'base_report_created_at' => nil, - 'base_report_out_of_date' => false, - 'head_report_created_at' => '2023-01-18T11:30:01.035Z', - 'added' => [ - { - 'id' => nil, - 'name' => 'Test vulnerability 1', - 'uuid' => new_uuid, - 'severity' => 'critical', - 'state' => 'detected', - 'severity_override' => nil - }, - { - 'id' => nil, - 'name' => 'Test vulnerability 2', - 'uuid' => confirmed_uuid, - 'severity' => 'critical', - 'state' => 'confirmed', - 'severity_override' => nil - } - ], - 'fixed' => [ - { - 'id' => nil, - 'name' => 'Test vulnerability 3', - 'uuid' => dismissed_uuid, - 'severity' => 'medium', - 'state' => 'dismissed', - 'severity_override' => nil - } - ] - } - } + context 'with data cached' do + before do + stub_report(mock_report) + end + + context 'when the report status is `parsing`' do + let(:report_status) { :parsing } + + it 'returns the report' do + expect(report).to eq(mock_report) + end end - it 'returns all the fields along with the calculated state of the findings' do - expect(report).to eq(expected_report) - expect(merge_request).to have_received(mr_report_method).with(nil) + context 'when the report status is `parsed`' do + let(:report_status) { :parsed } + let(:expected_report) do + { + status: report_status, + data: { + 'base_report_created_at' => nil, + 'base_report_out_of_date' => false, + 'head_report_created_at' => '2023-01-18T11:30:01.035Z', + 'added' => [ + { + 'id' => nil, + 'name' => 'Test vulnerability 1', + 'uuid' => new_uuid, + 'severity' => 'critical', + 'state' => 'detected', + 'severity_override' => nil + }, + { + 'id' => nil, + 'name' => 'Test vulnerability 2', + 'uuid' => confirmed_uuid, + 'severity' => 'critical', + 'state' => 'confirmed', + 'severity_override' => nil + } + ], + 'fixed' => [ + { + 'id' => nil, + 'name' => 'Test vulnerability 3', + 'uuid' => dismissed_uuid, + 'severity' => 'medium', + 'state' => 'dismissed', + 'severity_override' => nil + } + ] + } + } + end + + context 'when cached results is not latest' do + before do + allow(ReactiveCachingWorker).to receive(:perform_async).and_call_original + allow_next_instance_of(Ci::CompareSecurityReportsService) do |instance| + allow(instance).to receive(:latest?).and_return(false) + end + end + + it 'queues reactive cache update' do + expect_reactive_cache_update_queued(report_generation_service, params.stringify_keys) + + expect(report).to eq({ status: :parsing }) + end + end + + it 'returns all the fields along with the calculated state of the findings' do + expect(report).to eq(expected_report) + end end end end diff --git a/spec/support/helpers/reactive_caching_helpers.rb b/spec/support/helpers/reactive_caching_helpers.rb index 00192a5d58722c883304af7c830952f33f511d19..604abbe2edfd8efa57839ce37dd8ceba4b80e4dc 100644 --- a/spec/support/helpers/reactive_caching_helpers.rb +++ b/spec/support/helpers/reactive_caching_helpers.rb @@ -45,9 +45,9 @@ def start_reactive_cache_lifetime(subject, *qualifiers) Rails.cache.write(alive_reactive_cache_key(subject, *qualifiers), true) end - def expect_reactive_cache_update_queued(subject, worker_klass: ReactiveCachingWorker) + def expect_reactive_cache_update_queued(subject, *args, worker_klass: ReactiveCachingWorker) expect(worker_klass) .to receive(:perform_in) - .with(subject.class.reactive_cache_refresh_interval, subject.class.name, subject.id) + .with(subject.class.reactive_cache_refresh_interval, subject.class.name, subject.id, *args) end end