Commit 4586a76e authored by Adam Hegyi's avatar Adam Hegyi

Merge branch 'sk/335503-add-finder' into 'master'

Add VulnerabilityReadsFinder to speed up API responses

See merge request gitlab-org/gitlab!76220
parents fc754036 cf39e335
---
name: vulnerability_reads_table
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76220
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348151
milestone: '14.9'
type: development
group: group::threat insights
default_enabled: false
# frozen_string_literal: true
# Security::VulnerabilityReadsFinder
#
# Used to filter Vulnerability records for Vulnerabilities API from vulnerability_reads table
#
# Arguments:
# vulnerable: any object that has a #vulnerabilities method that returns a collection of `Vulnerability`s
# params: optional! a hash with one or more of the following:
# project_ids: if `vulnerable` includes multiple projects (like a Group), this filter will restrict
# the vulnerabilities returned to those in the group's projects that also match these IDs
# image: only return vulnerabilities with these location images
# report_types: only return vulnerabilities from these report types
# severities: only return vulnerabilities with these severities
# states: only return vulnerabilities in these states
# scanner: only return vulnerabilities with these external_id
# scanner_id: only return vulnerabilities with these scanner_ids
# has_resolution: only return vulnerabilities that have resolution
# has_issues: only return vulnerabilities that have issues linked
# cluster_agent_id: only return vulnerabilities with these cluster_agent_ids
# sort: return vulnerabilities ordered by severity_asc or severity_desc
module Security
class VulnerabilityReadsFinder
include FinderMethods
def initialize(vulnerable, params = {})
@params = params
@vulnerable = vulnerable
@vulnerability_reads = vulnerable.vulnerability_reads
end
def execute
filter_by_projects
filter_by_image
filter_by_report_types
filter_by_severities
filter_by_states
filter_by_scanner_external_id
filter_by_scanner_ids
filter_by_resolution
filter_by_issues
filter_by_cluster_agent_id
sort
end
private
attr_reader :params, :vulnerable, :vulnerability_reads
def filter_by_projects
return unless params[:project_id].present?
@vulnerability_reads = vulnerability_reads.for_projects(params[:project_id])
end
def filter_by_report_types
return unless params[:report_type].present?
@vulnerability_reads = vulnerability_reads.with_report_types(params[:report_type])
end
def filter_by_severities
return unless params[:severity].present?
@vulnerability_reads = vulnerability_reads.with_severities(params[:severity])
end
def filter_by_states
return unless params[:state].present?
@vulnerability_reads = vulnerability_reads.with_states(params[:state])
end
def filter_by_scanner_ids
return unless params[:scanner_id].present?
@vulnerability_reads = vulnerability_reads.by_scanner_ids(params[:scanner_id])
end
def filter_by_scanner_external_id
return unless params[:scanner].present?
@vulnerability_reads = vulnerability_reads.with_scanner_external_ids(params[:scanner])
end
def filter_by_resolution
return unless params[:has_resolution].in?([true, false])
@vulnerability_reads = vulnerability_reads.with_resolution(params[:has_resolution])
end
def filter_by_issues
return unless params[:has_issues].in?([true, false])
@vulnerability_reads = vulnerability_reads.with_issues(params[:has_issues])
end
def filter_by_image
return if vulnerable.is_a?(InstanceSecurityDashboard) || !params[:image].present?
@vulnerability_reads = vulnerability_reads.with_container_image(params[:image])
end
def filter_by_cluster_agent_id
return unless params[:cluster_agent_id].present?
@vulnerability_reads = vulnerability_reads.with_cluster_agent_ids(params[:cluster_agent_id])
end
def sort
@vulnerability_reads.order_by(params[:sort])
end
end
end
......@@ -73,19 +73,41 @@ module Resolvers
end
def unconditional_includes
[:findings]
if vulnerability_reads_enabled?
[{ vulnerability: [:findings] }]
else
[:findings]
end
end
def preloads
{
has_solutions: [{ findings: [:remediations] }]
}
if vulnerability_reads_enabled?
{
vulnerability: {
has_solutions: [{ findings: [:remediations] }]
}
}
else
{
has_solutions: [{ findings: [:remediations] }]
}
end
end
private
def vulnerabilities(params)
apply_lookahead(::Security::VulnerabilitiesFinder.new(vulnerable, params).execute)
if vulnerability_reads_enabled?
apply_lookahead(::Security::VulnerabilityReadsFinder.new(vulnerable, params).execute.as_vulnerabilities)
else
apply_lookahead(::Security::VulnerabilitiesFinder.new(vulnerable, params).execute)
end
end
def vulnerability_reads_enabled?
return false if vulnerable.nil? || vulnerable.is_a?(::InstanceSecurityDashboard)
Feature.enabled?(:vulnerability_reads_table, vulnerable, default_enabled: :yaml)
end
end
end
......@@ -53,7 +53,13 @@ module Resolvers
private
def vulnerabilities(filters)
Security::VulnerabilitiesFinder.new(vulnerable, filters).execute
finder = if !vulnerable.is_a?(::InstanceSecurityDashboard) && Feature.enabled?(:vulnerability_reads_table, vulnerable, default_enabled: :yaml)
Security::VulnerabilityReadsFinder
else
Security::VulnerabilitiesFinder
end
finder.new(vulnerable, filters).execute
end
end
end
......@@ -386,20 +386,19 @@ module EE
end
def vulnerabilities
::Vulnerability.where(
project: ::Project.for_group_and_its_subgroups(self).non_archived.without_deleted
)
::Vulnerability.where(project: projects_for_group_and_its_subgroups_without_deleted)
end
def vulnerability_reads
::Vulnerabilities::Read.where(project: projects_for_group_and_its_subgroups_without_deleted)
end
def vulnerability_scanners
::Vulnerabilities::Scanner.where(
project: ::Project.for_group_and_its_subgroups(self).non_archived.without_deleted
)
::Vulnerabilities::Scanner.where(project: projects_for_group_and_its_subgroups_without_deleted)
end
def vulnerability_historical_statistics
::Vulnerabilities::HistoricalStatistic
.for_project(::Project.for_group_and_its_subgroups(self).non_archived.without_deleted)
::Vulnerabilities::HistoricalStatistic.for_project(projects_for_group_and_its_subgroups_without_deleted)
end
def max_personal_access_token_lifetime_from_now
......@@ -700,6 +699,14 @@ module EE
::GroupMember.active_without_invites_and_requests.where(source_id: groups.self_and_ancestors)
end
def users_without_project_bots(members)
::User.where(id: members.distinct.select(:user_id)).without_project_bot
end
def projects_for_group_and_its_subgroups_without_deleted
::Project.for_group_and_its_subgroups(self).non_archived.without_deleted
end
override :_safe_read_repository_read_only_column
def _safe_read_repository_read_only_column
::NamespaceSetting.where(namespace: self).pick(:repository_read_only)
......
......@@ -69,6 +69,7 @@ module EE
# the rationale behind vulnerabilities and vulnerability_findings can be found here:
# https://gitlab.com/gitlab-org/gitlab/issues/10252#terminology
has_many :vulnerabilities
has_many :vulnerability_reads, class_name: 'Vulnerabilities::Read'
has_many :vulnerability_feedback, class_name: 'Vulnerabilities::Feedback'
has_many :vulnerability_historical_statistics, class_name: 'Vulnerabilities::HistoricalStatistic'
has_many :vulnerability_findings, class_name: 'Vulnerabilities::Finding', inverse_of: :project do
......
......@@ -41,6 +41,7 @@ module EE
belongs_to :confirmed_by, class_name: 'User'
has_one :group, through: :project
has_one :vulnerability_read, class_name: '::Vulnerabilities::Read'
has_many :findings, class_name: '::Vulnerabilities::Finding', inverse_of: :vulnerability
has_many :dismissed_findings, -> { dismissed }, class_name: 'Vulnerabilities::Finding', inverse_of: :vulnerability
......
......@@ -29,6 +29,12 @@ class InstanceSecurityDashboard
Vulnerability.for_projects(projects)
end
def vulnerability_reads
return Vulnerabilities::Read.none if projects.empty?
Vulnerabilities::Read.for_projects(projects)
end
def vulnerability_scanners
return Vulnerabilities::Scanner.none if projects.empty?
......
......@@ -23,5 +23,43 @@ module Vulnerabilities
enum state: ::Enums::Vulnerability.vulnerability_states
enum report_type: ::Enums::Vulnerability.report_types
enum severity: ::Enums::Vulnerability.severity_levels, _prefix: :severity
scope :order_severity_asc, -> { reorder(severity: :asc) }
scope :order_severity_desc, -> { reorder(severity: :desc) }
scope :order_detected_at_asc, -> { reorder(vulnerability_id: :asc) }
scope :order_detected_at_desc, -> { reorder(vulnerability_id: :desc) }
scope :by_scanner_ids, -> (scanner_ids) { where(scanner_id: scanner_ids) }
scope :for_projects, -> (project_ids) { where(project_id: project_ids) }
scope :grouped_by_severity, -> { reorder(severity: :desc).group(:severity) }
scope :with_report_types, -> (report_types) { where(report_type: report_types) }
scope :with_severities, -> (severities) { where(severity: severities) }
scope :with_states, -> (states) { where(state: states) }
scope :with_container_image, -> (images) { where(location_image: images) }
scope :with_cluster_agent_ids, -> (agent_ids) { where(cluster_agent_id: agent_ids) }
scope :with_resolution, -> (has_resolution = true) { where(resolved_on_default_branch: has_resolution) }
scope :with_issues, -> (has_issues = true) { where(has_issues: has_issues) }
scope :with_scanner_external_ids, -> (scanner_external_ids) { joins(:scanner).merge(::Vulnerabilities::Scanner.with_external_id(scanner_external_ids)) }
scope :with_findings_scanner_and_identifiers, -> { includes(vulnerability: { findings: [:scanner, :identifiers, finding_identifiers: :identifier] }) }
scope :with_created_issue_links_and_issues, -> { includes(vulnerability: { created_issue_links: :issue }) }
scope :as_vulnerabilities, -> do
preload(vulnerability: { project: [:route] }).current_scope.tap do |relation|
relation.define_singleton_method(:records) do
super().map(&:vulnerability)
end
end
end
def self.order_by(method)
case method.to_s
when 'severity_desc' then order_severity_desc
when 'severity_asc' then order_severity_asc
when 'detected_desc' then order_detected_at_desc
when 'detected_asc' then order_detected_at_asc
else
order_severity_desc
end
end
end
end
......@@ -12,7 +12,11 @@ module API
helpers do
def vulnerabilities_by(project)
Security::VulnerabilitiesFinder.new(project).execute
if Feature.enabled?(:vulnerability_reads_table, project)
Security::VulnerabilityReadsFinder.new(project).execute.as_vulnerabilities
else
Security::VulnerabilitiesFinder.new(project).execute
end
end
def find_vulnerability!
......
......@@ -8,7 +8,7 @@ RSpec.describe Projects::AutocompleteSourcesController do
let_it_be(:project) { create(:project, :public, group: group) }
let_it_be(:epic) { create(:epic, group: group) }
let_it_be(:epic2) { create(:epic, group: group2) }
let_it_be(:vulnerability) { create(:vulnerability, project: project) }
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
before do
sign_in(user)
......
......@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe Projects::Security::Vulnerabilities::NotesController do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) }
let_it_be(:vulnerability) { create(:vulnerability, project: project) }
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
let!(:note) { create(:note, noteable: vulnerability, project: project) }
......
......@@ -35,7 +35,7 @@ FactoryBot.modify do
trait :with_vulnerabilities do
after(:create) do |project|
create_list(:vulnerability, 2, :detected, project: project)
create_list(:vulnerability, 2, :with_finding, :detected, project: project)
end
end
......
......@@ -6,7 +6,7 @@ RSpec.describe Autocomplete::VulnerabilitiesAutocompleteFinder do
describe '#execute' do
let_it_be(:group, refind: true) { create(:group) }
let_it_be(:project, refind: true) { create(:project, group: group) }
let_it_be(:vulnerability) { create(:vulnerability, project: project) }
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
let(:params) { {} }
......@@ -44,7 +44,7 @@ RSpec.describe Autocomplete::VulnerabilitiesAutocompleteFinder do
context 'when multiple vulnerabilities are found' do
before do
create_list(:vulnerability, 10, project: project)
create_list(:vulnerability, 10, :with_finding, project: project)
end
it 'returns max 5 items' do
......
......@@ -77,7 +77,7 @@ RSpec.describe Security::VulnerabilitiesFinder do
context 'when filtered by project' do
let(:group) { create(:group) }
let(:another_project) { create(:project, namespace: group) }
let!(:another_vulnerability) { create(:vulnerability, project: another_project) }
let!(:another_vulnerability) { create(:vulnerability, :with_findings, project: another_project) }
let(:filters) { { project_id: [another_project.id] } }
let(:vulnerable) { group }
......@@ -148,7 +148,7 @@ RSpec.describe Security::VulnerabilitiesFinder do
context 'when filtered by more than one property' do
let_it_be(:vulnerability4) do
create(:vulnerability, severity: :medium, report_type: :sast, state: :detected, project: project)
create(:vulnerability, :with_findings, severity: :medium, report_type: :sast, state: :detected, project: project)
end
let(:filters) { { report_type: %w[sast], severity: %w[medium] } }
......@@ -160,7 +160,7 @@ RSpec.describe Security::VulnerabilitiesFinder do
context 'when filtered by image' do
let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, project: project, vulnerability: cluster_vulnerability) }
let(:filters) { { image: [finding.location['image']] } }
let(:feature_enabled) { true }
......@@ -188,7 +188,7 @@ RSpec.describe Security::VulnerabilitiesFinder do
context 'when filtered by cluster_id' do
let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, project: project, vulnerability: cluster_vulnerability) }
let(:filters) { { cluster_id: [finding.location['kubernetes_resource']['cluster_id']] } }
......@@ -207,7 +207,7 @@ RSpec.describe Security::VulnerabilitiesFinder do
context 'when filtered by cluster_agent_id' do
let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, project: project, vulnerability: cluster_vulnerability) }
let(:filters) { { cluster_agent_id: [finding.location['kubernetes_resource']['agent_id']] } }
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Security::VulnerabilityReadsFinder do
let_it_be(:project) { create(:project) }
let_it_be(:low_severity_vuln_read) do
create(:vulnerability, :with_finding, :with_issue_links, severity: :low, report_type: :sast, state: :detected, project: project).vulnerability_read
end
let_it_be(:high_severity_vuln_read) do
create(:vulnerability, :with_finding, resolved_on_default_branch: true, severity: :high, report_type: :dependency_scanning, state: :confirmed, project: project).vulnerability_read
end
let_it_be(:medium_severity_vuln_read) do
create(:vulnerability, :with_finding, severity: :medium, report_type: :dast, state: :dismissed, project: project).vulnerability_read
end
let(:filters) { {} }
let(:vulnerable) { project }
subject { described_class.new(vulnerable, filters).execute }
context 'when not given a second argument' do
subject { described_class.new(project).execute }
it 'does not filter the vulnerability list' do
expect(subject).to eq [high_severity_vuln_read, medium_severity_vuln_read, low_severity_vuln_read]
end
end
context 'when filtered by report type' do
let(:filters) { { report_type: %w[sast dast] } }
it 'only returns vulnerabilities matching the given report types' do
is_expected.to contain_exactly(low_severity_vuln_read, medium_severity_vuln_read)
end
end
context 'when filtered by severity' do
let(:filters) { { severity: %w[medium high] } }
it 'only returns vulnerabilities matching the given severities' do
is_expected.to contain_exactly(medium_severity_vuln_read, high_severity_vuln_read)
end
end
context 'when filtered by state' do
let(:filters) { { state: %w[detected confirmed] } }
it 'only returns vulnerabilities matching the given states' do
is_expected.to contain_exactly(low_severity_vuln_read, high_severity_vuln_read)
end
end
context 'when filtered by scanner external ID' do
let(:filters) { { scanner: [low_severity_vuln_read.vulnerability.finding_scanner_external_id, high_severity_vuln_read.vulnerability.finding_scanner_external_id] } }
it 'only returns vulnerabilities matching the given scanner IDs' do
is_expected.to contain_exactly(low_severity_vuln_read, high_severity_vuln_read)
end
end
context 'when filtered by scanner_id' do
let(:filters) { { scanner_id: [low_severity_vuln_read.vulnerability.finding_scanner_id, medium_severity_vuln_read.vulnerability.finding_scanner_id] } }
it 'only returns vulnerabilities matching the given scanner IDs' do
is_expected.to contain_exactly(low_severity_vuln_read, medium_severity_vuln_read)
end
end
context 'when filtered by project' do
let(:group) { create(:group) }
let(:another_project) { create(:project, namespace: group) }
let!(:another_vulnerability) { create(:vulnerability, :with_finding, project: another_project) }
let(:vulnerable) { group }
let(:filters) { { project_id: [another_project.id] } }
before do
project.update!(namespace: group)
end
it 'only returns vulnerabilities matching the given projects' do
is_expected.to contain_exactly(another_vulnerability.vulnerability_read)
end
end
context 'when sorted' do
let(:filters) { { sort: method } }
context 'when sort method is not given' do
let(:method) { nil }
it { is_expected.to eq [high_severity_vuln_read, medium_severity_vuln_read, low_severity_vuln_read]}
end
context 'ascending by severity' do
let(:method) { :severity_asc }
it { is_expected.to eq [low_severity_vuln_read, medium_severity_vuln_read, high_severity_vuln_read] }
end
context 'descending by severity' do
let(:method) { :severity_desc }
it { is_expected.to eq [high_severity_vuln_read, medium_severity_vuln_read, low_severity_vuln_read] }
end
context 'ascending by detected_at' do
let(:method) { :detected_asc }
it { is_expected.to eq [low_severity_vuln_read, high_severity_vuln_read, medium_severity_vuln_read] }
end
context 'descending by detected_at' do
let(:method) { :detected_desc }
it { is_expected.to eq [medium_severity_vuln_read, high_severity_vuln_read, low_severity_vuln_read] }
end
end
context 'when filtered by has_issues argument' do
let(:filters) { { has_issues: has_issues } }
context 'when has_issues is set to true' do
let(:has_issues) { true }
it 'only returns vulnerabilities that have issues' do
is_expected.to contain_exactly(low_severity_vuln_read)
end
end
context 'when has_issues is set to false' do
let(:has_issues) { false }
it 'only returns vulnerabilities that does not have issues' do
is_expected.to contain_exactly(high_severity_vuln_read, medium_severity_vuln_read)
end
end
end
context 'when filtered by has_resolution argument' do
let(:filters) { { has_resolution: has_resolution } }
context 'when has_resolution is set to true' do
let(:has_resolution) { true }
it 'only returns vulnerabilities that have resolution' do
is_expected.to contain_exactly(high_severity_vuln_read)
end
end
context 'when has_resolution is set to false' do
let(:has_resolution) { false }
it 'only returns vulnerabilities that do not have resolution' do
is_expected.to contain_exactly(low_severity_vuln_read, medium_severity_vuln_read)
end
end
end
context 'when filtered by more than one property' do
let_it_be(:read4) do
create(:vulnerability, :with_finding, severity: :medium, report_type: :sast, state: :detected, project: project).vulnerability_read
end
let(:filters) { { report_type: %w[sast], severity: %w[medium] } }
it 'only returns vulnerabilities matching all of the given filters' do
is_expected.to contain_exactly(read4)
end
end
context 'when filtered by image' do
let(:vulnerable) { project }
let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, project: project, vulnerability: cluster_vulnerability) }
let(:filters) { { image: [finding.location['image']] } }
it 'only returns vulnerabilities matching the given image' do
is_expected.to contain_exactly(cluster_vulnerability.vulnerability_read)
end
context 'when different report_type is passed' do
let(:filters) { { report_type: %w[dast], image: [finding.location['image']] }}
it 'returns an empty relation' do
is_expected.to be_empty
end
end
context 'when vulnerable is InstanceSecurityDashboard' do
let(:vulnerable) { InstanceSecurityDashboard.new(project.users.first) }
it 'does not include cluster vulnerability' do
is_expected.not_to contain_exactly(cluster_vulnerability.vulnerability_read)
end
end
end
context 'when filtered by cluster_agent_id' do
let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, project: project, vulnerability: cluster_vulnerability) }
let(:filters) { { cluster_agent_id: [finding.location['kubernetes_resource']['agent_id']] } }
it 'only returns vulnerabilities matching the given agent_id' do
is_expected.to contain_exactly(cluster_vulnerability.vulnerability_read)
end
context 'when different report_type is passed' do
let(:filters) { { report_type: %w[dast], cluster_agent_id: [finding.location['kubernetes_resource']['agent_id']] }}
it 'returns empty list' do
is_expected.to be_empty
end
end
end
end
......@@ -28,7 +28,7 @@ RSpec.describe GitlabSchema.types['Group'] do
let_it_be(:project) { create(:project, namespace: group) }
let_it_be(:user) { create(:user) }
let_it_be(:vulnerability) do
create(:vulnerability, :detected, :critical, project: project, title: 'A terrible one!')
create(:vulnerability, :detected, :critical, :with_finding, project: project, title: 'A terrible one!')
end
let_it_be(:query) do
......
......@@ -12,20 +12,25 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
let_it_be(:user) { create(:user, security_dashboard_projects: [project]) }
let_it_be(:low_vulnerability) do
create(:vulnerability, :with_findings, :detected, :low, :dast, :with_issue_links, project: project)
create(:vulnerability, :with_finding, :detected, :low, :dast, :with_issue_links, project: project)
end
let_it_be(:critical_vulnerability) do
create(:vulnerability, :with_findings, :detected, :critical, :sast, resolved_on_default_branch: true, project: project)
create(:vulnerability, :with_finding, :detected, :critical, :sast, resolved_on_default_branch: true, project: project)
end
let_it_be(:high_vulnerability) do
create(:vulnerability, :with_findings, :dismissed, :high, :container_scanning, project: project)
create(:vulnerability, :with_finding, :dismissed, :high, :container_scanning, project: project)
end
let(:current_user) { user }
let(:params) { {} }
let(:vulnerable) { project }
let(:vulnerability_reads_table_enabled) { false }
before do
stub_feature_flags(vulnerability_reads_table: vulnerability_reads_table_enabled)
end
context 'when given sort' do
context 'when sorting descending by severity' do
......@@ -136,7 +141,7 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
context 'when given project IDs' do
let_it_be(:group) { create(:group) }
let_it_be(:project2) { create(:project, namespace: group) }
let_it_be(:project2_vulnerability) { create(:vulnerability, project: project2) }
let_it_be(:project2_vulnerability) { create(:vulnerability, :with_finding, project: project2) }
let(:params) { { project_id: [project2.id] } }
let(:vulnerable) { group }
......@@ -194,7 +199,7 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
context 'when image is given' do
let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) }
let_it_be(:cluster_finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability) }
let_it_be(:cluster_finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability, project: project) }
let(:params) { { image: [cluster_finding.location['image']] } }
......@@ -213,27 +218,46 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
context 'when cluster_id is given' do
let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) }
let_it_be(:cluster_finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability) }
let_it_be(:cluster_finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability, project: project) }
let_it_be(:cluster_gid) { ::Gitlab::GlobalId.as_global_id(cluster_finding.location['kubernetes_resource']['cluster_id'].to_i, model_name: 'Clusters::Cluster') }
let(:params) { { cluster_id: [cluster_gid] } }
context 'when vulnerability_reads_table is disabled' do
before do
# cluster_id is not supported by vulnerability_reads
stub_feature_flags(vulnerability_reads_table: false)
end
let(:params) { { cluster_id: [cluster_gid] } }
it 'only returns vulnerabilities with given cluster' do
is_expected.to contain_exactly(cluster_vulnerability)
it 'only returns vulnerabilities with given cluster' do
is_expected.to contain_exactly(cluster_vulnerability)
end
context 'when different report_type is given along with cluster' do
let(:params) { { report_type: %w[sast], cluster_id: [cluster_gid] } }
it 'returns empty list' do
is_expected.to be_empty
end
end
end
context 'when different report_type is given along with cluster' do
let(:params) { { report_type: %w[sast], cluster_id: [cluster_gid] } }
context 'when vulnerability_reads_table is enabled' do
before do
stub_feature_flags(vulnerability_reads_table: true)
end
it 'returns empty list' do
is_expected.to be_empty
let(:params) { { cluster_id: [Gitlab::GlobalId.build(nil, model_name: 'Clusters::Cluster', id: non_existing_record_id)] } }
it 'ignores the filter and returns unmatching vulnerabilities' do
is_expected.to include(cluster_vulnerability)
end
end
end
context 'when cluster_agent_id is given' do
let_it_be(:cluster_vulnerability) { create(:vulnerability, :cluster_image_scanning, project: project) }
let_it_be(:cluster_finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, vulnerability: cluster_vulnerability) }
let_it_be(:cluster_finding) { create(:vulnerabilities_finding, :with_cluster_image_scanning_scanning_metadata, project: project, vulnerability: cluster_vulnerability) }
let_it_be(:cluster_gid) { ::Gitlab::GlobalId.as_global_id(cluster_finding.location['kubernetes_resource']['agent_id'].to_i, model_name: 'Clusters::Agent') }
let(:params) { { cluster_agent_id: [cluster_gid] } }
......@@ -250,5 +274,21 @@ RSpec.describe Resolvers::VulnerabilitiesResolver do
end
end
end
context 'when vulnerability_reads_table feature is enabled' do
let(:vulnerability_reads_table_enabled) { true }
let(:params) { { report_type: %w[sast dast] } }
let(:vulnerable) { project }
it 'returns vulnerabilities of a project' do
is_expected.to contain_exactly(low_vulnerability, critical_vulnerability)
end
it 'calls VulnerabilityReadsFinder' do
expect(Security::VulnerabilityReadsFinder).to receive(:new).with(vulnerable, params).and_call_original
subject
end
end
end
end
......@@ -26,6 +26,11 @@ RSpec.describe Resolvers::VulnerabilitySeveritiesCountResolver do
let(:current_user) { user }
let(:filters) { {} }
let(:vulnerable) { project }
let(:vulnerability_reads_table_enabled) { false }
before do
stub_feature_flags(vulnerability_reads_table: vulnerability_reads_table_enabled)
end
context 'when the user does not have access' do
it 'is redacted' do
......@@ -116,6 +121,21 @@ RSpec.describe Resolvers::VulnerabilitySeveritiesCountResolver do
is_expected.to eq('critical' => 1, 'low' => 1)
end
end
context 'when vulnerability_reads_table feature is enabled' do
let(:vulnerability_reads_table_enabled) { true }
let(:filters) { { report_type: %w[sast dast] } }
it 'returns vulnerabilities of a project' do
is_expected.to eq('critical' => 1, 'low' => 1)
end
it 'calls VulnerabilityReadsFinder' do
expect(Security::VulnerabilityReadsFinder).to receive(:new).with(vulnerable, filters).and_call_original
subject
end
end
end
context 'when resolving vulnerabilities for an instance security dashboard' do
......
......@@ -7,7 +7,7 @@ RSpec.describe GitlabSchema.types['Project'] do
let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) }
let_it_be(:vulnerability) { create(:vulnerability, project: project, severity: :high) }
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project, severity: :high) }
before do
stub_licensed_features(security_dashboard: true)
......@@ -71,7 +71,7 @@ RSpec.describe GitlabSchema.types['Project'] do
let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) }
let_it_be(:vulnerability) do
create(:vulnerability, :detected, :critical, project: project, title: 'A terrible one!')
create(:vulnerability, :detected, :critical, :with_finding, project: project, title: 'A terrible one!')
end
let_it_be(:query) do
......
......@@ -95,41 +95,44 @@ RSpec.describe GitlabSchema.types['Vulnerability'] do
)
end
context 'N+1 queries' do
RSpec.shared_examples "N+1 queries" do
it 'avoids N+1 database queries' do
# Execute the query once so we don't count selecting Projects and Namespaces
GitlabSchema.execute(query, context: { current_user: user })
# Count queries for the baseline which is a single Vulnerability with a remediation
# Should be 10
control_count = ActiveRecord::QueryRecorder.new { GitlabSchema.execute(query, context: { current_user: user }) }.count
expect(control_count).to eq(10)
expect(control_count).to eq(single_query_count)
create(:vulnerability, :with_remediation, project: project)
create(:vulnerability, :with_remediation, project: project)
create(:vulnerability, :with_remediation, project: project)
# Every additional Vulnerability seems to add TWO more database calls similar to
# SELECT
# MAX("project_authorizations"."access_level") AS maximum_access_level,
# "project_authorizations"."user_id" AS project_authorizations_user_id
# FROM "project_authorizations"
# WHERE
# "project_authorizations"."project_id" = 315
# AND
# "project_authorizations"."user_id" = 409
# GROUP BY
# "project_authorizations"."user_id"
#
# I have no idea where do they come from or if we could batch them
# so we have to increase the control_count by 2 * number of Vulnerabilities
expect { GitlabSchema.execute(query, context: { current_user: user }) }.not_to exceed_query_limit(control_count + (3 * 2))
expect { GitlabSchema.execute(query, context: { current_user: user }) }.not_to exceed_query_limit(multiple_queries_count)
result = GitlabSchema.execute(query, context: { current_user: user }).to_h
vulnerability = result.dig('data', 'project', 'vulnerabilities', 'nodes').first
expect(vulnerability['hasSolutions']).to be_truthy
end
end
context 'N+1 queries' do
context 'when vulnerability_reads_table is disabled' do
before do
stub_feature_flags(vulnerability_reads_table: false)
end
let(:single_query_count) { 10 }
let(:multiple_queries_count) { single_query_count + (2 * 3) }
it_behaves_like "N+1 queries"
end
context 'when vulnerability_reads_table is enabled' do
let(:single_query_count) { 14 }
let(:multiple_queries_count) { single_query_count + (3 * 3) }
it_behaves_like "N+1 queries"
end
end
end
describe 'false_positive' do
......
......@@ -359,6 +359,24 @@ RSpec.describe Group do
end
end
describe '#vulnerability_reads' do
subject { group.vulnerability_reads }
let(:subgroup) { create(:group, parent: group) }
let(:group_project) { create(:project, namespace: group) }
let(:subgroup_project) { create(:project, namespace: subgroup) }
let(:archived_project) { create(:project, :archived, namespace: group) }
let(:deleted_project) { create(:project, pending_delete: true, namespace: group) }
let!(:group_vulnerability) { create(:vulnerability, :with_findings, project: group_project) }
let!(:subgroup_vulnerability) { create(:vulnerability, :with_findings, project: subgroup_project) }
let!(:archived_vulnerability) { create(:vulnerability, :with_findings, project: archived_project) }
let!(:deleted_vulnerability) { create(:vulnerability, :with_findings, project: deleted_project) }
it 'returns vulnerabilities for all non-archived, non-deleted projects in the group and its subgroups' do
is_expected.to contain_exactly(group_vulnerability.vulnerability_read, subgroup_vulnerability.vulnerability_read)
end
end
describe '#vulnerability_scanners' do
subject { group.vulnerability_scanners }
......
......@@ -60,6 +60,7 @@ RSpec.describe Vulnerability do
it { is_expected.to belong_to(:confirmed_by).class_name('User') }
it { is_expected.to have_one(:group).through(:project) }
it { is_expected.to have_one(:vulnerability_read) }
it { is_expected.to have_many(:findings).class_name('Vulnerabilities::Finding').dependent(false) }
it { is_expected.to have_many(:notes).dependent(:delete_all) }
......
......@@ -160,6 +160,25 @@ RSpec.describe InstanceSecurityDashboard do
end
end
describe '#vulnerability_reads' do
let_it_be(:vulnerability1) { create(:vulnerability, :with_findings, project: project1) }
let_it_be(:vulnerability2) { create(:vulnerability, :with_findings, project: project2) }
context 'when the user cannot read all resources' do
it 'returns only vulnerability_reads from projects on their dashboard that they can read' do
expect(subject.vulnerability_reads).to contain_exactly(vulnerability1.vulnerability_read)
end
end
context 'when the user can read all resources' do
let(:user) { create(:auditor) }
it "returns vulnerability_reads from all projects on the user's dashboard" do
expect(subject.vulnerability_reads).to contain_exactly(vulnerability1.vulnerability_read, vulnerability2.vulnerability_read)
end
end
end
describe '#vulnerability_scanners' do
let_it_be(:vulnerability_scanner1) { create(:vulnerabilities_scanner, project: project1) }
let_it_be(:vulnerability_scanner2) { create(:vulnerabilities_scanner, project: project2) }
......
......@@ -51,6 +51,7 @@ RSpec.describe Project do
it { is_expected.to have_many(:downstream_projects) }
it { is_expected.to have_many(:vulnerability_historical_statistics).class_name('Vulnerabilities::HistoricalStatistic') }
it { is_expected.to have_many(:vulnerability_remediations).class_name('Vulnerabilities::Remediation') }
it { is_expected.to have_many(:vulnerability_reads).class_name('Vulnerabilities::Read') }
it { is_expected.to have_one(:github_integration) }
it { is_expected.to have_many(:project_aliases) }
......
This diff is collapsed.
......@@ -102,10 +102,10 @@ RSpec.describe 'getting group information' do
let_it_be(:public_project) { create(:project, group: public_group) }
let_it_be(:private_project) { create(:project, group: private_group) }
let_it_be(:vulnerability_1) { create(:vulnerability, :dismissed, :critical_severity, :with_notes, notes_count: 2, project: public_project) }
let_it_be(:vulnerability_2) { create(:vulnerability, :confirmed, :high_severity, :with_notes, notes_count: 3, project: public_project) }
let_it_be(:vulnerability_3) { create(:vulnerability, :dismissed, :medium_severity, :with_notes, notes_count: 4, project: private_project) }
let_it_be(:vulnerability_4) { create(:vulnerability, :confirmed, :low_severity, :with_notes, notes_count: 7, project: private_project) }
let_it_be(:vulnerability_1) { create(:vulnerability, :dismissed, :critical_severity, :with_notes, :with_finding, notes_count: 2, project: public_project) }
let_it_be(:vulnerability_2) { create(:vulnerability, :confirmed, :high_severity, :with_notes, :with_finding, notes_count: 3, project: public_project) }
let_it_be(:vulnerability_3) { create(:vulnerability, :dismissed, :medium_severity, :with_notes, :with_finding, notes_count: 4, project: private_project) }
let_it_be(:vulnerability_4) { create(:vulnerability, :confirmed, :low_severity, :with_notes, :with_finding, notes_count: 7, project: private_project) }
let_it_be(:vulnerability_statistic_1) { create(:vulnerability_statistic, :grade_c, project: public_project) }
let_it_be(:vulnerability_statistic_2) { create(:vulnerability_statistic, :grade_d, project: private_project) }
......
......@@ -83,9 +83,9 @@ RSpec.describe 'Query.instanceSecurityDashboard.projects' do
graphql_query_for('instanceSecurityDashboard', nil, fields)
end
let_it_be(:vulnerability_1) { create(:vulnerability, :dismissed, :critical_severity, :with_notes, notes_count: 2, project: project) }
let_it_be(:vulnerability_2) { create(:vulnerability, :confirmed, :high_severity, :with_notes, notes_count: 3, project: project) }
let_it_be(:vulnerability_3) { create(:vulnerability, :confirmed, :medium_severity, :with_notes, notes_count: 7, project: other_project) }
let_it_be(:vulnerability_1) { create(:vulnerability, :dismissed, :critical_severity, :with_notes, :with_finding, notes_count: 2, project: project) }
let_it_be(:vulnerability_2) { create(:vulnerability, :confirmed, :high_severity, :with_notes, :with_finding, notes_count: 3, project: project) }
let_it_be(:vulnerability_3) { create(:vulnerability, :confirmed, :medium_severity, :with_notes, :with_finding, notes_count: 7, project: other_project) }
let_it_be(:vulnerability_statistic_1) { create(:vulnerability_statistic, :grade_c, project: project) }
let_it_be(:vulnerability_statistic_2) { create(:vulnerability_statistic, :grade_d, project: other_project) }
......
......@@ -22,7 +22,7 @@ RSpec.describe 'Query.project(fullPath).vulnerabilitySeveritiesCount' do
before do
stub_licensed_features(security_dashboard: true)
create_list(:vulnerability, 2, :high, :with_issue_links, resolved_on_default_branch: true, project: project)
create_list(:vulnerability, 2, :high, :with_issue_links, :with_finding, resolved_on_default_branch: true, project: project)
project.add_developer(user)
end
......
......@@ -32,14 +32,30 @@ RSpec.describe API::Vulnerabilities do
expect(response.headers['X-Total']).to eq project.vulnerabilities.count.to_s
end
context 'when vulnerability_reads_table is disabled' do
before do
stub_feature_flags(vulnerability_reads_table: false)
end
it 'returns all vulnerabilities of a project' do
get_vulnerabilities
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/vulnerabilities', dir: 'ee')
expect(response.headers['X-Total']).to eq project.vulnerabilities.count.to_s
end
end
context 'with pagination' do
let(:project_vulnerabilities_path) { "#{super()}?page=2&per_page=1" }
let(:project_vulnerabilities_path) { "#{super()}?page=3&per_page=1" }
it 'paginates the vulnerabilities according to the pagination params' do
low_severity_vulnerability = create(:vulnerability, :with_finding, project: project, severity: :low)
get_vulnerabilities
expect(response).to have_gitlab_http_status(:ok)
expect(json_response.map { |v| v['id'] }).to contain_exactly(project.vulnerabilities.order_severity_desc.second.id)
expect(json_response.map { |v| v['id'] }).to contain_exactly(low_severity_vulnerability.id)
end
end
......@@ -62,7 +78,6 @@ RSpec.describe API::Vulnerabilities do
describe 'GET /vulnerabilities/:id' do
let_it_be(:project) { create(:project, :with_vulnerabilities) }
let_it_be(:vulnerability) { project.vulnerabilities.first }
let_it_be(:finding) { create(:vulnerabilities_finding, vulnerability: vulnerability) }
let(:vulnerability_id) { vulnerability.id }
......@@ -86,7 +101,7 @@ RSpec.describe API::Vulnerabilities do
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/vulnerability', dir: 'ee')
expect(json_response['finding']['id']).to eq finding.id
expect(json_response['finding']['id']).to eq vulnerability.finding.id
end
it_behaves_like 'responds with "not found" for an unknown vulnerability ID'
......@@ -241,7 +256,7 @@ RSpec.describe API::Vulnerabilities do
end
context 'if a vulnerability is already dismissed' do
let(:vulnerability) { create(:vulnerability, :dismissed, project: project) }
let(:vulnerability) { create(:vulnerability, :with_findings, :dismissed, project: project) }
it 'responds with 304 Not Modified' do
dismiss_vulnerability
......@@ -299,7 +314,7 @@ RSpec.describe API::Vulnerabilities do
it_behaves_like 'responds with "not found" for an unknown vulnerability ID'
context 'when the vulnerability is already resolved' do
let(:vulnerability) { create(:vulnerability, :resolved, project: project) }
let(:vulnerability) { create(:vulnerability, :with_findings, :resolved, project: project) }
it 'responds with 304 Not Modified response' do
resolve_vulnerability
......@@ -357,7 +372,7 @@ RSpec.describe API::Vulnerabilities do
it_behaves_like 'responds with "not found" for an unknown vulnerability ID'
context 'when the vulnerability is already confirmed' do
let(:vulnerability) { create(:vulnerability, :confirmed, project: project) }
let(:vulnerability) { create(:vulnerability, :with_findings, :confirmed, project: project) }
it 'responds with 304 Not Modified response' do
confirm_vulnerability
......@@ -388,7 +403,7 @@ RSpec.describe API::Vulnerabilities do
end
let_it_be(:project) { create(:project) }
let_it_be(:vulnerability) { create(:vulnerability, :dismissed, project: project) }
let_it_be(:vulnerability) { create(:vulnerability, :with_findings, :dismissed, project: project) }
let(:vulnerability_id) { vulnerability.id }
......@@ -445,7 +460,7 @@ RSpec.describe API::Vulnerabilities do
end
context 'if a vulnerability is already in detected state' do
let(:vulnerability) { create(:vulnerability, :detected, project: project) }
let(:vulnerability) { create(:vulnerability, :with_findings, :detected, project: project) }
it 'responds with 304 Not Modified' do
revert_vulnerability_to_detected
......
......@@ -73,7 +73,7 @@ RSpec.describe 'groups autocomplete' do
describe '#vulnerabilities' do
let_it_be_with_reload(:project) { create(:project, :private, group: group) }
let_it_be(:vulnerability) { create(:vulnerability, project: project) }
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
before do
project.add_developer(user)
......
......@@ -82,7 +82,7 @@ RSpec.describe Groups::AutocompleteService do
describe '#vulnerability' do
let_it_be_with_refind(:project) { create(:project, group: group) }
let_it_be(:vulnerability) { create(:vulnerability, project: project) }
let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
let_it_be(:guest) { create(:user) }
let(:autocomplete_user) { user }
......
......@@ -610,6 +610,7 @@ project:
- sync_events
- secure_files
- security_trainings
- vulnerability_reads
award_emoji:
- awardable
- user
......
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