Commit a9cb9333 authored by Craig Smith's avatar Craig Smith Committed by James Fargher

Fix scanned resources count in MR security model

The scanned resources count in the MR widget is
counts duplicate URLs twice, but the URLs
displayed are unique. The commit updates the count
to only consider unique URLs
parent 01bb455f
...@@ -13,14 +13,10 @@ module Security ...@@ -13,14 +13,10 @@ module Security
end end
def execute def execute
@pipeline.builds scanned_resources = ::Security::ScannedResourcesService.new(@pipeline, @report_types).execute
.security_scans_scanned_resources_count(@report_types) scanned_resources.transform_values do |scanned_resources|
.transform_keys { |k| Security::Scan.scan_types.key(k) } scanned_resources.length
.reverse_merge(no_counts)
end end
def no_counts
@report_types.zip([0].cycle).to_h
end end
end end
end end
---
title: Fix scanned resources count in MR security modal
merge_request: 37029
author:
type: fixed
...@@ -20,9 +20,6 @@ RSpec.describe Security::ReportSummaryService, '#execute' do ...@@ -20,9 +20,6 @@ RSpec.describe Security::ReportSummaryService, '#execute' do
create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) do |job| create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) do |job|
create(:ee_ci_job_artifact, :dependency_scanning, job: job, project: project) create(:ee_ci_job_artifact, :dependency_scanning, job: job, project: project)
end end
create_security_scan(project, pipeline, 'dast', 26)
create_security_scan(project, pipeline, 'sast', 12)
end end
before do before do
...@@ -83,7 +80,7 @@ RSpec.describe Security::ReportSummaryService, '#execute' do ...@@ -83,7 +80,7 @@ RSpec.describe Security::ReportSummaryService, '#execute' do
it 'returns the scanned_resources_count' do it 'returns the scanned_resources_count' do
expect(result).to match(a_hash_including( expect(result).to match(a_hash_including(
dast: a_hash_including(scanned_resources_count: 26), dast: a_hash_including(scanned_resources_count: 26),
sast: a_hash_including(scanned_resources_count: 12), sast: a_hash_including(scanned_resources_count: 0),
container_scanning: a_hash_including(scanned_resources_count: 0), container_scanning: a_hash_including(scanned_resources_count: 0),
dependency_scanning: a_hash_including(scanned_resources_count: 0) dependency_scanning: a_hash_including(scanned_resources_count: 0)
)) ))
...@@ -121,8 +118,3 @@ RSpec.describe Security::ReportSummaryService, '#execute' do ...@@ -121,8 +118,3 @@ RSpec.describe Security::ReportSummaryService, '#execute' do
end end
end end
end end
def create_security_scan(project, pipeline, report_type, scanned_resources_count)
dast_build = create(:ee_ci_build, :artifacts, project: project, pipeline: pipeline, name: report_type)
create(:security_scan, scan_type: report_type, scanned_resources_count: scanned_resources_count, build: dast_build)
end
...@@ -12,16 +12,20 @@ RSpec.describe Security::ScannedResourcesCountingService, '#execute' do ...@@ -12,16 +12,20 @@ RSpec.describe Security::ScannedResourcesCountingService, '#execute' do
context "The Pipeline has security builds" do context "The Pipeline has security builds" do
before_all do before_all do
create_security_scan(project, pipeline, 'dast', 34) create(:ci_build, :success, name: 'dast_job', pipeline: pipeline, project: project) do |job|
create_security_scan(project, pipeline, 'sast', 12) create(:ee_ci_job_artifact, :dast, job: job, project: project)
end
create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) do |job|
create(:ee_ci_job_artifact, :sast, job: job, project: project)
end
end end
context 'All report types are requested' do context 'All report types are requested' do
subject { described_class.new(pipeline, %w[sast dast container_scanning dependency_scanning]).execute } subject { described_class.new(pipeline, %w[sast dast container_scanning dependency_scanning]).execute }
it { it {
is_expected.to match(a_hash_including("sast" => 12, is_expected.to match(a_hash_including("sast" => 0,
"dast" => 34, "dast" => 6,
"container_scanning" => 0, "container_scanning" => 0,
"dependency_scanning" => 0)) "dependency_scanning" => 0))
} }
...@@ -31,7 +35,7 @@ RSpec.describe Security::ScannedResourcesCountingService, '#execute' do ...@@ -31,7 +35,7 @@ RSpec.describe Security::ScannedResourcesCountingService, '#execute' do
subject { described_class.new(pipeline, %w[dast]).execute } subject { described_class.new(pipeline, %w[dast]).execute }
it { it {
is_expected.to eq({ "dast" => 34 }) is_expected.to eq({ "dast" => 6 })
} }
end end
end end
...@@ -48,18 +52,4 @@ RSpec.describe Security::ScannedResourcesCountingService, '#execute' do ...@@ -48,18 +52,4 @@ RSpec.describe Security::ScannedResourcesCountingService, '#execute' do
"dependency_scanning" => 0)) "dependency_scanning" => 0))
} }
end end
context 'performance' do
subject { described_class.new(pipeline, %w[sast dast container_scanning dependency_scanning]).execute }
it 'performs only one query' do
count = ActiveRecord::QueryRecorder.new { subject }.count
expect(count).to eq(1)
end
end
end
def create_security_scan(project, pipeline, report_type, scanned_resources_count)
dast_build = create(:ee_ci_build, :artifacts, project: project, pipeline: pipeline, name: report_type)
create(:security_scan, scan_type: report_type, scanned_resources_count: scanned_resources_count, build: dast_build)
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