Migrate serializer:gob fields to serializer:json (#1855)

management/server/migration/migration.go

@@ -0,0 +1,101 @@
+package migration
+
+import (
+	"encoding/gob"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"strings"
+
+	log "github.com/sirupsen/logrus"
+	"gorm.io/gorm"
+)
+
+// MigrateFieldFromGobToJSON migrates a column from Gob 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 MigrateFieldFromGobToJSON[T any, S any](db *gorm.DB, fieldName string) error {
+
+	oldColumnName := fieldName
+	newColumnName := fieldName + "_tmp"
+
+	var model T
+
+	if !db.Migrator().HasTable(&model) {
+		log.Debugf("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.Debugf("No records in table %s, no migration needed", tableName)
+			return nil
+		}
+		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 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 {
+			return fmt.Errorf("find rows: %w", err)
+		}
+
+		for _, row := range rows {
+			var field S
+
+			str, ok := row[oldColumnName].(string)
+			if !ok {
+				return fmt.Errorf("type assertion failed")
+			}
+			reader := strings.NewReader(str)
+
+			if err := gob.NewDecoder(reader).Decode(&field); err != nil {
+				return fmt.Errorf("gob decode error: %w", err)
+			}
+
+			jsonValue, err := json.Marshal(field)
+			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 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.Infof("Migration of %s.%s from gob to json completed", tableName, fieldName)
+
+	return nil
+}

management/server/migration/migration_test.go

@@ -0,0 +1,91 @@
+package migration_test
+
+import (
+	"encoding/gob"
+	"net"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gorm.io/driver/sqlite"
+	"gorm.io/gorm"
+
+	"github.com/netbirdio/netbird/management/server"
+	"github.com/netbirdio/netbird/management/server/migration"
+	"github.com/netbirdio/netbird/route"
+)
+
+func setupDatabase(t *testing.T) *gorm.DB {
+	t.Helper()
+
+	db, err := gorm.Open(sqlite.Open("file::memory:?cache=shared"), &gorm.Config{
+		PrepareStmt: true,
+	})
+
+	require.NoError(t, err, "Failed to open database")
+	return db
+}
+
+func TestMigrateFieldFromGobToJSON_EmptyDB(t *testing.T) {
+	db := setupDatabase(t)
+	err := migration.MigrateFieldFromGobToJSON[server.Account, net.IPNet](db, "network_net")
+	require.NoError(t, err, "Migration should not fail for an empty database")
+}
+
+func TestMigrateFieldFromGobToJSON_WithGobData(t *testing.T) {
+	db := setupDatabase(t)
+
+	err := db.AutoMigrate(&server.Account{}, &route.Route{})
+	require.NoError(t, err, "Failed to auto-migrate tables")
+
+	_, ipnet, err := net.ParseCIDR("10.0.0.0/24")
+	require.NoError(t, err, "Failed to parse CIDR")
+
+	type network struct {
+		server.Network
+		Net net.IPNet `gorm:"serializer:gob"`
+	}
+
+	type account struct {
+		server.Account
+		Network *network `gorm:"embedded;embeddedPrefix:network_"`
+	}
+
+	err = db.Save(&account{Account: server.Account{Id: "123"}, Network: &network{Net: *ipnet}}).Error
+	require.NoError(t, err, "Failed to insert Gob data")
+
+	var gobStr string
+	err = db.Model(&server.Account{}).Select("network_net").First(&gobStr).Error
+	assert.NoError(t, err, "Failed to fetch Gob data")
+
+	err = gob.NewDecoder(strings.NewReader(gobStr)).Decode(&ipnet)
+	require.NoError(t, err, "Failed to decode Gob data")
+
+	err = migration.MigrateFieldFromGobToJSON[server.Account, net.IPNet](db, "network_net")
+	require.NoError(t, err, "Migration should not fail with Gob data")
+
+	var jsonStr string
+	db.Model(&server.Account{}).Select("network_net").First(&jsonStr)
+	assert.JSONEq(t, `{"IP":"10.0.0.0","Mask":"////AA=="}`, jsonStr, "Data should be migrated")
+}
+
+func TestMigrateFieldFromGobToJSON_WithJSONData(t *testing.T) {
+	db := setupDatabase(t)
+
+	err := db.AutoMigrate(&server.Account{}, &route.Route{})
+	require.NoError(t, err, "Failed to auto-migrate tables")
+
+	_, ipnet, err := net.ParseCIDR("10.0.0.0/24")
+	require.NoError(t, err, "Failed to parse CIDR")
+
+	err = db.Save(&server.Account{Network: &server.Network{Net: *ipnet}}).Error
+	require.NoError(t, err, "Failed to insert JSON data")
+
+	err = migration.MigrateFieldFromGobToJSON[server.Account, net.IPNet](db, "network_net")
+	require.NoError(t, err, "Migration should not fail with JSON data")
+
+	var jsonStr string
+	db.Model(&server.Account{}).Select("network_net").First(&jsonStr)
+	assert.JSONEq(t, `{"IP":"10.0.0.0","Mask":"////AA=="}`, jsonStr, "Data should be unchanged")
+}

management/server/network.go

@@ -36,7 +36,7 @@ type NetworkMap struct {
 
 type Network struct {
 	Identifier string    `json:"id"`
-	Net        net.IPNet `gorm:"serializer:gob"`
+	Net        net.IPNet `gorm:"serializer:json"`
 	Dns        string
 	// Serial is an ID that increments by 1 when any change to the network happened (e.g. new peer has been added).
 	// Used to synchronize state to the client apps.

management/server/sqlite_store.go

@@ -3,6 +3,8 @@ package server
 import (
 	"errors"
 	"fmt"
+	"net"
+	"net/netip"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -18,6 +20,7 @@ import (
 	nbdns "github.com/netbirdio/netbird/dns"
 	"github.com/netbirdio/netbird/management/server/account"
 	nbgroup "github.com/netbirdio/netbird/management/server/group"
+	"github.com/netbirdio/netbird/management/server/migration"
 	nbpeer "github.com/netbirdio/netbird/management/server/peer"
 	"github.com/netbirdio/netbird/management/server/posture"
 	"github.com/netbirdio/netbird/management/server/status"
@@ -40,6 +43,8 @@ type installation struct {
 	InstallationIDValue string
 }
 
+type migrationFunc func(*gorm.DB) error
+
 // NewSqliteStore restores a store from the file located in the datadir
 func NewSqliteStore(dataDir string, metrics telemetry.AppMetrics) (*SqliteStore, error) {
 	storeStr := "store.db?cache=shared"
@@ -64,13 +69,16 @@ func NewSqliteStore(dataDir string, metrics telemetry.AppMetrics) (*SqliteStore,
 	conns := runtime.NumCPU()
 	sql.SetMaxOpenConns(conns) // TODO: make it configurable
 
+	if err := migrate(db); err != nil {
+		return nil, fmt.Errorf("migrate: %w", err)
+	}
 	err = db.AutoMigrate(
 		&SetupKey{}, &nbpeer.Peer{}, &User{}, &PersonalAccessToken{}, &nbgroup.Group{},
 		&Account{}, &Policy{}, &PolicyRule{}, &route.Route{}, &nbdns.NameServerGroup{},
 		&installation{}, &account.ExtraSettings{}, &posture.Checks{}, &nbpeer.NetworkAddress{},
 	)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("auto migrate: %w", err)
 	}
 
 	return &SqliteStore{db: db, storeFile: file, metrics: metrics, installationPK: 1}, nil
@@ -542,3 +550,32 @@ func (s *SqliteStore) Close() error {
 func (s *SqliteStore) GetStoreEngine() StoreEngine {
 	return SqliteStoreEngine
 }
+
+// migrate migrates the SQLite database to the latest schema
+func migrate(db *gorm.DB) error {
+	migrations := getMigrations()
+
+	for _, m := range migrations {
+		if err := m(db); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func getMigrations() []migrationFunc {
+	return []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")
+		},
+	}
+}

management/server/sqlite_store_test.go

@@ -3,6 +3,7 @@ package server
 import (
 	"fmt"
 	"net"
+	"net/netip"
 	"path/filepath"
 	"runtime"
 	"testing"
@@ -12,6 +13,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	route2 "github.com/netbirdio/netbird/route"
+
 	"github.com/netbirdio/netbird/management/server/status"
 
 	nbpeer "github.com/netbirdio/netbird/management/server/peer"
@@ -349,6 +352,60 @@ func TestSqlite_GetUserByTokenID(t *testing.T) {
 	require.Equal(t, status.NotFound, parsedErr.Type(), "should return not found error")
 }
 
+func TestMigrate(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("The SQLite store is not properly supported by Windows yet")
+	}
+
+	store := newSqliteStore(t)
+
+	err := migrate(store.db)
+	require.NoError(t, err, "Migration should not fail on empty db")
+
+	_, ipnet, err := net.ParseCIDR("10.0.0.0/24")
+	require.NoError(t, err, "Failed to parse CIDR")
+
+	type network struct {
+		Network
+		Net net.IPNet `gorm:"serializer:gob"`
+	}
+
+	type account struct {
+		Account
+		Network *network `gorm:"embedded;embeddedPrefix:network_"`
+	}
+
+	act := &account{
+		Network: &network{
+			Net: *ipnet,
+		},
+	}
+
+	err = store.db.Save(act).Error
+	require.NoError(t, err, "Failed to insert Gob data")
+
+	type route struct {
+		route2.Route
+		Network    netip.Prefix `gorm:"serializer:gob"`
+		PeerGroups []string     `gorm:"serializer:gob"`
+	}
+
+	prefix := netip.MustParsePrefix("11.0.0.0/24")
+	rt := &route{
+		Network:    prefix,
+		PeerGroups: []string{"group1", "group2"},
+	}
+
+	err = store.db.Save(rt).Error
+	require.NoError(t, err, "Failed to insert Gob data")
+
+	err = migrate(store.db)
+	require.NoError(t, err, "Migration should not fail on gob populated db")
+
+	err = migrate(store.db)
+	require.NoError(t, err, "Migration should not fail on migrated db")
+}
+
 func newSqliteStore(t *testing.T) *SqliteStore {
 	t.Helper()
 

route/route.go

@@ -68,11 +68,11 @@ type Route struct {
 	ID string `gorm:"primaryKey"`
 	// AccountID is a reference to Account that this object belongs
 	AccountID   string       `gorm:"index"`
-	Network     netip.Prefix `gorm:"serializer:gob"`
+	Network     netip.Prefix `gorm:"serializer:json"`
 	NetID       string
 	Description string
 	Peer        string
-	PeerGroups  []string `gorm:"serializer:gob"`
+	PeerGroups  []string `gorm:"serializer:json"`
 	NetworkType NetworkType
 	Masquerade  bool
 	Metric      int