Commit a796af89 authored by Craig Smith's avatar Craig Smith

Update securityReportSummary to avoid expensive operation

It’s sometimes expensive to download security artifacts since they can be large, so this commits only downloads the artifact when it’s needed.

Changelog: changed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61221
EE: true
parent d2e6bb87
...@@ -19,7 +19,9 @@ module Resolvers ...@@ -19,7 +19,9 @@ module Resolvers
def selection_information(lookahead) def selection_information(lookahead)
lookahead.selections.each_with_object({}) do |report_type, response| lookahead.selections.each_with_object({}) do |report_type, response|
response[report_type.name.to_sym] = report_type.selections.map(&:name) next if report_type.name == :__typename
response[report_type.name.to_sym] = report_type.selections.map(&:name) - [:__typename] # ignore __typename meta field
end end
end end
end end
......
...@@ -22,7 +22,7 @@ module Security ...@@ -22,7 +22,7 @@ module Security
private private
def summary_counts_for_report_type(report_type, summary_types) def summary_counts_for_report_type(report_type, summary_types)
return unless report_exists?(report_type) return if @pipeline.security_findings.by_report_types(report_type).empty?
summary_types.each_with_object({}) do |summary_type, response| summary_types.each_with_object({}) do |summary_type, response|
case summary_type case summary_type
...@@ -73,9 +73,5 @@ module Security ...@@ -73,9 +73,5 @@ module Security
def grouped_scans def grouped_scans
@grouped_scans ||= @pipeline.security_scans.by_scan_types(@selection_information.keys).group_by(&:scan_type) @grouped_scans ||= @pipeline.security_scans.by_scan_types(@selection_information.keys).group_by(&:scan_type)
end end
def report_exists?(report_type)
@pipeline&.security_reports&.reports&.key?(report_type.to_s)
end
end end
end end
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe Resolvers::SecurityReportSummaryResolver do RSpec.describe Resolvers::SecurityReportSummaryResolver do
include GraphqlHelpers include GraphqlHelpers
let_it_be(:pipeline) { double('project') } let_it_be(:pipeline) { 'pipeline' }
describe '#resolve' do describe '#resolve' do
context 'All fields are requested' do context 'All fields are requested' do
...@@ -31,7 +31,35 @@ RSpec.describe Resolvers::SecurityReportSummaryResolver do ...@@ -31,7 +31,35 @@ RSpec.describe Resolvers::SecurityReportSummaryResolver do
) do |summary_service| ) do |summary_service|
expect(summary_service).to receive(:execute).and_return({}) expect(summary_service).to receive(:execute).and_return({})
end end
resolve(described_class, obj: pipeline, lookahead: lookahead)
end
end
context 'When lookahead includes :__typename' do
let(:lookahead) do
selection_info = {
dast: [:scanned_resources_count, :vulnerabilities_count, :scans, :__typename],
sast: [:scanned_resources_count, :vulnerabilities_count, :__typename],
'__typename': []
}
build_mock_lookahead(selection_info)
end
let(:expected_selection_info) do
{
dast: [:scanned_resources_count, :vulnerabilities_count, :scans],
sast: [:scanned_resources_count, :vulnerabilities_count]
}
end
it 'does not search for :__typename' do
expect_next_instance_of(
Security::ReportSummaryService,
pipeline,
expected_selection_info
) do |summary_service|
expect(summary_service).to receive(:execute).and_return({})
end
resolve(described_class, obj: pipeline, lookahead: lookahead) resolve(described_class, obj: pipeline, lookahead: lookahead)
end end
end end
...@@ -45,6 +73,5 @@ def build_mock_lookahead(structure) ...@@ -45,6 +73,5 @@ def build_mock_lookahead(structure)
end end
double(report_type, name: report_type, selections: stub_count_types) double(report_type, name: report_type, selections: stub_count_types)
end end
double('lookahead', selections: lookahead_selections) double('lookahead', selections: lookahead_selections)
end end
...@@ -99,6 +99,21 @@ RSpec.describe Security::ReportSummaryService, '#execute' do ...@@ -99,6 +99,21 @@ RSpec.describe Security::ReportSummaryService, '#execute' do
end end
end end
context 'when scanned resources are not requested' do
let(:selection_information) do
{
dast: [:vulnerabilities_count],
container_scanning: [:vulnerabilities_count]
}
end
it 'does not download the artifact' do
expect(pipeline).not_to receive(:security_reports)
result
end
end
context 'when the scans is requested' do context 'when the scans is requested' do
let(:selection_information) { { dast: [:scans] } } let(:selection_information) { { dast: [:scans] } }
...@@ -157,4 +172,45 @@ RSpec.describe Security::ReportSummaryService, '#execute' do ...@@ -157,4 +172,45 @@ RSpec.describe Security::ReportSummaryService, '#execute' do
end end
end end
end end
context 'When only the DAST scan ran' do
let_it_be(:pipeline) { create(:ci_pipeline, :success) }
let_it_be(:build_dast) { create(:ci_build, :success, name: 'dast', pipeline: pipeline) }
let_it_be(:artifact_dast) { create(:ee_ci_job_artifact, :dast_large_scanned_resources_field, job: build_dast) }
let_it_be(:report_dast) { create(:ci_reports_security_report, type: :dast) }
let_it_be(:scan_dast) { create(:security_scan, scan_type: :dast, build: build_dast) }
before do
stub_licensed_features(sast: true, dependency_scanning: true, container_scanning: true, dast: true)
dast_content = File.read(artifact_dast.file.path)
Gitlab::Ci::Parsers::Security::Dast.parse!(dast_content, report_dast)
report_dast.merge!(report_dast)
{ artifact_dast => report_dast }.each do |artifact, report|
report.findings.each do |finding|
create(:security_finding,
severity: finding.severity,
confidence: finding.confidence,
project_fingerprint: finding.project_fingerprint,
deduplicated: true,
scan: artifact.job.security_scans.first)
end
end
end
let(:selection_information) do
{
dast: [:scanned_resources_count, :vulnerabilities_count],
sast: [:vulnerabilities_count]
}
end
it 'returns nil for the other scans' do
expect(result[:dast]).not_to be_nil
expect(result[:sast]).to be_nil
expect(result[:container_scanning]).to be_nil
expect(result[:dependency_scanning]).to be_nil
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment