Commit c78ebd60 authored by Olivier Gonzalez's avatar Olivier Gonzalez

Fix vulnerabilities list of group dashboard

Only fetch the vulnerabilities for the latest pipeline of each project.
parent bf184d1f
......@@ -251,6 +251,10 @@ module Ci
end
end
def self.latest_successful_ids_per_project
success.group(:project_id).select('max(id) as id')
end
def self.truncate_sha(sha)
sha[0...8]
end
......
......@@ -233,6 +233,12 @@ class Namespace < ActiveRecord::Base
Project.inside_path(full_path)
end
# Includes pipelines from this namespace and pipelines from all subgroups
# that belongs to this namespace
def all_pipelines
Ci::Pipeline.where(project: all_projects)
end
def has_parent?
parent.present?
end
......
# frozen_string_literal: true
class Groups::Security::VulnerabilitiesController < Groups::Security::ApplicationController
def index
@vulnerabilities = group.all_vulnerabilities.ordered
@vulnerabilities = group.latest_vulnerabilities.ordered
.page(params[:page])
respond_to do |format|
......
......@@ -82,8 +82,9 @@ module EE
end
end
def all_vulnerabilities
Vulnerabilities::Occurrence.where(project: all_projects)
def latest_vulnerabilities
Vulnerabilities::Occurrence
.for_pipelines(all_pipelines.latest_successful_ids_per_project)
end
def human_ldap_access
......
......@@ -68,6 +68,11 @@ module Vulnerabilities
preload(:scanner, :identifiers, :project)
end
def self.for_pipelines(pipelines)
joins(:occurrence_pipelines)
.where(vulnerability_occurrence_pipelines: { pipeline_id: pipelines })
end
def feedback(feedback_type:)
params = {
project_id: project_id,
......
......@@ -15,6 +15,6 @@ class VulnerabilitySummaryEntity < Grape::Entity
private
def grouped_vulnerabilities
@grouped_by_report_and_severity ||= object.all_vulnerabilities.counted_by_report_and_severity
@grouped_by_report_and_severity ||= object.latest_vulnerabilities.counted_by_report_and_severity
end
end
......@@ -55,7 +55,7 @@ describe Groups::Security::VulnerabilitiesController do
context 'when no page request' do
before do
projects.each do |project|
create(:vulnerabilities_occurrence, project: project)
create_vulnerabilities(1, project)
end
end
......@@ -72,7 +72,7 @@ describe Groups::Security::VulnerabilitiesController do
context 'when page requested' do
before do
projects.each do |project|
create_list(:vulnerabilities_occurrence, 11, project: project)
create_vulnerabilities(11, project)
end
end
......@@ -87,11 +87,11 @@ describe Groups::Security::VulnerabilitiesController do
context 'with vulnerability feedback' do
it "avoids N+1 queries" do
create_vulnerabilities(2, project_dev)
create_vulnerabilities(2, project_dev, with_feedback: true)
control_count = ActiveRecord::QueryRecorder.new { get_summary }
create_vulnerabilities(2, project_guest)
create_vulnerabilities(2, project_guest, with_feedback: true)
expect { get_summary }.not_to exceed_all_query_limit(control_count)
end
......@@ -101,10 +101,13 @@ describe Groups::Security::VulnerabilitiesController do
def get_summary
get :index, group_id: group, format: :json
end
end
def create_vulnerabilities(count, project, options = {})
pipeline = create(:ci_pipeline, :success, project: project)
vulnerabilities = create_list(:vulnerabilities_occurrence, count, pipelines: [pipeline], project: project)
return vulnerabilities unless options[:with_feedback]
def create_vulnerabilities(count, project)
pipeline = create(:ci_empty_pipeline, project: project)
vulnerabilities = create_list(:vulnerabilities_occurrence, count, project: project)
vulnerabilities.each do |occurrence|
create(:vulnerability_feedback, :sast, :dismissal,
pipeline: pipeline,
......@@ -113,7 +116,7 @@ describe Groups::Security::VulnerabilitiesController do
create(:vulnerability_feedback, :sast, :issue,
pipeline: pipeline,
issue: create(:issue, project: project_dev),
issue: create(:issue, project: project),
project: project_dev,
project_fingerprint: occurrence.project_fingerprint)
end
......@@ -121,7 +124,6 @@ describe Groups::Security::VulnerabilitiesController do
end
end
end
end
describe 'GET summary.json' do
subject { get :summary, group_id: group, format: :json }
......@@ -142,20 +144,22 @@ describe Groups::Security::VulnerabilitiesController do
before do
stub_licensed_features(security_dashboard: true)
pipeline = create(:ci_pipeline, :success, project: project_dev)
create_list(:vulnerabilities_occurrence, 3,
project: project_dev, report_type: :sast, severity: :high)
pipelines: [pipeline], project: project_dev, report_type: :sast, severity: :high)
create_list(:vulnerabilities_occurrence, 1,
project: project_dev, report_type: :dependency_scanning, severity: :low)
pipelines: [pipeline], project: project_dev, report_type: :dependency_scanning, severity: :low)
create_list(:vulnerabilities_occurrence, 2,
project: project_guest, report_type: :dependency_scanning, severity: :low)
pipelines: [pipeline], project: project_guest, report_type: :dependency_scanning, severity: :low)
create_list(:vulnerabilities_occurrence, 1,
project: project_guest, report_type: :dast, severity: :medium)
pipelines: [pipeline], project: project_guest, report_type: :dast, severity: :medium)
create_list(:vulnerabilities_occurrence, 1,
project: project_other, report_type: :dast, severity: :low)
pipelines: [pipeline], project: project_other, report_type: :dast, severity: :low)
end
context 'when user has guest access' do
......
......@@ -259,4 +259,37 @@ describe Group do
end
end
end
describe '#latest_vulnerabilities' do
let(:project) { create(:project, namespace: group) }
let(:external_project) { create(:project) }
let(:failed_pipeline) { create(:ci_pipeline, :failed, project: project) }
let!(:old_vuln) { create_vulnerability(project) }
let!(:new_vuln) { create_vulnerability(project) }
let!(:external_vuln) { create_vulnerability(external_project) }
let!(:failed_vuln) { create_vulnerability(project, failed_pipeline) }
subject { group.latest_vulnerabilities }
def create_vulnerability(project, pipeline = nil)
pipeline ||= create(:ci_pipeline, :success, project: project)
create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project)
end
it 'returns vulns only for the latest successful pipelines of projects belonging to the group' do
is_expected.to contain_exactly(new_vuln)
end
context 'with vulnerabilities from other branches' do
let!(:branch_pipeline) { create(:ci_pipeline, :success, project: project, ref: 'feature-x') }
let!(:branch_vuln) { create(:vulnerabilities_occurrence, pipelines: [branch_pipeline], project: project) }
# TODO: This should actually fail and we must scope vulns
# per branch as soon as we store them for other branches
it 'includes vulnerabilities from all branches' do
is_expected.to contain_exactly(branch_vuln)
end
end
end
end
......@@ -1157,6 +1157,19 @@ describe Ci::Pipeline, :mailer do
end
end
describe '.latest_successful_ids_per_project' do
let(:projects) { create_list(:project, 2) }
let!(:pipeline1) { create(:ci_pipeline, :success, project: projects[0]) }
let!(:pipeline2) { create(:ci_pipeline, :success, project: projects[0]) }
let!(:pipeline3) { create(:ci_pipeline, :failed, project: projects[0]) }
let!(:pipeline4) { create(:ci_pipeline, :success, project: projects[1]) }
it 'returns expected pipeline ids' do
expect(described_class.latest_successful_ids_per_project)
.to contain_exactly(pipeline2, pipeline4)
end
end
describe '.internal_sources' do
subject { described_class.internal_sources }
......
......@@ -574,6 +574,17 @@ describe Namespace do
it { expect(group.all_projects.to_a).to match_array([project2, project1]) }
end
describe '#all_pipelines' do
let(:group) { create(:group) }
let(:child) { create(:group, parent: group) }
let!(:project1) { create(:project_empty_repo, namespace: group) }
let!(:project2) { create(:project_empty_repo, namespace: child) }
let!(:pipeline1) { create(:ci_empty_pipeline, project: project1) }
let!(:pipeline2) { create(:ci_empty_pipeline, project: project2) }
it { expect(group.all_pipelines.to_a).to match_array([pipeline1, pipeline2]) }
end
describe '#share_with_group_lock with subgroups', :nested_groups do
context 'when creating a subgroup' do
let(:subgroup) { create(:group, parent: root_group )}
......
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