Merge branch '217811-add-vulnerability-historical-statistcs-deletion-worker' into 'master'

Delete historical vulnerability statistics entries older than 90 days

See merge request gitlab-org/gitlab!37436

config/initializers/1_settings.rb

@@ -589,6 +589,9 @@ Gitlab.ee do
   Settings.cron_jobs['vulnerability_statistics_schedule_worker'] ||= Settingslogic.new({})
   Settings.cron_jobs['vulnerability_statistics_schedule_worker']['cron'] ||= '15 1 * * *'
   Settings.cron_jobs['vulnerability_statistics_schedule_worker']['job_class'] = 'Vulnerabilities::Statistics::ScheduleWorker'
+  Settings.cron_jobs['vulnerability_historical_statistics_deletion_worker'] ||= Settingslogic.new({})
+  Settings.cron_jobs['vulnerability_historical_statistics_deletion_worker']['cron'] ||= '15 3 * * *'
+  Settings.cron_jobs['vulnerability_historical_statistics_deletion_worker']['job_class'] = 'Vulnerabilities::HistoricalStatistics::DeletionWorker'
 end
 
 #

db/migrate/20200724100123_add_index_on_vulnerability_historical_statistics_date.rb

+# frozen_string_literal: true
+
+class AddIndexOnVulnerabilityHistoricalStatisticsDate < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :vulnerability_historical_statistics, [:date, :id]
+  end
+
+  def down
+    remove_concurrent_index :vulnerability_historical_statistics, [:date, :id]
+  end
+end

db/schema_migrations/20200724100123

+f55dccae8909110396882bd2c28be993eb32f33e880ed4a520d14071f70c9019
\ No newline at end of file

db/structure.sql

@@ -20802,6 +20802,8 @@ CREATE INDEX index_vulnerability_feedback_on_merge_request_id ON public.vulnerab
 
 CREATE INDEX index_vulnerability_feedback_on_pipeline_id ON public.vulnerability_feedback USING btree (pipeline_id);
 
+CREATE INDEX index_vulnerability_historical_statistics_on_date_and_id ON public.vulnerability_historical_statistics USING btree (date, id);
+
 CREATE UNIQUE INDEX index_vulnerability_identifiers_on_project_id_and_fingerprint ON public.vulnerability_identifiers USING btree (project_id, fingerprint);
 
 CREATE INDEX index_vulnerability_issue_links_on_issue_id ON public.vulnerability_issue_links USING btree (issue_id);

ee/app/models/vulnerabilities/historical_statistic.rb

@@ -2,6 +2,8 @@
 
 module Vulnerabilities
   class HistoricalStatistic < ApplicationRecord
+    include EachBatch
+
     self.table_name = 'vulnerability_historical_statistics'
 
     belongs_to :project, optional: false
@@ -17,5 +19,7 @@ module Vulnerabilities
     validates :info, numericality: { greater_than_or_equal_to: 0 }
 
     enum letter_grade: Vulnerabilities::Statistic.letter_grades
+
+    scope :older_than, ->(days:) { where('"vulnerability_historical_statistics"."date" < now() - interval ?', "#{days} days") }
   end
 end

ee/app/services/vulnerabilities/historical_statistics/deletion_service.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  module HistoricalStatistics
+    class DeletionService
+      def self.execute
+        new.execute
+      end
+
+      def execute
+        ::Vulnerabilities::HistoricalStatistic
+          .older_than(days: 100)
+          .each_batch { |relation| relation.delete_all }
+      end
+    end
+  end
+end

ee/app/workers/all_queues.yml

@@ -251,6 +251,14 @@
   :weight: 1
   :idempotent: 
   :tags: []
+- :name: cronjob:vulnerabilities_historical_statistics_deletion
+  :feature_category: :vulnerability_management
+  :has_external_dependencies: 
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: 
+  :tags: []
 - :name: cronjob:vulnerabilities_statistics_schedule
   :feature_category: :vulnerability_management
   :has_external_dependencies: 

ee/app/workers/vulnerabilities/historical_statistics/deletion_worker.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  module HistoricalStatistics
+    class DeletionWorker # rubocop:disable Scalability/IdempotentWorker
+      include ApplicationWorker
+      # rubocop:disable Scalability/CronWorkerContext
+      # This worker does not perform work scoped to a context
+      include CronjobQueue
+      # rubocop:enable Scalability/CronWorkerContext
+
+      feature_category :vulnerability_management
+
+      def perform
+        DeletionService.execute
+      end
+    end
+  end
+end

ee/changelogs/unreleased/217811-add-vulnerability-historical-statistcs-deletion-worker.yml

+---
+title: Delete historical vulnerability statistics entries older than 90 days
+merge_request: 37436
+author:
+type: added

ee/spec/models/vulnerabilities/historical_statistic_spec.rb

@@ -19,4 +19,14 @@ RSpec.describe Vulnerabilities::HistoricalStatistic do
     it { is_expected.to validate_numericality_of(:info).is_greater_than_or_equal_to(0) }
     it { is_expected.to define_enum_for(:letter_grade).with_values(%i(a b c d f)) }
   end
+
+  describe '.older_than' do
+    let_it_be(:statistic_1) { create(:vulnerability_historical_statistic, date: 99.days.ago) }
+    let_it_be(:statistic_2) { create(:vulnerability_historical_statistic, date: 100.days.ago) }
+    let_it_be(:statistic_3) { create(:vulnerability_historical_statistic, date: 101.days.ago) }
+
+    subject(:older_than) { described_class.older_than(days: 100) }
+
+    it { is_expected.to match_array([statistic_2, statistic_3]) }
+  end
 end

ee/spec/services/vulnerabilities/historical_statistics/deletion_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Vulnerabilities::HistoricalStatistics::DeletionService do
+  describe '.execute' do
+    let(:mock_service_object) { instance_double(described_class, execute: true) }
+
+    before do
+      allow(described_class).to receive(:new).and_return(mock_service_object)
+    end
+
+    it 'instantiates the service object and calls `execute`' do
+      described_class.execute
+
+      expect(mock_service_object).to have_received(:execute)
+    end
+  end
+
+  describe '#execute' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:other_project) { create(:project) }
+
+    subject(:delete_historical_statistics) { described_class.new.execute }
+
+    before do
+      create(:vulnerability_historical_statistic, project: project, date: 10.days.ago)
+      create(:vulnerability_historical_statistic, project: project, date: 20.days.ago)
+      create(:vulnerability_historical_statistic, project: other_project, date: 15.days.ago)
+      create(:vulnerability_historical_statistic, project: other_project, date: 25.days.ago)
+    end
+
+    context 'when there is no historical statistics older than 100 days' do
+      it 'does not delete historical statistics' do
+        expect { delete_historical_statistics }.not_to change { Vulnerabilities::HistoricalStatistic.count }
+      end
+    end
+
+    context 'when there is a historical statistic entry that was created 99 days ago' do
+      before do
+        create(:vulnerability_historical_statistic, project: project, date: 99.days.ago)
+        create(:vulnerability_historical_statistic, project: other_project, date: 99.days.ago)
+      end
+
+      it 'does not delete historical statistics' do
+        expect { delete_historical_statistics }.not_to change { Vulnerabilities::HistoricalStatistic.count }
+      end
+
+      context 'and there are more than one entries that are older than 100 days' do
+        before do
+          create(:vulnerability_historical_statistic, project: project, date: 101.days.ago)
+          create(:vulnerability_historical_statistic, project: project, date: 102.days.ago)
+          create(:vulnerability_historical_statistic, project: project, date: 103.days.ago)
+          create(:vulnerability_historical_statistic, project: other_project, date: 101.days.ago)
+          create(:vulnerability_historical_statistic, project: other_project, date: 102.days.ago)
+          create(:vulnerability_historical_statistic, project: other_project, date: 103.days.ago)
+        end
+
+        it 'deletes historical statistics older than 90 days', :aggregate_failures do
+          expect { delete_historical_statistics }.to change { Vulnerabilities::HistoricalStatistic.count }.by(-6)
+          expect(Vulnerabilities::HistoricalStatistic.pluck(:date)).to all(be >= 100.days.ago.to_date)
+        end
+      end
+    end
+  end
+end

ee/spec/workers/vulnerabilities/historical_statistics/deletion_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Vulnerabilities::HistoricalStatistics::DeletionWorker do
+  let(:worker) { described_class.new }
+
+  describe "#perform" do
+    before do
+      allow(Vulnerabilities::HistoricalStatistics::DeletionService).to receive(:execute)
+    end
+
+    it 'calls `Vulnerabilities::HistoricalStatistics::DeletionService`' do
+      worker.perform
+
+      expect(Vulnerabilities::HistoricalStatistics::DeletionService).to have_received(:execute).with(no_args)
+    end
+  end
+end