Merge branch '33257-prevent-accidental-deletions-via-soft-delete-for-groups-workers' into 'master'

Resolve "Prevent accidental deletions via soft delete for groups" - workers (MR: 2/n)

See merge request gitlab-org/gitlab!19679

changelogs/unreleased/33257-prevent-accidental-deletions-via-soft-delete-for-groups-workers.yml

+---
+title: Add workers for 'soft-delete for groups' feature
+merge_request: 19679
+author:
+type: added

config/initializers/1_settings.rb

@@ -469,6 +469,9 @@ Settings.cron_jobs['namespaces_prune_aggregation_schedules_worker']['cron'] ||=
 Settings.cron_jobs['namespaces_prune_aggregation_schedules_worker']['job_class'] = 'Namespaces::PruneAggregationSchedulesWorker'
 
 Gitlab.ee do
+  Settings.cron_jobs['adjourned_group_deletion_worker'] ||= Settingslogic.new({})
+  Settings.cron_jobs['adjourned_group_deletion_worker']['cron'] ||= '0 3 * * *'
+  Settings.cron_jobs['adjourned_group_deletion_worker']['job_class'] = 'AdjournedGroupDeletionWorker'
   Settings.cron_jobs['clear_shared_runners_minutes_worker'] ||= Settingslogic.new({})
   Settings.cron_jobs['clear_shared_runners_minutes_worker']['cron'] ||= '0 0 1 * *'
   Settings.cron_jobs['clear_shared_runners_minutes_worker']['job_class'] = 'ClearSharedRunnersMinutesWorker'

db/migrate/20191203121729_update_group_deletion_schedules_foreign_keys.rb

+# frozen_string_literal: true
+
+class UpdateGroupDeletionSchedulesForeignKeys < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key(:group_deletion_schedules, :users, column: :user_id, on_delete: :cascade, name: new_foreign_key_name)
+    remove_foreign_key_if_exists(:group_deletion_schedules, column: :user_id, on_delete: :nullify)
+  end
+
+  def down
+    add_concurrent_foreign_key(:group_deletion_schedules, :users, column: :user_id, on_delete: :nullify, name: existing_foreign_key_name)
+    remove_foreign_key_if_exists(:group_deletion_schedules, column: :user_id, on_delete: :cascade)
+  end
+
+  private
+
+  def new_foreign_key_name
+    concurrent_foreign_key_name(:group_deletion_schedules, :user_id)
+  end
+
+  def existing_foreign_key_name
+    'fk_group_deletion_schedules_users_user_id'
+  end
+end

db/schema.rb

@@ -4490,7 +4490,7 @@ ActiveRecord::Schema.define(version: 2019_12_06_122926) do
   add_foreign_key "grafana_integrations", "projects", on_delete: :cascade
   add_foreign_key "group_custom_attributes", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "group_deletion_schedules", "namespaces", column: "group_id", on_delete: :cascade
-  add_foreign_key "group_deletion_schedules", "users", on_delete: :nullify
+  add_foreign_key "group_deletion_schedules", "users", name: "fk_11e3ebfcdd", on_delete: :cascade
   add_foreign_key "group_group_links", "namespaces", column: "shared_group_id", on_delete: :cascade
   add_foreign_key "group_group_links", "namespaces", column: "shared_with_group_id", on_delete: :cascade
   add_foreign_key "identities", "saml_providers", name: "fk_aade90f0fc", on_delete: :cascade

ee/app/models/ee/group.rb

@@ -58,6 +58,7 @@ module EE
       validate :custom_project_templates_group_allowed, if: :custom_project_templates_group_id_changed?
 
       scope :aimed_for_deletion, -> (date) { joins(:deletion_schedule).where('group_deletion_schedules.marked_for_deletion_on <= ?', date) }
+      scope :with_deletion_schedule, -> { preload(:deletion_schedule) }
 
       scope :where_group_links_with_provider, ->(provider) do
         joins(:ldap_group_links).where(ldap_group_links: { provider: provider })

ee/app/workers/adjourned_group_deletion_worker.rb

+# frozen_string_literal: true
+
+class AdjournedGroupDeletionWorker
+  include ApplicationWorker
+  include CronjobQueue
+
+  INTERVAL = 5.minutes.to_i
+
+  feature_category :authentication_and_authorization
+
+  def perform
+    deletion_cutoff = Gitlab::CurrentSettings.deletion_adjourned_period.days.ago.to_date
+
+    Group.aimed_for_deletion(deletion_cutoff)
+      .with_deletion_schedule
+      .find_each(batch_size: 100) # rubocop: disable CodeReuse/ActiveRecord
+      .with_index do |group, index|
+
+      deletion_schedule = group.deletion_schedule
+      delay = index * INTERVAL
+
+      GroupDestroyWorker.perform_in(delay, group.id, deletion_schedule.user_id)
+    end
+  end
+end

ee/app/workers/all_queues.yml

 ---
+- cronjob:adjourned_group_deletion
 - cronjob:clear_shared_runners_minutes
 - cronjob:geo_file_download_dispatch
 - cronjob:geo_metrics_update

ee/spec/workers/adjourned_group_deletion_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AdjournedGroupDeletionWorker do
+  describe "#perform" do
+    subject(:worker) { described_class.new }
+
+    let_it_be(:user) { create(:user) }
+    let_it_be(:group_not_marked_for_deletion) { create(:group) }
+
+    let_it_be(:group_marked_for_deletion) do
+      create(:group_with_deletion_schedule,
+             marked_for_deletion_on: 14.days.ago,
+             deleting_user: user)
+    end
+
+    let_it_be(:group_marked_for_deletion_for_later) do
+      create(:group_with_deletion_schedule,
+             marked_for_deletion_on: 2.days.ago,
+             deleting_user: user)
+    end
+
+    before do
+      stub_application_setting(deletion_adjourned_period: 14)
+    end
+
+    it 'only schedules to delete groups marked for deletion on or before the specified `deletion_adjourned_period`' do
+      expect(GroupDestroyWorker).to receive(:perform_in).with(0, group_marked_for_deletion.id, user.id)
+
+      worker.perform
+    end
+
+    it 'does not schedule to delete a group not marked for deletion' do
+      expect(GroupDestroyWorker).not_to receive(:perform_in).with(0, group_not_marked_for_deletion.id, user.id)
+
+      worker.perform
+    end
+
+    it 'does not schedule to delete a group that is marked for deletion after the specified `deletion_adjourned_period`' do
+      expect(GroupDestroyWorker).not_to receive(:perform_in).with(0, group_marked_for_deletion_for_later.id, user.id)
+
+      worker.perform
+    end
+  end
+end