From 1a88a57d17233270a1f6fa5856d0cfb41c668667 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 9 Aug 2023 15:17:29 +0100 Subject: [PATCH] CSS-4745 Fix remove cloud failure (#1024) * Add migration `1_3.sql` files Signed-off-by: Babak K. Shandiz * Modify cloud credentials foreign key relationship as cascade on-delete Signed-off-by: Babak K. Shandiz * Remove unnecessary transaction block Signed-off-by: Babak K. Shandiz * Add test to verify migration v1.3 changes are applied Signed-off-by: Babak K. Shandiz * Increment schema version Signed-off-by: Babak K. Shandiz * Assert cloud credential is deleted after cascaded relationship Signed-off-by: Babak K. Shandiz --------- Signed-off-by: Babak K. Shandiz --- internal/dbmodel/cloudcredential.go | 2 +- internal/dbmodel/cloudcredential_test.go | 39 ++++++++++++++++++++++++ internal/dbmodel/sql/postgres/1_3.sql | 11 +++++++ internal/dbmodel/sql/sqlite/1_3.sql | 13 ++++++++ internal/dbmodel/version.go | 4 +-- 5 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 internal/dbmodel/sql/postgres/1_3.sql create mode 100644 internal/dbmodel/sql/sqlite/1_3.sql diff --git a/internal/dbmodel/cloudcredential.go b/internal/dbmodel/cloudcredential.go index be7b79390..f8adf20f3 100644 --- a/internal/dbmodel/cloudcredential.go +++ b/internal/dbmodel/cloudcredential.go @@ -19,7 +19,7 @@ type CloudCredential struct { // Cloud is the cloud this credential is for. CloudName string - Cloud Cloud `gorm:"foreignKey:CloudName;references:Name"` + Cloud Cloud `gorm:"foreignKey:CloudName;references:Name;constraint:OnDelete:CASCADE"` // Owner is the user that owns this credential. OwnerUsername string diff --git a/internal/dbmodel/cloudcredential_test.go b/internal/dbmodel/cloudcredential_test.go index 6f784e997..804d207f8 100644 --- a/internal/dbmodel/cloudcredential_test.go +++ b/internal/dbmodel/cloudcredential_test.go @@ -51,3 +51,42 @@ func TestCloudCredential(t *testing.T) { c.Check(cred.CloudName, qt.Equals, cred.Cloud.Name) c.Check(cred.OwnerUsername, qt.Equals, cred.Owner.Username) } + +// TestCloudCredentialsCascadeOnDelete As of database version 1.3 (see migrations), +// the foreign key relationship to the clouds, should be a cascade-on-delete relationship. +func TestCloudCredentialsCascadeOnDelete(t *testing.T) { + c := qt.New(t) + db := gormDB(c) + + cloud := dbmodel.Cloud{ + Name: "test-cloud", + Type: "test-provider", + } + result := db.Create(&cloud) + c.Assert(result.Error, qt.IsNil) + c.Check(result.RowsAffected, qt.Equals, int64(1)) + + cred := dbmodel.CloudCredential{ + Name: "test-credential", + Cloud: cloud, + Owner: dbmodel.User{ + Username: "bob@external", + }, + } + result = db.Create(&cred) + c.Assert(result.Error, qt.IsNil) + c.Check(result.RowsAffected, qt.Equals, int64(1)) + c.Check(cred.CloudName, qt.Equals, "test-cloud") + c.Check(cred.OwnerUsername, qt.Equals, "bob@external") + + result = db.Delete(&cloud) + c.Assert(result.Error, qt.IsNil) + c.Check(result.RowsAffected, qt.Equals, int64(1)) + + deletedCred := dbmodel.CloudCredential{ + Name: "test-credential", + } + result = db.Find(&deletedCred) + c.Assert(result.Error, qt.IsNil) + c.Assert(result.RowsAffected, qt.Equals, int64(0)) +} diff --git a/internal/dbmodel/sql/postgres/1_3.sql b/internal/dbmodel/sql/postgres/1_3.sql new file mode 100644 index 000000000..922264725 --- /dev/null +++ b/internal/dbmodel/sql/postgres/1_3.sql @@ -0,0 +1,11 @@ +-- 1_3.sql is a migration that alters the foreign key relationship `cloud_credentials.cloud_name -> clouds.name` to a cascade on-delete. + +alter table cloud_credentials +drop constraint cloud_credentials_cloud_name_fkey, +add constraint cloud_credentials_cloud_name_fkey + foreign key (cloud_name) + references clouds(name) + on delete cascade; +ALTER TABLE + +UPDATE versions SET major=1, minor=3 WHERE component='jimmdb'; diff --git a/internal/dbmodel/sql/sqlite/1_3.sql b/internal/dbmodel/sql/sqlite/1_3.sql new file mode 100644 index 000000000..96436d7ef --- /dev/null +++ b/internal/dbmodel/sql/sqlite/1_3.sql @@ -0,0 +1,13 @@ +-- 1_3.sql is a migration that alters the foreign key relationship `cloud_credentials.cloud_name -> clouds.name` to a cascade on-delete. +-- Followed official instructions under heading "Making Other Kinds Of Table Schema Changes" at: +-- - https://www.sqlite.org/lang_altertable.html +-- - http://web.archive.org/web/20230718062623/https://www.sqlite.org/lang_altertable.html + +PRAGMA schema_version; +PRAGMA writable_schema=ON; +UPDATE sqlite_schema SET sql=REPLACE(sql,'cloud_name TEXT NOT NULL REFERENCES clouds (name)','cloud_name TEXT NOT NULL REFERENCES clouds (name) ON DELETE CASCADE') WHERE type='table' AND name='cloud_credentials'; +PRAGMA schema_version=44; +PRAGMA writable_schema=OFF; +PRAGMA integrity_check; + +UPDATE versions SET major=1, minor=3 WHERE component='jimmdb'; diff --git a/internal/dbmodel/version.go b/internal/dbmodel/version.go index 550ff3e1b..97ebb17b3 100644 --- a/internal/dbmodel/version.go +++ b/internal/dbmodel/version.go @@ -19,8 +19,8 @@ const ( // Minor is the minor version of the model described in the dbmodel // package. It should be incremented for any change made to the - // database model from database model in a relesed JIMM. - Minor = 2 + // database model from database model in a released JIMM. + Minor = 3 ) type Version struct {