From a0fa3307187a43be673eca36ff146eef8ec1e446 Mon Sep 17 00:00:00 2001
From: Rutger Wessels <rwessels@gitlab.com>
Date: Tue, 4 Mar 2025 05:09:33 +0100
Subject: [PATCH] Add foreign key to organization_users.organization_id column
---
...8_add_fk_organization_users_organizations.rb | 17 +++++++++++++++++
db/schema_migrations/20250203130048 | 1 +
db/structure.sql | 3 +++
.../dependency_list_export/part_spec.rb | 4 ++++
.../models/vulnerabilities/export/part_spec.rb | 4 ++++
ee/spec/models/vulnerabilities/export_spec.rb | 4 ++++
lib/tasks/gitlab/update_templates.rake | 1 +
.../organizations/organization_user_spec.rb | 12 ++++++++----
8 files changed, 42 insertions(+), 4 deletions(-)
create mode 100644 db/migrate/20250203130048_add_fk_organization_users_organizations.rb
create mode 100644 db/schema_migrations/20250203130048
diff --git a/db/migrate/20250203130048_add_fk_organization_users_organizations.rb b/db/migrate/20250203130048_add_fk_organization_users_organizations.rb
new file mode 100644
index 0000000000000..cb923daff0061
--- /dev/null
+++ b/db/migrate/20250203130048_add_fk_organization_users_organizations.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class AddFkOrganizationUsersOrganizations < Gitlab::Database::Migration[2.2]
+ disable_ddl_transaction!
+ milestone '17.10'
+
+ def up
+ add_concurrent_foreign_key :organization_users, :organizations, column: :organization_id, on_delete: :restrict,
+ validate: false
+ end
+
+ def down
+ with_lock_retries do
+ remove_foreign_key_if_exists :organization_users, column: :organization_id
+ end
+ end
+end
diff --git a/db/schema_migrations/20250203130048 b/db/schema_migrations/20250203130048
new file mode 100644
index 0000000000000..50454c1b0f35d
--- /dev/null
+++ b/db/schema_migrations/20250203130048
@@ -0,0 +1 @@
+fcf6107996b68031f34dd3999ee18f164fcb26728efd80c40e8ab3d18f88c570
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 62156c0ff99fe..112a34b74ef9d 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -39764,6 +39764,9 @@ ALTER TABLE ONLY packages_npm_metadata
ALTER TABLE ONLY push_rules
ADD CONSTRAINT fk_83b29894de FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
+ALTER TABLE ONLY organization_users
+ ADD CONSTRAINT fk_8471abad75 FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE RESTRICT NOT VALID;
+
ALTER TABLE ONLY merge_request_diffs
ADD CONSTRAINT fk_8483f3258f FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE;
diff --git a/ee/spec/models/dependencies/dependency_list_export/part_spec.rb b/ee/spec/models/dependencies/dependency_list_export/part_spec.rb
index 26be54d51f9a3..70024079972e6 100644
--- a/ee/spec/models/dependencies/dependency_list_export/part_spec.rb
+++ b/ee/spec/models/dependencies/dependency_list_export/part_spec.rb
@@ -53,6 +53,10 @@
it_behaves_like 'cleanup by a loose foreign key' do
let_it_be(:parent) { create(:organization) }
let_it_be(:model) { create(:dependency_list_export_part, organization: parent) }
+
+ before do
+ parent.users.delete_all
+ end
end
end
diff --git a/ee/spec/models/vulnerabilities/export/part_spec.rb b/ee/spec/models/vulnerabilities/export/part_spec.rb
index 942e3c4f7325c..ba46abd505e60 100644
--- a/ee/spec/models/vulnerabilities/export/part_spec.rb
+++ b/ee/spec/models/vulnerabilities/export/part_spec.rb
@@ -33,6 +33,10 @@
it_behaves_like 'cleanup by a loose foreign key' do
let_it_be(:parent) { create(:organization) }
let_it_be(:model) { create(:vulnerability_export_part, organization: parent) }
+
+ before do
+ parent.users.delete_all
+ end
end
end
diff --git a/ee/spec/models/vulnerabilities/export_spec.rb b/ee/spec/models/vulnerabilities/export_spec.rb
index b018128c21b2a..c4398aa9e81d6 100644
--- a/ee/spec/models/vulnerabilities/export_spec.rb
+++ b/ee/spec/models/vulnerabilities/export_spec.rb
@@ -295,6 +295,10 @@
it_behaves_like 'cleanup by a loose foreign key' do
let_it_be(:parent) { create(:organization) }
let_it_be(:model) { create(:vulnerability_export, organization: parent) }
+
+ before do
+ parent.users.delete_all
+ end
end
end
diff --git a/lib/tasks/gitlab/update_templates.rake b/lib/tasks/gitlab/update_templates.rake
index 7b48cdbed63ca..d0c14d7842929 100644
--- a/lib/tasks/gitlab/update_templates.rake
+++ b/lib/tasks/gitlab/update_templates.rake
@@ -123,6 +123,7 @@ namespace :gitlab do
if tmp_organization
puts "Destroying temporary organization #{tmp_organization_path}"
+ tmp_organization.users.delete_all
tmp_organization.destroy
end
diff --git a/spec/models/organizations/organization_user_spec.rb b/spec/models/organizations/organization_user_spec.rb
index 0f0e0159ed1f7..98782eccc1c27 100644
--- a/spec/models/organizations/organization_user_spec.rb
+++ b/spec/models/organizations/organization_user_spec.rb
@@ -83,10 +83,14 @@
end
end
- context 'with loose foreign key on organization_users.organization_id' do
- it_behaves_like 'cleanup by a loose foreign key' do
- let_it_be(:parent) { create(:organization) }
- let_it_be(:model) { create(:organization_user, organization: parent) }
+ context 'with foreign key on organization_users.organization_id' do
+ let!(:organization) { create(:organization) }
+ let!(:organization_user) { create(:organization_user, organization: organization) }
+
+ context 'and organization is attempted to be destroyed' do
+ it 'prevents destroying the organization' do
+ expect { organization.destroy! }.to raise_error(ActiveRecord::InvalidForeignKey)
+ end
end
end
--
GitLab