Commit 69cbba0e authored by can eldem's avatar can eldem

Fix: Remove unused existing field

Existing field is not used by FE
we should remove it to reduce payload
parent 4acb76d3
...@@ -8,5 +8,4 @@ class Vulnerabilities::FindingReportsComparerEntity < Grape::Entity ...@@ -8,5 +8,4 @@ class Vulnerabilities::FindingReportsComparerEntity < Grape::Entity
expose :head_report_created_at expose :head_report_created_at
expose :added, using: Vulnerabilities::FindingEntity expose :added, using: Vulnerabilities::FindingEntity
expose :fixed, using: Vulnerabilities::FindingEntity expose :fixed, using: Vulnerabilities::FindingEntity
expose :existing, using: Vulnerabilities::FindingEntity
end end
...@@ -41,13 +41,6 @@ module Gitlab ...@@ -41,13 +41,6 @@ module Gitlab
base_report.findings - head_report.findings base_report.findings - head_report.findings
end end
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 end
end end
......
...@@ -46,34 +46,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do ...@@ -46,34 +46,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
end end
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 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(: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]) } 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 ...@@ -142,7 +114,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
it 'returns empty array when reports are not present' do it 'returns empty array when reports are not present' do
comparer = described_class.new(empty_report, empty_report) comparer = described_class.new(empty_report, empty_report)
expect(comparer.existing).to eq([])
expect(comparer.fixed).to eq([]) expect(comparer.fixed).to eq([])
expect(comparer.added).to eq([]) expect(comparer.added).to eq([])
end end
...@@ -150,7 +121,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do ...@@ -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 it 'returns added vulnerability when base is empty and head is not empty' do
comparer = described_class.new(empty_report, head_report) comparer = described_class.new(empty_report, head_report)
expect(comparer.existing).to eq([])
expect(comparer.fixed).to eq([]) expect(comparer.fixed).to eq([])
expect(comparer.added).to eq([head_vulnerability]) expect(comparer.added).to eq([head_vulnerability])
end end
...@@ -158,7 +128,6 @@ RSpec.describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do ...@@ -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 it 'returns fixed vulnerability when head is empty and base is not empty' do
comparer = described_class.new(base_report, empty_report) comparer = described_class.new(base_report, empty_report)
expect(comparer.existing).to eq([])
expect(comparer.fixed).to eq([base_vulnerability]) expect(comparer.fixed).to eq([base_vulnerability])
expect(comparer.added).to eq([]) expect(comparer.added).to eq([])
end end
......
...@@ -31,9 +31,8 @@ RSpec.describe Vulnerabilities::FindingReportsComparerEntity do ...@@ -31,9 +31,8 @@ RSpec.describe Vulnerabilities::FindingReportsComparerEntity do
allow(request).to receive(:current_user).and_return(user) allow(request).to receive(:current_user).and_return(user)
end 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(:added)
expect(subject.keys).to include(:existing)
expect(subject.keys).to include(:fixed) expect(subject.keys).to include(:fixed)
end end
......
...@@ -26,7 +26,6 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -26,7 +26,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
it 'reports new vulnerabilities' do it 'reports new vulnerabilities' do
expect(subject[:status]).to eq(:parsed) expect(subject[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(4) expect(subject[:data]['added'].count).to eq(4)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0) expect(subject[:data]['fixed'].count).to eq(0)
end end
end end
...@@ -40,7 +39,7 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -40,7 +39,7 @@ RSpec.describe Ci::CompareSecurityReportsService do
end end
it 'populates fields based on current_user' do 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_issue_path']).to be_present
expect(payload['create_vulnerability_feedback_merge_request_path']).to be_present expect(payload['create_vulnerability_feedback_merge_request_path']).to be_present
...@@ -54,10 +53,6 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -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')) expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('external_id' => 'CVE-2017-5946'))
end end
it 'reports existing dependency vulenerabilities' do
expect(subject[:data]['existing'].count).to eq(3)
end
it 'reports fixed dependency scanning vulnerabilities' do it 'reports fixed dependency scanning vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(1) expect(subject[:data]['fixed'].count).to eq(1)
compare_keys = collect_ids(subject[:data]['fixed']) compare_keys = collect_ids(subject[:data]['fixed'])
...@@ -97,10 +92,9 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -97,10 +92,9 @@ RSpec.describe Ci::CompareSecurityReportsService do
let_it_be(:base_pipeline) { create(:ee_ci_pipeline) } let_it_be(:base_pipeline) { create(:ee_ci_pipeline) }
let_it_be(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report, project: project) } 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[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(8) expect(subject[:data]['added'].count).to eq(8)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0) expect(subject[:data]['fixed'].count).to eq(0)
end end
end end
...@@ -123,10 +117,6 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -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')) expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('external_id' => 'CVE-2017-15650'))
end end
it 'reports existing container vulnerabilities' do
expect(subject[:data]['existing'].count).to eq(0)
end
it 'reports fixed container scanning vulnerabilities' do it 'reports fixed container scanning vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(8) expect(subject[:data]['fixed'].count).to eq(8)
compare_keys = collect_ids(subject[:data]['fixed']) compare_keys = collect_ids(subject[:data]['fixed'])
...@@ -149,10 +139,9 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -149,10 +139,9 @@ RSpec.describe Ci::CompareSecurityReportsService do
let_it_be(:base_pipeline) { create(:ee_ci_pipeline) } let_it_be(:base_pipeline) { create(:ee_ci_pipeline) }
let_it_be(:head_pipeline) { create(:ee_ci_pipeline, :with_dast_report, project: project) } 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[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(20) expect(subject[:data]['added'].count).to eq(20)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0) expect(subject[:data]['fixed'].count).to eq(0)
end end
end end
...@@ -177,11 +166,6 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -177,11 +166,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
expect(subject[:data]['added'].last['identifiers']).to include(a_hash_including('name' => 'CWE-201')) expect(subject[:data]['added'].last['identifiers']).to include(a_hash_including('name' => 'CWE-201'))
end 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 it 'reports fixed DAST vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(19) expect(subject[:data]['fixed'].count).to eq(19)
expect(subject[:data]['fixed']).to include( expect(subject[:data]['fixed']).to include(
...@@ -214,7 +198,6 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -214,7 +198,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
it 'reports new vulnerabilities' do it 'reports new vulnerabilities' do
expect(subject[:status]).to eq(:parsed) expect(subject[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(33) expect(subject[:data]['added'].count).to eq(33)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0) expect(subject[:data]['fixed'].count).to eq(0)
end end
end end
...@@ -224,7 +207,7 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -224,7 +207,7 @@ RSpec.describe Ci::CompareSecurityReportsService do
let_it_be(:head_pipeline) { create(:ee_ci_pipeline, :with_sast_feature_branch, project: project) } let_it_be(:head_pipeline) { create(:ee_ci_pipeline, :with_sast_feature_branch, project: project) }
it 'populates fields based on current_user' do 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_issue_path']).to be_present
expect(payload['create_vulnerability_feedback_merge_request_path']).to be_present expect(payload['create_vulnerability_feedback_merge_request_path']).to be_present
...@@ -238,10 +221,6 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -238,10 +221,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('name' => 'CWE-120')) expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('name' => 'CWE-120'))
end end
it 'reports existing sast vulnerabilities' do
expect(subject[:data]['existing'].count).to eq(29)
end
it 'reports fixed sast vulnerabilities' do it 'reports fixed sast vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(4) expect(subject[:data]['fixed'].count).to eq(4)
compare_keys = collect_ids(subject[:data]['fixed']) compare_keys = collect_ids(subject[:data]['fixed'])
...@@ -267,7 +246,6 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -267,7 +246,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
it 'reports new vulnerabilities' do it 'reports new vulnerabilities' do
expect(subject[:status]).to eq(:parsed) expect(subject[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(1) expect(subject[:data]['added'].count).to eq(1)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0) expect(subject[:data]['fixed'].count).to eq(0)
end end
end end
...@@ -277,7 +255,7 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -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) } let_it_be(:head_pipeline) { create(:ee_ci_pipeline, :with_secret_detection_feature_branch, project: project) }
it 'populates fields based on current_user' do it 'populates fields based on current_user' do
payload = subject[:data]['existing'].first payload = subject[:data]['added'].first
expect(payload).to be_nil expect(payload).to be_nil
expect(service.current_user).to eq(current_user) expect(service.current_user).to eq(current_user)
end end
...@@ -286,10 +264,6 @@ RSpec.describe Ci::CompareSecurityReportsService do ...@@ -286,10 +264,6 @@ RSpec.describe Ci::CompareSecurityReportsService do
expect(subject[:data]['added'].count).to eq(0) expect(subject[:data]['added'].count).to eq(0)
end end
it 'removes existing secret_detection vulnerabilities' do
expect(subject[:data]['existing'].count).to eq(0)
end
it 'reports fixed secret_detection vulnerabilities' do it 'reports fixed secret_detection vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(1) expect(subject[:data]['fixed'].count).to eq(1)
compare_keys = collect_ids(subject[:data]['fixed']) 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