Commit c1387ac8 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch '33899-remove-history-cache-ff' into 'master'

Remove cache_vulnerability_history feature flag

See merge request gitlab-org/gitlab!21349
parents 711c4bec d18ab4b8
...@@ -27,9 +27,9 @@ module Security ...@@ -27,9 +27,9 @@ module Security
# Silently swallow errors if there are any problems caching vulnerabilities # Silently swallow errors if there are any problems caching vulnerabilities
def cache_vulnerabilities def cache_vulnerabilities
project = @pipeline.project project = @pipeline.project
if Feature.enabled?(:cache_vulnerability_history, project.group)
Gitlab::Vulnerabilities::HistoryCache.new(project.group, project.id).fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE, force: true) Gitlab::Vulnerabilities::HistoryCache.new(project.group, project.id)
end .fetch(Gitlab::Vulnerabilities::History::HISTORY_RANGE, force: true)
rescue => err rescue => err
error("Failed to cache vulnerabilities for pipeline #{@pipeline.id}: #{err}") error("Failed to cache vulnerabilities for pipeline #{@pipeline.id}: #{err}")
end end
......
---
title: Cache vulnerability findings history endpoint for security dashboards
merge_request: 21349
author:
type: added
...@@ -15,7 +15,7 @@ module Gitlab ...@@ -15,7 +15,7 @@ module Gitlab
end end
def findings_counter def findings_counter
return cached_vulnerability_history if use_vulnerability_cache? return cached_vulnerability_history unless dynamic_filters_included?
findings = vulnerability_findings.count_by_day_and_severity(HISTORY_RANGE) findings = vulnerability_findings.count_by_day_and_severity(HISTORY_RANGE)
::Vulnerabilities::HistorySerializer.new.represent(findings) ::Vulnerabilities::HistorySerializer.new.represent(findings)
...@@ -48,10 +48,6 @@ module Gitlab ...@@ -48,10 +48,6 @@ module Gitlab
history history
end end
def use_vulnerability_cache?
Feature.enabled?(:cache_vulnerability_history, group) && !dynamic_filters_included?
end
def dynamic_filters_included? def dynamic_filters_included?
dynamic_filters = [:report_type, :confidence, :severity] dynamic_filters = [:report_type, :confidence, :severity]
filters.keys.any? { |key| dynamic_filters.include?(key.to_sym) } filters.keys.any? { |key| dynamic_filters.include?(key.to_sym) }
......
...@@ -16,67 +16,42 @@ describe Gitlab::Vulnerabilities::History do ...@@ -16,67 +16,42 @@ describe Gitlab::Vulnerabilities::History do
describe '#findings_counter', :use_clean_rails_memory_store_caching do describe '#findings_counter', :use_clean_rails_memory_store_caching do
subject(:counter) { described_class.new(group, filters).findings_counter } subject(:counter) { described_class.new(group, filters).findings_counter }
context 'feature disabled' do context 'filters are passed' do
before do let(:filters) { { report_type: :sast } }
stub_feature_flags(cache_vulnerability_history: false)
end
it 'does not call Gitlab::Vulnerabilities::HistoryCache' do it 'does not call Gitlab::Vulnerabilities::HistoryCache' do
expect(Gitlab::Vulnerabilities::HistoryCache).not_to receive(:new) expect(Gitlab::Vulnerabilities::HistoryCache).not_to receive(:new)
counter counter
end end
it 'returns the proper format for the history' do
Timecop.freeze do
expect(counter[:total]).to eq({ Date.today => 3 })
expect(counter[:high]).to eq({ Date.today => 2 })
end
end
end end
context 'feature enabled' do it 'calls Gitlab::Vulnerabilities::HistoryCache' do
before do expect(Gitlab::Vulnerabilities::HistoryCache).to receive(:new).twice.and_call_original
stub_feature_flags(cache_vulnerability_history: true)
end
context 'filters are passed' do
let(:filters) { { 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 counter
expect(Gitlab::Vulnerabilities::HistoryCache).to receive(:new).twice.and_call_original end
counter it 'returns the proper format for the history' do
Timecop.freeze do
expect(counter[:total]).to eq({ Date.today => 3 })
expect(counter[:high]).to eq({ Date.today => 2 })
end end
end
it 'returns the proper format for the history' do context 'multiple projects with vulnerabilities' do
Timecop.freeze do before do
expect(counter[:total]).to eq({ Date.today => 3 }) Timecop.freeze(Date.today - 1) do
expect(counter[:high]).to eq({ Date.today => 2 }) create_vulnerabilities(1, project1, { severity: :high })
end end
end Timecop.freeze(Date.today - 4) do
create_vulnerabilities(1, project2, { severity: :high })
context 'multiple projects with vulnerabilities' do
before do
Timecop.freeze(Date.today - 1) do
create_vulnerabilities(1, project1, { severity: :high })
end
Timecop.freeze(Date.today - 4) do
create_vulnerabilities(1, project2, { severity: :high })
end
end end
end
it 'sorts by date for each key' do it 'sorts by date for each key' do
Timecop.freeze do Timecop.freeze do
expect(counter[:high].keys).to eq([(Date.today - 4), (Date.today - 1), Date.today]) expect(counter[:high].keys).to eq([(Date.today - 4), (Date.today - 1), Date.today])
end
end end
end end
end end
......
...@@ -62,28 +62,10 @@ describe Security::StoreReportsService do ...@@ -62,28 +62,10 @@ describe Security::StoreReportsService do
expect { subject }.not_to raise_error expect { subject }.not_to raise_error
end end
context 'feature disabled' do it 'caches vulnerability history' do
before do expect_any_instance_of(Gitlab::Vulnerabilities::HistoryCache).to receive(:fetch)
stub_feature_flags(cache_vulnerability_history: false)
end
it 'does not cache vulnerability history' do
expect_any_instance_of(Gitlab::Vulnerabilities::HistoryCache).not_to receive(:fetch)
subject
end
end
context 'feature enabled' do subject
before do
stub_feature_flags(cache_vulnerability_history: true)
end
it 'caches vulnerability history' do
expect_any_instance_of(Gitlab::Vulnerabilities::HistoryCache).to receive(:fetch)
subject
end
end end
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