Skip to content

G
gitlab-ce
Commit 402c36b0 authored by Sean McGivern's avatar Sean McGivern
Browse files
Options

Merge branch 'remove-existing-field-from-report' into 'master' 

Fix: Remove unused existing field

See merge request gitlab-org/gitlab!51609
parents b699fe02 69cbba0e
Hide whitespace changes
Inline Side-by-side
Showing with 6 additions and 72 deletions
ee/app/serializers/vulnerabilities/finding_reports_comparer_entity.rb
View file @ 402c36b0
......@@ -8,5 +8,4 @@ class Vulnerabilities::FindingReportsComparerEntity < Grape::Entity
expose :head_report_created_at
expose :added, using: Vulnerabilities::FindingEntity
expose :fixed, using: Vulnerabilities::FindingEntity
expose :existing, using: Vulnerabilities::FindingEntity
end
ee/lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
View file @ 402c36b0
......@@ -41,13 +41,6 @@ module Gitlab
base_report.findings - head_report.findings
end
end
def existing
strong_memoize(:existing) do
# Existing vulnerabilities should point to source report for most recent information
head_report.findings & base_report.findings
end
end
end
end
end
......
ee/spec/lib/gitlab/ci/reports/security/vulnerability_reports_comparer_spec.rb
View file @ 402c36b0
......@@ -46,34 +46,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
end
end
describe '#existing' do
context 'with existing reports' do
it 'points to source tree' do
expect(subject.existing).to eq([head_vulnerability])
end
context 'when comparing reports with different fingerprints' do
let(:base_vulnerability) { build(:vulnerabilities_finding, report_type: :sast, identifiers: [identifier], location_fingerprint: 'A') }
let(:head_vulnerability) { build(:vulnerabilities_finding, report_type: :sast, identifiers: [identifier], location_fingerprint: 'B') }
it 'does not find any overlap' do
expect(subject.existing).to eq([])
end
end
context 'new vulnerabilities' do
let(:vuln) { build(:vulnerabilities_finding, report_type: :sast, identifiers: [identifier], location_fingerprint: '888', confidence: ::Enums::Vulnerability.confidence_levels[:medium]) }
let(:low_vuln) { build(:vulnerabilities_finding, report_type: :sast, identifiers: [identifier], location_fingerprint: '888', confidence: ::Enums::Vulnerability.confidence_levels[:low]) }
let(:base_report) { build(:ci_reports_security_aggregated_reports, findings: [base_vulnerability, vuln])}
let(:head_report) { build(:ci_reports_security_aggregated_reports, findings: [head_vulnerability, vuln, low_vuln])}
it 'does not change order' do
expect(subject.existing).to eq([head_vulnerability, vuln])
end
end
end
end
describe '#added' do
let(:vuln) { build(:vulnerabilities_finding, report_type: :sast, identifiers: [identifier], location_fingerprint: '888', confidence: ::Enums::Vulnerability.confidence_levels[:high], severity: Enums::Vulnerability.severity_levels[:critical]) }
let(:low_vuln) { build(:vulnerabilities_finding, report_type: :sast, identifiers: [identifier], location_fingerprint: '888', confidence: ::Enums::Vulnerability.confidence_levels[:high], severity: Enums::Vulnerability.severity_levels[:low]) }
......@@ -142,7 +114,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
it 'returns empty array when reports are not present' do
comparer = described_class.new(empty_report, empty_report)
expect(comparer.existing).to eq([])
expect(comparer.fixed).to eq([])
expect(comparer.added).to eq([])
end
......@@ -150,7 +121,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
it 'returns added vulnerability when base is empty and head is not empty' do
comparer = described_class.new(empty_report, head_report)
expect(comparer.existing).to eq([])
expect(comparer.fixed).to eq([])
expect(comparer.added).to eq([head_vulnerability])
end
......@@ -158,7 +128,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
it 'returns fixed vulnerability when head is empty and base is not empty' do
comparer = described_class.new(base_report, empty_report)
expect(comparer.existing).to eq([])
expect(comparer.fixed).to eq([base_vulnerability])
expect(comparer.added).to eq([])
end
......
ee/spec/serializers/vulnerabilities/finding_reports_comparer_entity_spec.rb
View file @ 402c36b0
......@@ -31,9 +31,8 @@ RSpec.describe Vulnerabilities::FindingReportsComparerEntity do
allow(request).to receive(:current_user).and_return(user)
end
it 'contains the added existing and fixed vulnerabilities for container scanning' do
it 'contains the added and fixed vulnerabilities for container scanning' do
expect(subject.keys).to include(:added)
expect(subject.keys).to include(:existing)
expect(subject.keys).to include(:fixed)
end
......
ee/spec/services/ci/compare_security_reports_service_spec.rb
View file @ 402c36b0
......@@ -26,7 +26,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
it 'reports new vulnerabilities' do
expect(subject[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(4)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0)
end
end
......@@ -40,7 +39,7 @@ RSpec.describe Ci::CompareSecurityReportsService do
end
it 'populates fields based on current_user' do
payload = subject[:data]['existing'].first
payload = subject[:data]['added'].first
expect(payload['create_vulnerability_feedback_issue_path']).to be_present
expect(payload['create_vulnerability_feedback_merge_request_path']).to be_present
......@@ -54,10 +53,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('external_id' => 'CVE-2017-5946'))
end
it 'reports existing dependency vulenerabilities' do
expect(subject[:data]['existing'].count).to eq(3)
end
it 'reports fixed dependency scanning vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(1)
compare_keys = collect_ids(subject[:data]['fixed'])
......@@ -97,10 +92,9 @@ RSpec.describe Ci::CompareSecurityReportsService do
let_it_be(:base_pipeline) { create(:ee_ci_pipeline) }
let_it_be(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report, project: project) }
it 'reports new, existing and fixed vulnerabilities' do
it 'reports new and fixed vulnerabilities' do
expect(subject[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(8)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0)
end
end
......@@ -123,10 +117,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('external_id' => 'CVE-2017-15650'))
end
it 'reports existing container vulnerabilities' do
expect(subject[:data]['existing'].count).to eq(0)
end
it 'reports fixed container scanning vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(8)
compare_keys = collect_ids(subject[:data]['fixed'])
......@@ -149,10 +139,9 @@ RSpec.describe Ci::CompareSecurityReportsService do
let_it_be(:base_pipeline) { create(:ee_ci_pipeline) }
let_it_be(:head_pipeline) { create(:ee_ci_pipeline, :with_dast_report, project: project) }
it 'reports the new vulnerabilities, while not changing the counts of existing and fixed vulnerabilities' do
it 'reports the new vulnerabilities, while not changing the counts of fixed vulnerabilities' do
expect(subject[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(20)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0)
end
end
......@@ -177,11 +166,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
expect(subject[:data]['added'].last['identifiers']).to include(a_hash_including('name' => 'CWE-201'))
end
it 'reports existing DAST vulnerabilities' do
expect(subject[:data]['existing'].count).to eq(1)
expect(subject[:data]['existing'].last['identifiers']).to include(a_hash_including('name' => 'CWE-120'))
end
it 'reports fixed DAST vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(19)
expect(subject[:data]['fixed']).to include(
......@@ -214,7 +198,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
it 'reports new vulnerabilities' do
expect(subject[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(33)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0)
end
end
......@@ -224,7 +207,7 @@ RSpec.describe Ci::CompareSecurityReportsService do
let_it_be(:head_pipeline) { create(:ee_ci_pipeline, :with_sast_feature_branch, project: project) }
it 'populates fields based on current_user' do
payload = subject[:data]['existing'].first
payload = subject[:data]['added'].first
expect(payload['create_vulnerability_feedback_issue_path']).to be_present
expect(payload['create_vulnerability_feedback_merge_request_path']).to be_present
......@@ -238,10 +221,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('name' => 'CWE-120'))
end
it 'reports existing sast vulnerabilities' do
expect(subject[:data]['existing'].count).to eq(29)
end
it 'reports fixed sast vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(4)
compare_keys = collect_ids(subject[:data]['fixed'])
......@@ -267,7 +246,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
it 'reports new vulnerabilities' do
expect(subject[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(1)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0)
end
end
......@@ -277,7 +255,7 @@ RSpec.describe Ci::CompareSecurityReportsService do
let_it_be(:head_pipeline) { create(:ee_ci_pipeline, :with_secret_detection_feature_branch, project: project) }
it 'populates fields based on current_user' do
payload = subject[:data]['existing'].first
payload = subject[:data]['added'].first
expect(payload).to be_nil
expect(service.current_user).to eq(current_user)
end
......@@ -286,10 +264,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
expect(subject[:data]['added'].count).to eq(0)
end
it 'removes existing secret_detection vulnerabilities' do
expect(subject[:data]['existing'].count).to eq(0)
end
it 'reports fixed secret_detection vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(1)
compare_keys = collect_ids(subject[:data]['fixed'])
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7