Skip to content

G
gitlab-ce
Commit 16918b44 authored by Peter Leitzen's avatar Peter Leitzen
Browse files
Options

Merge branch 'cleanup-comparison-reports-logic' into 'master' 

Remove unused classes for report comparison logic

See merge request gitlab-org/gitlab-ee!16045
parents 2d3e84ef e68ae3b3
Hide whitespace changes
Inline Side-by-side
Showing with 5 additions and 359 deletions
ee/app/models/ee/ci/pipeline.rb
View file @ 16918b44
......@@ -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 @ 16918b44
---
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