Move StoreKind under own StoreConfig configuration and rename to Engine (#1219)

* Move StoreKind under own StoreConfig configuration parameter

* Rename StoreKind option to Engine

* Rename StoreKind internal methods and types to Engine

* Add template engine value test

---------

Co-authored-by: Maycon Santos <mlsmaycon@gmail.com>

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

@@ -14,7 +14,7 @@ jobs:
   test:
     strategy:
       matrix:
-        store: ['JsonFile', 'Sqlite']
+        store: ['jsonfile', 'sqlite']
     runs-on: macos-latest
     steps:
       - name: Install Go
@@ -36,4 +36,4 @@ jobs:
         run: go mod tidy
 
       - name: Test
-        run: NETBIRD_STORE_KIND=${{ 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' -timeout 5m -p 1 ./...

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

@@ -15,7 +15,7 @@ jobs:
     strategy:
       matrix:
         arch: ['386','amd64']
-        store: ['JsonFile', 'Sqlite']
+        store: ['jsonfile', 'sqlite']
     runs-on: ubuntu-latest
     steps:
       - name: Install Go
@@ -42,7 +42,7 @@ jobs:
         run: go mod tidy
 
       - name: Test
-        run: CGO_ENABLED=1 GOARCH=${{ matrix.arch }} NETBIRD_STORE_KIND=${{ 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' -timeout 5m -p 1 ./...
 
   test_client_on_docker:
     runs-on: ubuntu-20.04
@@ -102,10 +102,10 @@ jobs:
         run: docker run -t --cap-add=NET_ADMIN --privileged --rm -v $PWD:/ci -w /ci/client/firewall --entrypoint /busybox/sh gcr.io/distroless/base:debug -c /ci/nftablesmanager-testing.bin  -test.timeout 5m -test.parallel 1
 
       - name: Run Engine tests in docker with file store
-        run: docker run -t --cap-add=NET_ADMIN --privileged --rm -v $PWD:/ci -w /ci/client/internal -e NETBIRD_STORE_KIND="JsonFile" --entrypoint /busybox/sh gcr.io/distroless/base:debug -c /ci/engine-testing.bin  -test.timeout 5m -test.parallel 1
+        run: docker run -t --cap-add=NET_ADMIN --privileged --rm -v $PWD:/ci -w /ci/client/internal -e NETBIRD_STORE_ENGINE="jsonfile" --entrypoint /busybox/sh gcr.io/distroless/base:debug -c /ci/engine-testing.bin  -test.timeout 5m -test.parallel 1
       
       - name: Run Engine tests in docker with sqlite store
-        run: docker run -t --cap-add=NET_ADMIN --privileged --rm -v $PWD:/ci -w /ci/client/internal -e NETBIRD_STORE_KIND="Sqlite" --entrypoint /busybox/sh gcr.io/distroless/base:debug -c /ci/engine-testing.bin  -test.timeout 5m -test.parallel 1
+        run: docker run -t --cap-add=NET_ADMIN --privileged --rm -v $PWD:/ci -w /ci/client/internal -e NETBIRD_STORE_ENGINE="sqlite" --entrypoint /busybox/sh gcr.io/distroless/base:debug -c /ci/engine-testing.bin  -test.timeout 5m -test.parallel 1
 
       - name: Run Peer tests in docker
         run: docker run -t --cap-add=NET_ADMIN --privileged --rm -v $PWD:/ci -w /ci/client/internal/peer  --entrypoint /busybox/sh gcr.io/distroless/base:debug -c /ci/peer-testing.bin  -test.timeout 5m -test.parallel 1

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

@@ -16,7 +16,7 @@ jobs:
   test:
     strategy:
       matrix:
-        store: ['JsonFile', 'Sqlite']
+        store: ['jsonfile', 'sqlite']
     runs-on: windows-latest
     steps:
       - name: Checkout code

.github/workflows/test-infrastructure-files.yml

@@ -56,6 +56,7 @@ jobs:
           CI_NETBIRD_IDP_MGMT_CLIENT_ID: testing.client.id
           CI_NETBIRD_IDP_MGMT_CLIENT_SECRET: testing.client.secret
           CI_NETBIRD_AUTH_SUPPORTED_SCOPES: "openid profile email offline_access api email_verified"
+          CI_NETBIRD_STORE_CONFIG_ENGINE: "sqlite"
 
       - name: check values
         working-directory: infrastructure_files
@@ -81,6 +82,7 @@ jobs:
           CI_NETBIRD_IDP_MGMT_CLIENT_ID: testing.client.id
           CI_NETBIRD_IDP_MGMT_CLIENT_SECRET: testing.client.secret
           CI_NETBIRD_SIGNAL_PORT: 12345
+          CI_NETBIRD_STORE_CONFIG_ENGINE: "sqlite"
 
         run: |
           grep AUTH_CLIENT_ID docker-compose.yml | grep $CI_NETBIRD_AUTH_CLIENT_ID
@@ -97,7 +99,8 @@ jobs:
           grep NETBIRD_TOKEN_SOURCE docker-compose.yml | grep $CI_NETBIRD_TOKEN_SOURCE
           grep AuthUserIDClaim management.json | grep $CI_NETBIRD_AUTH_USER_ID_CLAIM
           grep -A 3 DeviceAuthorizationFlow management.json | grep -A 1 ProviderConfig | grep Audience | grep $CI_NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE
-          grep -A 8 DeviceAuthorizationFlow management.json | grep -A 6 ProviderConfig | grep Scope | grep "$CI_NETBIRD_AUTH_DEVICE_AUTH_SCOPE"
+          grep -A 3 DeviceAuthorizationFlow management.json | grep -A 1 ProviderConfig | grep Audience | grep $CI_NETBIRD_AUTH_DEVICE_AUTH_AUDIENCE
+          grep Engine management.json  | grep "$CI_NETBIRD_STORE_CONFIG_ENGINE"
           grep UseIDToken management.json | grep false
           grep -A 1 IdpManagerConfig management.json | grep ManagerType | grep $CI_NETBIRD_MGMT_IDP 
           grep -A 3 IdpManagerConfig management.json | grep -A 1 ClientConfig | grep Issuer | grep $CI_NETBIRD_AUTH_AUTHORITY

infrastructure_files/base.setup.env

@@ -55,6 +55,9 @@ NETBIRD_AUTH_PKCE_AUDIENCE=$NETBIRD_AUTH_AUDIENCE
 NETBIRD_DASH_AUTH_USE_AUDIENCE=${NETBIRD_DASH_AUTH_USE_AUDIENCE:-true}
 NETBIRD_DASH_AUTH_AUDIENCE=$NETBIRD_AUTH_AUDIENCE
 
+# Store config
+NETBIRD_STORE_CONFIG_ENGINE=${NETBIRD_STORE_CONFIG_ENGINE:-"jsonfile"}
+
 # exports
 export NETBIRD_DOMAIN
 export NETBIRD_AUTH_CLIENT_ID
@@ -97,4 +100,5 @@ export NETBIRD_AUTH_PKCE_AUTHORIZATION_ENDPOINT
 export NETBIRD_AUTH_PKCE_USE_ID_TOKEN
 export NETBIRD_AUTH_PKCE_AUDIENCE
 export NETBIRD_DASH_AUTH_USE_AUDIENCE
-export NETBIRD_DASH_AUTH_AUDIENCE
+export NETBIRD_DASH_AUTH_AUDIENCE
+export NETBIRD_STORE_CONFIG_ENGINE

infrastructure_files/management.json.tmpl

@@ -28,6 +28,9 @@
     },
     "Datadir": "",
     "DataStoreEncryptionKey": "$NETBIRD_DATASTORE_ENC_KEY",
+    "StoreConfig": {
+        "Engine": "$NETBIRD_STORE_CONFIG_ENGINE"
+    },
     "HttpConfig": {
         "Address": "0.0.0.0:$NETBIRD_MGMT_API_PORT",
         "AuthIssuer": "$NETBIRD_AUTH_AUTHORITY",

infrastructure_files/tests/setup.env

@@ -22,4 +22,5 @@ NETBIRD_AUTH_DEVICE_AUTH_SCOPE="openid email"
 NETBIRD_MGMT_IDP=$CI_NETBIRD_MGMT_IDP
 NETBIRD_IDP_MGMT_CLIENT_ID=$CI_NETBIRD_IDP_MGMT_CLIENT_ID
 NETBIRD_IDP_MGMT_CLIENT_SECRET=$CI_NETBIRD_IDP_MGMT_CLIENT_SECRET
-NETBIRD_SIGNAL_PORT=12345
+NETBIRD_SIGNAL_PORT=12345
+NETBIRD_STORE_CONFIG_ENGINE=$CI_NETBIRD_STORE_CONFIG_ENGINE

management/cmd/management.go

@@ -126,7 +126,7 @@ var (
 			if err != nil {
 				return err
 			}
-			store, err := server.NewStore(config.StoreKind, config.Datadir, appMetrics)
+			store, err := server.NewStore(config.StoreConfig.Engine, config.Datadir, appMetrics)
 			if err != nil {
 				return fmt.Errorf("failed creating Store: %s: %v", config.Datadir, err)
 			}

management/server/config.go

@@ -46,7 +46,7 @@ type Config struct {
 
 	PKCEAuthorizationFlow *PKCEAuthorizationFlow
 
-	StoreKind StoreKind
+	StoreConfig StoreConfig
 }
 
 // GetAuthAudiences returns the audience from the http config and device authorization flow config
@@ -138,6 +138,11 @@ type ProviderConfig struct {
 	RedirectURLs []string
 }
 
+// StoreConfig contains Store configuration
+type StoreConfig struct {
+	Engine StoreEngine
+}
+
 // validateURL validates input http url
 func validateURL(httpURL string) bool {
 	_, err := url.ParseRequestURI(httpURL)

management/server/file_store.go

@@ -615,7 +615,7 @@ func (s *FileStore) Close() error {
 	return s.persist(s.storeFile)
 }
 
-// GetStoreKind returns FileStoreKind
+// GetStoreEngine returns FileStoreEngine
-func (s *FileStore) GetStoreKind() StoreKind {
+func (s *FileStore) GetStoreEngine() StoreEngine {
-	return FileStoreKind
+	return FileStoreEngine
 }

management/server/metrics/selfhosted.go

@@ -48,7 +48,7 @@ type properties map[string]interface{}
 // DataSource metric data source
 type DataSource interface {
 	GetAllAccounts() []*server.Account
-	GetStoreKind() server.StoreKind
+	GetStoreEngine() server.StoreEngine
 }
 
 // ConnManager peer connection manager that holds state for current active connections
@@ -296,7 +296,7 @@ func (w *Worker) generateProperties() properties {
 	metricsProperties["max_active_peer_version"] = maxActivePeerVersion
 	metricsProperties["ui_clients"] = uiClient
 	metricsProperties["idp_manager"] = w.idpManager
-	metricsProperties["store_kind"] = w.dataSource.GetStoreKind()
+	metricsProperties["store_engine"] = w.dataSource.GetStoreEngine()
 
 	for protocol, count := range rulesProtocol {
 		metricsProperties["rules_protocol_"+protocol] = count

management/server/metrics/selfhosted_test.go

@@ -151,9 +151,9 @@ func (mockDatasource) GetAllAccounts() []*server.Account {
 	}
 }
 
-// GetStoreKind returns FileStoreKind
+// GetStoreEngine returns FileStoreEngine
-func (mockDatasource) GetStoreKind() server.StoreKind {
+func (mockDatasource) GetStoreEngine() server.StoreEngine {
-	return server.FileStoreKind
+	return server.FileStoreEngine
 }
 
 // TestGenerateProperties tests and validate the properties generation by using the mockDatasource for the Worker.generateProperties
@@ -242,7 +242,7 @@ func TestGenerateProperties(t *testing.T) {
 		t.Errorf("expected 2 user_peers, got %d", properties["user_peers"])
 	}
 
-	if properties["store_kind"] != server.FileStoreKind {
+	if properties["store_engine"] != server.FileStoreEngine {
-		t.Errorf("expected JsonFile, got %s", properties["store_kind"])
+		t.Errorf("expected JsonFile, got %s", properties["store_engine"])
 	}
 }

management/server/sqlite_store.go

@@ -451,7 +451,7 @@ func (s *SqliteStore) Close() error {
 	return nil
 }
 
-// GetStoreKind returns SqliteStoreKind
+// GetStoreEngine returns SqliteStoreEngine
-func (s *SqliteStore) GetStoreKind() StoreKind {
+func (s *SqliteStore) GetStoreEngine() StoreEngine {
-	return SqliteStoreKind
+	return SqliteStoreEngine
 }

management/server/store.go

@@ -3,6 +3,7 @@ package server
 import (
 	"fmt"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/netbirdio/netbird/management/server/telemetry"
@@ -31,42 +32,43 @@ type Store interface {
 	SaveUserLastLogin(accountID, userID string, lastLogin time.Time) error
 	// Close should close the store persisting all unsaved data.
 	Close() error
-	// GetStoreKind should return StoreKind of the current store implementation.
+	// GetStoreEngine should return StoreEngine of the current store implementation.
 	// This is also a method of metrics.DataSource interface.
-	GetStoreKind() StoreKind
+	GetStoreEngine() StoreEngine
 }
 
-type StoreKind string
+type StoreEngine string
 
 const (
-	FileStoreKind   StoreKind = "JsonFile"
+	FileStoreEngine   StoreEngine = "jsonfile"
-	SqliteStoreKind StoreKind = "Sqlite"
+	SqliteStoreEngine StoreEngine = "sqlite"
 )
 
-func GetStoreKindFromEnv() StoreKind {
+func getStoreEngineFromEnv() StoreEngine {
-	kind, ok := os.LookupEnv("NETBIRD_STORE_KIND")
+	// 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 FileStoreKind
+		return FileStoreEngine
 	}
 
-	value := StoreKind(kind)
+	value := StoreEngine(strings.ToLower(kind))
 
-	if value == FileStoreKind || value == SqliteStoreKind {
+	if value == FileStoreEngine || value == SqliteStoreEngine {
 		return value
 	}
 
-	return FileStoreKind
+	return FileStoreEngine
 }
 
-func NewStore(kind StoreKind, dataDir string, metrics telemetry.AppMetrics) (Store, error) {
+func NewStore(kind StoreEngine, dataDir string, metrics telemetry.AppMetrics) (Store, error) {
 	if kind == "" {
 		// fallback to env. Normally this only should be used from tests
-		kind = GetStoreKindFromEnv()
+		kind = getStoreEngineFromEnv()
 	}
 	switch kind {
-	case FileStoreKind:
+	case FileStoreEngine:
 		return NewFileStore(dataDir, metrics)
-	case SqliteStoreKind:
+	case SqliteStoreEngine:
 		return NewSqliteStore(dataDir, metrics)
 	default:
 		return nil, fmt.Errorf("unsupported kind of store %s", kind)
@@ -79,15 +81,14 @@ func NewStoreFromJson(dataDir string, metrics telemetry.AppMetrics) (Store, erro
 		return nil, err
 	}
 
-	kind := GetStoreKindFromEnv()
+	kind := getStoreEngineFromEnv()
 
 	switch kind {
-	case FileStoreKind:
+	case FileStoreEngine:
 		return fstore, nil
-	case SqliteStoreKind:
+	case SqliteStoreEngine:
 		return NewSqliteStoreFromFileStore(fstore, dataDir, metrics)
 	default:
-		return nil, fmt.Errorf("unsupported kind of store %s", kind)
+		return nil, fmt.Errorf("unsupported store engine %s", kind)
 	}
-
 }