From bce4196389048c1f348c2ee7ed79c38692ccbf57 Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 13 Nov 2025 13:06:44 -0700 Subject: [PATCH 1/8] Add constraints to `spam_logs.user_id` Changelog: changed --- ...ot_null_constraint_on_spam_logs_user_id.rb | 14 ++++ ...7_add_foreign_key_to_users_on_spam_logs.rb | 16 +++++ ...0251124174132_remove_orphaned_spam_logs.rb | 18 +++++ db/schema_migrations/20251124174123 | 1 + db/schema_migrations/20251124174127 | 1 + db/schema_migrations/20251124174132 | 1 + db/structure.sql | 6 ++ ...13203202_remove_orphaned_spam_logs_spec.rb | 67 +++++++++++++++++++ 8 files changed, 124 insertions(+) create mode 100644 db/migrate/20251124174123_add_not_null_constraint_on_spam_logs_user_id.rb create mode 100644 db/migrate/20251124174127_add_foreign_key_to_users_on_spam_logs.rb create mode 100644 db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb create mode 100644 db/schema_migrations/20251124174123 create mode 100644 db/schema_migrations/20251124174127 create mode 100644 db/schema_migrations/20251124174132 create mode 100644 spec/migrations/20251113203202_remove_orphaned_spam_logs_spec.rb diff --git a/db/migrate/20251124174123_add_not_null_constraint_on_spam_logs_user_id.rb b/db/migrate/20251124174123_add_not_null_constraint_on_spam_logs_user_id.rb new file mode 100644 index 00000000000000..b674cbf299d77e --- /dev/null +++ b/db/migrate/20251124174123_add_not_null_constraint_on_spam_logs_user_id.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddNotNullConstraintOnSpamLogsUserId < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.7' + + def up + add_not_null_constraint :spam_logs, :user_id, validate: false + end + + def down + remove_not_null_constraint :spam_logs, :user_id + end +end diff --git a/db/migrate/20251124174127_add_foreign_key_to_users_on_spam_logs.rb b/db/migrate/20251124174127_add_foreign_key_to_users_on_spam_logs.rb new file mode 100644 index 00000000000000..1a67caa645d0f7 --- /dev/null +++ b/db/migrate/20251124174127_add_foreign_key_to_users_on_spam_logs.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddForeignKeyToUsersOnSpamLogs < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.7' + + def up + add_concurrent_foreign_key :spam_logs, :users, column: :user_id, on_delete: :cascade, validate: false + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :spam_logs, column: :user_id + end + end +end diff --git a/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb b/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb new file mode 100644 index 00000000000000..bedf6934fe2687 --- /dev/null +++ b/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class RemoveOrphanedSpamLogs < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.7' + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + define_batchable_model(:spam_logs).each_batch(of: 500) do |batch| + batch + .joins('LEFT OUTER JOIN users ON spam_logs.user_id = users.id') + .where(users: { id: nil }) + .delete_all + end + end + + def down; end +end diff --git a/db/schema_migrations/20251124174123 b/db/schema_migrations/20251124174123 new file mode 100644 index 00000000000000..5af7144fd4bf1c --- /dev/null +++ b/db/schema_migrations/20251124174123 @@ -0,0 +1 @@ +a2f8cd47c3bd4df35ae9b071e7e665589b5ea8161ab6bdf6b549be66b46b447a \ No newline at end of file diff --git a/db/schema_migrations/20251124174127 b/db/schema_migrations/20251124174127 new file mode 100644 index 00000000000000..8b0b64b4129e8c --- /dev/null +++ b/db/schema_migrations/20251124174127 @@ -0,0 +1 @@ +af91fa2d07448d87922d3601121795dd57382cd8c15fb4fcb560575864882186 \ No newline at end of file diff --git a/db/schema_migrations/20251124174132 b/db/schema_migrations/20251124174132 new file mode 100644 index 00000000000000..7c86e3baa24a8c --- /dev/null +++ b/db/schema_migrations/20251124174132 @@ -0,0 +1 @@ +65e3b5e48d2352f97d9177ef97abb023551e00ffd3b2ea187b698deb725e7056 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c16e1c87280c42..fa27d1e9972af6 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -34479,6 +34479,9 @@ ALTER TABLE user_details ALTER TABLE diff_note_positions ADD CONSTRAINT check_4c86140f48 CHECK ((namespace_id IS NOT NULL)) NOT VALID; +ALTER TABLE spam_logs + ADD CONSTRAINT check_56d1d910ee CHECK ((user_id IS NOT NULL)) NOT VALID; + ALTER TABLE ONLY instance_type_ci_runners ADD CONSTRAINT check_5c34a3c1db UNIQUE (id); @@ -49407,6 +49410,9 @@ ALTER TABLE ONLY board_user_preferences ALTER TABLE ONLY approval_policy_rule_project_links ADD CONSTRAINT fk_1c78796d52 FOREIGN KEY (approval_policy_rule_id) REFERENCES approval_policy_rules(id) ON DELETE CASCADE; +ALTER TABLE ONLY spam_logs + ADD CONSTRAINT fk_1cb83308b1 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE NOT VALID; + ALTER TABLE ONLY issue_links ADD CONSTRAINT fk_1cce06b868 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; diff --git a/spec/migrations/20251113203202_remove_orphaned_spam_logs_spec.rb b/spec/migrations/20251113203202_remove_orphaned_spam_logs_spec.rb new file mode 100644 index 00000000000000..49d245db445487 --- /dev/null +++ b/spec/migrations/20251113203202_remove_orphaned_spam_logs_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe RemoveOrphanedSpamLogs, feature_category: :insider_threat do + let(:connection) { ApplicationRecord.connection } + let(:spam_logs) { table(:spam_logs) } + let(:organizations) { table(:organizations) } + let(:users) { table(:users) } + let(:constraint_name) { 'fk_1cb83308b1' } + + let!(:organization) { organizations.create!(name: 'org', path: 'org') } + let!(:user) { create_user('user') } + + let!(:spam_log_with_valid_user) { create_spam_log(user.id) } + let!(:spam_log_with_orphaned_user) { without_constraint { create_spam_log(users.maximum(:id).to_i + 1) } } + + describe '#up' do + it 'removes orphaned spam_log records' do + expect { migrate! }.to change { spam_logs.count }.from(2).to(1) + + expect(spam_logs.find_by(id: spam_log_with_valid_user.id)).not_to be_nil + expect(spam_logs.find_by(id: spam_log_with_orphaned_user.id)).to be_nil + end + end + + describe '#down' do + it 'is a no-op' do + migrate! + + expect { schema_migrate_down! }.not_to change { spam_logs.count } + end + end + + private + + def create_user(username) + users.create!( + email: "#{username}@example.com", + username: username, + organization_id: organization.id, + projects_limit: 10 + ) + end + + def create_spam_log(user_id) + spam_logs.create!(user_id: user_id) + end + + def drop_constraint + connection.execute("ALTER TABLE spam_logs DROP CONSTRAINT IF EXISTS #{constraint_name}") + end + + def recreate_constraint + connection.execute(<<~SQL) + ALTER TABLE spam_logs ADD CONSTRAINT #{constraint_name} FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE NOT VALID + SQL + end + + def without_constraint + drop_constraint + yield + ensure + recreate_constraint + end +end -- GitLab From 8f4e79fd08d7cb646cd86d5c5fff31cd8183c4ab Mon Sep 17 00:00:00 2001 From: mo khan Date: Mon, 24 Nov 2025 11:26:43 -0700 Subject: [PATCH 2/8] Prepare async check constraint validation --- ...ot_null_constraint_on_spam_logs_for_user_id.rb | 15 +++++++++++++++ db/schema_migrations/20251124182336 | 1 + 2 files changed, 16 insertions(+) create mode 100644 db/post_migrate/20251124182336_prepare_async_check_not_null_constraint_on_spam_logs_for_user_id.rb create mode 100644 db/schema_migrations/20251124182336 diff --git a/db/post_migrate/20251124182336_prepare_async_check_not_null_constraint_on_spam_logs_for_user_id.rb b/db/post_migrate/20251124182336_prepare_async_check_not_null_constraint_on_spam_logs_for_user_id.rb new file mode 100644 index 00000000000000..b158c28aceb6fd --- /dev/null +++ b/db/post_migrate/20251124182336_prepare_async_check_not_null_constraint_on_spam_logs_for_user_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class PrepareAsyncCheckNotNullConstraintOnSpamLogsForUserId < Gitlab::Database::Migration[2.3] + milestone '18.7' + + CONSTRAINT_NAME = 'check_56d1d910ee' + + def up + prepare_async_check_constraint_validation :spam_logs, name: CONSTRAINT_NAME + end + + def down + unprepare_async_check_constraint_validation :spam_logs, name: CONSTRAINT_NAME + end +end diff --git a/db/schema_migrations/20251124182336 b/db/schema_migrations/20251124182336 new file mode 100644 index 00000000000000..7ec6a11954cb7a --- /dev/null +++ b/db/schema_migrations/20251124182336 @@ -0,0 +1 @@ +30b533d2baf9271b14951a0b35de89d6191ceed58d476c518542bd30b2c5f1be \ No newline at end of file -- GitLab From fc6dcac826f6c799e67800a8a3beb11d8b1eb546 Mon Sep 17 00:00:00 2001 From: mo khan Date: Mon, 24 Nov 2025 11:30:48 -0700 Subject: [PATCH 3/8] Add migration to validate foreign key constraint --- ...ign_key_constraint_on_spam_logs_for_user_id.rb | 15 +++++++++++++++ db/schema_migrations/20251124182820 | 1 + 2 files changed, 16 insertions(+) create mode 100644 db/post_migrate/20251124182820_prepare_async_foreign_key_constraint_on_spam_logs_for_user_id.rb create mode 100644 db/schema_migrations/20251124182820 diff --git a/db/post_migrate/20251124182820_prepare_async_foreign_key_constraint_on_spam_logs_for_user_id.rb b/db/post_migrate/20251124182820_prepare_async_foreign_key_constraint_on_spam_logs_for_user_id.rb new file mode 100644 index 00000000000000..e8adb282fec70e --- /dev/null +++ b/db/post_migrate/20251124182820_prepare_async_foreign_key_constraint_on_spam_logs_for_user_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class PrepareAsyncForeignKeyConstraintOnSpamLogsForUserId < Gitlab::Database::Migration[2.3] + milestone '18.7' + + FK_NAME = :fk_1cb83308b1 + + def up + prepare_async_foreign_key_validation :spam_logs, :user_id, name: FK_NAME + end + + def down + unprepare_async_foreign_key_validation :spam_logs, :user_id, name: FK_NAME + end +end diff --git a/db/schema_migrations/20251124182820 b/db/schema_migrations/20251124182820 new file mode 100644 index 00000000000000..b0e258590d13ab --- /dev/null +++ b/db/schema_migrations/20251124182820 @@ -0,0 +1 @@ +e30ec62c83a62987f6509340db539559beb1de7fbff4a0fe227f3d4729445557 \ No newline at end of file -- GitLab From 648b0582526e9e59e92ee552acda386ce245c1b8 Mon Sep 17 00:00:00 2001 From: mo khan Date: Mon, 24 Nov 2025 13:58:29 -0700 Subject: [PATCH 4/8] Fix failing spec --- spec/db/schema_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 3473bce022f9d7..7cd46ac9ab40b2 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -175,7 +175,7 @@ p_sent_notifications: %w[project_id noteable_id recipient_id commit_id in_reply_to_discussion_id], slack_integrations: %w[team_id user_id bot_user_id], # these are external Slack IDs snippets: %w[author_id], - spam_logs: %w[user_id target_id], + spam_logs: %w[target_id], status_check_responses: %w[external_approval_rule_id], subscriptions: %w[subscribable_id], suggestions: %w[commit_id], -- GitLab From c73d999d3bd64876110e70ffc4fec206d6e1985f Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 26 Nov 2025 16:51:52 -0700 Subject: [PATCH 5/8] Increase batch size to 1K --- db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb b/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb index bedf6934fe2687..57b35a768c5d10 100644 --- a/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb +++ b/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb @@ -6,7 +6,7 @@ class RemoveOrphanedSpamLogs < Gitlab::Database::Migration[2.3] restrict_gitlab_migration gitlab_schema: :gitlab_main def up - define_batchable_model(:spam_logs).each_batch(of: 500) do |batch| + define_batchable_model(:spam_logs).each_batch(of: 1_000) do |batch| batch .joins('LEFT OUTER JOIN users ON spam_logs.user_id = users.id') .where(users: { id: nil }) -- GitLab From 00ca31fcb90fee78132ffd6e521b5a68469a05c2 Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 27 Nov 2025 11:05:11 -0700 Subject: [PATCH 6/8] Bump to 2K rows per batch --- db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb b/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb index 57b35a768c5d10..f0e2a3e84f73b3 100644 --- a/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb +++ b/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb @@ -6,7 +6,7 @@ class RemoveOrphanedSpamLogs < Gitlab::Database::Migration[2.3] restrict_gitlab_migration gitlab_schema: :gitlab_main def up - define_batchable_model(:spam_logs).each_batch(of: 1_000) do |batch| + define_batchable_model(:spam_logs).each_batch(of: 2_000) do |batch| batch .joins('LEFT OUTER JOIN users ON spam_logs.user_id = users.id') .where(users: { id: nil }) -- GitLab From 4578d77ffb15d0804d6082e07e9d7d832b7c8949 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 28 Nov 2025 09:05:17 -0700 Subject: [PATCH 7/8] Apply 1 suggestion(s) to 1 file(s) --- db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb b/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb index f0e2a3e84f73b3..57b35a768c5d10 100644 --- a/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb +++ b/db/post_migrate/20251124174132_remove_orphaned_spam_logs.rb @@ -6,7 +6,7 @@ class RemoveOrphanedSpamLogs < Gitlab::Database::Migration[2.3] restrict_gitlab_migration gitlab_schema: :gitlab_main def up - define_batchable_model(:spam_logs).each_batch(of: 2_000) do |batch| + define_batchable_model(:spam_logs).each_batch(of: 1_000) do |batch| batch .joins('LEFT OUTER JOIN users ON spam_logs.user_id = users.id') .where(users: { id: nil }) -- GitLab From 6d0d60735a524477b1e4235cd242d64c81eb7bfe Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 28 Nov 2025 13:03:09 -0700 Subject: [PATCH 8/8] Remove fk exclusion to fix failing spec --- ee/spec/lib/gitlab/database/desired_sharding_key_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/ee/spec/lib/gitlab/database/desired_sharding_key_spec.rb b/ee/spec/lib/gitlab/database/desired_sharding_key_spec.rb index 7f5da74d536c1b..d115649ae05309 100644 --- a/ee/spec/lib/gitlab/database/desired_sharding_key_spec.rb +++ b/ee/spec/lib/gitlab/database/desired_sharding_key_spec.rb @@ -12,7 +12,6 @@ # below table will get a foreign key after its partitioning backfill # finishes, before the application begins using it. # - 'spam_logs.user_id', 'merge_request_diff_files_99208b8fac.merge_request_diff_id' ] end -- GitLab