From 055df9854c8af522919f91eed59c8f12b730cbe5 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Sun, 4 May 2025 20:58:04 +0200 Subject: [PATCH] [management] add gorm tag for primary key for the networks objects (#3758) --- management/server/migration/migration.go | 21 ++++++++++++++++++ management/server/migration/migration_test.go | 22 +++++++++++++++++++ .../networks/resources/types/resource.go | 2 +- .../server/networks/routers/types/router.go | 2 +- management/server/networks/types/network.go | 2 +- management/server/store/sql_store.go | 4 ++++ management/server/store/sql_store_test.go | 6 ++--- management/server/store/store.go | 9 ++++++++ management/server/types/group.go | 2 +- 9 files changed, 63 insertions(+), 7 deletions(-) diff --git a/management/server/migration/migration.go b/management/server/migration/migration.go index d7abbad47..c8a852e0a 100644 --- a/management/server/migration/migration.go +++ b/management/server/migration/migration.go @@ -352,3 +352,24 @@ func MigrateNewField[T any](ctx context.Context, db *gorm.DB, columnName string, log.WithContext(ctx).Infof("Migration of empty %s to default value in table %s completed", columnName, tableName) return nil } + +func DropIndex[T any](ctx context.Context, db *gorm.DB, indexName string) error { + var model T + + if !db.Migrator().HasTable(&model) { + log.WithContext(ctx).Debugf("table for %T does not exist, no migration needed", model) + return nil + } + + if !db.Migrator().HasIndex(&model, indexName) { + log.WithContext(ctx).Debugf("index %s does not exist in table %T, no migration needed", indexName, model) + return nil + } + + if err := db.Migrator().DropIndex(&model, indexName); err != nil { + return fmt.Errorf("failed to drop index %s: %w", indexName, err) + } + + log.WithContext(ctx).Infof("dropped index %s from table %T", indexName, model) + return nil +} diff --git a/management/server/migration/migration_test.go b/management/server/migration/migration_test.go index e907d6853..94377930a 100644 --- a/management/server/migration/migration_test.go +++ b/management/server/migration/migration_test.go @@ -227,3 +227,25 @@ func TestMigrateSetupKeyToHashedSetupKey_ForAlreadyMigratedKey_Case2(t *testing. assert.Equal(t, "9+FQcmNd2GCxIK+SvHmtp6PPGV4MKEicDS+xuSQmvlE=", key.Key, "Key should be hashed") } + +func TestDropIndex(t *testing.T) { + db := setupDatabase(t) + + err := db.AutoMigrate(&types.SetupKey{}) + require.NoError(t, err, "Failed to auto-migrate tables") + + err = db.Save(&types.SetupKey{ + Id: "1", + Key: "9+FQcmNd2GCxIK+SvHmtp6PPGV4MKEicDS+xuSQmvlE=", + }).Error + require.NoError(t, err, "Failed to insert setup key") + + exist := db.Migrator().HasIndex(&types.SetupKey{}, "idx_setup_keys_account_id") + assert.True(t, exist, "Should have the index") + + err = migration.DropIndex[types.SetupKey](context.Background(), db, "idx_setup_keys_account_id") + require.NoError(t, err, "Migration should not fail to remove index") + + exist = db.Migrator().HasIndex(&types.SetupKey{}, "idx_setup_keys_account_id") + assert.False(t, exist, "Should not have the index") +} diff --git a/management/server/networks/resources/types/resource.go b/management/server/networks/resources/types/resource.go index ecac0a724..04c63608d 100644 --- a/management/server/networks/resources/types/resource.go +++ b/management/server/networks/resources/types/resource.go @@ -30,7 +30,7 @@ func (p NetworkResourceType) String() string { } type NetworkResource struct { - ID string `gorm:"index"` + ID string `gorm:"primaryKey"` NetworkID string `gorm:"index"` AccountID string `gorm:"index"` Name string diff --git a/management/server/networks/routers/types/router.go b/management/server/networks/routers/types/router.go index 5158ebb12..71465868f 100644 --- a/management/server/networks/routers/types/router.go +++ b/management/server/networks/routers/types/router.go @@ -10,7 +10,7 @@ import ( ) type NetworkRouter struct { - ID string `gorm:"index"` + ID string `gorm:"primaryKey"` NetworkID string `gorm:"index"` AccountID string `gorm:"index"` Peer string diff --git a/management/server/networks/types/network.go b/management/server/networks/types/network.go index a4ba7b821..d1c7f2b33 100644 --- a/management/server/networks/types/network.go +++ b/management/server/networks/types/network.go @@ -7,7 +7,7 @@ import ( ) type Network struct { - ID string `gorm:"index"` + ID string `gorm:"primaryKey"` AccountID string `gorm:"index"` Name string Description string diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index dd39cf77d..d0adad6ee 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -82,6 +82,10 @@ func NewSqlStore(ctx context.Context, db *gorm.DB, storeEngine types.Engine, met log.WithContext(ctx).Warnf("setting NB_SQL_MAX_OPEN_CONNS is not supported for sqlite, using default value 1") } conns = 1 + _, err = sql.Exec("PRAGMA foreign_keys = ON") + if err != nil { + return nil, fmt.Errorf("failed to set foreign keys for sqlite: %w", err) + } } sql.SetMaxOpenConns(conns) diff --git a/management/server/store/sql_store_test.go b/management/server/store/sql_store_test.go index 8bd8ce098..8e99b34e1 100644 --- a/management/server/store/sql_store_test.go +++ b/management/server/store/sql_store_test.go @@ -60,10 +60,10 @@ func Test_NewStore(t *testing.T) { runTestForAllEngines(t, "", func(t *testing.T, store Store) { if store == nil { - t.Errorf("expected to create a new Store") + t.Fatalf("expected to create a new Store") } if len(store.GetAllAccounts(context.Background())) != 0 { - t.Errorf("expected to create a new empty Accounts map when creating a new FileStore") + t.Fatalf("expected to create a new empty Accounts map when creating a new FileStore") } }) } @@ -1115,7 +1115,7 @@ func TestSqlite_CreateAndGetObjectInTransaction(t *testing.T) { group := &types.Group{ ID: "group-id", - AccountID: "account-id", + AccountID: "bf1c8084-ba50-4ce7-9439-34653001fc3b", Name: "group-name", Issued: "api", Peers: nil, diff --git a/management/server/store/store.go b/management/server/store/store.go index ca332a493..6da623956 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -315,6 +315,15 @@ func getMigrations(ctx context.Context) []migrationFunc { func(db *gorm.DB) error { return migration.MigrateNewField[routerTypes.NetworkRouter](ctx, db, "enabled", true) }, + func(db *gorm.DB) error { + return migration.DropIndex[networkTypes.Network](ctx, db, "idx_networks_id") + }, + func(db *gorm.DB) error { + return migration.DropIndex[resourceTypes.NetworkResource](ctx, db, "idx_network_resources_id") + }, + func(db *gorm.DB) error { + return migration.DropIndex[routerTypes.NetworkRouter](ctx, db, "idx_network_routers_id") + }, } } diff --git a/management/server/types/group.go b/management/server/types/group.go index 00a28fa77..1b321387c 100644 --- a/management/server/types/group.go +++ b/management/server/types/group.go @@ -14,7 +14,7 @@ const ( // Group of the peers for ACL type Group struct { // ID of the group - ID string + ID string `gorm:"primaryKey"` // AccountID is a reference to Account that this object belongs AccountID string `json:"-" gorm:"index"`