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 0000000000000000000000000000000000000000..b674cbf299d77eb999a6cfe04c1ee7fa3d492ef5 --- /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 0000000000000000000000000000000000000000..1a67caa645d0f717ff9d7958d65dc89c27b98a54 --- /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 0000000000000000000000000000000000000000..57b35a768c5d106beed9d6e57cab093bb2431fb1 --- /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: 1_000) 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/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 0000000000000000000000000000000000000000..b158c28aceb6fdcd727895c7cfee5569530bbffc --- /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/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 0000000000000000000000000000000000000000..e8adb282fec70e08422f1de43f0ddc4065030620 --- /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/20251124174123 b/db/schema_migrations/20251124174123 new file mode 100644 index 0000000000000000000000000000000000000000..5af7144fd4bf1c78d0ae8d16f8ff38bfe436bd84 --- /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 0000000000000000000000000000000000000000..8b0b64b4129e8cf56a94e89699c1c6b6b07b59f6 --- /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 0000000000000000000000000000000000000000..7c86e3baa24a8cbfb27cf0226643a9f462f50b01 --- /dev/null +++ b/db/schema_migrations/20251124174132 @@ -0,0 +1 @@ +65e3b5e48d2352f97d9177ef97abb023551e00ffd3b2ea187b698deb725e7056 \ No newline at end of file diff --git a/db/schema_migrations/20251124182336 b/db/schema_migrations/20251124182336 new file mode 100644 index 0000000000000000000000000000000000000000..7ec6a11954cb7aeb1726b534959a6bab12c41149 --- /dev/null +++ b/db/schema_migrations/20251124182336 @@ -0,0 +1 @@ +30b533d2baf9271b14951a0b35de89d6191ceed58d476c518542bd30b2c5f1be \ No newline at end of file diff --git a/db/schema_migrations/20251124182820 b/db/schema_migrations/20251124182820 new file mode 100644 index 0000000000000000000000000000000000000000..b0e258590d13ab1eeddf7e9054d68c3d357ad206 --- /dev/null +++ b/db/schema_migrations/20251124182820 @@ -0,0 +1 @@ +e30ec62c83a62987f6509340db539559beb1de7fbff4a0fe227f3d4729445557 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c16e1c87280c4209b013f16e5d7cb5fe453662ec..fa27d1e9972af657929b290c9eed15fb7ef23a9d 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/ee/spec/lib/gitlab/database/desired_sharding_key_spec.rb b/ee/spec/lib/gitlab/database/desired_sharding_key_spec.rb index 7f5da74d536c1bf070245d85d9df01dc0487aecc..d115649ae05309e7114e9ba43eb253bf7b723c1a 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 diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 3473bce022f9d710ec2b56030ad1bdefe465a345..7cd46ac9ab40b26270baea9b99bc868aa2e14dd7 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], 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 0000000000000000000000000000000000000000..49d245db445487ca6ac28f7b97304f3a764557bd --- /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