fix: resolve panic when DSN="memory" (#574)

Executing the migration logic in registry.go cause a panic as the registry is not initalized at that point. Therefore we decided to move the handling to driver_default.go, after the registry has been initialized.

.circleci/config.yml

@@ -136,6 +136,12 @@ workflows:
           filters:
             tags:
               only: /.*/
+      - test-e2e:
+          name: test-e2e-memory
+          flavor: memory
+          filters:
+            tags:
+              only: /.*/
       - test-e2e:
           name: test-e2e-postgres
           flavor: postgres
@@ -164,6 +170,7 @@ workflows:
           requires:
             - test
             - test-e2e-sqlite
+            - test-e2e-memory
             - test-e2e-postgres
             - test-e2e-mysql
             - test-e2e-cockroach
@@ -177,6 +184,7 @@ workflows:
           requires:
             - test
             - test-e2e-sqlite
+            - test-e2e-memory
             - test-e2e-postgres
             - test-e2e-mysql
             - test-e2e-cockroach
@@ -190,6 +198,7 @@ workflows:
           requires:
             - test
             - test-e2e-sqlite
+            - test-e2e-memory
             - test-e2e-postgres
             - test-e2e-mysql
             - test-e2e-cockroach
@@ -205,6 +214,7 @@ workflows:
           requires:
             - test
             - test-e2e-sqlite
+            - test-e2e-memory
             - test-e2e-postgres
             - test-e2e-mysql
             - test-e2e-cockroach
@@ -225,6 +235,7 @@ workflows:
             - goreleaser/test
             - test
             - test-e2e-sqlite
+            - test-e2e-memory
             - test-e2e-postgres
             - test-e2e-mysql
             - test-e2e-cockroach

Makefile

@@ -91,7 +91,7 @@ format: deps
 # Runs tests in short mode, without database adapters
 .PHONY: docker
 docker:
-		docker build -f .docker/Dockerfile-build -t oryd/kratos:latest .
+		docker build -f .docker/Dockerfile-build -t oryd/kratos:latest-sqlite .
 
 .PHONY: test-e2e
 test-e2e: test-resetdb

driver/configuration/provider_viper.go

@@ -34,6 +34,8 @@ var _ Provider = new(ViperProvider)
 
 const DefaultBrowserReturnURL = "default_browser_return_url"
 
+const DefaultSQLiteMemoryDSN = "sqlite://:memory:?_fk=true"
+
 const (
 	ViperKeyDSN = "dsn"
 
@@ -170,7 +172,7 @@ func (p *ViperProvider) DSN() string {
 	dsn := viperx.GetString(p.l, ViperKeyDSN, "")
 
 	if dsn == "memory" {
-		return "sqlite://mem.db?mode=memory&_fk=true&cache=shared"
+		return DefaultSQLiteMemoryDSN
 	}
 
 	if len(dsn) > 0 {

driver/configuration/provider_viper_test.go

@@ -426,7 +426,7 @@ func TestViperProvider_DSN(t *testing.T) {
 		l := logrusx.New("", "")
 		p := configuration.NewViperProvider(l, false)
 
-		assert.Equal(t, "sqlite://mem.db?mode=memory&_fk=true&cache=shared", p.DSN())
+		assert.Equal(t, configuration.DefaultSQLiteMemoryDSN, p.DSN())
 	})
 
 	t.Run("case=dsn: not memory", func(t *testing.T) {

driver/driver_default.go

@@ -1,6 +1,8 @@
 package driver
 
 import (
+	"context"
+
 	"github.com/pkg/errors"
 
 	"github.com/ory/x/logrusx"
@@ -13,6 +15,11 @@ type DefaultDriver struct {
 	r Registry
 }
 
+// IsSQLiteMemoryMode returns true if SQLite if configured to use inmemory mode
+func IsSQLiteMemoryMode(dsn string) bool {
+	return dsn == configuration.DefaultSQLiteMemoryDSN
+}
+
 func NewDefaultDriver(l *logrusx.Logger, version, build, date string, dev bool) (Driver, error) {
 	if l == nil {
 		l = logrusx.New("ORY Kratos", version)
@@ -24,7 +31,6 @@ func NewDefaultDriver(l *logrusx.Logger, version, build, date string, dev bool)
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to instantiate service registry")
 	}
-
 	r.
 		WithConfig(c).
 		WithLogger(l).
@@ -35,6 +41,15 @@ func NewDefaultDriver(l *logrusx.Logger, version, build, date string, dev bool)
 		return nil, errors.Wrap(err, "unable to initialize service registry")
 	}
 
+	dsn := c.DSN()
+	// if dsn is memory we have to run the migrations on every start
+	if IsSQLiteMemoryMode(dsn) {
+		l.Print("Kratos is running migrations on every startup as DSN is memory.\n")
+		l.Print("This means your data is lost when Kratos terminates.\n")
+		if err := r.Persister().MigrateUp(context.Background()); err != nil {
+			return nil, err
+		}
+	}
 	return &DefaultDriver{r: r, c: c}, nil
 }
 

driver/registry_test.go → driver/driver_default_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	driver "github.com/ory/kratos/driver"
+	"github.com/ory/kratos/driver/configuration"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -14,7 +15,7 @@ func TestDriverDefault_SQLiteMemoryMode(t *testing.T) {
 			dsn string
 			boo bool
 		}{
-			{dsn: "sqlite://mem.db?mode=memory&_fk=true&cache=shared", boo: true},
+			{dsn: configuration.DefaultSQLiteMemoryDSN, boo: true},
 			{dsn: "sqlite://mem.db?mode=asd&_fk=true&cache=shared", boo: false},
 			{dsn: "invalidurl", boo: false},
 		} {

driver/registry.go

@@ -1,10 +1,6 @@
 package driver
 
 import (
-	"context"
-	"net/url"
-	"strings"
-
 	"github.com/gorilla/sessions"
 	"github.com/pkg/errors"
 
@@ -126,17 +122,6 @@ type Registry interface {
 	x.CSRFTokenGeneratorProvider
 }
 
-// IsSQLiteMemoryMode returns true if SQLite if configured to use memory mode
-func IsSQLiteMemoryMode(dsn string) bool {
-	if urlParts := strings.SplitN(dsn, "?", 2); len(urlParts) == 2 && strings.HasPrefix(dsn, "sqlite://") {
-		queryVals, err := url.ParseQuery(urlParts[1])
-		if err == nil && queryVals.Get("mode") == "memory" {
-			return true
-		}
-	}
-	return false
-}
-
 func NewRegistry(c configuration.Provider) (Registry, error) {
 	dsn := c.DSN()
 	driver, err := dbal.GetDriverFor(dsn)
@@ -149,13 +134,5 @@ func NewRegistry(c configuration.Provider) (Registry, error) {
 		return nil, errors.Errorf("driver of type %T does not implement interface Registry", driver)
 	}
 
-	// if dsn is memory we have to run the migrations on every start
-	if IsSQLiteMemoryMode(dsn) {
-		registry.Logger().Print("Kratos is running migrations on every startup as DSN is memory.\n")
-		registry.Logger().Print("This means your data is lost when Kratos terminates.\n")
-		if err := registry.Persister().MigrateUp(context.Background()); err != nil {
-			return nil, err
-		}
-	}
 	return registry, nil
 }

test.sh

@@ -0,0 +1,4 @@
+#!/bin/sh
+set -x
+#docker run -it -e DSN="sqlite3://:memory:?_fk=true" --mount type=bind,source=/Users/andreas/Documents/dev/ory.sh/kratos-config,target=/home/ory oryd/mykratos:latest-sqlite
+docker run -it -e DSN="memory" --mount type=bind,source=/Users/andreas/Documents/dev/ory.sh/kratos-config,target=/home/ory oryd/mykratos:latest-sqlite

test/e2e/run.sh

@@ -18,6 +18,7 @@ if [ -z ${TEST_DATABASE_POSTGRESQL+x} ]; then
   export TEST_DATABASE_MYSQL="mysql://root:secret@(127.0.0.1:3444)/mysql?parseTime=true&multiStatements=true"
   export TEST_DATABASE_POSTGRESQL="postgres://postgres:secret@127.0.0.1:3445/postgres?sslmode=disable"
   export TEST_DATABASE_COCKROACHDB="cockroach://root@127.0.0.1:3446/defaultdb?sslmode=disable"
+
 fi
 
 ! nc -zv 127.0.0.1 4434
@@ -105,11 +106,13 @@ run() {
     PORT=4446 HYDRA_ADMIN_URL=http://127.0.0.1:4445 ./hydra-login-consent > "${base}/test/e2e/hydra-ui.e2e.log" 2>&1 &)
 
   export DSN=${1}
-  $kratos migrate sql -e --yes
+  if [ "$DSN" != "memory" ]; then
+    $kratos migrate sql -e --yes
+  fi
 
   yq merge test/e2e/profiles/kratos.base.yml "test/e2e/profiles/${profile}/.kratos.yml" > test/e2e/kratos.generated.yml
   ($kratos serve --dev -c test/e2e/kratos.generated.yml > "${base}/test/e2e/kratos.${profile}.e2e.log" 2>&1 &)
-
+  
   npm run wait-on -- -t 10000 http-get://127.0.0.1:4434/health/ready \
     http-get://127.0.0.1:4455/health \
     http-get://127.0.0.1:4445/health/ready \
@@ -179,11 +182,17 @@ if [[ $dev = "yes" ]]; then
 fi
 
 export TEST_DATABASE_SQLITE="sqlite:///$(mktemp -d -t ci-XXXXXXXXXX)/db.sqlite?_fk=true"
+export TEST_DATABASE_MEMORY="memory"
+
 case "$1" in
         sqlite)
           db="${TEST_DATABASE_SQLITE}"
           ;;
 
+        memory)
+          db="${TEST_DATABASE_MEMORY}"
+          ;;
+
         mysql)
           db="${TEST_DATABASE_MYSQL}"
           ;;