Skip to content

Commit

Permalink
Migrate serializer:gob fields to serializer:json (#1855)
Browse files Browse the repository at this point in the history
  • Loading branch information
lixmal authored Apr 18, 2024
1 parent e3b7644 commit 77488ad
Show file tree
Hide file tree
Showing 6 changed files with 290 additions and 4 deletions.
101 changes: 101 additions & 0 deletions management/server/migration/migration.go
Original file line number Diff line number Diff line change
@@ -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
}
91 changes: 91 additions & 0 deletions management/server/migration/migration_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
2 changes: 1 addition & 1 deletion management/server/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
39 changes: 38 additions & 1 deletion management/server/sqlite_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package server
import (
"errors"
"fmt"
"net"
"net/netip"
"path/filepath"
"runtime"
"strings"
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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")
},
}
}
57 changes: 57 additions & 0 deletions management/server/sqlite_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"fmt"
"net"
"net/netip"
"path/filepath"
"runtime"
"testing"
Expand All @@ -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"
Expand Down Expand Up @@ -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()

Expand Down
4 changes: 2 additions & 2 deletions route/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 77488ad

Please sign in to comment.