From f99477d3db3953b8f90ab9d670f70eba2539f003 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 29 Apr 2024 15:45:46 +0300 Subject: [PATCH 1/8] serialize net.IP as json --- management/server/peer/peer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/management/server/peer/peer.go b/management/server/peer/peer.go index e6284318030..ff927a43174 100644 --- a/management/server/peer/peer.go +++ b/management/server/peer/peer.go @@ -19,7 +19,7 @@ type Peer struct { // A setup key this peer was registered with SetupKey string // IP address of the Peer - IP net.IP `gorm:"uniqueIndex:idx_peers_account_id_ip"` + IP net.IP `gorm:"uniqueIndex:idx_peers_account_id_ip;serializer:json"` // Meta is a Peer system meta data Meta PeerSystemMeta `gorm:"embedded;embeddedPrefix:meta_"` // Name is peer's name (machine name) @@ -61,7 +61,7 @@ type PeerStatus struct { //nolint:revive // Location is a geo location information of a Peer based on public connection IP type Location struct { - ConnectionIP net.IP // from grpc peer or reverse proxy headers depends on setup + ConnectionIP net.IP `gorm:"serializer:json"` // from grpc peer or reverse proxy headers depends on setup CountryCode string CityName string GeoNameID uint // city level geoname id From 8415064758631b71b69adf51ce19e33b4085fd47 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 29 Apr 2024 23:20:22 +0300 Subject: [PATCH 2/8] migrate net ip field from blob to json --- management/server/migration/migration.go | 94 ++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/management/server/migration/migration.go b/management/server/migration/migration.go index ba31ee45990..60c923352c9 100644 --- a/management/server/migration/migration.go +++ b/management/server/migration/migration.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "net" "strings" log "github.com/sirupsen/logrus" @@ -99,3 +100,96 @@ func MigrateFieldFromGobToJSON[T any, S any](db *gorm.DB, fieldName string) erro return nil } + +// MigrateNetIPFieldFromBlobToJSON migrates a Net IP column from Blob encoding to JSON encoding. +// T is the type of the model that contains the field to be migrated. +// S is the type of the field to be migrated. +func MigrateNetIPFieldFromBlobToJSON[T any](db *gorm.DB, fieldName string, indexName string) error { + oldColumnName := fieldName + newColumnName := fieldName + "_tmp" + + var model T + + if !db.Migrator().HasTable(&model) { + log.Printf("Table for %T does not exist, no migration needed", model) + return nil + } + + stmt := &gorm.Statement{DB: db} + err := stmt.Parse(&model) + if err != nil { + return fmt.Errorf("parse model: %w", err) + } + tableName := stmt.Schema.Table + + var item string + if err := db.Model(&model).Select(oldColumnName).First(&item).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + log.Printf("No records in table %s, no migration needed", tableName) + return nil + } + return fmt.Errorf("fetch first record: %w", err) + } + if len(item) < 0 { + return fmt.Errorf("no records fetched") + } + + var js json.RawMessage + var syntaxError *json.SyntaxError + err = json.Unmarshal([]byte(item), &js) + if err == nil || !errors.As(err, &syntaxError) { + log.Debugf("No migration needed for %s, %s", tableName, fieldName) + return nil + } + + if err := db.Transaction(func(tx *gorm.DB) error { + if err := tx.Exec(fmt.Sprintf("ALTER TABLE %s ADD COLUMN %s TEXT", tableName, newColumnName)).Error; err != nil { + return fmt.Errorf("add column %s: %w", newColumnName, err) + } + + var rows []map[string]any + if err := tx.Table(tableName).Select("id", oldColumnName).Find(&rows).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + log.Printf("No records in table %s, no migration needed", tableName) + return nil + } + return fmt.Errorf("find rows: %w", err) + } + + for _, row := range rows { + blob, ok := row[oldColumnName].(string) + if !ok { + return fmt.Errorf("type assertion failed") + } + + jsonValue, err := json.Marshal(net.IP(blob)) + if err != nil { + return fmt.Errorf("re-encode to JSON: %w", err) + } + + if err := tx.Table(tableName).Where("id = ?", row["id"]).Update(newColumnName, jsonValue).Error; err != nil { + return fmt.Errorf("update row: %w", err) + } + } + + if indexName != "" { + if err := tx.Migrator().DropIndex(&model, indexName); err != nil { + return fmt.Errorf("drop index %s: %w", indexName, err) + } + } + + if err := tx.Exec(fmt.Sprintf("ALTER TABLE %s DROP COLUMN %s", tableName, oldColumnName)).Error; err != nil { + return fmt.Errorf("drop column %s: %w", oldColumnName, err) + } + if err := tx.Exec(fmt.Sprintf("ALTER TABLE %s RENAME COLUMN %s TO %s", tableName, newColumnName, oldColumnName)).Error; err != nil { + return fmt.Errorf("rename column %s to %s: %w", newColumnName, oldColumnName, err) + } + return nil + }); err != nil { + return err + } + + log.Printf("Migration of %s.%s from blob to json completed", tableName, fieldName) + + return nil +} From bf8c6b95def75b305d0dad76f20207d5cab63e10 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 29 Apr 2024 23:20:43 +0300 Subject: [PATCH 3/8] run net ip migration --- management/server/sqlite_store.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/management/server/sqlite_store.go b/management/server/sqlite_store.go index bfde82a6de7..b02e4cdc364 100644 --- a/management/server/sqlite_store.go +++ b/management/server/sqlite_store.go @@ -571,13 +571,17 @@ func getMigrations() []migrationFunc { func(db *gorm.DB) error { return migration.MigrateFieldFromGobToJSON[Account, net.IPNet](db, "network_net") }, - func(db *gorm.DB) error { return migration.MigrateFieldFromGobToJSON[route.Route, netip.Prefix](db, "network") }, - func(db *gorm.DB) error { return migration.MigrateFieldFromGobToJSON[route.Route, []string](db, "peer_groups") }, + func(db *gorm.DB) error { + return migration.MigrateNetIPFieldFromBlobToJSON[nbpeer.Peer](db, "ip", "idx_peers_account_id_ip") + }, + func(db *gorm.DB) error { + return migration.MigrateNetIPFieldFromBlobToJSON[nbpeer.Peer](db, "location_connection_ip", "") + }, } } From 90ebf0017bdd0b6b2dd5aaa49ee6197b5027a9be Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Mon, 29 Apr 2024 23:21:00 +0300 Subject: [PATCH 4/8] remove duplicate index --- management/server/peer/peer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/management/server/peer/peer.go b/management/server/peer/peer.go index ff927a43174..0d0eabb2c0e 100644 --- a/management/server/peer/peer.go +++ b/management/server/peer/peer.go @@ -13,13 +13,13 @@ type Peer struct { // ID is an internal ID of the peer ID string `gorm:"primaryKey"` // AccountID is a reference to Account that this object belongs - AccountID string `json:"-" gorm:"index;uniqueIndex:idx_peers_account_id_ip"` + AccountID string `json:"-" gorm:"index"` // WireGuard public key Key string `gorm:"index"` // A setup key this peer was registered with SetupKey string // IP address of the Peer - IP net.IP `gorm:"uniqueIndex:idx_peers_account_id_ip;serializer:json"` + IP net.IP `gorm:"serializer:json"` // Meta is a Peer system meta data Meta PeerSystemMeta `gorm:"embedded;embeddedPrefix:meta_"` // Name is peer's name (machine name) From 7aff0e828bb6364fd4668f3b0a8ea744d50585a5 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 30 Apr 2024 10:07:40 +0300 Subject: [PATCH 5/8] Refactor --- management/server/migration/migration.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/management/server/migration/migration.go b/management/server/migration/migration.go index 60c923352c9..a14c24eea06 100644 --- a/management/server/migration/migration.go +++ b/management/server/migration/migration.go @@ -103,7 +103,6 @@ func MigrateFieldFromGobToJSON[T any, S any](db *gorm.DB, fieldName string) erro // MigrateNetIPFieldFromBlobToJSON migrates a Net IP column from Blob encoding to JSON encoding. // T is the type of the model that contains the field to be migrated. -// S is the type of the field to be migrated. func MigrateNetIPFieldFromBlobToJSON[T any](db *gorm.DB, fieldName string, indexName string) error { oldColumnName := fieldName newColumnName := fieldName + "_tmp" @@ -130,9 +129,6 @@ func MigrateNetIPFieldFromBlobToJSON[T any](db *gorm.DB, fieldName string, index } return fmt.Errorf("fetch first record: %w", err) } - if len(item) < 0 { - return fmt.Errorf("no records fetched") - } var js json.RawMessage var syntaxError *json.SyntaxError From cb9a8e9fa45679d79e9be88a84567247a5fe44cf Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 30 Apr 2024 13:27:14 +0300 Subject: [PATCH 6/8] Add tests --- management/server/migration/migration_test.go | 69 +++++++++++++++++++ management/server/sqlite_store_test.go | 19 ++++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/management/server/migration/migration_test.go b/management/server/migration/migration_test.go index 4bef41b86a4..d529c88f2d2 100644 --- a/management/server/migration/migration_test.go +++ b/management/server/migration/migration_test.go @@ -13,6 +13,7 @@ import ( "github.com/netbirdio/netbird/management/server" "github.com/netbirdio/netbird/management/server/migration" + nbpeer "github.com/netbirdio/netbird/management/server/peer" "github.com/netbirdio/netbird/route" ) @@ -89,3 +90,71 @@ func TestMigrateFieldFromGobToJSON_WithJSONData(t *testing.T) { db.Model(&server.Account{}).Select("network_net").First(&jsonStr) assert.JSONEq(t, `{"IP":"10.0.0.0","Mask":"////AA=="}`, jsonStr, "Data should be unchanged") } + +func TestMigrateNetIPFieldFromBlobToJSON_EmptyDB(t *testing.T) { + db := setupDatabase(t) + err := migration.MigrateNetIPFieldFromBlobToJSON[nbpeer.Peer](db, "ip", "idx_peers_account_id_ip") + require.NoError(t, err, "Migration should not fail for an empty database") +} + +func TestMigrateNetIPFieldFromBlobToJSON_WithBlobData(t *testing.T) { + db := setupDatabase(t) + + err := db.AutoMigrate(&server.Account{}, &nbpeer.Peer{}) + require.NoError(t, err, "Failed to auto-migrate tables") + + type location struct { + nbpeer.Location + ConnectionIP net.IP + } + + type peer struct { + nbpeer.Peer + Location location `gorm:"embedded;embeddedPrefix:location_"` + } + + type account struct { + server.Account + Peers []peer `gorm:"foreignKey:AccountID;references:id"` + } + + err = db.Save(&account{ + Account: server.Account{Id: "123"}, + Peers: []peer{ + {Location: location{ConnectionIP: net.IP{10, 0, 0, 1}}}, + }}, + ).Error + require.NoError(t, err, "Failed to insert blob data") + + var blobValue string + err = db.Model(&nbpeer.Peer{}).Select("location_connection_ip").First(&blobValue).Error + assert.NoError(t, err, "Failed to fetch blob data") + + err = migration.MigrateNetIPFieldFromBlobToJSON[nbpeer.Peer](db, "location_connection_ip", "") + require.NoError(t, err, "Migration should not fail with net.IP blob data") + + var jsonStr string + db.Model(&nbpeer.Peer{}).Select("location_connection_ip").First(&jsonStr) + assert.JSONEq(t, `"10.0.0.1"`, jsonStr, "Data should be migrated") +} + +func TestMigrateNetIPFieldFromBlobToJSON_WithJSONData(t *testing.T) { + db := setupDatabase(t) + + err := db.AutoMigrate(&server.Account{}, &nbpeer.Peer{}) + require.NoError(t, err, "Failed to auto-migrate tables") + + err = db.Save(&server.Account{ + PeersG: []nbpeer.Peer{ + {Location: nbpeer.Location{ConnectionIP: net.IP{10, 0, 0, 1}}}, + }}, + ).Error + require.NoError(t, err, "Failed to insert JSON data") + + err = migration.MigrateNetIPFieldFromBlobToJSON[nbpeer.Peer](db, "location_connection_ip", "") + require.NoError(t, err, "Migration should not fail with net.IP JSON data") + + var jsonStr string + db.Model(&nbpeer.Peer{}).Select("location_connection_ip").First(&jsonStr) + assert.JSONEq(t, `"10.0.0.1"`, jsonStr, "Data should be unchanged") +} diff --git a/management/server/sqlite_store_test.go b/management/server/sqlite_store_test.go index 8a1bcd10aeb..9fb8824e7e4 100644 --- a/management/server/sqlite_store_test.go +++ b/management/server/sqlite_store_test.go @@ -2,8 +2,6 @@ package server import ( "fmt" - nbdns "github.com/netbirdio/netbird/dns" - nbgroup "github.com/netbirdio/netbird/management/server/group" "math/rand" "net" "net/netip" @@ -12,6 +10,9 @@ import ( "testing" "time" + nbdns "github.com/netbirdio/netbird/dns" + nbgroup "github.com/netbirdio/netbird/management/server/group" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -518,15 +519,29 @@ func TestMigrate(t *testing.T) { Net net.IPNet `gorm:"serializer:gob"` } + type location struct { + nbpeer.Location + ConnectionIP net.IP + } + + type peer struct { + nbpeer.Peer + Location location `gorm:"embedded;embeddedPrefix:location_"` + } + type account struct { Account Network *network `gorm:"embedded;embeddedPrefix:network_"` + Peers []peer `gorm:"foreignKey:AccountID;references:id"` } act := &account{ Network: &network{ Net: *ipnet, }, + Peers: []peer{ + {Location: location{ConnectionIP: net.IP{10, 0, 0, 1}}}, + }, } err = store.db.Save(act).Error From 4df1d556c86d512f9e2ddac6e6edb35775ad09be Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Tue, 30 Apr 2024 13:35:49 +0300 Subject: [PATCH 7/8] fix tests --- management/server/migration/migration_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/management/server/migration/migration_test.go b/management/server/migration/migration_test.go index d529c88f2d2..45757e9d6fb 100644 --- a/management/server/migration/migration_test.go +++ b/management/server/migration/migration_test.go @@ -145,6 +145,7 @@ func TestMigrateNetIPFieldFromBlobToJSON_WithJSONData(t *testing.T) { require.NoError(t, err, "Failed to auto-migrate tables") err = db.Save(&server.Account{ + Id: "1234", PeersG: []nbpeer.Peer{ {Location: nbpeer.Location{ConnectionIP: net.IP{10, 0, 0, 1}}}, }}, From 5b042eee74ef23fa9a804a933213ecb7a25f7569 Mon Sep 17 00:00:00 2001 From: bcmmbaga Date: Fri, 3 May 2024 14:07:54 +0300 Subject: [PATCH 8/8] migrate null blob values --- management/server/migration/migration.go | 35 ++++++++++++++++-------- management/server/sqlite_store.go | 4 +-- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/management/server/migration/migration.go b/management/server/migration/migration.go index a14c24eea06..9776418ad6b 100644 --- a/management/server/migration/migration.go +++ b/management/server/migration/migration.go @@ -1,6 +1,7 @@ package migration import ( + "database/sql" "encoding/gob" "encoding/json" "errors" @@ -121,7 +122,7 @@ func MigrateNetIPFieldFromBlobToJSON[T any](db *gorm.DB, fieldName string, index } tableName := stmt.Schema.Table - var item string + var item sql.NullString if err := db.Model(&model).Select(oldColumnName).First(&item).Error; err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { log.Printf("No records in table %s, no migration needed", tableName) @@ -130,12 +131,14 @@ func MigrateNetIPFieldFromBlobToJSON[T any](db *gorm.DB, fieldName string, index return fmt.Errorf("fetch first record: %w", err) } - var js json.RawMessage - var syntaxError *json.SyntaxError - err = json.Unmarshal([]byte(item), &js) - if err == nil || !errors.As(err, &syntaxError) { - log.Debugf("No migration needed for %s, %s", tableName, fieldName) - return nil + if item.Valid { + var js json.RawMessage + var syntaxError *json.SyntaxError + err = json.Unmarshal([]byte(item.String), &js) + if err == nil || !errors.As(err, &syntaxError) { + log.Debugf("No migration needed for %s, %s", tableName, fieldName) + return nil + } } if err := db.Transaction(func(tx *gorm.DB) error { @@ -153,12 +156,22 @@ func MigrateNetIPFieldFromBlobToJSON[T any](db *gorm.DB, fieldName string, index } for _, row := range rows { - blob, ok := row[oldColumnName].(string) - if !ok { - return fmt.Errorf("type assertion failed") + var blobValue string + if columnValue := row[oldColumnName]; columnValue != nil { + value, ok := columnValue.(string) + if !ok { + return fmt.Errorf("type assertion failed") + } + blobValue = value + } + + columnIpValue := net.IP(blobValue) + if net.ParseIP(columnIpValue.String()) == nil { + log.Debugf("failed to parse %s as ip, fallback to ipv6 loopback", oldColumnName) + columnIpValue = net.IPv6loopback } - jsonValue, err := json.Marshal(net.IP(blob)) + jsonValue, err := json.Marshal(columnIpValue) if err != nil { return fmt.Errorf("re-encode to JSON: %w", err) } diff --git a/management/server/sqlite_store.go b/management/server/sqlite_store.go index b02e4cdc364..36cb9b67190 100644 --- a/management/server/sqlite_store.go +++ b/management/server/sqlite_store.go @@ -578,10 +578,10 @@ func getMigrations() []migrationFunc { return migration.MigrateFieldFromGobToJSON[route.Route, []string](db, "peer_groups") }, func(db *gorm.DB) error { - return migration.MigrateNetIPFieldFromBlobToJSON[nbpeer.Peer](db, "ip", "idx_peers_account_id_ip") + return migration.MigrateNetIPFieldFromBlobToJSON[nbpeer.Peer](db, "location_connection_ip", "") }, func(db *gorm.DB) error { - return migration.MigrateNetIPFieldFromBlobToJSON[nbpeer.Peer](db, "location_connection_ip", "") + return migration.MigrateNetIPFieldFromBlobToJSON[nbpeer.Peer](db, "ip", "idx_peers_account_id_ip") }, } }