From 381447b8d6d272f47d4fa7a133dda941ae123e4e Mon Sep 17 00:00:00 2001
From: Maycon Santos <mlsmaycon@gmail.com>
Date: Tue, 18 Jun 2024 15:39:54 +0200
Subject: [PATCH] Fix store migration on empty string (#2149)

* Fix store migration on empty string

when fetching empty values from the database to check for migration our parser failed to handle null strings preventing the service from start

this uses sql.NullString to handle that and check for empty string resulted from null data

---------

Co-authored-by: Viktor Liu <17948409+lixmal@users.noreply.github.com>
---
 management/server/migration/migration.go | 10 ++++++----
 management/server/sql_store_test.go      | 21 +++++++++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/management/server/migration/migration.go b/management/server/migration/migration.go
index 9776418ad..8a2d4219e 100644
--- a/management/server/migration/migration.go
+++ b/management/server/migration/migration.go
@@ -35,8 +35,8 @@ func MigrateFieldFromGobToJSON[T any, S any](db *gorm.DB, fieldName string) erro
 	}
 	tableName := stmt.Schema.Table
 
-	var item string
-	if err := db.Model(model).Select(oldColumnName).First(&item).Error; err != nil {
+	var sqliteItem sql.NullString
+	if err := db.Model(model).Select(oldColumnName).First(&sqliteItem).Error; err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
 			log.Debugf("No records in table %s, no migration needed", tableName)
 			return nil
@@ -44,10 +44,13 @@ func MigrateFieldFromGobToJSON[T any, S any](db *gorm.DB, fieldName string) erro
 		return fmt.Errorf("fetch first record: %w", err)
 	}
 
+	item := sqliteItem.String
+
 	var js json.RawMessage
 	var syntaxError *json.SyntaxError
 	err = json.Unmarshal([]byte(item), &js)
-	if err == nil || !errors.As(err, &syntaxError) {
+	// if the item is JSON parsable or an empty string it can not be gob encoded
+	if err == nil || !errors.As(err, &syntaxError) || item == "" {
 		log.Debugf("No migration needed for %s, %s", tableName, fieldName)
 		return nil
 	}
@@ -74,7 +77,6 @@ func MigrateFieldFromGobToJSON[T any, S any](db *gorm.DB, fieldName string) erro
 			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)
diff --git a/management/server/sql_store_test.go b/management/server/sql_store_test.go
index fc2743986..a41195206 100644
--- a/management/server/sql_store_test.go
+++ b/management/server/sql_store_test.go
@@ -559,6 +559,7 @@ func TestMigrate(t *testing.T) {
 	rt := &route{
 		Network:    prefix,
 		PeerGroups: []string{"group1", "group2"},
+		Route:      route2.Route{ID: "route1"},
 	}
 
 	err = store.db.Save(rt).Error
@@ -569,6 +570,26 @@ func TestMigrate(t *testing.T) {
 
 	err = migrate(store.db)
 	require.NoError(t, err, "Migration should not fail on migrated db")
+
+	err = store.db.Delete(rt).Where("id = ?", "route1").Error
+	require.NoError(t, err, "Failed to delete Gob data")
+
+	prefix = netip.MustParsePrefix("12.0.0.0/24")
+	nRT := &route2.Route{
+		Network: prefix,
+		ID:      "route2",
+		Peer:    "peer-id",
+	}
+
+	err = store.db.Save(nRT).Error
+	require.NoError(t, err, "Failed to insert json nil slice data")
+
+	err = migrate(store.db)
+	require.NoError(t, err, "Migration should not fail on json nil slice populated db")
+
+	err = migrate(store.db)
+	require.NoError(t, err, "Migration should not fail on migrated db")
+
 }
 
 func newSqliteStore(t *testing.T) *SqlStore {