Merge branch 'if-token_expiry_enforcement_revert_16-3' into '16-3-stable-ee'

Do not enqueue PAT expiry enforcement migration

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/153457



Merged-by: John Skarbek <jskarbek@gitlab.com>
Approved-by: Alper Akgun <aakgun@gitlab.com>
Approved-by: Smriti Garg <sgarg@gitlab.com>
Approved-by: Sofia Vistas <svistas@gitlab.com>
Co-authored-by: Imre Farkas <ifarkas@gitlab.com>
Co-authored-by: agius <andrew@atevans.com>

db/post_migrate/20230522073230_add_not_null_constraint_to_personal_access_tokens_expires_at.rb

@@ -3,10 +3,13 @@
 class AddNotNullConstraintToPersonalAccessTokensExpiresAt < Gitlab::Database::Migration[2.1]
   disable_ddl_transaction!
-  def up
-    add_not_null_constraint :personal_access_tokens, :expires_at, validate: false
-  end
+  def up; end
+  # required because specs like
+  # /spec/lib/gitlab/background_migration/backfill_admin_mode_scope_for_personal_access_tokens_spec.rb
+  # run against old schemas, thus a DOWN migration counterpart to
+  # /db/migrate/20231027084327_change_personal_access_tokens_remove_not_null_expires_at.rb#L15
+  # is required to achieve the correct db schema for these specs
   def down
     remove_not_null_constraint :personal_access_tokens, :expires_at
   end

db/post_migrate/20230608071301_requeue_cleanup_personal_access_tokens_with_nil_expires_at.rb

 # frozen_string_literal: true
 class RequeueCleanupPersonalAccessTokensWithNilExpiresAt < Gitlab::Database::Migration[2.1]
-  MIGRATION = "CleanupPersonalAccessTokensWithNilExpiresAt"
-  DELAY_INTERVAL = 2.minutes
-  BATCH_SIZE = 50_000
-  restrict_gitlab_migration gitlab_schema: :gitlab_main
-
-  def up
-    delete_batched_background_migration(MIGRATION, :personal_access_tokens, :id, [])
-
-    queue_batched_background_migration(
-      MIGRATION,
-      :personal_access_tokens,
-      :id,
-      job_interval: DELAY_INTERVAL,
-      batch_size: BATCH_SIZE
-    )
-  end
-
-  def down
-    delete_batched_background_migration(MIGRATION, :personal_access_tokens, :id, [])
-  end
+  def up; end
+  def down; end
 end

db/structure.sql

@@ -27287,9 +27287,6 @@ ALTER TABLE ONLY chat_teams
 ALTER TABLE vulnerability_scanners
     ADD CONSTRAINT check_37608c9db5 CHECK ((char_length(vendor) <= 255)) NOT VALID;
-ALTER TABLE personal_access_tokens
-    ADD CONSTRAINT check_b8d60815eb CHECK ((expires_at IS NOT NULL)) NOT VALID;
-
 ALTER TABLE sprints
     ADD CONSTRAINT check_ccd8a1eae0 CHECK ((start_date IS NOT NULL)) NOT VALID;

lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb

@@ -7,16 +7,7 @@ module BackgroundMigration
     class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob
       feature_category :system_access
-      EXPIRES_AT_DEFAULT = 365.days.from_now
-
-      scope_to ->(relation) { relation.where(expires_at: nil) }
-      operation_name :update_all
-
-      def perform
-        each_sub_batch do |sub_batch|
-          sub_batch.update_all(expires_at: EXPIRES_AT_DEFAULT)
-        end
-      end
+      def perform; end
     end
   end
 end

spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithNilExpiresAt, schema: 20230510062503, feature_category: :system_access do # rubocop:disable Layout/LineLength
-  let(:personal_access_tokens_table) { table(:personal_access_tokens) }
-  let(:users_table) { table(:users) }
-  let(:expires_at_default) { described_class::EXPIRES_AT_DEFAULT }
-
-  subject(:perform_migration) do
-    described_class.new(
-      start_id: 1,
-      end_id: 30,
-      batch_table: :personal_access_tokens,
-      batch_column: :id,
-      sub_batch_size: 3,
-      pause_ms: 0,
-      connection: ActiveRecord::Base.connection
-    ).perform
-  end
-
-  before do
-    user = users_table.create!(name: 'PAT_USER', email: 'pat_user@gmail.com', username: "pat_user1", projects_limit: 0)
-    personal_access_tokens_table.create!(user_id: user.id, name: "PAT#1", expires_at: expires_at_default + 1.day)
-    personal_access_tokens_table.create!(user_id: user.id, name: "PAT#2", expires_at: nil)
-    personal_access_tokens_table.create!(user_id: user.id, name: "PAT#3", expires_at: Time.zone.now + 2.days)
-  end
-
-  it 'adds expiry to personal access tokens', :aggregate_failures do
-    freeze_time do
-      expect(ActiveRecord::QueryRecorder.new { perform_migration }.count).to eq(3)
-
-      expect(personal_access_tokens_table.find_by_name("PAT#1").expires_at).to eq(expires_at_default.to_date + 1.day)
-      expect(personal_access_tokens_table.find_by_name("PAT#2").expires_at).to eq(expires_at_default.to_date)
-      expect(personal_access_tokens_table.find_by_name("PAT#3").expires_at).to eq(Time.zone.now.to_date + 2.days)
-    end
-  end
-end

spec/migrations/20230608071301_requeue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-require_migration!
-
-RSpec.describe RequeueCleanupPersonalAccessTokensWithNilExpiresAt, feature_category: :system_access do
-  let!(:batched_migration) { described_class::MIGRATION }
-
-  it 'schedules a new batched migration' do
-    reversible_migration do |migration|
-      migration.before -> {
-        expect(batched_migration).not_to have_scheduled_batched_migration
-      }
-
-      migration.after -> {
-        expect(batched_migration).to have_scheduled_batched_migration(
-          table_name: :personal_access_tokens,
-          column_name: :id,
-          interval: described_class::DELAY_INTERVAL,
-          batch_size: described_class::BATCH_SIZE
-        )
-      }
-    end
-  end
-end