Transition vulnerability feedback to gitlab_sec

Changelog: changed
EE: true

config/gitlab_loose_foreign_keys.yml

@@ -593,6 +593,21 @@ vulnerability_feedback:
   - table: ci_pipelines
     column: pipeline_id
     on_delete: async_nullify
+  - table: merge_requests
+    column: merge_request_id
+    on_delete: async_nullify
+  - table: users
+    column: comment_author_id
+    on_delete: async_nullify
+  - table: users
+    column: author_id
+    on_delete: async_delete
+  - table: issues
+    column: issue_id
+    on_delete: async_nullify
+  - table: projects
+    column: project_id
+    on_delete: async_delete
 vulnerability_finding_evidences:
   - table: projects
     column: project_id

db/docs/vulnerability_feedback.yml

@@ -7,8 +7,6 @@ feature_categories:
 description: Stores information about the confirm, dismiss, or create issue to investigate actions taken on vulnerabilities
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/5452
 milestone: '10.8'
-gitlab_schema: gitlab_main_cell
-allow_cross_foreign_keys:
-- gitlab_main_clusterwide
+gitlab_schema: gitlab_sec
 sharding_key:
   project_id: projects

db/post_migrate/20240404084304_backfill_vulnerability_feedback_pipeline_id_bigint.rb

@@ -8,10 +8,14 @@ class BackfillVulnerabilityFeedbackPipelineIdBigint < Gitlab::Database::Migratio
   COLUMN = :pipeline_id
   def up
-    backfill_conversion_of_integer_to_bigint(TABLE, COLUMN)
+    Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.with_suppressed do
+      backfill_conversion_of_integer_to_bigint(TABLE, COLUMN)
+    end
   end
   def down
-    revert_backfill_conversion_of_integer_to_bigint(TABLE, COLUMN)
+    Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.with_suppressed do
+      revert_backfill_conversion_of_integer_to_bigint(TABLE, COLUMN)
+    end
   end
 end

db/post_migrate/20241014062327_remove_merge_requests_vulnerability_feedback_merge_request_id_fk.rb

+# frozen_string_literal: true
+
+class RemoveMergeRequestsVulnerabilityFeedbackMergeRequestIdFk < Gitlab::Database::Migration[2.2]
+  milestone '17.6'
+  disable_ddl_transaction!
+
+  FOREIGN_KEY_NAME = "fk_563ff1912e"
+
+  def up
+    with_lock_retries do
+      remove_foreign_key_if_exists(:vulnerability_feedback, :merge_requests,
+        name: FOREIGN_KEY_NAME, reverse_lock_order: true)
+    end
+  end
+
+  def down
+    add_concurrent_foreign_key(:vulnerability_feedback, :merge_requests,
+      name: FOREIGN_KEY_NAME, column: :merge_request_id,
+      target_column: :id, on_delete: :nullify)
+  end
+end

db/post_migrate/20241014062801_remove_users_vulnerability_feedback_comment_author_id_fk.rb

+# frozen_string_literal: true
+
+class RemoveUsersVulnerabilityFeedbackCommentAuthorIdFk < Gitlab::Database::Migration[2.2]
+  milestone '17.6'
+  disable_ddl_transaction!
+
+  FOREIGN_KEY_NAME = "fk_94f7c8a81e"
+
+  def up
+    with_lock_retries do
+      remove_foreign_key_if_exists(:vulnerability_feedback, :users,
+        name: FOREIGN_KEY_NAME, reverse_lock_order: true)
+    end
+  end
+
+  def down
+    add_concurrent_foreign_key(:vulnerability_feedback, :users,
+      name: FOREIGN_KEY_NAME, column: :comment_author_id,
+      target_column: :id, on_delete: :nullify)
+  end
+end

db/post_migrate/20241014062942_remove_users_vulnerability_feedback_author_id_fk.rb

+# frozen_string_literal: true
+
+class RemoveUsersVulnerabilityFeedbackAuthorIdFk < Gitlab::Database::Migration[2.2]
+  milestone '17.6'
+  disable_ddl_transaction!
+
+  FOREIGN_KEY_NAME = "fk_rails_472f69b043"
+
+  def up
+    with_lock_retries do
+      remove_foreign_key_if_exists(:vulnerability_feedback, :users,
+        name: FOREIGN_KEY_NAME, reverse_lock_order: true)
+    end
+  end
+
+  def down
+    add_concurrent_foreign_key(:vulnerability_feedback, :users,
+      name: FOREIGN_KEY_NAME, column: :author_id,
+      target_column: :id, on_delete: :cascade)
+  end
+end

db/post_migrate/20241014063215_remove_issues_vulnerability_feedback_issue_id_fk.rb

+# frozen_string_literal: true
+
+class RemoveIssuesVulnerabilityFeedbackIssueIdFk < Gitlab::Database::Migration[2.2]
+  milestone '17.6'
+  disable_ddl_transaction!
+
+  FOREIGN_KEY_NAME = "fk_rails_8c77e5891a"
+
+  def up
+    with_lock_retries do
+      remove_foreign_key_if_exists(:vulnerability_feedback, :issues,
+        name: FOREIGN_KEY_NAME, reverse_lock_order: true)
+    end
+  end
+
+  def down
+    add_concurrent_foreign_key(:vulnerability_feedback, :issues,
+      name: FOREIGN_KEY_NAME, column: :issue_id,
+      target_column: :id, on_delete: :nullify)
+  end
+end

db/post_migrate/20241014063346_remove_projects_vulnerability_feedback_project_id_fk.rb

+# frozen_string_literal: true
+
+class RemoveProjectsVulnerabilityFeedbackProjectIdFk < Gitlab::Database::Migration[2.2]
+  milestone '17.6'
+  disable_ddl_transaction!
+
+  FOREIGN_KEY_NAME = "fk_rails_debd54e456"
+
+  def up
+    with_lock_retries do
+      remove_foreign_key_if_exists(:vulnerability_feedback, :projects,
+        name: FOREIGN_KEY_NAME, reverse_lock_order: true)
+    end
+  end
+
+  def down
+    add_concurrent_foreign_key(:vulnerability_feedback, :projects,
+      name: FOREIGN_KEY_NAME, column: :project_id,
+      target_column: :id, on_delete: :cascade)
+  end
+end

db/schema_migrations/20241014062327

+5902c993e582c48dc3767d4f1aafb6d877bf7dd233d2e212e2c8452a8c2f666e
\ No newline at end of file

db/schema_migrations/20241014062801

+ff5aab751c76a3b4af697a6d9aa58812cf3eb37c15bdf8bdcc7ce20a7149bde8
\ No newline at end of file

db/schema_migrations/20241014062942

+7d7762b1250ffc612b4b87f4664a62bbc868af4a9965498c639135d09fbf9e80
\ No newline at end of file

db/schema_migrations/20241014063215

+b88a736690a44b83d485d6517773d13e41d091437ca2fd820aa57fe75a0906fb
\ No newline at end of file

db/schema_migrations/20241014063346

+4524d9598427adcf0d84e3cae9767511721d2500d56ade663056b52c9b8fda38
\ No newline at end of file

db/structure.sql

@@ -34578,9 +34578,6 @@ ALTER TABLE ONLY status_check_responses
 ALTER TABLE ONLY merge_request_metrics
     ADD CONSTRAINT fk_56067dcb44 FOREIGN KEY (target_project_id) REFERENCES projects(id) ON DELETE CASCADE;
-ALTER TABLE ONLY vulnerability_feedback
-    ADD CONSTRAINT fk_563ff1912e FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE SET NULL;
-
 ALTER TABLE ONLY merge_request_diffs
     ADD CONSTRAINT fk_56ac6fc9c0 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
@@ -34947,9 +34944,6 @@ ALTER TABLE ONLY workspaces_agent_configs
 ALTER TABLE ONLY dast_site_profiles_builds
     ADD CONSTRAINT fk_94e80df60e FOREIGN KEY (dast_site_profile_id) REFERENCES dast_site_profiles(id) ON DELETE CASCADE;
-ALTER TABLE ONLY vulnerability_feedback
-    ADD CONSTRAINT fk_94f7c8a81e FOREIGN KEY (comment_author_id) REFERENCES users(id) ON DELETE SET NULL;
-
 ALTER TABLE ONLY milestones
     ADD CONSTRAINT fk_95650a40d4 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
@@ -36213,9 +36207,6 @@ ALTER TABLE ONLY protected_environment_deploy_access_levels
 ALTER TABLE ONLY smartcard_identities
     ADD CONSTRAINT fk_rails_4689f889a9 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
-ALTER TABLE ONLY vulnerability_feedback
-    ADD CONSTRAINT fk_rails_472f69b043 FOREIGN KEY (author_id) REFERENCES users(id) ON DELETE CASCADE;
-
 ALTER TABLE ONLY user_custom_attributes
     ADD CONSTRAINT fk_rails_47b91868a8 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
@@ -36729,9 +36720,6 @@ ALTER TABLE ONLY work_item_progresses
 ALTER TABLE ONLY packages_conan_metadata
     ADD CONSTRAINT fk_rails_8c68cfec8b FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE;
-ALTER TABLE ONLY vulnerability_feedback
-    ADD CONSTRAINT fk_rails_8c77e5891a FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE SET NULL;
-
 ALTER TABLE ONLY import_placeholder_memberships
     ADD CONSTRAINT fk_rails_8cdeffd260 FOREIGN KEY (source_user_id) REFERENCES import_source_users(id) ON DELETE CASCADE;
@@ -37320,9 +37308,6 @@ ALTER TABLE ONLY user_callouts
 ALTER TABLE ONLY scan_result_policies
     ADD CONSTRAINT fk_rails_de9e5d2ce6 FOREIGN KEY (security_orchestration_policy_configuration_id) REFERENCES security_orchestration_policy_configurations(id) ON DELETE CASCADE;
-ALTER TABLE ONLY vulnerability_feedback
-    ADD CONSTRAINT fk_rails_debd54e456 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
-
 ALTER TABLE ONLY service_desk_custom_email_verifications
     ADD CONSTRAINT fk_rails_debe4c4acc FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;

ee/app/models/vulnerabilities/feedback.rb

 # frozen_string_literal: true
 module Vulnerabilities
-  class Feedback < ApplicationRecord
+  class Feedback < Gitlab::Database::SecApplicationRecord
     include EachBatch
     self.table_name = 'vulnerability_feedback'

ee/spec/models/vulnerabilities/feedback_spec.rb

@@ -303,4 +303,41 @@
       let!(:model) { create(:vulnerability_feedback, project: parent.project, pipeline: parent) }
     end
   end
+
+  context 'with loose foreign key on vulnerability_feedback.merge_request_id' do
+    it_behaves_like 'cleanup by a loose foreign key' do
+      let_it_be(:parent) { create(:merge_request) }
+      let_it_be(:model) { create(:vulnerability_feedback, merge_request: parent) }
+    end
+  end
+
+  context 'with loose foreign key on vulnerability_feedback.comment_author_id' do
+    it_behaves_like 'cleanup by a loose foreign key' do
+      let_it_be(:parent) { create(:user) }
+      let_it_be(:model) { create(:vulnerability_feedback, comment_author: parent) }
+      let_it_be(:lfk_column) { :comment_author_id }
+    end
+  end
+
+  context 'with loose foreign key on vulnerability_feedback.author_id' do
+    it_behaves_like 'cleanup by a loose foreign key' do
+      let_it_be(:parent) { create(:user) }
+      let_it_be(:model) { create(:vulnerability_feedback, author: parent) }
+      let_it_be(:lfk_column) { :author_id }
+    end
+  end
+
+  context 'with loose foreign key on vulnerability_feedback.issue_id' do
+    it_behaves_like 'cleanup by a loose foreign key' do
+      let_it_be(:parent) { create(:issue) }
+      let_it_be(:model) { create(:vulnerability_feedback, issue: parent) }
+    end
+  end
+
+  context 'with loose foreign key on vulnerability_feedback.project_id' do
+    it_behaves_like 'cleanup by a loose foreign key' do
+      let_it_be(:parent) { create(:project) }
+      let_it_be(:model) { create(:vulnerability_feedback, project: parent) }
+    end
+  end
 end

spec/support/shared_contexts/loose_foreign_keys_shared_context.rb

@@ -3,9 +3,16 @@
 RSpec.shared_context 'for loose foreign keys' do
   include LooseForeignKeysHelper
+  # Generally it's reasonable to assume only one FK between tables. If there is more than one, you need
+  # to specify which column you want to be testing with `lfk_column`.
+  let(:lfk_column) { nil }
   let(:foreign_key_definition) do
     foreign_keys_for_parent = Gitlab::Database::LooseForeignKeys.definitions_by_table[parent.class.table_name]
-    foreign_keys_for_parent.find { |definition| definition.from_table == model.class.table_name }
+
+    foreign_keys_for_parent.find do |definition|
+      definition.from_table == model.class.table_name &&
+        (lfk_column.nil? || definition.options[:column].to_sym == lfk_column.to_sym)
+    end
   end
   def find_model