From 0fbf72434e9b91d9a4d0ec4bb82cca93508e919f Mon Sep 17 00:00:00 2001
From: Yury Gargay <yury.gargay@gmail.com>
Date: Tue, 20 Feb 2024 15:06:32 +0100
Subject: [PATCH] Make SQLite default for new installations (#1529)

* Make SQLite default for new installations

* if var is not set, return empty string

this allows getStoreEngineFromDatadir to detect json store files

---------

Co-authored-by: Maycon Santos <mlsmaycon@gmail.com>
---
 .github/workflows/golang-test-darwin.yml  |  2 +-
 .github/workflows/golang-test-linux.yml   |  2 +-
 .github/workflows/golang-test-windows.yml |  1 +
 management/server/sqlite_store.go         |  6 +++++-
 management/server/store.go                | 25 +++++++++++++++++------
 5 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/.github/workflows/golang-test-darwin.yml b/.github/workflows/golang-test-darwin.yml
index 77a907185..202e462cd 100644
--- a/.github/workflows/golang-test-darwin.yml
+++ b/.github/workflows/golang-test-darwin.yml
@@ -36,4 +36,4 @@ jobs:
         run: go mod tidy
 
       - name: Test
-        run: NETBIRD_STORE_ENGINE=${{ matrix.store }} go test -exec 'sudo --preserve-env=CI' -timeout 5m -p 1 ./...
+        run: NETBIRD_STORE_ENGINE=${{ matrix.store }} go test -exec 'sudo --preserve-env=CI,NETBIRD_STORE_ENGINE' -timeout 5m -p 1 ./...
diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml
index 1f83e856d..2e9941349 100644
--- a/.github/workflows/golang-test-linux.yml
+++ b/.github/workflows/golang-test-linux.yml
@@ -42,7 +42,7 @@ jobs:
         run: go mod tidy
 
       - name: Test
-        run: CGO_ENABLED=1 GOARCH=${{ matrix.arch }} NETBIRD_STORE_ENGINE=${{ matrix.store }} go test -exec 'sudo --preserve-env=CI' -timeout 5m -p 1 ./...
+        run: CGO_ENABLED=1 GOARCH=${{ matrix.arch }} NETBIRD_STORE_ENGINE=${{ matrix.store }} go test -exec 'sudo --preserve-env=CI,NETBIRD_STORE_ENGINE' -timeout 5m -p 1 ./...
 
   test_client_on_docker:
     runs-on: ubuntu-20.04
diff --git a/.github/workflows/golang-test-windows.yml b/.github/workflows/golang-test-windows.yml
index 6027d3626..a50c81918 100644
--- a/.github/workflows/golang-test-windows.yml
+++ b/.github/workflows/golang-test-windows.yml
@@ -44,6 +44,7 @@ jobs:
 
       - run: PsExec64 -s -w ${{ github.workspace }} C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe env -w GOMODCACHE=C:\Users\runneradmin\go\pkg\mod
       - run: PsExec64 -s -w ${{ github.workspace }} C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe env -w GOCACHE=C:\Users\runneradmin\AppData\Local\go-build
+      - run: "[Environment]::SetEnvironmentVariable('NETBIRD_STORE_ENGINE', 'jsonfile', 'Machine')"
 
       - name: test
         run: PsExec64 -s -w ${{ github.workspace }} cmd.exe /c "C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe test -timeout 5m -p 1 ./... > test-out.txt 2>&1"
diff --git a/management/server/sqlite_store.go b/management/server/sqlite_store.go
index 8e43b3e6b..ab6f88c2b 100644
--- a/management/server/sqlite_store.go
+++ b/management/server/sqlite_store.go
@@ -485,7 +485,11 @@ func (s *SqliteStore) SaveUserLastLogin(accountID, userID string, lastLogin time
 
 // Close is noop in Sqlite
 func (s *SqliteStore) Close() error {
-	return nil
+	sql, err := s.db.DB()
+	if err != nil {
+		return err
+	}
+	return sql.Close()
 }
 
 // GetStoreEngine returns SqliteStoreEngine
diff --git a/management/server/store.go b/management/server/store.go
index 3a96c3240..e3a945c64 100644
--- a/management/server/store.go
+++ b/management/server/store.go
@@ -3,6 +3,7 @@ package server
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -50,10 +51,10 @@ const (
 )
 
 func getStoreEngineFromEnv() StoreEngine {
-	// NETBIRD_STORE_ENGINE supposed to be used in tests. Otherwise rely on the config file.
+	// NETBIRD_STORE_ENGINE supposed to be used in tests. Otherwise, rely on the config file.
 	kind, ok := os.LookupEnv("NETBIRD_STORE_ENGINE")
 	if !ok {
-		return FileStoreEngine
+		return ""
 	}
 
 	value := StoreEngine(strings.ToLower(kind))
@@ -62,13 +63,26 @@ func getStoreEngineFromEnv() StoreEngine {
 		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) {
 	if kind == "" {
-		// fallback to env. Normally this only should be used from tests
+		// 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)
+		}
 	}
 	switch kind {
 	case FileStoreEngine:
@@ -82,15 +96,14 @@ func NewStore(kind StoreEngine, dataDir string, metrics telemetry.AppMetrics) (S
 	}
 }
 
+// NewStoreFromJson is only used in tests
 func NewStoreFromJson(dataDir string, metrics telemetry.AppMetrics) (Store, error) {
 	fstore, err := NewFileStore(dataDir, nil)
 	if err != nil {
 		return nil, err
 	}
 
-	kind := getStoreEngineFromEnv()
-
-	switch kind {
+	switch kind := getStoreEngineFromEnv(); kind {
 	case FileStoreEngine:
 		return fstore, nil
 	case SqliteStoreEngine: