From 91bba2376d8e88246204d2992bcd1999372dac1d Mon Sep 17 00:00:00 2001 From: smriti Date: Wed, 10 May 2023 12:38:15 +0530 Subject: [PATCH 01/22] Update expires_at column value to 365 days from now when its nil As per issue we cannot support PATs which do not expire ever. With this migration change we are making sure no such tokens are there in system with nil expiry date Changelog: changed MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239/ --- ...onal_access_tokens_with_nil_expires_at.yml | 6 ++++ ...sonal_access_tokens_with_nil_expires_at.rb | 32 +++++++++++++++++ db/schema_migrations/20230510062502 | 1 + ...sonal_access_tokens_with_nil_expires_at.rb | 27 ++++++++++++++ ..._access_tokens_with_nil_expires_at_spec.rb | 36 +++++++++++++++++++ ..._access_tokens_with_nil_expires_at_spec.rb | 26 ++++++++++++++ 6 files changed, 128 insertions(+) create mode 100644 db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml create mode 100644 db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb create mode 100644 db/schema_migrations/20230510062502 create mode 100644 lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb create mode 100644 spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb create mode 100644 spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb diff --git a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml new file mode 100644 index 00000000000000..190e4df68cf5fe --- /dev/null +++ b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml @@ -0,0 +1,6 @@ +--- +migration_job_name: CleanupPersonalAccessTokensWithNilExpiresAt +description: Updates value of expires_at column to 365 when its nil for PersonalAccessTokens +feature_category: authentication_and_authorization +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239/ +milestone: 16.0 diff --git a/db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb b/db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb new file mode 100644 index 00000000000000..ba4228564a6cab --- /dev/null +++ b/db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/database/batched_background_migrations.html +# for more information on when/how to queue batched background migrations + +# Update below commented lines with appropriate values. + +class QueueCleanupPersonalAccessTokensWithNilExpiresAt < Gitlab::Database::Migration[2.1] + MIGRATION = "CleanupPersonalAccessTokensWithNilExpiresAt" + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 50_000 + SUB_BATCH_SIZE = 100 + + disable_ddl_transaction! + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + queue_batched_background_migration( + MIGRATION, + :personal_access_tokens, + :expires_at, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :personal_access_tokens, :expires_at, []) + end +end diff --git a/db/schema_migrations/20230510062502 b/db/schema_migrations/20230510062502 new file mode 100644 index 00000000000000..d224e66d064c7b --- /dev/null +++ b/db/schema_migrations/20230510062502 @@ -0,0 +1 @@ +91b3b2c88b467558162b71662b8acf2ecdeac644617b8a0de154a4eed6a3549a \ No newline at end of file diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb new file mode 100644 index 00000000000000..3cdcf065733f3f --- /dev/null +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/database/batched_background_migrations.html +# for more information on how to use batched background migrations + +# Update below commented lines with appropriate values. + +module Gitlab + module BackgroundMigration + # Clean up personal access tokens with nil expires_at and set the value to new default 365 days + class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob + # operation_name :my_operation + # scope_to ->(relation) { relation.where(column: "value") } + feature_category :system_access + + scope_to ->(relation) { relation.where(expires_at: nil) } + operation_name :update_all + feature_category :database + + def perform + each_sub_batch do |sub_batch| + sub_batch.update_all(expires_at: Time.now + 365.days) + end + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb new file mode 100644 index 00000000000000..349f3de1ee2b02 --- /dev/null +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithNilExpiresAt, schema: 20230510062502, feature_category: :system_access do # rubocop:disable Layout/LineLength + let(:migration) { described_class.new } + let(:personal_access_tokens_table) { table(:personal_access_tokens) } + let(:users) { table(:users) } + + subject(:perform_migration) do + described_class.new( + start_id: 1, + end_id: 30, + batch_table: :personal_access_tokens, + batch_column: :id, + sub_batch_size: 2, + pause_ms: 0, + connection: ActiveRecord::Base.connection + ).perform + end + + before do + user = users.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: nil) + 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 oauth tokens', :aggregate_failures 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((Time.zone.now + 365.days).to_date) + expect(personal_access_tokens_table.find_by_name("PAT#2").expires_at).to eq((Time.zone.now + 365.days).to_date) + expect(personal_access_tokens_table.find_by_name("PAT#3").expires_at).to eq((Time.zone.now + 2.days).to_date) + end +end diff --git a/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb new file mode 100644 index 00000000000000..1e00f695f4da80 --- /dev/null +++ b/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueueCleanupPersonalAccessTokensWithNilExpiresAt, 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: :expires_at, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + } + end + end +end -- GitLab From d74ab2173929fd03113308e3d5d047b6342fd8b8 Mon Sep 17 00:00:00 2001 From: smriti Date: Wed, 10 May 2023 18:14:09 +0530 Subject: [PATCH 02/22] Added new index to table for optimization Rubocop fixes Updated where class in logic class Reverted changes for expires_at greater than an year --- ...index_personal_access_tokens_expires_at.rb | 18 +++++++++++++++++ ...sonal_access_tokens_with_nil_expires_at.rb | 5 ----- db/schema_migrations/20230510062501 | 1 + ...sonal_access_tokens_with_nil_expires_at.rb | 7 +++---- ..._access_tokens_with_nil_expires_at_spec.rb | 14 ++++++------- ..._personal_access_tokens_expires_at_spec.rb | 20 +++++++++++++++++++ 6 files changed, 49 insertions(+), 16 deletions(-) create mode 100644 db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb create mode 100644 db/schema_migrations/20230510062501 create mode 100644 spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb diff --git a/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb b/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb new file mode 100644 index 00000000000000..b56f48b7508651 --- /dev/null +++ b/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +class AddTempIndexPersonalAccessTokensExpiresAt < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'tmp_index_personal_access_tokens_on_expires_at' + TABLE = :personal_access_tokens + CONDITIONS = "expires_at IS NULL" + + disable_ddl_transaction! + + def up + add_concurrent_index TABLE, 'expires_at', where: CONDITIONS, name: INDEX_NAME + + connection.execute("ANALYZE #{TABLE}") + end + + def down + remove_concurrent_index TABLE, :expires_at, name: INDEX_NAME + end +end diff --git a/db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb b/db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb index ba4228564a6cab..96994d67482c08 100644 --- a/db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -1,10 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/database/batched_background_migrations.html -# for more information on when/how to queue batched background migrations - -# Update below commented lines with appropriate values. - class QueueCleanupPersonalAccessTokensWithNilExpiresAt < Gitlab::Database::Migration[2.1] MIGRATION = "CleanupPersonalAccessTokensWithNilExpiresAt" DELAY_INTERVAL = 2.minutes diff --git a/db/schema_migrations/20230510062501 b/db/schema_migrations/20230510062501 new file mode 100644 index 00000000000000..305e44a8620bd5 --- /dev/null +++ b/db/schema_migrations/20230510062501 @@ -0,0 +1 @@ +8102b00724f4ef9779ef9757aeba4f4b9138d6e684a9ede26807c0ac799455c2 \ No newline at end of file diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb index 3cdcf065733f3f..03cb429326e3ad 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -7,10 +7,9 @@ module Gitlab module BackgroundMigration - # Clean up personal access tokens with nil expires_at and set the value to new default 365 days + # Clean up personal access tokens with expires_at value is or greater than 1 year + # and set the value to new default 365 days class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob - # operation_name :my_operation - # scope_to ->(relation) { relation.where(column: "value") } feature_category :system_access scope_to ->(relation) { relation.where(expires_at: nil) } @@ -19,7 +18,7 @@ class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob def perform each_sub_batch do |sub_batch| - sub_batch.update_all(expires_at: Time.now + 365.days) + sub_batch.update_all(expires_at: 1.year.from_now) end end end diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb index 349f3de1ee2b02..7bc031f8837dd1 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithNilExpiresAt, schema: 20230510062502, feature_category: :system_access do # rubocop:disable Layout/LineLength - let(:migration) { described_class.new } let(:personal_access_tokens_table) { table(:personal_access_tokens) } - let(:users) { table(:users) } + let(:users_table) { table(:users) } + let(:expires_at_default) { 1.year.from_now } subject(:perform_migration) do described_class.new( @@ -13,24 +13,24 @@ end_id: 30, batch_table: :personal_access_tokens, batch_column: :id, - sub_batch_size: 2, + sub_batch_size: 3, pause_ms: 0, connection: ActiveRecord::Base.connection ).perform end before do - user = users.create!(name: 'PAT_USER', email: 'pat_user@gmail.com', username: "pat_user1", projects_limit: 0) + 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: nil) 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 oauth tokens', :aggregate_failures do + it 'adds expiry to personal access tokens', :aggregate_failures 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((Time.zone.now + 365.days).to_date) - expect(personal_access_tokens_table.find_by_name("PAT#2").expires_at).to eq((Time.zone.now + 365.days).to_date) + expect(personal_access_tokens_table.find_by_name("PAT#1").expires_at).to eq(expires_at_default.to_date) + 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 + 2.days).to_date) end end diff --git a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb new file mode 100644 index 00000000000000..65fd3a9d90acf6 --- /dev/null +++ b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe AddTempIndexPersonalAccessTokensExpiresAt, feature_category: :projects do + it 'correctly migrates up and down' do + reversible_migration do |migration| + migration.before -> { + expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') + .map(&:name)).not_to include('tmp_index_personal_access_tokens_on_expires_at') + } + + migration.after -> { + expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') + .map(&:name)).to include('tmp_index_personal_access_tokens_on_expires_at') + } + end + end +end -- GitLab From a2f6c2f383e6a50eb3b6e26850cd7ce31b5f706c Mon Sep 17 00:00:00 2001 From: smriti Date: Wed, 10 May 2023 22:19:17 +0530 Subject: [PATCH 03/22] Added a new migration for updating token with expiry 1 year Rubocop fixes Removed all the comments Spec failure resolved Rubocop fixes --- ...cess_tokens_with_expiry_more_than_year.yml | 6 ++++ ...onal_access_tokens_with_nil_expires_at.yml | 2 +- ...index_personal_access_tokens_expires_at.rb | 1 + ...ccess_tokens_with_expiry_more_than_year.rb | 27 ++++++++++++++ db/schema_migrations/20230510162550 | 1 + ...ccess_tokens_with_expiry_more_than_year.rb | 24 +++++++++++++ ...sonal_access_tokens_with_nil_expires_at.rb | 8 ++--- ..._tokens_with_expiry_more_than_year_spec.rb | 36 +++++++++++++++++++ ..._access_tokens_with_nil_expires_at_spec.rb | 4 +-- ..._tokens_with_expiry_more_than_year_spec.rb | 26 ++++++++++++++ 10 files changed, 126 insertions(+), 9 deletions(-) create mode 100644 db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml create mode 100644 db/post_migrate/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year.rb create mode 100644 db/schema_migrations/20230510162550 create mode 100644 lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb create mode 100644 spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb create mode 100644 spec/migrations/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb diff --git a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml new file mode 100644 index 00000000000000..2bb266e73cbfd9 --- /dev/null +++ b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml @@ -0,0 +1,6 @@ +--- +migration_job_name: CleanupPersonalAccessTokensWithExpiryMoreThanYear +description: Updates value of expires_at column to 365 when its expiry is more than a year for PersonalAccessTokens +feature_category: system_access +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239/ +milestone: 16.0 diff --git a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml index 190e4df68cf5fe..d5bd907d9cc906 100644 --- a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml +++ b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml @@ -1,6 +1,6 @@ --- migration_job_name: CleanupPersonalAccessTokensWithNilExpiresAt description: Updates value of expires_at column to 365 when its nil for PersonalAccessTokens -feature_category: authentication_and_authorization +feature_category: system_access introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239/ milestone: 16.0 diff --git a/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb b/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb index b56f48b7508651..704f2c31b62fab 100644 --- a/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb +++ b/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + class AddTempIndexPersonalAccessTokensExpiresAt < Gitlab::Database::Migration[2.1] INDEX_NAME = 'tmp_index_personal_access_tokens_on_expires_at' TABLE = :personal_access_tokens diff --git a/db/post_migrate/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year.rb b/db/post_migrate/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year.rb new file mode 100644 index 00000000000000..eef401e458ccc2 --- /dev/null +++ b/db/post_migrate/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class QueueCleanupPersonalAccessTokensWithExpiryMoreThanYear < Gitlab::Database::Migration[2.1] + MIGRATION = "CleanupPersonalAccessTokensWithExpiryMoreThanYear" + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 50_000 + SUB_BATCH_SIZE = 100 + + disable_ddl_transaction! + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + queue_batched_background_migration( + MIGRATION, + :personal_access_tokens, + :expires_at, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :personal_access_tokens, :expires_at, []) + end +end diff --git a/db/schema_migrations/20230510162550 b/db/schema_migrations/20230510162550 new file mode 100644 index 00000000000000..d0f6de3550dc18 --- /dev/null +++ b/db/schema_migrations/20230510162550 @@ -0,0 +1 @@ +8fb7fa9e6e4b3994134a8ca6012d9aef11f1ef5c40a3b4463398dbf62ed9ad25 \ No newline at end of file diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb new file mode 100644 index 00000000000000..10775ca3ca22f4 --- /dev/null +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Clean up personal access tokens with expires_at value is or greater than 1 year + # and set the value to new default 365 days + # This query is expected to process some 500 rows + class CleanupPersonalAccessTokensWithExpiryMoreThanYear < BatchedMigrationJob + feature_category :system_access + + EXPIRES_AT_DEFAULT = 1.year.from_now + + scope_to ->(relation) { relation.where('expires_at > ?', EXPIRES_AT_DEFAULT) } + operation_name :update_all + feature_category :database + + def perform + each_sub_batch do |sub_batch| + sub_batch.update_all(expires_at: EXPIRES_AT_DEFAULT) + end + end + end + end +end diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb index 03cb429326e3ad..e96bb566faa2f1 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -1,14 +1,10 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/database/batched_background_migrations.html -# for more information on how to use batched background migrations - -# Update below commented lines with appropriate values. - module Gitlab module BackgroundMigration - # Clean up personal access tokens with expires_at value is or greater than 1 year + # Clean up personal access tokens with expires_at value is nil # and set the value to new default 365 days + # This query is expected to process some 3738072 rows class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob feature_category :system_access diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb new file mode 100644 index 00000000000000..9a0151c2c7a799 --- /dev/null +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithExpiryMoreThanYear, schema: 20230510162550, 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) { 1.year.from_now } + + 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: nil) + personal_access_tokens_table.create!(user_id: user.id, name: "PAT#2", expires_at: 1.year.from_now + 2.days) + 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 + expect(ActiveRecord::QueryRecorder.new { perform_migration }.count).to eq(3) + + expect(personal_access_tokens_table.find_by_name("PAT#1").expires_at).to eq(nil) + 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 + 2.days).to_date) + end +end diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb index 7bc031f8837dd1..4394a8baf2e768 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -21,7 +21,7 @@ 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: nil) + personal_access_tokens_table.create!(user_id: user.id, name: "PAT#1", expires_at: 1.year.from_now + 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 @@ -29,7 +29,7 @@ it 'adds expiry to personal access tokens', :aggregate_failures 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) + expect(personal_access_tokens_table.find_by_name("PAT#1").expires_at).to eq((1.year.from_now + 1.day).to_date) 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 + 2.days).to_date) end diff --git a/spec/migrations/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb b/spec/migrations/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb new file mode 100644 index 00000000000000..4085ff18c0f20b --- /dev/null +++ b/spec/migrations/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueueCleanupPersonalAccessTokensWithExpiryMoreThanYear, 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: :expires_at, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + } + end + end +end -- GitLab From e9a42fa3a57873b67e2e83a5225a68cd733fefea Mon Sep 17 00:00:00 2001 From: smriti Date: Thu, 11 May 2023 17:50:22 +0530 Subject: [PATCH 04/22] changed 1.year to 365 days --- ...nup_personal_access_tokens_with_expiry_more_than_year.rb | 2 +- .../cleanup_personal_access_tokens_with_nil_expires_at.rb | 2 +- ...ersonal_access_tokens_with_expiry_more_than_year_spec.rb | 4 ++-- ...eanup_personal_access_tokens_with_nil_expires_at_spec.rb | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb index 10775ca3ca22f4..8315d597452423 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb @@ -8,7 +8,7 @@ module BackgroundMigration class CleanupPersonalAccessTokensWithExpiryMoreThanYear < BatchedMigrationJob feature_category :system_access - EXPIRES_AT_DEFAULT = 1.year.from_now + EXPIRES_AT_DEFAULT = 365.days.from_now scope_to ->(relation) { relation.where('expires_at > ?', EXPIRES_AT_DEFAULT) } operation_name :update_all diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb index e96bb566faa2f1..4de33e35f716fb 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -14,7 +14,7 @@ class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob def perform each_sub_batch do |sub_batch| - sub_batch.update_all(expires_at: 1.year.from_now) + sub_batch.update_all(expires_at: 365.days.from_now) end end end diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb index 9a0151c2c7a799..75099c2a8419aa 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithExpiryMoreThanYear, schema: 20230510162550, 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) { 1.year.from_now } + let(:expires_at_default) { 365.days.from_now } subject(:perform_migration) do described_class.new( @@ -22,7 +22,7 @@ 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: nil) - personal_access_tokens_table.create!(user_id: user.id, name: "PAT#2", expires_at: 1.year.from_now + 2.days) + personal_access_tokens_table.create!(user_id: user.id, name: "PAT#2", expires_at: expires_at_default + 2.days) personal_access_tokens_table.create!(user_id: user.id, name: "PAT#3", expires_at: Time.zone.now + 2.days) end diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb index 4394a8baf2e768..b78204b814e206 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithNilExpiresAt, schema: 20230510062502, 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) { 1.year.from_now } + let(:expires_at_default) { 365.days.from_now } subject(:perform_migration) do described_class.new( @@ -21,7 +21,7 @@ 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: 1.year.from_now + 1.day) + 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 @@ -29,7 +29,7 @@ it 'adds expiry to personal access tokens', :aggregate_failures 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((1.year.from_now + 1.day).to_date) + expect(personal_access_tokens_table.find_by_name("PAT#1").expires_at).to eq((expires_at_default + 1.day).to_date) 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 + 2.days).to_date) end -- GitLab From 3b82b0cfed1af68eca54d694272f9355c447a18b Mon Sep 17 00:00:00 2001 From: Jessie Young Date: Thu, 11 May 2023 11:45:25 -0700 Subject: [PATCH 05/22] Freeze time in specs to prevent flakiness --- ...al_access_tokens_with_expiry_more_than_year_spec.rb | 10 ++++++---- ..._personal_access_tokens_with_nil_expires_at_spec.rb | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb index 75099c2a8419aa..cbc0743ec45b12 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb @@ -27,10 +27,12 @@ end it 'adds expiry to personal access tokens', :aggregate_failures do - expect(ActiveRecord::QueryRecorder.new { perform_migration }.count).to eq(3) + 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(nil) - 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 + 2.days).to_date) + expect(personal_access_tokens_table.find_by_name("PAT#1").expires_at).to eq(nil) + 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 diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb index b78204b814e206..550c26211c2645 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -27,10 +27,12 @@ end it 'adds expiry to personal access tokens', :aggregate_failures do - expect(ActiveRecord::QueryRecorder.new { perform_migration }.count).to eq(3) + 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 + 1.day).to_date) - 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 + 2.days).to_date) + 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 -- GitLab From 06cc5561fb2ba569d81a19f564c0d5c2a6cdda1f Mon Sep 17 00:00:00 2001 From: smriti Date: Fri, 12 May 2023 07:01:34 +0530 Subject: [PATCH 06/22] Changed the feature category --- .../cleanup_personal_access_tokens_with_expiry_more_than_year.rb | 1 - .../cleanup_personal_access_tokens_with_nil_expires_at.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb index 8315d597452423..48a96d9ebaada5 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb @@ -12,7 +12,6 @@ class CleanupPersonalAccessTokensWithExpiryMoreThanYear < BatchedMigrationJob scope_to ->(relation) { relation.where('expires_at > ?', EXPIRES_AT_DEFAULT) } operation_name :update_all - feature_category :database def perform each_sub_batch do |sub_batch| diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb index 4de33e35f716fb..db32862c0de038 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -10,7 +10,6 @@ class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob scope_to ->(relation) { relation.where(expires_at: nil) } operation_name :update_all - feature_category :database def perform each_sub_batch do |sub_batch| -- GitLab From 771d7090e60efefc6f622a5dc9f35804676a2750 Mon Sep 17 00:00:00 2001 From: smriti Date: Fri, 12 May 2023 07:44:51 +0530 Subject: [PATCH 07/22] Added new index for expires_at 365 days --- ..._tokens_expires_at_greater_than365_days.rb | 19 ++++++++++++++++++ ...ccess_tokens_with_expiry_more_than_year.rb | 2 +- ...ns_expires_at_greater_than365_days_spec.rb | 20 +++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 db/post_migrate/20230510062500_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb create mode 100644 spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb diff --git a/db/post_migrate/20230510062500_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb b/db/post_migrate/20230510062500_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb new file mode 100644 index 00000000000000..e61fa7a791300d --- /dev/null +++ b/db/post_migrate/20230510062500_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddTempIndexPersonalAccessTokensExpiresAtGreaterThan365Days < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'tmp_index_person_access_tokens_expires_at_2' + TABLE = :personal_access_tokens + CONDITIONS = "expires_at > '#{365.days.from_now.to_date}'::date" + + disable_ddl_transaction! + + def up + add_concurrent_index TABLE, 'expires_at', where: CONDITIONS, name: INDEX_NAME + + connection.execute("ANALYZE #{TABLE}") + end + + def down + remove_concurrent_index TABLE, :expires_at, name: INDEX_NAME + end +end diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb index 48a96d9ebaada5..2f5cb9468f3f8e 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb @@ -4,7 +4,7 @@ module Gitlab module BackgroundMigration # Clean up personal access tokens with expires_at value is or greater than 1 year # and set the value to new default 365 days - # This query is expected to process some 500 rows + # This query is expected to process some 268546 rows class CleanupPersonalAccessTokensWithExpiryMoreThanYear < BatchedMigrationJob feature_category :system_access diff --git a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb new file mode 100644 index 00000000000000..25868d1738a900 --- /dev/null +++ b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe AddTempIndexPersonalAccessTokensExpiresAtGreaterThan365Days, feature_category: :projects do + it 'correctly migrates up and down' do + reversible_migration do |migration| + migration.before -> { + expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') + .map(&:name)).not_to include('tmp_index_person_access_tokens_expires_at_2') + } + + migration.after -> { + expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') + .map(&:name)).to include('tmp_index_person_access_tokens_expires_at_2') + } + end + end +end -- GitLab From 94b50b5a1d187d178add0406d47a9f28f0d71374 Mon Sep 17 00:00:00 2001 From: smriti Date: Fri, 12 May 2023 08:13:36 +0530 Subject: [PATCH 08/22] Structure.sql with indexes --- db/structure.sql | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/db/structure.sql b/db/structure.sql index a57d1cbebdd251..30dcce52f11866 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -33105,6 +33105,10 @@ CREATE INDEX tmp_index_oauth_access_tokens_on_id_where_expires_in_null ON oauth_ CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING btree (id) WHERE (state <> 2); +CREATE INDEX tmp_index_person_access_tokens_expires_at_2 ON personal_access_tokens USING btree (expires_at) WHERE (expires_at > '2024-05-11'::date); + +CREATE INDEX tmp_index_personal_access_tokens_on_expires_at ON personal_access_tokens USING btree (expires_at) WHERE (expires_at IS NULL); + CREATE INDEX tmp_index_project_statistics_cont_registry_size ON project_statistics USING btree (project_id) WHERE (container_registry_size = 0); CREATE INDEX tmp_index_vulnerability_dismissal_info ON vulnerabilities USING btree (id) WHERE ((state = 2) AND ((dismissed_at IS NULL) OR (dismissed_by_id IS NULL))); -- GitLab From 28baa1109627bf6b5a07513e81fbf06e14150dff Mon Sep 17 00:00:00 2001 From: smriti Date: Fri, 12 May 2023 08:39:09 +0530 Subject: [PATCH 09/22] Comitting schema migration Re arranged schema versions --- ...personal_access_tokens_expires_at_greater_than365_days.rb} | 2 +- ...eue_cleanup_personal_access_tokens_with_nil_expires_at.rb} | 0 db/schema_migrations/20230510062503 | 1 + db/structure.sql | 2 +- ...onal_access_tokens_expires_at_greater_than365_days_spec.rb | 4 ++-- 5 files changed, 5 insertions(+), 4 deletions(-) rename db/post_migrate/{20230510062500_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb => 20230510062502_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb} (86%) rename db/post_migrate/{20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb => 20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb} (100%) create mode 100644 db/schema_migrations/20230510062503 diff --git a/db/post_migrate/20230510062500_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb b/db/post_migrate/20230510062502_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb similarity index 86% rename from db/post_migrate/20230510062500_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb rename to db/post_migrate/20230510062502_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb index e61fa7a791300d..27deb1ba602905 100644 --- a/db/post_migrate/20230510062500_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb +++ b/db/post_migrate/20230510062502_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class AddTempIndexPersonalAccessTokensExpiresAtGreaterThan365Days < Gitlab::Database::Migration[2.1] - INDEX_NAME = 'tmp_index_person_access_tokens_expires_at_2' + INDEX_NAME = 'tmp_index_person_access_tokens_expires_more_than_365_days' TABLE = :personal_access_tokens CONDITIONS = "expires_at > '#{365.days.from_now.to_date}'::date" diff --git a/db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb b/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb similarity index 100% rename from db/post_migrate/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb rename to db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb diff --git a/db/schema_migrations/20230510062503 b/db/schema_migrations/20230510062503 new file mode 100644 index 00000000000000..f6be2a733925ac --- /dev/null +++ b/db/schema_migrations/20230510062503 @@ -0,0 +1 @@ +2bd476bf0389b70aa5736ff69023993d37d54c4d333e3a91de9e57370935d6ec \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 30dcce52f11866..864c14d74b2b12 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -33105,7 +33105,7 @@ CREATE INDEX tmp_index_oauth_access_tokens_on_id_where_expires_in_null ON oauth_ CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING btree (id) WHERE (state <> 2); -CREATE INDEX tmp_index_person_access_tokens_expires_at_2 ON personal_access_tokens USING btree (expires_at) WHERE (expires_at > '2024-05-11'::date); +CREATE INDEX tmp_index_person_access_tokens_expires_more_than_365_days ON personal_access_tokens USING btree (expires_at) WHERE (expires_at > '2024-05-11'::date); CREATE INDEX tmp_index_personal_access_tokens_on_expires_at ON personal_access_tokens USING btree (expires_at) WHERE (expires_at IS NULL); diff --git a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb index 25868d1738a900..234a864379d627 100644 --- a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb +++ b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb @@ -8,12 +8,12 @@ reversible_migration do |migration| migration.before -> { expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') - .map(&:name)).not_to include('tmp_index_person_access_tokens_expires_at_2') + .map(&:name)).not_to include('tmp_index_person_access_tokens_expires_more_than_365_days') } migration.after -> { expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') - .map(&:name)).to include('tmp_index_person_access_tokens_expires_at_2') + .map(&:name)).to include('tmp_index_person_access_tokens_expires_more_than_365_days') } end end -- GitLab From a632d2d32c4c13b27ccba24a693714e58b380dff Mon Sep 17 00:00:00 2001 From: smriti Date: Fri, 12 May 2023 16:46:07 +0530 Subject: [PATCH 10/22] Updated index for expires_at --- ...index_personal_access_tokens_expires_at.rb | 10 +++++----- ..._tokens_expires_at_greater_than365_days.rb | 19 ------------------ db/schema_migrations/20230510062502 | 1 - db/structure.sql | 4 +--- ...ns_expires_at_greater_than365_days_spec.rb | 20 ------------------- ..._personal_access_tokens_expires_at_spec.rb | 4 ++-- 6 files changed, 8 insertions(+), 50 deletions(-) delete mode 100644 db/post_migrate/20230510062502_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb delete mode 100644 db/schema_migrations/20230510062502 delete mode 100644 spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb diff --git a/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb b/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb index 704f2c31b62fab..406f53f4548c02 100644 --- a/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb +++ b/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb @@ -1,16 +1,16 @@ # frozen_string_literal: true +# This is a temporary index to execute batch migration for personal access tokens cleanup +# as per issue - https://gitlab.com/gitlab-org/gitlab/-/issues/369123. +# It will be removed later class AddTempIndexPersonalAccessTokensExpiresAt < Gitlab::Database::Migration[2.1] - INDEX_NAME = 'tmp_index_personal_access_tokens_on_expires_at' + INDEX_NAME = 'tmp_index_personal_access_tokens_expires_at' TABLE = :personal_access_tokens - CONDITIONS = "expires_at IS NULL" disable_ddl_transaction! def up - add_concurrent_index TABLE, 'expires_at', where: CONDITIONS, name: INDEX_NAME - - connection.execute("ANALYZE #{TABLE}") + add_concurrent_index TABLE, 'expires_at', name: INDEX_NAME end def down diff --git a/db/post_migrate/20230510062502_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb b/db/post_migrate/20230510062502_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb deleted file mode 100644 index 27deb1ba602905..00000000000000 --- a/db/post_migrate/20230510062502_add_temp_index_personal_access_tokens_expires_at_greater_than365_days.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -class AddTempIndexPersonalAccessTokensExpiresAtGreaterThan365Days < Gitlab::Database::Migration[2.1] - INDEX_NAME = 'tmp_index_person_access_tokens_expires_more_than_365_days' - TABLE = :personal_access_tokens - CONDITIONS = "expires_at > '#{365.days.from_now.to_date}'::date" - - disable_ddl_transaction! - - def up - add_concurrent_index TABLE, 'expires_at', where: CONDITIONS, name: INDEX_NAME - - connection.execute("ANALYZE #{TABLE}") - end - - def down - remove_concurrent_index TABLE, :expires_at, name: INDEX_NAME - end -end diff --git a/db/schema_migrations/20230510062502 b/db/schema_migrations/20230510062502 deleted file mode 100644 index d224e66d064c7b..00000000000000 --- a/db/schema_migrations/20230510062502 +++ /dev/null @@ -1 +0,0 @@ -91b3b2c88b467558162b71662b8acf2ecdeac644617b8a0de154a4eed6a3549a \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 864c14d74b2b12..8c7527bdc1ba6d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -33105,9 +33105,7 @@ CREATE INDEX tmp_index_oauth_access_tokens_on_id_where_expires_in_null ON oauth_ CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING btree (id) WHERE (state <> 2); -CREATE INDEX tmp_index_person_access_tokens_expires_more_than_365_days ON personal_access_tokens USING btree (expires_at) WHERE (expires_at > '2024-05-11'::date); - -CREATE INDEX tmp_index_personal_access_tokens_on_expires_at ON personal_access_tokens USING btree (expires_at) WHERE (expires_at IS NULL); +CREATE INDEX tmp_index_personal_access_tokens_expires_at ON personal_access_tokens USING btree (expires_at); CREATE INDEX tmp_index_project_statistics_cont_registry_size ON project_statistics USING btree (project_id) WHERE (container_registry_size = 0); diff --git a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb deleted file mode 100644 index 234a864379d627..00000000000000 --- a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_greater_than365_days_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require_migration! - -RSpec.describe AddTempIndexPersonalAccessTokensExpiresAtGreaterThan365Days, feature_category: :projects do - it 'correctly migrates up and down' do - reversible_migration do |migration| - migration.before -> { - expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') - .map(&:name)).not_to include('tmp_index_person_access_tokens_expires_more_than_365_days') - } - - migration.after -> { - expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') - .map(&:name)).to include('tmp_index_person_access_tokens_expires_more_than_365_days') - } - end - end -end diff --git a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb index 65fd3a9d90acf6..8fcaddb4d49e8b 100644 --- a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb +++ b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb @@ -8,12 +8,12 @@ reversible_migration do |migration| migration.before -> { expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') - .map(&:name)).not_to include('tmp_index_personal_access_tokens_on_expires_at') + .map(&:name)).not_to include('tmp_index_personal_access_tokens_expires_at') } migration.after -> { expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') - .map(&:name)).to include('tmp_index_personal_access_tokens_on_expires_at') + .map(&:name)).to include('tmp_index_personal_access_tokens_expires_at') } end end -- GitLab From c89ec51d318a0c5a3cc7d2f4710f9767a882d343 Mon Sep 17 00:00:00 2001 From: smriti Date: Fri, 12 May 2023 19:27:35 +0530 Subject: [PATCH 11/22] Formatting suggestions applied --- ...anup_personal_access_tokens_with_expiry_more_than_year.yml | 2 +- .../cleanup_personal_access_tokens_with_nil_expires_at.yml | 2 +- .../cleanup_personal_access_tokens_with_nil_expires_at.rb | 4 +++- ...cleanup_personal_access_tokens_with_nil_expires_at_spec.rb | 4 ++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml index 2bb266e73cbfd9..13ffd07a15c6af 100644 --- a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml +++ b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml @@ -1,6 +1,6 @@ --- migration_job_name: CleanupPersonalAccessTokensWithExpiryMoreThanYear -description: Updates value of expires_at column to 365 when its expiry is more than a year for PersonalAccessTokens +description: Updates value of expires_at column to 365 days from now when its expiry is more than that for PersonalAccessTokens feature_category: system_access introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239/ milestone: 16.0 diff --git a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml index d5bd907d9cc906..25995f8ffb9f1c 100644 --- a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml +++ b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml @@ -1,6 +1,6 @@ --- migration_job_name: CleanupPersonalAccessTokensWithNilExpiresAt -description: Updates value of expires_at column to 365 when its nil for PersonalAccessTokens +description: Updates value of expires_at column to 365 days from now when its nil for PersonalAccessTokens feature_category: system_access introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239/ milestone: 16.0 diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb index db32862c0de038..7ca972e3fe5f25 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -8,12 +8,14 @@ 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: 365.days.from_now) + sub_batch.update_all(expires_at: EXPIRES_AT_DEFAULT) end end end diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb index 550c26211c2645..ade16c0a780a7b 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithNilExpiresAt, schema: 20230510062502, feature_category: :system_access do # rubocop:disable Layout/LineLength +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) { 365.days.from_now } + let(:expires_at_default) { described_class::EXPIRES_AT_DEFAULT } subject(:perform_migration) do described_class.new( -- GitLab From 7b1c5d398ecc29dd1b5003d986aa52811dc19f8c Mon Sep 17 00:00:00 2001 From: Simon Tomlinson Date: Fri, 12 May 2023 12:20:05 -0500 Subject: [PATCH 12/22] Batch by id, fix both types of invalid data --- ..._cleanup_personal_access_tokens_with_nil_expires_at.rb | 8 +++----- .../cleanup_personal_access_tokens_with_nil_expires_at.rb | 2 +- ...nup_personal_access_tokens_with_nil_expires_at_spec.rb | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb b/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb index 96994d67482c08..418be66a69c052 100644 --- a/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -4,7 +4,6 @@ class QueueCleanupPersonalAccessTokensWithNilExpiresAt < Gitlab::Database::Migra MIGRATION = "CleanupPersonalAccessTokensWithNilExpiresAt" DELAY_INTERVAL = 2.minutes BATCH_SIZE = 50_000 - SUB_BATCH_SIZE = 100 disable_ddl_transaction! @@ -14,14 +13,13 @@ def up queue_batched_background_migration( MIGRATION, :personal_access_tokens, - :expires_at, + :id, job_interval: DELAY_INTERVAL, - batch_size: BATCH_SIZE, - sub_batch_size: SUB_BATCH_SIZE + batch_size: BATCH_SIZE ) end def down - delete_batched_background_migration(MIGRATION, :personal_access_tokens, :expires_at, []) + delete_batched_background_migration(MIGRATION, :personal_access_tokens, :id, []) end end diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb index 7ca972e3fe5f25..ea6fefe278ff50 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -10,7 +10,7 @@ class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob EXPIRES_AT_DEFAULT = 365.days.from_now - scope_to ->(relation) { relation.where(expires_at: nil) } + scope_to ->(relation) { relation.where(expires_at: nil).or(relation.where('expires_at > ?', EXPIRES_AT_DEFAULT)) } operation_name :update_all def perform diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb index ade16c0a780a7b..f0a80b1f33dc7d 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -30,7 +30,7 @@ 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#1").expires_at).to eq(expires_at_default.to_date) 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 -- GitLab From 57609438759ab44cae94f3d0afe22ac1c108b591 Mon Sep 17 00:00:00 2001 From: Simon Tomlinson Date: Fri, 12 May 2023 12:33:42 -0500 Subject: [PATCH 13/22] update comment --- .../cleanup_personal_access_tokens_with_nil_expires_at.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb index ea6fefe278ff50..4ed119a30cb29c 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -3,6 +3,7 @@ module Gitlab module BackgroundMigration # Clean up personal access tokens with expires_at value is nil + # or greater than the new default of 365 days # and set the value to new default 365 days # This query is expected to process some 3738072 rows class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob -- GitLab From a16bb439aedc090075f4dea00875a9b1689d591c Mon Sep 17 00:00:00 2001 From: Simon Tomlinson Date: Fri, 12 May 2023 12:38:03 -0500 Subject: [PATCH 14/22] remove migration for expires_at gt 1 year --- ...ccess_tokens_with_expiry_more_than_year.rb | 27 ------------- db/schema_migrations/20230510162550 | 1 - ...ccess_tokens_with_expiry_more_than_year.rb | 23 ----------- ..._tokens_with_expiry_more_than_year_spec.rb | 38 ------------------- 4 files changed, 89 deletions(-) delete mode 100644 db/post_migrate/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year.rb delete mode 100644 db/schema_migrations/20230510162550 delete mode 100644 lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb delete mode 100644 spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb diff --git a/db/post_migrate/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year.rb b/db/post_migrate/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year.rb deleted file mode 100644 index eef401e458ccc2..00000000000000 --- a/db/post_migrate/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -class QueueCleanupPersonalAccessTokensWithExpiryMoreThanYear < Gitlab::Database::Migration[2.1] - MIGRATION = "CleanupPersonalAccessTokensWithExpiryMoreThanYear" - DELAY_INTERVAL = 2.minutes - BATCH_SIZE = 50_000 - SUB_BATCH_SIZE = 100 - - disable_ddl_transaction! - - restrict_gitlab_migration gitlab_schema: :gitlab_main - - def up - queue_batched_background_migration( - MIGRATION, - :personal_access_tokens, - :expires_at, - job_interval: DELAY_INTERVAL, - batch_size: BATCH_SIZE, - sub_batch_size: SUB_BATCH_SIZE - ) - end - - def down - delete_batched_background_migration(MIGRATION, :personal_access_tokens, :expires_at, []) - end -end diff --git a/db/schema_migrations/20230510162550 b/db/schema_migrations/20230510162550 deleted file mode 100644 index d0f6de3550dc18..00000000000000 --- a/db/schema_migrations/20230510162550 +++ /dev/null @@ -1 +0,0 @@ -8fb7fa9e6e4b3994134a8ca6012d9aef11f1ef5c40a3b4463398dbf62ed9ad25 \ No newline at end of file diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb deleted file mode 100644 index 2f5cb9468f3f8e..00000000000000 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module BackgroundMigration - # Clean up personal access tokens with expires_at value is or greater than 1 year - # and set the value to new default 365 days - # This query is expected to process some 268546 rows - class CleanupPersonalAccessTokensWithExpiryMoreThanYear < BatchedMigrationJob - feature_category :system_access - - EXPIRES_AT_DEFAULT = 365.days.from_now - - scope_to ->(relation) { relation.where('expires_at > ?', EXPIRES_AT_DEFAULT) } - operation_name :update_all - - def perform - each_sub_batch do |sub_batch| - sub_batch.update_all(expires_at: EXPIRES_AT_DEFAULT) - end - end - end - end -end diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb deleted file mode 100644 index cbc0743ec45b12..00000000000000 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithExpiryMoreThanYear, schema: 20230510162550, 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) { 365.days.from_now } - - 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: nil) - personal_access_tokens_table.create!(user_id: user.id, name: "PAT#2", expires_at: expires_at_default + 2.days) - 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(nil) - 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 -- GitLab From 188d03037957486ad26b578cc595441efa488e2b Mon Sep 17 00:00:00 2001 From: Simon Tomlinson Date: Fri, 12 May 2023 12:41:03 -0500 Subject: [PATCH 15/22] remove temp index --- ...index_personal_access_tokens_expires_at.rb | 19 ------------------ db/schema_migrations/20230510062501 | 1 - db/structure.sql | 2 -- ..._personal_access_tokens_expires_at_spec.rb | 20 ------------------- 4 files changed, 42 deletions(-) delete mode 100644 db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb delete mode 100644 db/schema_migrations/20230510062501 delete mode 100644 spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb diff --git a/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb b/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb deleted file mode 100644 index 406f53f4548c02..00000000000000 --- a/db/post_migrate/20230510062501_add_temp_index_personal_access_tokens_expires_at.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -# This is a temporary index to execute batch migration for personal access tokens cleanup -# as per issue - https://gitlab.com/gitlab-org/gitlab/-/issues/369123. -# It will be removed later -class AddTempIndexPersonalAccessTokensExpiresAt < Gitlab::Database::Migration[2.1] - INDEX_NAME = 'tmp_index_personal_access_tokens_expires_at' - TABLE = :personal_access_tokens - - disable_ddl_transaction! - - def up - add_concurrent_index TABLE, 'expires_at', name: INDEX_NAME - end - - def down - remove_concurrent_index TABLE, :expires_at, name: INDEX_NAME - end -end diff --git a/db/schema_migrations/20230510062501 b/db/schema_migrations/20230510062501 deleted file mode 100644 index 305e44a8620bd5..00000000000000 --- a/db/schema_migrations/20230510062501 +++ /dev/null @@ -1 +0,0 @@ -8102b00724f4ef9779ef9757aeba4f4b9138d6e684a9ede26807c0ac799455c2 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 8c7527bdc1ba6d..a57d1cbebdd251 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -33105,8 +33105,6 @@ CREATE INDEX tmp_index_oauth_access_tokens_on_id_where_expires_in_null ON oauth_ CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING btree (id) WHERE (state <> 2); -CREATE INDEX tmp_index_personal_access_tokens_expires_at ON personal_access_tokens USING btree (expires_at); - CREATE INDEX tmp_index_project_statistics_cont_registry_size ON project_statistics USING btree (project_id) WHERE (container_registry_size = 0); CREATE INDEX tmp_index_vulnerability_dismissal_info ON vulnerabilities USING btree (id) WHERE ((state = 2) AND ((dismissed_at IS NULL) OR (dismissed_by_id IS NULL))); diff --git a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb b/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb deleted file mode 100644 index 8fcaddb4d49e8b..00000000000000 --- a/spec/migrations/add_temp_index_personal_access_tokens_expires_at_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require_migration! - -RSpec.describe AddTempIndexPersonalAccessTokensExpiresAt, feature_category: :projects do - it 'correctly migrates up and down' do - reversible_migration do |migration| - migration.before -> { - expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') - .map(&:name)).not_to include('tmp_index_personal_access_tokens_expires_at') - } - - migration.after -> { - expect(ActiveRecord::Base.connection.indexes('personal_access_tokens') - .map(&:name)).to include('tmp_index_personal_access_tokens_expires_at') - } - end - end -end -- GitLab From 41fc911dbb5e976e674430fd05348fc1de38d92c Mon Sep 17 00:00:00 2001 From: Jessie Young Date: Fri, 12 May 2023 10:50:05 -0700 Subject: [PATCH 16/22] remove files associated with deleted migrations --- ...cess_tokens_with_expiry_more_than_year.yml | 6 ----- ..._tokens_with_expiry_more_than_year_spec.rb | 26 ------------------- 2 files changed, 32 deletions(-) delete mode 100644 db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml delete mode 100644 spec/migrations/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb diff --git a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml deleted file mode 100644 index 13ffd07a15c6af..00000000000000 --- a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_expiry_more_than_year.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -migration_job_name: CleanupPersonalAccessTokensWithExpiryMoreThanYear -description: Updates value of expires_at column to 365 days from now when its expiry is more than that for PersonalAccessTokens -feature_category: system_access -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239/ -milestone: 16.0 diff --git a/spec/migrations/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb b/spec/migrations/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb deleted file mode 100644 index 4085ff18c0f20b..00000000000000 --- a/spec/migrations/20230510162550_queue_cleanup_personal_access_tokens_with_expiry_more_than_year_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require_migration! - -RSpec.describe QueueCleanupPersonalAccessTokensWithExpiryMoreThanYear, 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: :expires_at, - interval: described_class::DELAY_INTERVAL, - batch_size: described_class::BATCH_SIZE, - sub_batch_size: described_class::SUB_BATCH_SIZE - ) - } - end - end -end -- GitLab From 04dd68d6adb0bd961a9a00eeb9a99a831cb2079d Mon Sep 17 00:00:00 2001 From: Jessie Young Date: Fri, 12 May 2023 11:19:17 -0700 Subject: [PATCH 17/22] Fix spec for migration updates --- ...e_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb index 1e00f695f4da80..45ef85a49cf11f 100644 --- a/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb +++ b/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -15,7 +15,7 @@ migration.after -> { expect(batched_migration).to have_scheduled_batched_migration( table_name: :personal_access_tokens, - column_name: :expires_at, + column_name: :id, interval: described_class::DELAY_INTERVAL, batch_size: described_class::BATCH_SIZE, sub_batch_size: described_class::SUB_BATCH_SIZE -- GitLab From 8879d4348b3c0607a2fda2d0fc01c219d50f82d8 Mon Sep 17 00:00:00 2001 From: Jessie Young Date: Fri, 12 May 2023 12:07:02 -0700 Subject: [PATCH 18/22] Rename migrations to reflect new behavior --- ...anup_personal_access_tokens_with_invalid_expires_at.yml} | 6 +++--- ...eanup_personal_access_tokens_with_invalid_expires_at.rb} | 4 ++-- ...eanup_personal_access_tokens_with_invalid_expires_at.rb} | 2 +- ...anup_personal_access_tokens_with_nil_invalid_at_spec.rb} | 2 +- ..._personal_access_tokens_with_invalid_expires_at_spec.rb} | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) rename db/docs/batched_background_migrations/{cleanup_personal_access_tokens_with_nil_expires_at.yml => cleanup_personal_access_tokens_with_invalid_expires_at.yml} (53%) rename db/post_migrate/{20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb => 20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb} (74%) rename lib/gitlab/background_migration/{cleanup_personal_access_tokens_with_nil_expires_at.rb => cleanup_personal_access_tokens_with_invalid_expires_at.rb} (89%) rename spec/lib/gitlab/background_migration/{cleanup_personal_access_tokens_with_nil_expires_at_spec.rb => cleanup_personal_access_tokens_with_nil_invalid_at_spec.rb} (92%) rename spec/migrations/{20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb => 20230510062502_queue_cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb} (86%) diff --git a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_invalid_expires_at.yml similarity index 53% rename from db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml rename to db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_invalid_expires_at.yml index 25995f8ffb9f1c..96f6ff0de4b1c9 100644 --- a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml +++ b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_invalid_expires_at.yml @@ -1,6 +1,6 @@ --- -migration_job_name: CleanupPersonalAccessTokensWithNilExpiresAt -description: Updates value of expires_at column to 365 days from now when its nil for PersonalAccessTokens +migration_job_name: CleanupPersonalAccessTokensWithInvalidExpiresAt +description: Updates value of expires_at column to 365 days from now when it's nil or > 365 days in the future for PersonalAccessTokens feature_category: system_access -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239/ +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239 milestone: 16.0 diff --git a/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb b/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb similarity index 74% rename from db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb rename to db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb index 418be66a69c052..63d97a81a9d900 100644 --- a/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -class QueueCleanupPersonalAccessTokensWithNilExpiresAt < Gitlab::Database::Migration[2.1] - MIGRATION = "CleanupPersonalAccessTokensWithNilExpiresAt" +class QueueCleanupPersonalAccessTokensWithInvalidExpiresAt < Gitlab::Database::Migration[2.1] + MIGRATION = "CleanupPersonalAccessTokensWithInvalidExpiresAt" DELAY_INTERVAL = 2.minutes BATCH_SIZE = 50_000 diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at.rb similarity index 89% rename from lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb rename to lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at.rb index 4ed119a30cb29c..edfd06b1560507 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at.rb @@ -6,7 +6,7 @@ module BackgroundMigration # or greater than the new default of 365 days # and set the value to new default 365 days # This query is expected to process some 3738072 rows - class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob + class CleanupPersonalAccessTokensWithInvalidExpiresAt < BatchedMigrationJob feature_category :system_access EXPIRES_AT_DEFAULT = 365.days.from_now diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_invalid_at_spec.rb similarity index 92% rename from spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb rename to spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_invalid_at_spec.rb index f0a80b1f33dc7d..51779e28545c07 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_invalid_at_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithNilExpiresAt, schema: 20230510062503, feature_category: :system_access do # rubocop:disable Layout/LineLength +RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithInvalidExpiresAt, 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 } diff --git a/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb b/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb similarity index 86% rename from spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb rename to spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb index 45ef85a49cf11f..c0625822003813 100644 --- a/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb +++ b/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require_migration! -RSpec.describe QueueCleanupPersonalAccessTokensWithNilExpiresAt, feature_category: :system_access do +RSpec.describe QueueCleanupPersonalAccessTokensWithInvalidExpiresAt, feature_category: :system_access do let!(:batched_migration) { described_class::MIGRATION } it 'schedules a new batched migration' do -- GitLab From 8712ca1cf953b612c6a5c3269f826243f11f3f55 Mon Sep 17 00:00:00 2001 From: Jessie Young Date: Fri, 12 May 2023 12:08:38 -0700 Subject: [PATCH 19/22] remove disable_ddl_transaction --- ...ue_cleanup_personal_access_tokens_with_invalid_expires_at.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb b/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb index 63d97a81a9d900..e78ae85a627517 100644 --- a/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb +++ b/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb @@ -5,8 +5,6 @@ class QueueCleanupPersonalAccessTokensWithInvalidExpiresAt < Gitlab::Database::M DELAY_INTERVAL = 2.minutes BATCH_SIZE = 50_000 - disable_ddl_transaction! - restrict_gitlab_migration gitlab_schema: :gitlab_main def up -- GitLab From ae3ff964a781229d32df17a9799f2fa03fded3fa Mon Sep 17 00:00:00 2001 From: Jessie Young Date: Fri, 12 May 2023 12:27:43 -0700 Subject: [PATCH 20/22] Rubocop --- ...leanup_personal_access_tokens_with_invalid_expires_at_spec.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/lib/gitlab/background_migration/{cleanup_personal_access_tokens_with_nil_invalid_at_spec.rb => cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb} (100%) diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_invalid_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb similarity index 100% rename from spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_invalid_at_spec.rb rename to spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb -- GitLab From 350ae82b2f55878504c7177cfe2733ffd7af68de Mon Sep 17 00:00:00 2001 From: Jessie Young Date: Fri, 12 May 2023 13:40:09 -0700 Subject: [PATCH 21/22] Only update tokens with expires_at: nil --- ... cleanup_personal_access_tokens_with_nil_expires_at.yml} | 2 +- ...e_cleanup_personal_access_tokens_with_nil_expires_at.rb} | 4 ++-- ...> cleanup_personal_access_tokens_with_nil_expires_at.rb} | 6 ++---- ...anup_personal_access_tokens_with_nil_expires_at_spec.rb} | 4 ++-- ...anup_personal_access_tokens_with_nil_expires_at_spec.rb} | 2 +- 5 files changed, 8 insertions(+), 10 deletions(-) rename db/docs/batched_background_migrations/{cleanup_personal_access_tokens_with_invalid_expires_at.yml => cleanup_personal_access_tokens_with_nil_expires_at.yml} (78%) rename db/post_migrate/{20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb => 20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb} (73%) rename lib/gitlab/background_migration/{cleanup_personal_access_tokens_with_invalid_expires_at.rb => cleanup_personal_access_tokens_with_nil_expires_at.rb} (60%) rename spec/lib/gitlab/background_migration/{cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb => cleanup_personal_access_tokens_with_nil_expires_at_spec.rb} (89%) rename spec/migrations/{20230510062502_queue_cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb => 20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb} (86%) diff --git a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_invalid_expires_at.yml b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml similarity index 78% rename from db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_invalid_expires_at.yml rename to db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml index 96f6ff0de4b1c9..b7765de25e5326 100644 --- a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_invalid_expires_at.yml +++ b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml @@ -1,6 +1,6 @@ --- migration_job_name: CleanupPersonalAccessTokensWithInvalidExpiresAt -description: Updates value of expires_at column to 365 days from now when it's nil or > 365 days in the future for PersonalAccessTokens +description: Updates value of expires_at column to 365 days from now when it's for PersonalAccessTokens feature_category: system_access introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239 milestone: 16.0 diff --git a/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb b/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb similarity index 73% rename from db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb rename to db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb index e78ae85a627517..a7dd1bda9dbd09 100644 --- a/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_invalid_expires_at.rb +++ b/db/post_migrate/20230510062503_queue_cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -class QueueCleanupPersonalAccessTokensWithInvalidExpiresAt < Gitlab::Database::Migration[2.1] - MIGRATION = "CleanupPersonalAccessTokensWithInvalidExpiresAt" +class QueueCleanupPersonalAccessTokensWithNilExpiresAt < Gitlab::Database::Migration[2.1] + MIGRATION = "CleanupPersonalAccessTokensWithNilExpiresAt" DELAY_INTERVAL = 2.minutes BATCH_SIZE = 50_000 diff --git a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at.rb b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb similarity index 60% rename from lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at.rb rename to lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb index edfd06b1560507..e8ee2a4c251890 100644 --- a/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at.rb +++ b/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at.rb @@ -3,15 +3,13 @@ module Gitlab module BackgroundMigration # Clean up personal access tokens with expires_at value is nil - # or greater than the new default of 365 days # and set the value to new default 365 days - # This query is expected to process some 3738072 rows - class CleanupPersonalAccessTokensWithInvalidExpiresAt < BatchedMigrationJob + class CleanupPersonalAccessTokensWithNilExpiresAt < BatchedMigrationJob feature_category :system_access EXPIRES_AT_DEFAULT = 365.days.from_now - scope_to ->(relation) { relation.where(expires_at: nil).or(relation.where('expires_at > ?', EXPIRES_AT_DEFAULT)) } + scope_to ->(relation) { relation.where(expires_at: nil) } operation_name :update_all def perform diff --git a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb similarity index 89% rename from spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb rename to spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb index 51779e28545c07..ade16c0a780a7b 100644 --- a/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb +++ b/spec/lib/gitlab/background_migration/cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::BackgroundMigration::CleanupPersonalAccessTokensWithInvalidExpiresAt, schema: 20230510062503, feature_category: :system_access do # rubocop:disable Layout/LineLength +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 } @@ -30,7 +30,7 @@ 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) + 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 diff --git a/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb b/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb similarity index 86% rename from spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb rename to spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb index c0625822003813..45ef85a49cf11f 100644 --- a/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_invalid_expires_at_spec.rb +++ b/spec/migrations/20230510062502_queue_cleanup_personal_access_tokens_with_nil_expires_at_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require_migration! -RSpec.describe QueueCleanupPersonalAccessTokensWithInvalidExpiresAt, feature_category: :system_access do +RSpec.describe QueueCleanupPersonalAccessTokensWithNilExpiresAt, feature_category: :system_access do let!(:batched_migration) { described_class::MIGRATION } it 'schedules a new batched migration' do -- GitLab From 2248f3db65f43d4e3a34e10fc7cdc7845b925d31 Mon Sep 17 00:00:00 2001 From: Jessie Young Date: Fri, 12 May 2023 15:13:05 -0700 Subject: [PATCH 22/22] improve text --- .../cleanup_personal_access_tokens_with_nil_expires_at.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml index b7765de25e5326..630aeccd6e1fc6 100644 --- a/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml +++ b/db/docs/batched_background_migrations/cleanup_personal_access_tokens_with_nil_expires_at.yml @@ -1,6 +1,6 @@ --- migration_job_name: CleanupPersonalAccessTokensWithInvalidExpiresAt -description: Updates value of expires_at column to 365 days from now when it's for PersonalAccessTokens +description: Updates value of expires_at column to 365 days from now when it's nil for PersonalAccessTokens feature_category: system_access introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120239 milestone: 16.0 -- GitLab