Revert temporary change for sending expiration email for all ssh keys

We started sending SSH key expiration notification emails for all
expired keys in the
MR https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62114.
This change is no longer needed and, therefore, changing back to
sending notifications only for the keys that are expiring today.
Also removing the index that is not required anymore and
running the 'ssh_keys_expired_notification_worker' cron twice
daily as this is an idempotent job.

Changelog: changed

app/models/key.rb

@@ -46,7 +46,7 @@ class Key < ApplicationRecord
   scope :order_last_used_at_desc, -> { reorder(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) }
 
   # Date is set specifically in this scope to improve query time.
-  scope :expired_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') BETWEEN '2000-01-01' AND CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) }
+  scope :expired_today_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') = CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) }
   scope :expiring_soon_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') > CURRENT_DATE AND date(expires_at AT TIME ZONE 'UTC') < ? AND before_expiry_notification_delivered_at IS NULL", DAYS_TO_EXPIRE.days.from_now.to_date]) }
 
   def self.regular_keys

app/models/user.rb

@@ -123,7 +123,7 @@ class User < ApplicationRecord
 
   # Profile
   has_many :keys, -> { regular_keys }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-  has_many :expired_and_unnotified_keys, -> { expired_and_not_notified }, class_name: 'Key'
+  has_many :expired_today_and_unnotified_keys, -> { expired_today_and_not_notified }, class_name: 'Key'
   has_many :expiring_soon_and_unnotified_keys, -> { expiring_soon_and_not_notified }, class_name: 'Key'
   has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent
   has_many :group_deploy_keys

app/workers/ssh_keys/expired_notification_worker.rb

@@ -29,7 +29,7 @@ module SshKeys
         )
       ])
 
-      scope = Key.expired_and_not_notified.order(order)
+      scope = Key.expired_today_and_not_notified.order(order)
 
       iterator = Gitlab::Pagination::Keyset::Iterator.new(scope: scope, use_union_optimization: true)
       iterator.each_batch(of: BATCH_SIZE) do |relation|
@@ -37,7 +37,7 @@ module SshKeys
 
         users.each do |user|
           with_context(user: user) do
-            Keys::ExpiryNotificationService.new(user, { keys: user.expired_and_unnotified_keys, expiring_soon: false }).execute
+            Keys::ExpiryNotificationService.new(user, { keys: user.expired_today_and_unnotified_keys, expiring_soon: false }).execute
           end
         end
       end

config/initializers/1_settings.rb

@@ -571,7 +571,7 @@ Settings.cron_jobs['user_status_cleanup_batch_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['user_status_cleanup_batch_worker']['cron'] ||= '* * * * *'
 Settings.cron_jobs['user_status_cleanup_batch_worker']['job_class'] = 'UserStatusCleanup::BatchWorker'
 Settings.cron_jobs['ssh_keys_expired_notification_worker'] ||= Settingslogic.new({})
-Settings.cron_jobs['ssh_keys_expired_notification_worker']['cron'] ||= '0 2 * * *'
+Settings.cron_jobs['ssh_keys_expired_notification_worker']['cron'] ||= '0 2,14 * * *'
 Settings.cron_jobs['ssh_keys_expired_notification_worker']['job_class'] = 'SshKeys::ExpiredNotificationWorker'
 Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['namespaces_in_product_marketing_emails_worker']['cron'] ||= '0 16 * * *'

db/post_migrate/20211027112901_drop_index_keys_on_expires_at_and_before_expiry_notification_undelivered.rb

+# frozen_string_literal: true
+
+class DropIndexKeysOnExpiresAtAndBeforeExpiryNotificationUndelivered < Gitlab::Database::Migration[1.0]
+  DOWNTIME = false
+  INDEX_NAME = 'index_keys_on_expires_at_and_expiry_notification_undelivered'
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index_by_name(:keys, INDEX_NAME)
+  end
+
+  def down
+    add_concurrent_index :keys,
+                         "date(timezone('UTC', expires_at)), expiry_notification_delivered_at",
+                         where: 'expiry_notification_delivered_at IS NULL', name: INDEX_NAME
+  end
+end

db/schema_migrations/20211027112901

+83c1d699e5de98007ef815b5f9dfbc9a644c6289bf86832af1d5b8a6d3d83659
\ No newline at end of file

db/structure.sql

@@ -25674,8 +25674,6 @@ CREATE INDEX index_jira_imports_on_user_id ON jira_imports USING btree (user_id)
 
 CREATE INDEX index_jira_tracker_data_on_service_id ON jira_tracker_data USING btree (service_id);
 
-CREATE INDEX index_keys_on_expires_at_and_expiry_notification_undelivered ON keys USING btree (date(timezone('UTC'::text, expires_at)), expiry_notification_delivered_at) WHERE (expiry_notification_delivered_at IS NULL);
-
 CREATE INDEX index_keys_on_expires_at_and_id ON keys USING btree (date(timezone('UTC'::text, expires_at)), id) WHERE (expiry_notification_delivered_at IS NULL);
 
 CREATE UNIQUE INDEX index_keys_on_fingerprint ON keys USING btree (fingerprint);

spec/models/key_spec.rb

@@ -85,9 +85,9 @@ RSpec.describe Key, :mailer do
       let_it_be(:expiring_soon_notified) { create(:key, expires_at: 4.days.from_now, user: user, before_expiry_notification_delivered_at: Time.current) }
       let_it_be(:future_expiry) { create(:key, expires_at: 1.month.from_now, user: user) }
 
-      describe '.expired_and_not_notified' do
+      describe '.expired_today_and_not_notified' do
         it 'returns keys that expire today and in the past' do
-          expect(described_class.expired_and_not_notified).to contain_exactly(expired_today_not_notified, expired_yesterday)
+          expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified)
         end
       end
 

spec/models/user_spec.rb

@@ -98,7 +98,7 @@ RSpec.describe User do
     it { is_expected.to have_many(:group_members) }
     it { is_expected.to have_many(:groups) }
     it { is_expected.to have_many(:keys).dependent(:destroy) }
-    it { is_expected.to have_many(:expired_and_unnotified_keys) }
+    it { is_expected.to have_many(:expired_today_and_unnotified_keys) }
     it { is_expected.to have_many(:deploy_keys).dependent(:nullify) }
     it { is_expected.to have_many(:group_deploy_keys) }
     it { is_expected.to have_many(:events).dependent(:delete_all) }

spec/workers/ssh_keys/expired_notification_worker_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe SshKeys::ExpiredNotificationWorker, type: :worker do
         stub_const("SshKeys::ExpiredNotificationWorker::BATCH_SIZE", 5)
       end
 
-      let_it_be_with_reload(:keys) { create_list(:key, 20, expires_at: 3.days.ago, user: user) }
+      let_it_be_with_reload(:keys) { create_list(:key, 20, expires_at: Time.current, user: user) }
 
       it 'updates all keys regardless of batch size' do
         worker.perform
@@ -54,8 +54,8 @@ RSpec.describe SshKeys::ExpiredNotificationWorker, type: :worker do
     context 'when key has expired in the past' do
       let_it_be(:expired_past) { create(:key, expires_at: 1.day.ago, user: user) }
 
-      it 'does update notified column' do
-        expect { worker.perform }.to change { expired_past.reload.expiry_notification_delivered_at }
+      it 'does not update notified column' do
+        expect { worker.perform }.not_to change { expired_past.reload.expiry_notification_delivered_at }
       end
 
       context 'when key has already been notified of expiration' do