Deprecate FileStore engine support (#2119)

* Removejsonfile' from test matrix in workflows

* Remove sqlite to json migration command

* Refactor store engine implementation to remove JSON file store support

The codebase has been refactored to remove support for JSON file store storage engine, with SQLite serving as the default store engine. New functions have been added to handle unsupported store engines and to migrate data from file store to SQLite.

* Remove 'downCmd' from migration commands

* Refactoring

* Add sqlite cleanup

* Remove comment

.github/workflows/golang-test-darwin.yml

@@ -14,7 +14,7 @@ jobs:
   test:
     strategy:
       matrix:
-        store: ['jsonfile', 'sqlite']
+        store: ['sqlite']
     runs-on: macos-latest
     steps:
       - name: Install Go

.github/workflows/golang-test-linux.yml

@@ -15,7 +15,7 @@ jobs:
     strategy:
       matrix:
         arch: [ '386','amd64' ]
-        store: [ 'jsonfile', 'sqlite', 'postgres']
+        store: [ 'sqlite', 'postgres']
     runs-on: ubuntu-latest
     steps:
       - name: Install Go

client/internal/engine_test.go

@@ -817,7 +817,7 @@ func TestEngine_MultiplePeers(t *testing.T) {
 		return
 	}
 	defer sigServer.Stop()
-	mgmtServer, mgmtAddr, err := startManagement(dir)
+	mgmtServer, mgmtAddr, err := startManagement(t, dir)
 	if err != nil {
 		t.Fatal(err)
 		return
@@ -1037,7 +1037,9 @@ func startSignal(t *testing.T) (*grpc.Server, string, error) {
 	return s, lis.Addr().String(), nil
 }
 
-func startManagement(dataDir string) (*grpc.Server, string, error) {
+func startManagement(t *testing.T, dataDir string) (*grpc.Server, string, error) {
+	t.Helper()
+
 	config := &server.Config{
 		Stuns:      []*server.Host{},
 		TURNConfig: &server.TURNConfig{},
@@ -1054,10 +1056,12 @@ func startManagement(dataDir string) (*grpc.Server, string, error) {
 		return nil, "", err
 	}
 	s := grpc.NewServer(grpc.KeepaliveEnforcementPolicy(kaep), grpc.KeepaliveParams(kasp))
-	store, _, err := server.NewTestStoreFromJson(config.Datadir)
+
+	store, cleanUp, err := server.NewTestStoreFromJson(config.Datadir)
 	if err != nil {
 		return nil, "", err
 	}
+	t.Cleanup(cleanUp)
 
 	peersUpdateManager := server.NewPeersUpdateManager(nil)
 	eventStore := &activity.InMemoryEventStore{}

management/cmd/migration_down.go

@@ -1,67 +0,0 @@
-package cmd
-
-import (
-	"errors"
-	"flag"
-	"fmt"
-	"os"
-	"path"
-
-	log "github.com/sirupsen/logrus"
-	"github.com/spf13/cobra"
-
-	"github.com/netbirdio/netbird/management/server"
-	"github.com/netbirdio/netbird/util"
-)
-
-var shortDown = "Rollback SQLite store to JSON file store. Please make a backup of the SQLite file before running this command."
-
-var downCmd = &cobra.Command{
-	Use:     "downgrade [--datadir directory] [--log-file console]",
-	Aliases: []string{"down"},
-	Short:   shortDown,
-	Long: shortDown +
-		"\n\n" +
-		"This command reads the content of {datadir}/store.db and migrates it to {datadir}/store.json that can be used by File store driver.",
-	RunE: func(cmd *cobra.Command, args []string) error {
-		flag.Parse()
-		err := util.InitLog(logLevel, logFile)
-		if err != nil {
-			return fmt.Errorf("failed initializing log %v", err)
-		}
-
-		sqliteStorePath := path.Join(mgmtDataDir, "store.db")
-		if _, err := os.Stat(sqliteStorePath); errors.Is(err, os.ErrNotExist) {
-			return fmt.Errorf("%s doesn't exist, couldn't continue the operation", sqliteStorePath)
-		}
-
-		fileStorePath := path.Join(mgmtDataDir, "store.json")
-		if _, err := os.Stat(fileStorePath); err == nil {
-			return fmt.Errorf("%s already exists, couldn't continue the operation", fileStorePath)
-		}
-
-		sqlStore, err := server.NewSqliteStore(mgmtDataDir, nil)
-		if err != nil {
-			return fmt.Errorf("failed creating file store: %s: %v", mgmtDataDir, err)
-		}
-
-		sqliteStoreAccounts := len(sqlStore.GetAllAccounts())
-		log.Infof("%d account will be migrated from sqlite store %s to file store %s",
-			sqliteStoreAccounts, sqliteStorePath, fileStorePath)
-
-		store, err := server.NewFilestoreFromSqliteStore(sqlStore, mgmtDataDir, nil)
-		if err != nil {
-			return fmt.Errorf("failed creating file store: %s: %v", mgmtDataDir, err)
-		}
-
-		fsStoreAccounts := len(store.GetAllAccounts())
-		if fsStoreAccounts != sqliteStoreAccounts {
-			return fmt.Errorf("failed to migrate accounts from sqlite to file[]. Expected accounts: %d, got: %d",
-				sqliteStoreAccounts, fsStoreAccounts)
-		}
-
-		log.Info("Migration finished successfully")
-
-		return nil
-	},
-}

management/cmd/migration_up.go

@@ -1,11 +1,8 @@
 package cmd
 
 import (
-	"errors"
 	"flag"
 	"fmt"
-	"os"
-	"path"
 
 	"github.com/netbirdio/netbird/management/server"
 	"github.com/netbirdio/netbird/util"
@@ -29,36 +26,9 @@ var upCmd = &cobra.Command{
 			return fmt.Errorf("failed initializing log %v", err)
 		}
 
-		fileStorePath := path.Join(mgmtDataDir, "store.json")
-		if _, err := os.Stat(fileStorePath); errors.Is(err, os.ErrNotExist) {
-			return fmt.Errorf("%s doesn't exist, couldn't continue the operation", fileStorePath)
+		if err := server.MigrateFileStoreToSqlite(mgmtDataDir); err != nil {
+			return err
 		}
-
-		sqlStorePath := path.Join(mgmtDataDir, "store.db")
-		if _, err := os.Stat(sqlStorePath); err == nil {
-			return fmt.Errorf("%s already exists, couldn't continue the operation", sqlStorePath)
-		}
-
-		fstore, err := server.NewFileStore(mgmtDataDir, nil)
-		if err != nil {
-			return fmt.Errorf("failed creating file store: %s: %v", mgmtDataDir, err)
-		}
-
-		fsStoreAccounts := len(fstore.GetAllAccounts())
-		log.Infof("%d account will be migrated from file store %s to sqlite store %s",
-			fsStoreAccounts, fileStorePath, sqlStorePath)
-
-		store, err := server.NewSqliteStoreFromFileStore(fstore, mgmtDataDir, nil)
-		if err != nil {
-			return fmt.Errorf("failed creating file store: %s: %v", mgmtDataDir, err)
-		}
-
-		sqliteStoreAccounts := len(store.GetAllAccounts())
-		if fsStoreAccounts != sqliteStoreAccounts {
-			return fmt.Errorf("failed to migrate accounts from file to sqlite. Expected accounts: %d, got: %d",
-				fsStoreAccounts, sqliteStoreAccounts)
-		}
-
 		log.Info("Migration finished successfully")
 
 		return nil

management/cmd/root.go

@@ -75,7 +75,6 @@ func init() {
 	migrationCmd.MarkFlagRequired("datadir") //nolint
 
 	migrationCmd.AddCommand(upCmd)
-	migrationCmd.AddCommand(downCmd)
 
 	rootCmd.AddCommand(migrationCmd)
 }

management/server/sql_store.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -27,6 +28,10 @@ import (
 	"github.com/netbirdio/netbird/route"
 )
 
+const (
+	storeSqliteFileName = "store.db"
+)
+
 // SqlStore represents an account storage backed by a Sql DB persisted to disk
 type SqlStore struct {
 	db                *gorm.DB
@@ -623,10 +628,10 @@ func (s *SqlStore) GetStoreEngine() StoreEngine {
 
 // NewSqliteStore creates a new SQLite store.
 func NewSqliteStore(dataDir string, metrics telemetry.AppMetrics) (*SqlStore, error) {
-	storeStr := "store.db?cache=shared"
+	storeStr := fmt.Sprintf("%s?cache=shared", storeSqliteFileName)
 	if runtime.GOOS == "windows" {
 		// Vo avoid `The process cannot access the file because it is being used by another process` on Windows
-		storeStr = "store.db"
+		storeStr = storeSqliteFileName
 	}
 
 	file := filepath.Join(dataDir, storeStr)
@@ -655,6 +660,15 @@ func NewPostgresqlStore(dsn string, metrics telemetry.AppMetrics) (*SqlStore, er
 	return NewSqlStore(db, PostgresStoreEngine, metrics)
 }
 
+// newPostgresStore initializes a new Postgres store.
+func newPostgresStore(metrics telemetry.AppMetrics) (Store, error) {
+	dsn, ok := os.LookupEnv(postgresDsnEnv)
+	if !ok {
+		return nil, fmt.Errorf("%s is not set", postgresDsnEnv)
+	}
+	return NewPostgresqlStore(dsn, metrics)
+}
+
 // NewSqliteStoreFromFileStore restores a store from FileStore and stores SQLite DB in the file located in datadir.
 func NewSqliteStoreFromFileStore(fileStore *FileStore, dataDir string, metrics telemetry.AppMetrics) (*SqlStore, error) {
 	store, err := NewSqliteStore(dataDir, metrics)

management/server/store.go

@@ -1,21 +1,24 @@
 package server
 
 import (
+	"errors"
 	"fmt"
 	"net"
 	"net/netip"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"time"
 
+	"github.com/netbirdio/netbird/management/server/telemetry"
+	"github.com/netbirdio/netbird/util"
 	log "github.com/sirupsen/logrus"
 	"gorm.io/gorm"
 
 	"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/telemetry"
 	"github.com/netbirdio/netbird/management/server/testutil"
 	"github.com/netbirdio/netbird/route"
 )
@@ -76,50 +79,74 @@ func getStoreEngineFromEnv() StoreEngine {
 	}
 
 	value := StoreEngine(strings.ToLower(kind))
-	if value == FileStoreEngine || value == SqliteStoreEngine || value == PostgresStoreEngine {
+	if value == SqliteStoreEngine || value == PostgresStoreEngine {
 		return value
 	}
 
 	return SqliteStoreEngine
 }
 
-func getStoreEngineFromDatadir(dataDir string) StoreEngine {
-	storeFile := filepath.Join(dataDir, storeFileName)
-	if _, err := os.Stat(storeFile); err != nil {
-		// json file not found then use sqlite as default
-		return SqliteStoreEngine
-	}
-	return FileStoreEngine
-}
-
-func NewStore(kind StoreEngine, dataDir string, metrics telemetry.AppMetrics) (Store, error) {
+// getStoreEngine determines the store engine to use
+func getStoreEngine(kind StoreEngine) StoreEngine {
 	if kind == "" {
-		// if store engine is not set in the config we first try to evaluate NETBIRD_STORE_ENGINE
 		kind = getStoreEngineFromEnv()
 		if kind == "" {
-			// NETBIRD_STORE_ENGINE is not set we evaluate default based on dataDir
-			kind = getStoreEngineFromDatadir(dataDir)
+			kind = SqliteStoreEngine
 		}
 	}
+	return kind
+}
+
+// NewStore creates a new store based on the provided engine type, data directory, and telemetry metrics
+func NewStore(kind StoreEngine, dataDir string, metrics telemetry.AppMetrics) (Store, error) {
+	kind = getStoreEngine(kind)
+
+	if err := checkFileStoreEngine(kind, dataDir); err != nil {
+		return nil, err
+	}
+
 	switch kind {
-	case FileStoreEngine:
-		log.Info("using JSON file store engine")
-		return NewFileStore(dataDir, metrics)
 	case SqliteStoreEngine:
 		log.Info("using SQLite store engine")
 		return NewSqliteStore(dataDir, metrics)
 	case PostgresStoreEngine:
 		log.Info("using Postgres store engine")
-		dsn, ok := os.LookupEnv(postgresDsnEnv)
-		if !ok {
-			return nil, fmt.Errorf("%s is not set", postgresDsnEnv)
-		}
-		return NewPostgresqlStore(dsn, metrics)
+		return newPostgresStore(metrics)
 	default:
-		return nil, fmt.Errorf("unsupported kind of store %s", kind)
+		return handleUnsupportedStoreEngine(kind, dataDir, metrics)
 	}
 }
 
+func checkFileStoreEngine(kind StoreEngine, dataDir string) error {
+	if kind == FileStoreEngine {
+		storeFile := filepath.Join(dataDir, storeFileName)
+		if util.FileExists(storeFile) {
+			return fmt.Errorf("%s is not supported. Please refer to the documentation for migrating to SQLite: "+
+				"https://docs.netbird.io/selfhosted/sqlite-store#migrating-from-json-store-to-sq-lite-store", FileStoreEngine)
+		}
+	}
+	return nil
+}
+
+// handleUnsupportedStoreEngine handles cases where the store engine is unsupported
+func handleUnsupportedStoreEngine(kind StoreEngine, dataDir string, metrics telemetry.AppMetrics) (Store, error) {
+	jsonStoreFile := filepath.Join(dataDir, storeFileName)
+	sqliteStoreFile := filepath.Join(dataDir, storeSqliteFileName)
+
+	if util.FileExists(jsonStoreFile) && !util.FileExists(sqliteStoreFile) {
+		log.Warnf("unsupported store engine, but found %s. Automatically migrating to SQLite.", jsonStoreFile)
+
+		if err := MigrateFileStoreToSqlite(dataDir); err != nil {
+			return nil, fmt.Errorf("failed to migrate data to SQLite store: %w", err)
+		}
+
+		log.Info("using SQLite store engine")
+		return NewSqliteStore(dataDir, metrics)
+	}
+
+	return nil, fmt.Errorf("unsupported kind of store: %s", kind)
+}
+
 // migrate migrates the SQLite database to the latest schema
 func migrate(db *gorm.DB) error {
 	migrations := getMigrations()
@@ -160,25 +187,18 @@ func NewTestStoreFromJson(dataDir string) (Store, func(), error) {
 		return nil, nil, err
 	}
 
-	cleanUp := func() {}
-
 	// if store engine is not set in the config we first try to evaluate NETBIRD_STORE_ENGINE
 	kind := getStoreEngineFromEnv()
 	if kind == "" {
-		// NETBIRD_STORE_ENGINE is not set we evaluate default based on dataDir
-		kind = getStoreEngineFromDatadir(dataDir)
+		kind = SqliteStoreEngine
 	}
 
-	switch kind {
-	case FileStoreEngine:
-		return fstore, cleanUp, nil
-	case SqliteStoreEngine:
-		store, err := NewSqliteStoreFromFileStore(fstore, dataDir, nil)
-		if err != nil {
-			return nil, nil, err
-		}
-		return store, cleanUp, nil
-	case PostgresStoreEngine:
+	var (
+		store   Store
+		cleanUp func()
+	)
+
+	if kind == PostgresStoreEngine {
 		cleanUp, err = testutil.CreatePGDB()
 		if err != nil {
 			return nil, nil, err
@@ -189,16 +209,52 @@ func NewTestStoreFromJson(dataDir string) (Store, func(), error) {
 			return nil, nil, fmt.Errorf("%s is not set", postgresDsnEnv)
 		}
 
-		store, err := NewPostgresqlStoreFromFileStore(fstore, dsn, nil)
+		store, err = NewPostgresqlStoreFromFileStore(fstore, dsn, nil)
 		if err != nil {
 			return nil, nil, err
 		}
-		return store, cleanUp, nil
-	default:
-		store, err := NewSqliteStoreFromFileStore(fstore, dataDir, nil)
+	} else {
+		store, err = NewSqliteStoreFromFileStore(fstore, dataDir, nil)
 		if err != nil {
 			return nil, nil, err
 		}
-		return store, cleanUp, nil
+		cleanUp = func() { store.Close() }
 	}
+
+	return store, cleanUp, nil
+}
+
+// MigrateFileStoreToSqlite migrates the file store to the SQLite store.
+func MigrateFileStoreToSqlite(dataDir string) error {
+	fileStorePath := path.Join(dataDir, storeFileName)
+	if _, err := os.Stat(fileStorePath); errors.Is(err, os.ErrNotExist) {
+		return fmt.Errorf("%s doesn't exist, couldn't continue the operation", fileStorePath)
+	}
+
+	sqlStorePath := path.Join(dataDir, storeSqliteFileName)
+	if _, err := os.Stat(sqlStorePath); err == nil {
+		return fmt.Errorf("%s already exists, couldn't continue the operation", sqlStorePath)
+	}
+
+	fstore, err := NewFileStore(dataDir, nil)
+	if err != nil {
+		return fmt.Errorf("failed creating file store: %s: %v", dataDir, err)
+	}
+
+	fsStoreAccounts := len(fstore.GetAllAccounts())
+	log.Infof("%d account will be migrated from file store %s to sqlite store %s",
+		fsStoreAccounts, fileStorePath, sqlStorePath)
+
+	store, err := NewSqliteStoreFromFileStore(fstore, dataDir, nil)
+	if err != nil {
+		return fmt.Errorf("failed creating file store: %s: %v", dataDir, err)
+	}
+
+	sqliteStoreAccounts := len(store.GetAllAccounts())
+	if fsStoreAccounts != sqliteStoreAccounts {
+		return fmt.Errorf("failed to migrate accounts from file to sqlite. Expected accounts: %d, got: %d",
+			fsStoreAccounts, sqliteStoreAccounts)
+	}
+
+	return nil
 }