Replace DBAL layer with gobuffalo/pop (#130)

This is a major refactoring of the internal DBAL. After a successful proof of concept and evaluation of gobuffalo/pop, we believe this to be the best DBAL for Go at the moment. It abstracts a lot of boilerplate code away.

As with all sophisticated DBALs, pop too has its quirks. There are several issues that have been discovered during testing and adoption: gobuffalo/pop#136 gobuffalo/pop#476 gobuffalo/pop#473 gobuffalo/pop#469 gobuffalo/pop#466

However, the upside of moving much of the hard database/sql plumbing into another library cleans up the code base significantly and reduces complexity.

As part of this change, the "ephermal" DBAL ("in memory") will be removed and sqlite will be used instead. This further reduces complexity of the code base and code-duplication.

To support sqlite, CGO is required, which means that we need to run tests with `go test -tags sqlite` on a machine that has g++ installed. This also means that we need a Docker Image with `alpine` as opposed to pure `scratch`. While this is certainly a downside, the upside of less maintenance and "free" support for SQLite, PostgreSQL, MySQL, and CockroachDB simply outweighs any downsides that come with CGO.

.circleci/config.yml

@@ -5,75 +5,103 @@ version: 2
 jobs:
   lint:
     docker:
-      - image: circleci/golang:1.13
+      -
+        image: circleci/golang:1.13
         environment:
           - GO111MODULE=on
     working_directory: /go/src/github.com/ory/kratos
     steps:
       - checkout
-      - restore_cache:
+      -
+        restore_cache:
           keys:
             - go-mod-v1-{{ checksum "go.sum" }}
-      - run: go mod download
-      - save_cache:
+      -
+        run: go mod download
+      -
+        save_cache:
           key: go-mod-v1-{{ checksum "go.sum" }}
           paths:
             - "/go/pkg/mod"
-      - run: curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.20.1
-      - run: make lint
+      -
+        run: curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.20.1
+      -
+        run: make lint
 
   test:
     docker:
-      - image: circleci/golang:1.13
+      -
+        image: circleci/golang:1.13
         environment:
           - GO111MODULE=on
           - TEST_SELFSERVICE_OIDC_HYDRA_ADMIN=http://127.0.0.1:4445
           - TEST_SELFSERVICE_OIDC_HYDRA_PUBLIC=http://127.0.0.1:4444
           - TEST_SELFSERVICE_OIDC_HYDRA_INTEGRATION_ADDR=127.0.0.1:4499
-          - TEST_DATABASE_POSTGRESQL=postgres://test:test@localhost:5432/hydra?sslmode=disable
-      - image: oryd/hydra:v1.0.0
+          - TEST_DATABASE_POSTGRESQL=postgres://test:test@localhost:5432/postgres?sslmode=disable
+          - TEST_DATABASE_MYSQL=mysql://root:test@(localhost:3306)/mysql?parseTime=true
+#          - TEST_DATABASE_COCKROACHDB=cockroach://root@localhost:26257/defaultdb?sslmode=disable
+      -
+        image: postgres:9.6
+        environment:
+          - POSTGRES_USER=test
+          - POSTGRES_PASSWORD=test
+          - POSTGRES_DB=postgres
+      -
+        image: mysql:5.7
+        environment:
+          - MYSQL_ROOT_PASSWORD=test
+#      -
+#        image: cockroachdb/cockroach:v19.2.0
+#        command: start --insecure
+      -
+        image: oryd/hydra:v1.0.0
         environment:
           - DSN=memory
           - URLS_SELF_ISSUER=http://127.0.0.1:4444/
           - URLS_LOGIN=http://127.0.0.1:4499/login
           - URLS_CONSENT=http://127.0.0.1:4499/consent
         command: serve all --dangerous-force-http
-      - image: postgres:9.6
-        environment:
-          - POSTGRES_USER=test
-          - POSTGRES_PASSWORD=test
-          - POSTGRES_DB=hydra
     working_directory: /go/src/github.com/ory/kratos
     steps:
       - checkout
       - setup_remote_docker
-      - run:
+      -
+        run:
           command: |
             ./.circleci/release_name.bash
             echo 'export DOCKER_SHORT_TAG=$CIRCLE_SHA1' >> $BASH_ENV
             source $BASH_ENV
-      - run: GO111MODULE=off go get github.com/mattn/goveralls github.com/ory/go-acc
-      - restore_cache:
+      -
+        run: GO111MODULE=off go get github.com/mattn/goveralls github.com/ory/go-acc
+      -
+        restore_cache:
           keys:
             - go-v1-{{ checksum "go.sum" }}
-      - run: go mod download
-      - save_cache:
+      -
+        run: go mod download
+      -
+        save_cache:
           key: go-v1-{{ checksum "go.sum" }}
           paths:
             - "/go/pkg/mod"
-      - run: timeout 15 sh -c 'until nc -z $0 $1; do sleep 1; done' 127.0.0.1 4444
-      - run: go-acc -o coverage.txt ./... -- -v -failfast -timeout=20m
-      - run: test -z "$CIRCLE_PR_NUMBER" && goveralls -service=circle-ci -coverprofile=coverage.txt -repotoken=$COVERALLS_REPO_TOKEN || echo "forks are not allowed to push to coveralls"
+      -
+        run: timeout 15 sh -c 'until nc -z $0 $1; do sleep 1; done' 127.0.0.1 4444
+      -
+        run: go-acc -o coverage.txt ./... -- -v -failfast -timeout=20m -tags sqlite
+      -
+        run: test -z "$CIRCLE_PR_NUMBER" && goveralls -service=circle-ci -coverprofile=coverage.txt -repotoken=$COVERALLS_REPO_TOKEN || echo "forks are not allowed to push to coveralls"
 
 workflows:
   version: 2
   "test":
     jobs:
-      - lint:
+      -
+        lint:
           filters:
             tags:
               only: /.*/
-      - test:
+      -
+        test:
           filters:
             tags:
               only: /.*/

.dockerignore

@@ -6,3 +6,4 @@ tmp
 scripts
 .idea
 .git/
+database.yaml

.golangci.yml

@@ -1,4 +1,18 @@
 linters:
+  enabled:
+    - deadcode
+    - errcheck
+    - gosimple
+    - govet
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unused
+    - bodyclose
+    - dupl
+    - gosec
+    - varcheck
+    - godox
   disable:
     - ineffassign
 

Dockerfile

@@ -1,10 +1,6 @@
 FROM alpine:3.10
 
 RUN apk add -U --no-cache ca-certificates
-
-FROM scratch
-
-COPY --from=0 /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
 COPY kratos /usr/bin/kratos
 
 USER 1000

Makefile

@@ -65,4 +65,4 @@ sqlbin:
 resetdb:
 		docker kill hydra_test_database_postgres || true
 		docker rm -f hydra_test_database_postgres || true
-		docker run --rm --name hydra_test_database_postgres -p 3445:5432 -e POSTGRES_PASSWORD=secret -e POSTGRES_DB=hydra -d postgres:9.6
+		docker run --rm --name hydra_test_database_postgres -p 3445:5432 -e POSTGRES_PASSWORD=secret -e POSTGRES_DB=postgres -d postgres:9.6

README.md

@@ -67,4 +67,3 @@ changes in [UPGRADE.md](./UPGRADE.md) and [CHANGELOG.md](./CHANGELOG.md).
 ### Command line documentation
 
 Run `kratos -h` or `kratos help`.
-

cmd/client/migrate.go

@@ -1,15 +1,12 @@
 package client
 
 import (
-	"bufio"
+	"context"
 	"fmt"
 	"os"
-	"strings"
 
 	"github.com/spf13/cobra"
 
-	"github.com/ory/x/sqlcon"
-
 	"github.com/ory/viper"
 	"github.com/ory/x/cmdx"
 	"github.com/ory/x/flagx"
@@ -29,7 +26,7 @@ func (h *MigrateHandler) MigrateSQL(cmd *cobra.Command, args []string) {
 	var d driver.Driver
 
 	if flagx.MustGetBool(cmd, "read-from-env") {
-		d = driver.NewDefaultDriver(logrusx.New(), "", "", "")
+		d = driver.MustNewDefaultDriver(logrusx.New(), "", "", "")
 		if len(d.Configuration().DSN()) == 0 {
 			fmt.Println(cmd.UsageString())
 			fmt.Println("")
@@ -44,54 +41,57 @@ func (h *MigrateHandler) MigrateSQL(cmd *cobra.Command, args []string) {
 			return
 		}
 		viper.Set(configuration.ViperKeyDSN, args[0])
-		d = driver.NewDefaultDriver(logrusx.New(), "", "", "")
-	}
-
-	reg, ok := d.Registry().(*driver.RegistrySQL)
-	if !ok {
-		fmt.Println(cmd.UsageString())
-		fmt.Println("")
-		fmt.Printf("Migrations can only be executed against a SQL-compatible driver but DSN is not a SQL source.\n")
-		os.Exit(1)
-		return
+		d = driver.MustNewDefaultDriver(logrusx.New(), "", "", "")
 	}
 
-	scheme := sqlcon.GetDriverName(d.Configuration().DSN())
-	plan, err := reg.SchemaMigrationPlan(scheme)
-	cmdx.Must(err, "An error occurred planning migrations: %s", err)
-
-	fmt.Println("The following migration is planned:")
-	fmt.Println("")
-	plan.Render()
-
-	if !flagx.MustGetBool(cmd, "yes") {
-		fmt.Println("")
-		fmt.Println("To skip the next question use flag --yes (at your own risk).")
-		if !askForConfirmation("Do you wish to execute this migration plan?") {
-			fmt.Println("Migration aborted.")
-			return
-		}
-	}
-
-	n, err := reg.CreateSchemas(scheme)
+	err := d.Registry().Persister().MigrateUp(context.Background())
 	cmdx.Must(err, "An error occurred while connecting to SQL: %s", err)
-	fmt.Printf("Successfully applied %d SQL migrations!\n", n)
+	fmt.Println("Successfully applied SQL migrations!")
+
+	// if !ok {
+	// 	fmt.Println(cmd.UsageString())
+	// 	fmt.Println("")
+	// 	fmt.Printf("Migrations can only be executed against a SQL-compatible driver but DSN is not a SQL source.\n")
+	// 	os.Exit(1)
+	// 	return
+	// }
+	//
+	// scheme := sqlcon.GetDriverName(d.Configuration().DSN())
+	// plan, err := reg.SchemaMigrationPlan(scheme)
+	// cmdx.Must(err, "An error occurred planning migrations: %s", err)
+	//
+	// fmt.Println("The following migration is planned:")
+	// fmt.Println("")
+	// plan.Render()
+	//
+	// if !flagx.MustGetBool(cmd, "yes") {
+	// 	fmt.Println("")
+	// 	fmt.Println("To skip the next question use flag --yes (at your own risk).")
+	// 	if !askForConfirmation("Do you wish to execute this migration plan?") {
+	// 		fmt.Println("Migration aborted.")
+	// 		return
+	// 	}
+	// }
+	//
+	// n, err := reg.CreateSchemas(scheme)
+	// cmdx.Must(err, "An error occurred while connecting to SQL: %s", err)
+	// fmt.Printf("Successfully applied %d SQL migrations!\n", n)
 }
 
-func askForConfirmation(s string) bool {
-	reader := bufio.NewReader(os.Stdin)
-
-	for {
-		fmt.Printf("%s [y/n]: ", s)
-
-		response, err := reader.ReadString('\n')
-		cmdx.Must(err, "%s", err)
-
-		response = strings.ToLower(strings.TrimSpace(response))
-		if response == "y" || response == "yes" {
-			return true
-		} else if response == "n" || response == "no" {
-			return false
-		}
-	}
-}
+// func askForConfirmation(s string) bool {
+// 	reader := bufio.NewReader(os.Stdin)
+//
+// 	for {
+// 		fmt.Printf("%s [y/n]: ", s)
+//
+// 		response, err := reader.ReadString('\n')
+// 		cmdx.Must(err, "%s", err)
+//
+// 		response = strings.ToLower(strings.TrimSpace(response))
+// 		if response == "y" || response == "yes" {
+// 			return true
+// 		} else if response == "n" || response == "no" {
+// 			return false
+// 		}
+// 	}
+// }

cmd/serve.go

@@ -28,7 +28,7 @@ import (
 var serveCmd = &cobra.Command{
 	Use: "serve",
 	Run: func(cmd *cobra.Command, args []string) {
-		daemon.ServeAll(driver.NewDefaultDriver(logger, BuildVersion, BuildTime, BuildGitHash))(cmd, args)
+		daemon.ServeAll(driver.MustNewDefaultDriver(logger, BuildVersion, BuildTime, BuildGitHash))(cmd, args)
 	},
 }
 

contrib/sql/.soda.yml

@@ -0,0 +1,14 @@
+development:
+  url: sqlite://a/b
+
+sqlite:
+  url: sqlite://a/b
+
+postgres:
+  url: postgres://a/b
+
+mysql:
+  url: mysql://tcp(a)/b?parseTime=true&multiStatements=true
+
+cockroach:
+  url: crdb://a/b

contrib/sql/README.md

@@ -0,0 +1,25 @@
+# SQL Migrations
+
+To create a new [fizz](https://gobuffalo.io/en/docs/db/fizz/) migration run in the project root:
+
+```
+$ soda generate fizz -c ./contrib/sql/.soda.yml -p ./contrib/sql/migrations [name]
+```
+
+To create SQL migrations, target each database individually and run
+
+```
+$ soda generate sql -e mysql -c ./contrib/sql/.soda.yml -p ./contrib/sql/migrations [name]
+$ soda generate sql -e sqlite -c ./contrib/sql/.soda.yml -p ./contrib/sql/migrations [name]
+$ soda generate sql -e postgres -c ./contrib/sql/.soda.yml -p ./contrib/sql/migrations [name]
+$ soda generate sql -e cockroach -c ./contrib/sql/.soda.yml -p ./contrib/sql/migrations [name]
+```
+
+or, alternative run 
+
+```
+$ soda generate sql -c ./contrib/sql/.soda.yml -p ./contrib/sql/migrations 
+```
+
+and remove the `sqlite` part from the newly generated file to create a SQL migrations that works with all
+aforementioned databases.

contrib/sql/migrations/20191120000000_identities.down.fizz

@@ -0,0 +1 @@
+drop_table("identities")

contrib/sql/migrations/20191120000000_identities.up.fizz

@@ -0,0 +1,41 @@
+create_table("identities") {
+	t.Column("id", "uuid", {primary: true})
+    t.Column("traits_schema_url", "string", {"size": 2048})
+    t.Column("traits", "json")
+
+	t.Timestamps()
+}
+
+create_table("identity_credential_types") {
+ 	t.Column("id", "uuid", {primary: true})
+    t.Column("name", "string", { "size": 32 })
+
+    t.DisableTimestamps()
+}
+
+add_index("identity_credential_types", "name", {"unique": true})
+
+create_table("identity_credentials") {
+ 	t.Column("id", "uuid", {primary: true})
+    t.Column("config", "json")
+
+    t.Column("identity_credential_type_id", "uuid", { "size": 32 })
+    t.Column("identity_id", "uuid")
+
+ 	t.Timestamps()
+
+    t.ForeignKey("identity_id", {"identities": ["id"]}, {"on_delete": "cascade"})
+    t.ForeignKey("identity_credential_type_id", {"identity_credential_types": ["id"]}, {"on_delete": "cascade"})
+}
+
+create_table("identity_credential_identifiers") {
+ 	t.Column("id", "uuid", {primary: true})
+    t.Column("identifier", "string", {"size": 255})
+    t.Column("identity_credential_id", "uuid")
+
+ 	t.Timestamps()
+
+    t.ForeignKey("identity_credential_id", {"identity_credentials": ["id"]}, {"on_delete": "cascade"})
+}
+
+add_index("identity_credential_identifiers", "identifier", {"unique": true})