Skip to content

G
gitlab-ce
Commit e68ae3b3 authored by Can Eldem's avatar Can Eldem Committed by Peter Leitzen
Browse files
Options

Remove unused classes for report comparison logic 

Remove servies for comparing sec reports in MR widget
Remove depricated serializers
parent 2d3e84ef
Hide whitespace changes
Inline Side-by-side
Showing with 5 additions and 359 deletions
ee/app/models/ee/ci/pipeline.rb
View file @ e68ae3b3
......@@ -52,41 +52,6 @@ module EE
metrics: %i[metrics_reports]
}.freeze
# Deprecated, to be removed in 12.0
# A hash of Ci::JobArtifact file_types
# With mapping to the legacy job names,
# that has to contain given files
LEGACY_REPORT_FORMATS = {
codequality: {
names: %w(codeclimate codequality code_quality),
files: %w(codeclimate.json gl-code-quality-report.json)
},
sast: {
names: %w(deploy sast),
files: %w(gl-sast-report.json)
},
dependency_scanning: {
names: %w(dependency_scanning),
files: %w(gl-dependency-scanning-report.json)
},
container_scanning: {
names: %w(sast:container container_scanning),
files: %w(gl-sast-container-report.json gl-container-scanning-report.json)
},
dast: {
names: %w(dast),
files: %w(gl-dast-report.json)
},
performance: {
names: %w(performance deploy),
files: %w(performance.json)
},
license_management: {
names: %w(license_management),
files: %w(gl-license-management-report.json)
}
}.freeze
state_machine :status do
after_transition any => ::Ci::Pipeline.completed_statuses do |pipeline|
next unless pipeline.has_reports?(::Ci::JobArtifact.security_reports)
......
ee/app/serializers/vulnerabilities/occurrence_report_entity.rb deleted 100644 → 0
View file @ 2d3e84ef
# frozen_string_literal: true
class Vulnerabilities::OccurrenceReportEntity < Grape::Entity
expose :report_type, :name, :severity, :confidence, :compare_key, :identifiers, :scanner, :project_fingerprint, :uuid, :metadata_version, :location
end
ee/app/services/security/compare_reports_base_service.rb deleted 100644 → 0
View file @ 2d3e84ef
# frozen_string_literal: true
module Security
class CompareReportsBaseService
attr_reader :base_report, :head_report, :report_diff
def initialize(base_report, head_report)
@base_report = base_report
@head_report = head_report
@report_diff = ::Gitlab::Ci::Reports::Security::ReportsDiff.new
end
def execute
# If there is nothing to compare with, just consider all
# head occurrences as added
if base_report.occurrences.blank?
report_diff.added = head_report.occurrences
return report_diff
end
update_base_occurrence_locations
report_diff.added = head_report.occurrences - base_report.occurrences
report_diff.fixed = base_report.occurrences - head_report.occurrences
report_diff.existing = base_report.occurrences & head_report.occurrences
report_diff
end
private
def update_base_occurrence_locations
# Override this method with an update strategy in subclass if any?
end
end
end
ee/app/services/security/compare_reports_sast_service.rb deleted 100644 → 0
View file @ 2d3e84ef
# frozen_string_literal: true
module Security
class CompareReportsSastService < CompareReportsBaseService
include Gitlab::Utils::StrongMemoize
attr_reader :project
def initialize(base_report, head_report, project)
@project = project
super(base_report, head_report)
end
private
# Update location for base report occurrences by leveraging the git diff
def update_base_occurrence_locations
return unless git_diff
# Group by file path to optimize the usage of Diff::File and Diff::LineMapper
base_report.occurrences.group_by(&:file_path).each do |file_path, occurrences|
update_locations_by_file(git_diff, file_path, occurrences)
end
end
def update_locations_by_file(git_diff, file_path, occurrences)
diff_file = git_diff.diff_file_with_old_path(file_path)
return if diff_file.nil? || diff_file.deleted_file?
update_locations(diff_file, occurrences)
end
def update_locations(diff_file, occurrences)
line_mapper = Gitlab::Diff::LineMapper.new(diff_file)
occurrences.each do |occurrence|
new_path = line_mapper.diff_file.new_path
new_start_line = line_mapper.old_to_new(occurrence.start_line)
new_end_line = occurrence.end_line.present? ? line_mapper.old_to_new(occurrence.end_line) : nil
# skip if the line has been removed
# NB: if the line's content has changed it will be nil too
next if new_start_line.nil?
new_location = Gitlab::Ci::Reports::Security::Locations::Sast.new(
file_path: new_path,
start_line: new_start_line,
end_line: new_end_line
)
occurrence.update_location(new_location)
end
end
def git_diff
strong_memoize(:git_diff) do
compare = CompareService.new(project, head_report.commit_sha).execute(project, base_report.commit_sha, straight: false)
next unless compare
compare.diffs(expanded: true)
end
end
end
end
ee/changelogs/unreleased/cleanup-comparison-reports-logic.yml 0 → 100644
View file @ e68ae3b3
---
title: Remove unused classes for report comparison
merge_request: 16045
author:
type: other
ee/lib/gitlab/ci/reports/security/reports_diff.rb deleted 100644 → 0
View file @ 2d3e84ef
# frozen_string_literal: true
module Gitlab
module Ci
module Reports
module Security
class ReportsDiff
attr_accessor :added, :existing, :fixed
def initialize
@added = []
@existing = []
@fixed = []
end
end
end
end
end
end
ee/spec/services/security/compare_reports_base_service_spec.rb deleted 100644 → 0
View file @ 2d3e84ef
# frozen_string_literal: true
require 'spec_helper'
describe Security::CompareReportsBaseService, '#execute' do
let(:occurrence_1) { create(:ci_reports_security_occurrence, :dynamic) }
let(:occurrence_2) { create(:ci_reports_security_occurrence, :dynamic) }
let(:occurrence_3) { create(:ci_reports_security_occurrence, :dynamic) }
let(:base_report) { create(:ci_reports_security_report, occurrences: [occurrence_1, occurrence_2]) }
let(:head_report) { create(:ci_reports_security_report, occurrences: [occurrence_2, occurrence_3]) }
subject { described_class.new(base_report, head_report).execute }
it 'exposes added occurrences' do
expect(subject.added).to contain_exactly(occurrence_3)
end
it 'exposes existing occurrences' do
expect(subject.existing).to contain_exactly(occurrence_2)
end
it 'exposes fixed occurrences' do
expect(subject.fixed).to contain_exactly(occurrence_1)
end
end
ee/spec/services/security/compare_reports_sast_service_spec.rb deleted 100644 → 0
View file @ 2d3e84ef
# frozen_string_literal: true
require 'spec_helper'
describe Security::CompareReportsSastService, '#execute' do
# Setup a project with 2 reports having 1 common vulnerability whose location is updated
let(:project) { create(:project, :repository) }
let(:branch_name) { 'master' }
let(:identifier) { create(:ci_reports_security_identifier) }
let(:location) { create(:ci_reports_security_locations_sast, file_path: 'a.js', start_line: 2, end_line: 4) }
let(:location_updated) { create(:ci_reports_security_locations_sast, file_path: 'a.js', start_line: 3, end_line: 5) }
let(:occurrence_1) { create(:ci_reports_security_occurrence, :dynamic, name: 'occurrence_1') }
let(:occurrence_2) { create(:ci_reports_security_occurrence, name: 'occurrence_2', location: location, identifiers: [identifier]) }
let(:occurrence_2_updated) { create(:ci_reports_security_occurrence, name: 'occurrence_2_updated', location: location_updated, identifiers: [identifier]) }
let(:occurrence_3) { create(:ci_reports_security_occurrence, :dynamic, name: 'occurrence_3') }
let(:base_report) { create(:ci_reports_security_report, commit_sha: base_sha, occurrences: [occurrence_1, occurrence_2]) }
let(:head_report) { create(:ci_reports_security_report, commit_sha: head_sha, occurrences: [occurrence_2_updated, occurrence_3]) }
let(:file_content) do
<<-DIFF.strip_heredoc
var auth = "Jane";
if (userInput == auth) {
console.log(userInput);
}
DIFF
end
let(:file_content_updated) do
<<-DIFF.strip_heredoc
var auth = "Jane";
// Add a comment
if (userInput == auth) {
console.log(userInput);
}
DIFF
end
let(:base_sha) do
create_file('a.js', file_content)
project.commit(branch_name).id
end
subject { described_class.new(base_report, head_report, project).execute }
shared_examples 'report diff with existing occurrence' do
it 'returns added, existing and fixed occurrences' do
expect(subject.added).to contain_exactly(occurrence_3)
expect(subject.existing).to contain_exactly(occurrence_2)
expect(subject.fixed).to contain_exactly(occurrence_1)
end
it 'returns existing occurrence with old location set' do
expect(subject.existing.first.old_location).to eq(location)
end
end
shared_examples 'report diff without existing occurrence' do
it 'returns added and fixed occurrences but no existing ones' do
expect(subject.added).to contain_exactly(occurrence_3, occurrence_2_updated)
expect(subject.existing).to be_empty
expect(subject.fixed).to contain_exactly(occurrence_1, occurrence_2)
end
end
context 'when commit_sha are equal' do
let(:head_sha) { base_sha }
it_behaves_like 'report diff without existing occurrence'
end
context 'when there is no git diff available' do
let(:head_sha) do
update_file('a.js', file_content_updated)
project.commit(branch_name).id
end
before do
compare_service = spy
allow(CompareService).to receive(:new).and_return(compare_service)
allow(compare_service).to receive(:execute).and_return(nil)
end
it_behaves_like 'report diff without existing occurrence'
end
context 'when vulnerability line numbers are updated' do
let(:head_sha) do
update_file('a.js', file_content_updated)
project.commit(branch_name).id
end
it_behaves_like 'report diff with existing occurrence'
context 'without end_line' do
let(:location) { create(:ci_reports_security_locations_sast, file_path: 'a.js', start_line: 2, end_line: nil) }
let(:location_updated) { create(:ci_reports_security_locations_sast, file_path: 'a.js', start_line: 3, end_line: nil) }
it_behaves_like 'report diff with existing occurrence'
end
context 'when line content is updated' do
let(:file_content_updated) do
<<-DIFF.strip_heredoc
var auth = "Jane";
// Add a comment
if (input == auth) { // Add a comment here to change line content
console.log(userInput);
}
DIFF
end
it_behaves_like 'report diff without existing occurrence'
end
end
context 'when vulnerability file path is updated' do
let(:location_updated) { create(:ci_reports_security_locations_sast, file_path: 'b.js', start_line: 2, end_line: 4) }
let(:head_sha) do
delete_file('a.js')
create_file('b.js', file_content)
project.commit(branch_name).id
end
it_behaves_like 'report diff with existing occurrence'
end
context 'when vulnerability file path and lines are updated' do
let(:location_updated) { create(:ci_reports_security_locations_sast, file_path: 'b.js', start_line: 3, end_line: 5) }
let(:head_sha) do
delete_file('a.js')
create_file('b.js', file_content_updated)
project.commit(branch_name).id
end
it_behaves_like 'report diff with existing occurrence'
end
def create_file(file_name, content)
Files::CreateService.new(
project,
project.owner,
commit_message: 'Update',
start_branch: branch_name,
branch_name: branch_name,
file_path: file_name,
file_content: content
).execute
end
def update_file(file_name, content)
Files::UpdateService.new(
project,
project.owner,
commit_message: 'Update',
start_branch: branch_name,
branch_name: branch_name,
file_path: file_name,
file_content: content
).execute
end
def delete_file(file_name)
Files::DeleteService.new(
project,
project.owner,
commit_message: 'Update',
start_branch: branch_name,
branch_name: branch_name,
file_path: file_name
).execute
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7