Commit c53694fb authored by Stan Hu's avatar Stan Hu Committed by Kamil Trzciński

Remove support for checking legacy security reports

The checking of the existence of the legacy security reports
was using a significant number of SQL queries in the merge request
widget. This commit removes support for them so that they will
no longer be accessible from the widget, but the data remains
intact.

Part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63228
parent bba9011a
...@@ -130,7 +130,7 @@ module EE ...@@ -130,7 +130,7 @@ module EE
end end
def any_report_artifact_for_type(file_type) def any_report_artifact_for_type(file_type)
report_artifact_for_file_type(file_type) || legacy_report_artifact_for_file_type(file_type) report_artifact_for_file_type(file_type)
end end
def report_artifact_for_file_type(file_type) def report_artifact_for_file_type(file_type)
...@@ -139,24 +139,6 @@ module EE ...@@ -139,24 +139,6 @@ module EE
job_artifacts.where(file_type: ::Ci::JobArtifact.file_types[file_type]).last job_artifacts.where(file_type: ::Ci::JobArtifact.file_types[file_type]).last
end end
def legacy_report_artifact_for_file_type(file_type)
return unless available_licensed_report_type?(file_type)
legacy_names = LEGACY_REPORT_FORMATS[file_type]
return unless legacy_names
builds.success.latest.where(name: legacy_names[:names]).each do |build|
legacy_names[:files].each do |file_name|
next unless build.has_artifact?(file_name)
return OpenStruct.new(build: build, path: file_name)
end
end
# In case there is no artifact return nil
nil
end
def expose_license_management_data? def expose_license_management_data?
any_report_artifact_for_type(:license_management) any_report_artifact_for_type(:license_management)
end end
......
...@@ -27,20 +27,12 @@ module EE ...@@ -27,20 +27,12 @@ module EE
def downloadable_path_for_report_type(file_type) def downloadable_path_for_report_type(file_type)
if (job_artifact = batch_lookup_report_artifact_for_file_type(file_type)) && if (job_artifact = batch_lookup_report_artifact_for_file_type(file_type)) &&
can?(current_user, :read_build, job_artifact.job) can?(current_user, :read_build, job_artifact.job)
return download_project_job_artifacts_path( download_project_job_artifacts_path(
job_artifact.project, job_artifact.project,
job_artifact.job, job_artifact.job,
file_type: file_type, file_type: file_type,
proxy: true) proxy: true)
end end
if (build_artifact = legacy_report_artifact_for_file_type(file_type)) &&
can?(current_user, :read_build, build_artifact.build)
return raw_project_job_artifacts_path(
build_artifact.build.project,
build_artifact.build,
path: build_artifact.path)
end
end end
end end
end end
......
---
title: Remove support for checking legacy security reports
merge_request: 14291
author:
type: performance
...@@ -200,7 +200,7 @@ describe Projects::PipelinesController do ...@@ -200,7 +200,7 @@ describe Projects::PipelinesController do
describe 'GET security' do describe 'GET security' do
context 'with a sast artifact' do context 'with a sast artifact' do
before do before do
create(:ee_ci_build, :legacy_sast, pipeline: pipeline) create(:ee_ci_build, :sast, pipeline: pipeline)
end end
context 'with feature enabled' do context 'with feature enabled' do
......
...@@ -98,7 +98,7 @@ describe 'Pipeline', :js do ...@@ -98,7 +98,7 @@ describe 'Pipeline', :js do
context 'with a sast artifact' do context 'with a sast artifact' do
before do before do
create(:ee_ci_build, :legacy_sast, pipeline: pipeline) create(:ee_ci_build, :sast, pipeline: pipeline)
visit security_project_pipeline_path(project, pipeline) visit security_project_pipeline_path(project, pipeline)
end end
...@@ -135,7 +135,7 @@ describe 'Pipeline', :js do ...@@ -135,7 +135,7 @@ describe 'Pipeline', :js do
context 'with a license management artifact' do context 'with a license management artifact' do
before do before do
create(:ee_ci_build, :legacy_license_management, pipeline: pipeline) create(:ee_ci_build, :license_management, pipeline: pipeline)
visit licenses_project_pipeline_path(project, pipeline) visit licenses_project_pipeline_path(project, pipeline)
end end
...@@ -143,7 +143,7 @@ describe 'Pipeline', :js do ...@@ -143,7 +143,7 @@ describe 'Pipeline', :js do
it 'shows jobs tab pane as active' do it 'shows jobs tab pane as active' do
expect(page).to have_content('Licenses') expect(page).to have_content('Licenses')
expect(page).to have_css('#js-tab-licenses') expect(page).to have_css('#js-tab-licenses')
expect(find('.js-licenses-counter')).to have_content('0') expect(find('.js-licenses-counter')).to have_content('4')
end end
it 'shows security report section' do it 'shows security report section' do
......
...@@ -26,26 +26,6 @@ describe Ci::Pipeline do ...@@ -26,26 +26,6 @@ describe Ci::Pipeline do
end end
end end
describe '#with_legacy_security_reports scope' do
let(:pipeline_1) { create(:ci_pipeline_without_jobs, project: project) }
let(:pipeline_2) { create(:ci_pipeline_without_jobs, project: project) }
let(:pipeline_3) { create(:ci_pipeline_without_jobs, project: project) }
let(:pipeline_4) { create(:ci_pipeline_without_jobs, project: project) }
let(:pipeline_5) { create(:ci_pipeline_without_jobs, project: project) }
before do
create(:ee_ci_build, :legacy_sast, pipeline: pipeline_1)
create(:ee_ci_build, :legacy_dependency_scanning, pipeline: pipeline_2)
create(:ee_ci_build, :legacy_container_scanning, pipeline: pipeline_3)
create(:ee_ci_build, :legacy_dast, pipeline: pipeline_4)
create(:ee_ci_build, :success, :artifacts, name: 'foobar', pipeline: pipeline_5)
end
it "returns pipeline with security reports" do
expect(described_class.with_legacy_security_reports).to contain_exactly(pipeline_1, pipeline_2, pipeline_3, pipeline_4)
end
end
describe '#with_vulnerabilities scope' do describe '#with_vulnerabilities scope' do
let!(:pipeline_1) { create(:ci_pipeline_without_jobs, project: project) } let!(:pipeline_1) { create(:ci_pipeline_without_jobs, project: project) }
let!(:pipeline_2) { create(:ci_pipeline_without_jobs, project: project) } let!(:pipeline_2) { create(:ci_pipeline_without_jobs, project: project) }
...@@ -146,45 +126,6 @@ describe Ci::Pipeline do ...@@ -146,45 +126,6 @@ describe Ci::Pipeline do
end end
end end
describe '#legacy_report_artifact_for_file_type' do
let(:build_name) { ::EE::Ci::Pipeline::LEGACY_REPORT_FORMATS[file_type][:names].first }
let(:artifact_path) { ::EE::Ci::Pipeline::LEGACY_REPORT_FORMATS[file_type][:files].first }
let!(:build) do
create(
:ci_build,
:success,
:artifacts,
name: build_name,
pipeline: pipeline,
options: {
artifacts: {
paths: [artifact_path]
}
}
)
end
subject { pipeline.legacy_report_artifact_for_file_type(file_type) }
described_class::LEGACY_REPORT_FORMATS.each do |file_type, _|
licensed_features = described_class::REPORT_LICENSED_FEATURES[file_type]
context "for file_type: #{file_type}" do
let(:file_type) { file_type }
let(:expected) { OpenStruct.new(build: build, path: artifact_path) }
if licensed_features.nil?
it_behaves_like 'unlicensed report type'
elsif licensed_features.size == 1
it_behaves_like 'licensed report type', licensed_features.first
else
it_behaves_like 'multi-licensed report type', licensed_features
end
end
end
end
describe '#security_reports' do describe '#security_reports' do
subject { pipeline.security_reports } subject { pipeline.security_reports }
......
...@@ -97,16 +97,6 @@ describe MergeRequestWidgetEntity do ...@@ -97,16 +97,6 @@ describe MergeRequestWidgetEntity do
end end
end end
context "with legacy report artifacts" do
before do
create(:ee_ci_build, :"legacy_#{artifact_type}", pipeline: pipeline)
end
it "has data entry" do
expect(subject.as_json).to include(json_entry)
end
end
context "without artifacts" do context "without artifacts" do
it "does not have data entry" do it "does not have data entry" do
expect(subject.as_json).not_to include(json_entry) expect(subject.as_json).not_to include(json_entry)
...@@ -167,26 +157,11 @@ describe MergeRequestWidgetEntity do ...@@ -167,26 +157,11 @@ describe MergeRequestWidgetEntity do
end end
end end
context 'when legacy artifact is defined' do
before do
create(:ee_ci_build, :legacy_license_management, pipeline: pipeline)
end
it 'is included, if license manage management features are on' do
expect(subject.as_json).to include(:license_management)
expect(subject.as_json[:license_management]).to include(:head_path)
expect(subject.as_json[:license_management]).to include(:base_path)
expect(subject.as_json[:license_management]).to include(:managed_licenses_path)
expect(subject.as_json[:license_management]).to include(:can_manage_licenses)
expect(subject.as_json[:license_management]).to include(:license_management_full_report_path)
end
end
describe '#managed_licenses_path' do describe '#managed_licenses_path' do
let(:managed_licenses_path) { expose_path(api_v4_projects_managed_licenses_path(id: project.id)) } let(:managed_licenses_path) { expose_path(api_v4_projects_managed_licenses_path(id: project.id)) }
before do before do
create(:ee_ci_build, :legacy_license_management, pipeline: pipeline) create(:ee_ci_build, :license_management, pipeline: pipeline)
end end
it 'is a path for target project' do it 'is a path for target project' do
......
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