Commit 605fad39 authored by Mayra Cabrera's avatar Mayra Cabrera

Merge branch '207448_cleanup_vulnerabilities_history_and_summary_logic' into 'master'

Remove vulnerabilities summary & history related logic

See merge request gitlab-org/gitlab!39077
parents 54ea9a29 9ad882c4
......@@ -110,14 +110,6 @@ module Vulnerabilities
.where(vulnerability_occurrence_pipelines: { pipeline_id: pipelines })
end
def self.count_by_day_and_severity(period)
joins(:finding_pipelines)
.select('CAST(vulnerability_occurrence_pipelines.created_at AS DATE) AS day', :severity, 'COUNT(distinct vulnerability_occurrences.id) as count')
.where(['vulnerability_occurrence_pipelines.created_at >= ?', Time.zone.now.beginning_of_day - period])
.group(:day, :severity)
.order('day')
end
def self.counted_by_severity
group(:severity).count.transform_keys do |severity|
SEVERITY_LEVELS[severity]
......
# frozen_string_literal: true
class Vulnerabilities::HistoryEntity < Grape::Entity
present_collection true
Vulnerabilities::Finding::SEVERITY_LEVELS.keys.each do |level|
expose level do |object|
counts(by_severity[level]&.group_by(&:day) || {})
end
end
expose :total do |object|
counts(by_days)
end
private
def by_days
items.group_by(&:day)
end
def by_severity
items.group_by(&:severity)
end
def items
object[:items]
end
def counts(hash)
hash.transform_values { |items| items.sum(&:count) } # rubocop: disable CodeReuse/ActiveRecord
end
end
# frozen_string_literal: true
class Vulnerabilities::HistorySerializer < BaseSerializer
entity Vulnerabilities::HistoryEntity
end
# frozen_string_literal: true
class VulnerabilitySummaryEntity < Grape::Entity
Vulnerabilities::Finding::SEVERITY_LEVELS.each do |severity_name, severity|
expose severity_name do |param|
object[severity] || 0
end
end
end
# frozen_string_literal: true
class VulnerabilitySummarySerializer < BaseSerializer
entity VulnerabilitySummaryEntity
end
......@@ -15,23 +15,11 @@ module Security
errors << result[:message] if result[:status] == :error
end
cache_vulnerabilities
if errors.any?
error(errors.join(", "))
else
success
end
end
# Silently swallow errors if there are any problems caching vulnerabilities
def cache_vulnerabilities
project = @pipeline.project
Gitlab::Vulnerabilities::HistoryCache.new(project.group, project.id)
.fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE, force: true)
rescue => err
error("Failed to cache vulnerabilities for pipeline #{@pipeline.id}: #{err}")
end
end
end
# frozen_string_literal: true
require 'vulnerabilities/history_serializer'
module Gitlab
module Vulnerabilities
class History
attr_reader :vulnerable, :filters
HISTORY_RANGE = 3.months
NoProjectIDsError = Class.new(StandardError)
def initialize(vulnerable, params:)
@vulnerable = vulnerable
@filters = params
end
def findings_counter
return cached_vulnerability_history unless dynamic_filters_included?
findings = vulnerability_findings.count_by_day_and_severity(HISTORY_RANGE)
::Vulnerabilities::HistorySerializer.new.represent(findings)
end
private
def vulnerability_findings
::Security::VulnerabilityFindingsFinder.new(pipeline_ids, params: filters).execute
end
def cached_vulnerability_history
history = { info: {}, unknown: {}, low: {}, medium: {}, high: {}, critical: {}, total: {} }
project_ids_to_fetch.each do |project_id|
project_history = Gitlab::Vulnerabilities::HistoryCache.new(vulnerable, project_id).fetch(HISTORY_RANGE)
history.each do |key, value|
value.merge!(project_history[key]) { |k, aggregate, project_count| aggregate + project_count }
end
end
sort_by_date_for_each_key(history)
end
def sort_by_date_for_each_key(history)
history.each do |key, value|
history[key] = value.sort_by { |date, count| date }.to_h
end
history
end
def dynamic_filters_included?
dynamic_filters = [:report_type, :confidence, :severity]
filters.keys.any? { |key| dynamic_filters.include?(key.to_sym) }
end
def project_ids_to_fetch
return filters[:project_id] if filters.key?('project_id')
vulnerable.project_ids_with_security_reports
end
def pipeline_ids
vulnerable.all_pipelines.with_vulnerabilities.success
end
end
end
end
# frozen_string_literal: true
module Gitlab
module Vulnerabilities
class HistoryCache
attr_reader :vulnerable, :project_id
def initialize(vulnerable, project_id)
@vulnerable = vulnerable
@project_id = project_id
end
def fetch(range, force: false)
Rails.cache.fetch(cache_key, force: force, expires_in: 1.day) do
findings = ::Security::VulnerabilityFindingsFinder
.new(pipeline_ids, params: { project_id: [project_id] })
.execute
.count_by_day_and_severity(range)
::Vulnerabilities::HistorySerializer.new.represent(findings)
end
end
private
def cache_key
# TODO: rename 'vulnerabilities' to 'findings' in the cache key, but carefully
# https://gitlab.com/gitlab-org/gitlab/issues/32963
['projects', project_id, 'vulnerabilities']
end
def pipeline_ids
vulnerable.all_pipelines.with_vulnerabilities.success
end
end
end
end
# frozen_string_literal: true
module Gitlab
module Vulnerabilities
class Summary
attr_reader :vulnerable, :filters
def initialize(vulnerable, params)
@filters = params
@vulnerable = vulnerable
end
def findings_counter
return cached_vulnerability_summary if use_vulnerability_cache?
vulnerabilities = found_vulnerabilities.counted_by_severity
VulnerabilitySummarySerializer.new.represent(vulnerabilities)
end
private
def cached_vulnerability_summary
summary = {
info: 0,
unknown: 0,
low: 0,
medium: 0,
high: 0,
critical: 0
}
summary_keys = ::Vulnerabilities::Finding::SEVERITY_LEVELS.keys.map(&:to_sym)
project_ids_to_fetch.each do |project_id|
project_summary = Gitlab::Vulnerabilities::SummaryCache
.new(vulnerable, project_id)
.fetch
summary_keys.each do |key|
summary[key] += project_summary[key] unless project_summary[key].nil?
end
end
summary
end
def found_vulnerabilities
::Security::VulnerabilityFindingsFinder.new(pipeline_ids, params: filters).execute
end
def use_vulnerability_cache?
Feature.enabled?(:cache_vulnerability_summary) && !dynamic_filters_included?
end
def dynamic_filters_included?
dynamic_filters = [:report_type, :confidence, :severity]
filters.keys.any? { |k| dynamic_filters.include?(k.to_sym) }
end
def project_ids_to_fetch
return [vulnerable.id] if vulnerable.is_a?(Project)
if filters.key?('project_id')
vulnerable.project_ids_with_security_reports & filters[:project_id].map(&:to_i)
else
vulnerable.project_ids_with_security_reports
end
end
def pipeline_ids
vulnerable.all_pipelines.with_vulnerabilities.latest_successful_ids_per_project
end
end
end
end
# frozen_string_literal: true
module Gitlab
module Vulnerabilities
class SummaryCache
attr_reader :vulnerable, :project_id
def initialize(vulnerable, project_id)
@vulnerable = vulnerable
@project_id = project_id
end
def fetch(force: false)
Rails.cache.fetch(cache_key, force: force, expires_in: 1.day) do
findings = ::Security::VulnerabilityFindingsFinder
.new(pipeline_ids, params: { project_id: [project_id] })
.execute
.counted_by_severity
VulnerabilitySummarySerializer.new.represent(findings)
end
end
private
def cache_key
['projects', project_id, 'findings-summary']
end
def pipeline_ids
vulnerable.all_pipelines.with_vulnerabilities.success
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Vulnerabilities::HistoryCache do
describe '#fetch', :use_clean_rails_memory_store_caching do
shared_examples 'the history cache when given an expected Vulnerable' do
let(:project) { create(:project, :public, namespace: group) }
let(:project_cache_key) { described_class.new(vulnerable, project.id).send(:cache_key) }
let(:today) { '1980-10-31' }
before do
Timecop.freeze(today) do
create_vulnerabilities(1, project)
end
end
it 'reads from cache when records are cached' do
history_cache = described_class.new(vulnerable, project.id)
expect(Rails.cache.fetch(project_cache_key, raw: true)).to be_nil
control_count = ActiveRecord::QueryRecorder.new { history_cache.fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE) }
expect { 2.times { history_cache.fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE) } }.not_to exceed_query_limit(control_count)
end
it 'returns the proper format for uncached history' do
Timecop.freeze(today) do
fetched_history = described_class.new(vulnerable, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE)
expect(fetched_history[:total]).to eq( Date.today => 1 )
expect(fetched_history[:high]).to eq( Date.today => 1 )
end
end
it 'returns the proper format for cached history' do
Timecop.freeze(today) do
described_class.new(vulnerable, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE)
fetched_history = described_class.new(vulnerable, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE)
expect(fetched_history[:total]).to eq( Date.today => 1 )
expect(fetched_history[:high]).to eq( Date.today => 1 )
end
end
def create_vulnerabilities(count, project, options = {})
report_type = options[:report_type] || :sast
pipeline = create(:ci_pipeline, :success, project: project)
create_list(:vulnerabilities_occurrence, count, report_type: report_type, pipelines: [pipeline], project: project)
end
end
context 'when given a Group' do
it_behaves_like 'the history cache when given an expected Vulnerable' do
let(:group) { create(:group) }
let(:vulnerable) { group }
end
end
context 'when given an InstanceSecurityDashboard' do
it_behaves_like 'the history cache when given an expected Vulnerable' do
let(:group) { create(:group) }
let(:user) { create(:user) }
let(:vulnerable) { InstanceSecurityDashboard.new(user) }
before do
project.add_developer(user)
user.security_dashboard_projects << project
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Vulnerabilities::History do
describe '#findings_counter', :use_clean_rails_memory_store_caching do
shared_examples 'the history cache when given an expected Vulnerable' do
let(:filters) { ActionController::Parameters.new }
let(:today) { Date.parse('20191031') }
before do
Timecop.freeze(today) do
create_vulnerabilities(1, project1, { severity: :medium, report_type: :sast })
create_vulnerabilities(2, project2, { severity: :high, report_type: :sast })
end
end
subject(:counter) { described_class.new(vulnerable, params: filters).findings_counter }
context 'when filters are passed' do
let(:filters) { ActionController::Parameters.new({ 'report_type' => ['sast'] }) }
it 'does not call Gitlab::Vulnerabilities::HistoryCache' do
expect(Gitlab::Vulnerabilities::HistoryCache).not_to receive(:new)
counter
end
end
it 'calls Gitlab::Vulnerabilities::HistoryCache' do
expect(Gitlab::Vulnerabilities::HistoryCache).to receive(:new).twice.and_call_original
counter
end
it 'returns the proper format for the history' do
Timecop.freeze(today) do
expect(counter[:total]).to eq({ today => 3 })
expect(counter[:high]).to eq({ today => 2 })
end
end
context 'when there are multiple projects with vulnerabilities' do
before do
Timecop.freeze(today - 1) do
create_vulnerabilities(1, project1, { severity: :high })
end
Timecop.freeze(today - 4) do
create_vulnerabilities(1, project2, { severity: :high })
end
end
it 'sorts by date for each key' do
Timecop.freeze(today) do
expect(counter[:high].keys).to eq([(today - 4), (today - 1), today])
end
end
end
context 'when a project_id filter is passed' do
let(:filters) { ActionController::Parameters.new({ 'project_id' => [project1] }) }
it 'only fetches history for the filtered by projects' do
expect(Gitlab::Vulnerabilities::HistoryCache).to receive(:new).once.and_call_original
Timecop.freeze(today) do
expect(counter[:total]).to eq({ today => 1 })
expect(counter[:high]).to eq({})
expect(counter[:medium]).to eq({ today => 1 })
end
end
end
def create_vulnerabilities(count, project, options = {})
report_type = options[:report_type] || :sast
severity = options[:severity] || :high
pipeline = create(:ci_pipeline, :success, project: project)
created_at = options[:created_at] || today
create_list(:vulnerabilities_occurrence, count, report_type: report_type, severity: severity, pipelines: [pipeline], project: project, created_at: created_at)
end
end
context 'when the given vulnerable is a Group' do
it_behaves_like 'the history cache when given an expected Vulnerable' do
let(:group) { create(:group) }
let(:project1) { create(:project, :public, namespace: group) }
let(:project2) { create(:project, :public, namespace: group) }
let(:vulnerable) { group }
end
end
context 'when given an InstanceSecurityDashboard' do
it_behaves_like 'the history cache when given an expected Vulnerable' do
let(:group) { create(:group) }
let(:project1) { create(:project, :public, namespace: group) }
let(:project2) { create(:project, :public, namespace: group) }
let(:user) { create(:user) }
let(:vulnerable) { InstanceSecurityDashboard.new(user) }
before do
project1.add_developer(user)
project2.add_developer(user)
user.security_dashboard_projects << [project1, project2]
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Vulnerabilities::SummaryCache do
let(:group) { create(:group) }
let(:project) { create(:project, :public, namespace: group) }
let(:project_cache_key) { described_class.new(group, project.id).send(:cache_key) }
before do
create_vulnerabilities(1, project)
end
describe '#fetch', :use_clean_rails_memory_store_caching do
it 'reads from cache when records are cached' do
summary_cache = described_class.new(group, project.id)
expect(Rails.cache.fetch(project_cache_key, raw: true)).to be_nil
control_count = ActiveRecord::QueryRecorder.new { summary_cache.fetch }
expect { 2.times { summary_cache.fetch } }.not_to exceed_query_limit(control_count)
end
it 'returns the proper format for uncached summary' do
Timecop.freeze do
fetched_history = described_class.new(group, project.id).fetch
expect(fetched_history[:high]).to eq(1)
end
end
it 'returns the proper format for cached summary' do
Timecop.freeze do
described_class.new(group, project.id).fetch
fetched_history = described_class.new(group, project.id).fetch
expect(fetched_history[:high]).to eq(1)
end
end
def create_vulnerabilities(count, project, options = {})
pipeline = create(:ci_pipeline, :success, project: project)
create_list(
:vulnerabilities_occurrence,
count,
report_type: options[:report_type] || :sast,
pipelines: [pipeline],
project: project
)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Vulnerabilities::Summary do
let(:group) { create(:group) }
let(:project1) { create(:project, :public, namespace: group) }
let(:project2) { create(:project, :public, namespace: group) }
let(:filters) { {} }
before do
create_vulnerabilities(1, project1, { severity: :medium, report_type: :sast })
create_vulnerabilities(2, project2, { severity: :high, report_type: :sast })
end
describe '#findings_counter', :use_clean_rails_memory_store_caching do
subject(:counter) { described_class.new(group, filters).findings_counter }
context 'feature disabled' do
before do
stub_feature_flags(cache_vulnerability_summary: false)
end
it 'does not call Gitlab::Vulnerabilities::SummaryCache' do
expect(Gitlab::Vulnerabilities::SummaryCache).not_to receive(:new)
counter
end
it 'returns the proper format for the summary' do
Timecop.freeze do
expect(counter[:high]).to eq(2)
end
end
end
context 'feature enabled' do
before do
stub_feature_flags(cache_vulnerability_history: true)
end
context 'filters are passed' do
let(:filters) { { report_type: :sast } }
it 'does not call Gitlab::Vulnerabilities::SummaryCache' do
expect(Gitlab::Vulnerabilities::SummaryCache).not_to receive(:new)
counter
end
end
it 'calls Gitlab::Vulnerabilities::SummaryCache' do
expect(Gitlab::Vulnerabilities::SummaryCache).to receive(:new).twice.and_call_original
counter
end
it 'returns the proper format for the summary' do
Timecop.freeze do
expect(counter[:high]).to eq(2)
end
end
context 'when a project_id param is passed' do
let(:filters) { ActionController::Parameters.new({ project_id: [project1.id.to_s] }) }
it 'only fetches findings for the given projects' do
expect(counter[:high]).to eq(0)
expect(counter[:medium]).to eq(1)
end
end
end
def create_vulnerabilities(count, project, options = {})
pipeline = create(:ci_pipeline, :success, project: project)
create_list(
:vulnerabilities_occurrence,
count,
report_type: options[:report_type] || :sast,
severity: options[:severity] || :high,
pipelines: [pipeline],
project: project,
created_at: options[:created_at] || Date.today
)
end
end
end
......@@ -108,82 +108,6 @@ RSpec.describe Vulnerabilities::Finding do
end
end
describe '.count_by_day_and_severity' do
let(:project) { create(:project) }
let(:date_1) { Time.zone.parse('2018-11-11') }
let(:date_2) { Time.zone.parse('2018-11-12') }
before do
travel_to(date_1) do
pipeline = create(:ci_pipeline, :success, project: project)
create_list(:vulnerabilities_finding, 2,
pipelines: [pipeline], project: project, report_type: :sast, severity: :high)
end
travel_to(date_2) do
pipeline = create(:ci_pipeline, :success, project: project)
create_list(:vulnerabilities_finding, 2,
pipelines: [pipeline], project: project, report_type: :dependency_scanning, severity: :low)
create_list(:vulnerabilities_finding, 1,
pipelines: [pipeline], project: project, report_type: :dast, severity: :medium)
create_list(:vulnerabilities_finding, 1,
pipelines: [pipeline], project: project, report_type: :dast, severity: :low)
create_list(:vulnerabilities_finding, 2,
pipelines: [pipeline], project: project, report_type: :secret_detection, severity: :critical)
end
end
subject do
travel_to(Time.zone.parse('2018-11-15')) do
described_class.count_by_day_and_severity(range)
end
end
context 'within 3-day period' do
let(:range) { 3.days }
it 'returns expected counts for findings' do
first, second, third = subject
expect(first.day).to eq(date_2)
expect(first.severity).to eq('low')
expect(first.count).to eq(3)
expect(second.day).to eq(date_2)
expect(second.severity).to eq('medium')
expect(second.count).to eq(1)
expect(third.day).to eq(date_2)
expect(third.severity).to eq('critical')
expect(third.count).to eq(2)
end
end
context 'within 4-day period' do
let(:range) { 4.days }
it 'returns expected counts for findings' do
first, second, third, forth = subject
expect(first.day).to eq(date_1)
expect(first.severity).to eq('high')
expect(first.count).to eq(2)
expect(second.day).to eq(date_2)
expect(second.severity).to eq('low')
expect(second.count).to eq(3)
expect(third.day).to eq(date_2)
expect(third.severity).to eq('medium')
expect(third.count).to eq(1)
expect(forth.day).to eq(date_2)
expect(forth.severity).to eq('critical')
expect(forth.count).to eq(2)
end
end
end
describe '.by_report_types' do
let!(:vulnerability_sast) { create(:vulnerabilities_finding, report_type: :sast) }
let!(:vulnerability_secret_detection) { create(:vulnerabilities_finding, report_type: :secret_detection) }
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Vulnerabilities::HistoryEntity do
let(:project) { create(:project) }
let(:time) { Time.zone.parse('2018-11-10') }
let(:entity) do
travel_to(Time.zone.parse('2018-11-15')) do
described_class.represent(project.vulnerability_findings.count_by_day_and_severity(3.months))
end
end
before do
travel_to(time) do
pipeline_1 = create(:ci_pipeline, :success, project: project)
create_list(:vulnerabilities_occurrence, 2,
pipelines: [pipeline_1], project: project, report_type: :sast, severity: :high)
create_list(:vulnerabilities_occurrence, 1,
pipelines: [pipeline_1], project: project, report_type: :dependency_scanning, severity: :low)
end
end
describe '#as_json' do
subject { entity.as_json }
it 'contains required fields' do
expect(subject[:total]).to eq({ time.to_date => 3 })
expect(subject[:high]).to eq({ time.to_date => 2 })
expect(subject[:low]).to eq({ time.to_date => 1 })
end
end
end
......@@ -58,22 +58,5 @@ RSpec.describe Security::StoreReportsService do
end
end
end
context 'history caching' do
it 'swallows errors' do
allow( Gitlab::Vulnerabilities::HistoryCache).to receive(:new)
.and_raise("error")
expect { subject }.not_to raise_error
end
it 'caches vulnerability history' do
expect_next_instance_of(Gitlab::Vulnerabilities::HistoryCache) do |instance|
expect(instance).to receive(:fetch)
end
subject
end
end
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