Skip to content

Commit

Permalink
all: add cockroachdb support
Browse files Browse the repository at this point in the history 
add cockroachdb support

Signed-off-by: David López <not4rent@gmail.com>
  • Loading branch information
@lopezator
lopezator committed Apr 12, 2019
1 parent 6829a58 commit d7ce922
Show file tree
Hide file tree
Showing 36 changed files with 1,047 additions and 288 deletions.
13 changes: 13 additions & 0 deletions .circleci/config.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ jobs:
environment:
- TEST_DATABASE_POSTGRESQL=postgres://test:test@localhost:5432/hydra?sslmode=disable
- TEST_DATABASE_MYSQL=root:test@(localhost:3306)/mysql?parseTime=true
- TEST_DATABASE_COCKROACHDB=postgres://root@localhost:26257/defaultdb?sslmode=disable

This comment has been minimized.

Sign in to view

Copy link
@aeneasr

aeneasr Apr 15, 2019

Why is this postgres?
- image: postgres:9.6
environment:
- POSTGRES_USER=test
Expand All @@ -42,6 +43,8 @@ jobs:
- image: mysql:5.7
environment:
- MYSQL_ROOT_PASSWORD=test
- image: cockroachdb/cockroach:v2.1.6
command: start --insecure
working_directory: /go/src/github.com/ory/hydra
steps:
- run:
Expand All @@ -63,6 +66,7 @@ jobs:
environment:
- DATABASE_URL_POSTGRES=postgres://test:test@localhost:5432/hydra?sslmode=disable
- DATABASE_URL_MYSQL=mysql://root:test@(localhost:3306)/mysql?parseTime=true
- DATABASE_URL_COCKROACH=cockroach://root@localhost:26257/defaultdb?sslmode=disable
- image: postgres:9.5
environment:
- POSTGRES_USER=test
Expand All @@ -71,6 +75,8 @@ jobs:
- image: mysql:5.7
environment:
- MYSQL_ROOT_PASSWORD=test
- image: cockroachdb/cockroach:v2.1.6
command: start --insecure
working_directory: /go/src/github.com/ory/hydra
steps:
- run:
Expand All @@ -83,8 +89,10 @@ jobs:
- run: go install github.com/ory/hydra
- run: hydra migrate sql $DATABASE_URL_POSTGRES
- run: hydra migrate sql $DATABASE_URL_MYSQL
- run: hydra migrate sql $DATABASE_URL_COCKROACH
- run: DATABASE_URL=$DATABASE_URL_POSTGRES ./scripts/test-e2e.sh
- run: DATABASE_URL=$DATABASE_URL_MYSQL ./scripts/test-e2e.sh
- run: DATABASE_URL=$DATABASE_URL_COCKROACH ./scripts/test-e2e.sh
# See https://github.com/ory/hydra/issues/1179
# - run: DATABASE_URL=memory ./scripts/test-e2e.sh

Expand All @@ -110,6 +118,7 @@ jobs:
environment:
- DATABASE_URL_POSTGRES=postgres://test:test@localhost:5432/hydra?sslmode=disable
- DATABASE_URL_MYSQL=mysql://root:test@(localhost:3306)/mysql?parseTime=true
- DATABASE_URL_COCKROACH=cockroach://root@localhost:26257/defaultdb?sslmode=disable
- image: postgres:9.5
environment:
- POSTGRES_USER=test
Expand All @@ -118,6 +127,8 @@ jobs:
- image: mysql:5.7
environment:
- MYSQL_ROOT_PASSWORD=test
- image: cockroachdb/cockroach:v2.1.6
command: start --insecure
working_directory: /go/src/github.com/ory/hydra
steps:
- run:
Expand All @@ -130,8 +141,10 @@ jobs:
- run: go install github.com/ory/hydra
- run: hydra migrate sql $DATABASE_URL_POSTGRES
- run: hydra migrate sql $DATABASE_URL_MYSQL
- run: hydra migrate sql $DATABASE_URL_COCKROACH
- run: OAUTH2_ACCESS_TOKEN_STRATEGY=jwt DATABASE_URL=$DATABASE_URL_POSTGRES ./scripts/test-e2e.sh
- run: OAUTH2_ACCESS_TOKEN_STRATEGY=jwt DATABASE_URL=$DATABASE_URL_MYSQL ./scripts/test-e2e.sh
- run: OAUTH2_ACCESS_TOKEN_STRATEGY=jwt DATABASE_URL=$DATABASE_URL_COCKROACH ./scripts/test-e2e.sh
# See https://github.com/ory/hydra/issues/1179
# - run: OAUTH2_ACCESS_TOKEN_STRATEGY=jwt DATABASE_URL=memory ./scripts/test-e2e.sh

Expand Down
20 changes: 14 additions & 6 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,34 @@ SHELL=/bin/bash -o pipefail
test:
docker kill hydra_test_database_mysql || true
docker kill hydra_test_database_postgres || true
docker kill hydra_test_database_cockroach || true
docker rm -f hydra_test_database_mysql || true
docker rm -f hydra_test_database_postgres || true
docker rm -f hydra_test_database_cockroach || true
docker run --rm --name hydra_test_database_mysql -p 3444:3306 -e MYSQL_ROOT_PASSWORD=secret -d mysql:5.7
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_cockroach -p 3446:26257 -d cockroachdb/cockroach:v2.1.6 start --insecure
make sqlbin
TEST_DATABASE_MYSQL='root:secret@(127.0.0.1:3444)/mysql?parseTime=true' \
TEST_DATABASE_POSTGRESQL='postgres://postgres:secret@127.0.0.1:3445/hydra?sslmode=disable' \
go-acc ./... -- -failfast -timeout=20m
TEST_DATABASE_POSTGRESQL='postgres://postgres:secret@127.0.0.1:3445/hydra?sslmode=disable' \
TEST_DATABASE_COCKROACHDB='postgres://root@127.0.0.1:3446/defaultdb?sslmode=disable' \

This comment has been minimized.

Sign in to view

Copy link
@aeneasr

aeneasr Apr 15, 2019

Shouldn't this be cockroach?
go-acc ./oauth2/... -- -v -failfast -timeout=20m
docker rm -f hydra_test_database_mysql
docker rm -f hydra_test_database_postgres
docker rm -f hydra_test_database_cockroach

# Resets the test databases
.PHONY: test-resetdb
test-resetdb:
docker kill hydra_test_database_mysql || true
docker kill hydra_test_database_postgres || true
docker kill hydra_test_database_cockroach || true
docker rm -f hydra_test_database_mysql || true
docker rm -f hydra_test_database_postgres || true
docker rm -f hydra_test_database_cockroach || true
docker run --rm --name hydra_test_database_mysql -p 3444:3306 -e MYSQL_ROOT_PASSWORD=secret -d mysql:5.7
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_cockroach -p 3446:26257 -d cockroachdb/cockroach:v2.1.6 start --insecure

# Runs tests in short mode, without database adapters
.PHONY: quicktest
Expand All @@ -47,10 +55,10 @@ mocks:
# Adds sql files to the binary using go-bindata
.PHONY: sqlbin
sqlbin:
cd client; go-bindata -o sql_migration_files.go -pkg client ./migrations/sql/shared ./migrations/sql/mysql ./migrations/sql/postgres ./migrations/sql/tests
cd consent; go-bindata -o sql_migration_files.go -pkg consent ./migrations/sql/shared ./migrations/sql/mysql ./migrations/sql/postgres ./migrations/sql/tests
cd jwk; go-bindata -o sql_migration_files.go -pkg jwk ./migrations/sql/shared ./migrations/sql/mysql ./migrations/sql/postgres ./migrations/sql/tests
cd oauth2; go-bindata -o sql_migration_files.go -pkg oauth2 ./migrations/sql/shared ./migrations/sql/mysql ./migrations/sql/postgres ./migrations/sql/tests
cd client; go-bindata -o sql_migration_files.go -pkg client ./migrations/sql/shared ./migrations/sql/mysql ./migrations/sql/postgres ./migrations/sql/cockroach ./migrations/sql/tests ./migrations/sql/crdb
cd consent; go-bindata -o sql_migration_files.go -pkg consent ./migrations/sql/shared ./migrations/sql/mysql ./migrations/sql/postgres ./migrations/sql/cockroach ./migrations/sql/tests ./migrations/sql/crdb
cd jwk; go-bindata -o sql_migration_files.go -pkg jwk ./migrations/sql/shared ./migrations/sql/mysql ./migrations/sql/postgres ./migrations/sql/cockroach ./migrations/sql/tests ./migrations/sql/crdb
cd oauth2; go-bindata -o sql_migration_files.go -pkg oauth2 ./migrations/sql/shared ./migrations/sql/mysql ./migrations/sql/postgres ./migrations/sql/cockroach ./migrations/sql/tests ./migrations/sql/crdb

# Runs all code generators
.PHONY: gen
Expand Down
39 changes: 30 additions & 9 deletions client/manager_sql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package client

import (
"context"
"database/sql"
"encoding/json"
"fmt"
"strings"
Expand All @@ -31,7 +32,7 @@ import (
"github.com/pkg/errors"
migrate "github.com/rubenv/sql-migrate"
"github.com/sirupsen/logrus"
"gopkg.in/square/go-jose.v2"
jose "gopkg.in/square/go-jose.v2"

"github.com/ory/fosite"
"github.com/ory/go-convenience/stringsx"
Expand All @@ -40,8 +41,9 @@ import (
)

var Migrations = map[string]*dbal.PackrMigrationSource{
dbal.DriverMySQL: dbal.NewMustPackerMigrationSource(logrus.New(), AssetNames(), Asset, []string{"migrations/sql/shared", "migrations/sql/mysql"}, true),
dbal.DriverPostgreSQL: dbal.NewMustPackerMigrationSource(logrus.New(), AssetNames(), Asset, []string{"migrations/sql/shared", "migrations/sql/postgres"}, true),
dbal.DriverMySQL: dbal.NewMustPackerMigrationSource(logrus.New(), AssetNames(), Asset, []string{"migrations/sql/shared", "migrations/sql/mysql"}, true),
dbal.DriverPostgreSQL: dbal.NewMustPackerMigrationSource(logrus.New(), AssetNames(), Asset, []string{"migrations/sql/shared", "migrations/sql/postgres"}, true),
dbal.DriverCockroachDB: dbal.NewMustPackerMigrationSource(logrus.New(), AssetNames(), Asset, []string{"migrations/sql/cockroach"}, true),
}

func NewSQLManager(db *sqlx.DB, r InternalRegistry) *SQLManager {
Expand Down Expand Up @@ -206,8 +208,13 @@ func (d *sqlData) ToClient() (*Client, error) {
}

func (m *SQLManager) CreateSchemas() (int, error) {
driver := m.DB.DriverName()
switch driver {
case "cockroach":
driver = "postgres"
}
migrate.SetTable("hydra_client_migration")
n, err := migrate.Exec(m.DB.DB, m.DB.DriverName(), Migrations[dbal.Canonicalize(m.DB.DriverName())], migrate.Up)
n, err := migrate.Exec(m.DB.DB, driver, Migrations[dbal.Canonicalize(m.DB.DriverName())], migrate.Up)
if err != nil {
return 0, errors.Wrapf(err, "Could not migrate sql schema, applied %d Migrations", n)
}
Expand All @@ -216,7 +223,7 @@ func (m *SQLManager) CreateSchemas() (int, error) {

func (m *SQLManager) GetConcreteClient(ctx context.Context, id string) (*Client, error) {
var d sqlData
if err := m.DB.GetContext(ctx, &d, m.DB.Rebind("SELECT * FROM hydra_client WHERE id=?"), id); err != nil {
if err := m.DB.GetContext(ctx, &d, m.Rebind("SELECT * FROM hydra_client WHERE id=?"), id); err != nil {
return nil, sqlcon.HandleError(err)
}

Expand Down Expand Up @@ -253,7 +260,7 @@ func (m *SQLManager) UpdateClient(ctx context.Context, c *Client) error {
query = append(query, fmt.Sprintf("%s=:%s", param, param))
}

if _, err := m.DB.NamedExecContext(ctx, fmt.Sprintf(`UPDATE hydra_client SET %s WHERE id=:id`, strings.Join(query, ", ")), s); err != nil {
if _, err := m.NamedExecContext(ctx, fmt.Sprintf(`UPDATE hydra_client SET %s WHERE id=:id`, strings.Join(query, ", ")), s); err != nil {
return sqlcon.HandleError(err)
}
return nil
Expand Down Expand Up @@ -284,7 +291,7 @@ func (m *SQLManager) CreateClient(ctx context.Context, c *Client) error {
return errors.WithStack(err)
}

if _, err := m.DB.NamedExecContext(ctx, fmt.Sprintf(
if _, err := m.NamedExecContext(ctx, fmt.Sprintf(
"INSERT INTO hydra_client (%s) VALUES (%s)",
strings.Join(sqlParams, ", "),
":"+strings.Join(sqlParams, ", :"),
Expand All @@ -296,7 +303,7 @@ func (m *SQLManager) CreateClient(ctx context.Context, c *Client) error {
}

func (m *SQLManager) DeleteClient(ctx context.Context, id string) error {
if _, err := m.DB.ExecContext(ctx, m.DB.Rebind(`DELETE FROM hydra_client WHERE id=?`), id); err != nil {
if _, err := m.DB.ExecContext(ctx, m.Rebind(`DELETE FROM hydra_client WHERE id=?`), id); err != nil {
return sqlcon.HandleError(err)
}
return nil
Expand All @@ -306,7 +313,7 @@ func (m *SQLManager) GetClients(ctx context.Context, limit, offset int) (clients
d := make([]sqlData, 0)
clients = make(map[string]Client)

if err := m.DB.SelectContext(ctx, &d, m.DB.Rebind("SELECT * FROM hydra_client ORDER BY id LIMIT ? OFFSET ?"), limit, offset); err != nil {
if err := m.DB.SelectContext(ctx, &d, m.Rebind("SELECT * FROM hydra_client ORDER BY id LIMIT ? OFFSET ?"), limit, offset); err != nil {
return nil, sqlcon.HandleError(err)
}

Expand All @@ -320,3 +327,17 @@ func (m *SQLManager) GetClients(ctx context.Context, limit, offset int) (clients
}
return clients, nil
}

func (m *SQLManager) Rebind(query string) string {
if m.DB.DriverName() == "cockroach" {
m.DB = sqlx.NewDb(m.DB.DB, "postgres")
}
return m.DB.Rebind(query)
}

func (m *SQLManager) NamedExecContext(ctx context.Context, query string, arg interface{}) (sql.Result, error) {
if m.DB.DriverName() == "cockroach" {
m.DB = sqlx.NewDb(m.DB.DB, "postgres")
}
return m.DB.NamedExecContext(ctx, query, arg)
}
15 changes: 15 additions & 0 deletions client/manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@ func connectToPG() {
m.Unlock()
}

func connectToCRDB() {
db, err := dockertest.ConnectToTestCockroachDB()
if err != nil {
log.Fatalf("Could not connect to database: %v", err)
}

conf := internal.NewConfigurationWithDefaults()
reg := internal.NewRegistrySQL(conf, db)

m.Lock()
clientManagers["cockroach"] = reg.ClientManager()
m.Unlock()
}

func TestManagers(t *testing.T) {
conf := internal.NewConfigurationWithDefaults()
reg := internal.NewRegistry(conf)
Expand All @@ -84,6 +98,7 @@ func TestManagers(t *testing.T) {
dockertest.Parallel([]func(){
connectToPG,
connectToMySQL,
connectToCRDB,
})
}

Expand Down
34 changes: 34 additions & 0 deletions client/migrations/sql/cockroach/1.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
-- +migrate Up
CREATE TABLE IF NOT EXISTS hydra_client (
pk SERIAL PRIMARY KEY,
id varchar(255) NOT NULL,
client_name text NOT NULL,
client_secret text NOT NULL,
redirect_uris text NOT NULL,
grant_types text NOT NULL,
response_types text NOT NULL,
scope text NOT NULL,
owner text NOT NULL,
policy_uri text NOT NULL,
tos_uri text NOT NULL,
client_uri text NOT NULL,
logo_uri text NOT NULL,
contacts text NOT NULL,
client_secret_expires_at INTEGER NOT NULL DEFAULT 0,
sector_identifier_uri text NOT NULL,
jwks text NOT NULL,
jwks_uri text NOT NULL,
request_uris text NOT NULL,
token_endpoint_auth_method VARCHAR(25) NOT NULL DEFAULT '',
request_object_signing_alg VARCHAR(10) NOT NULL DEFAULT '',
userinfo_signed_response_alg VARCHAR(10) NOT NULL DEFAULT '',
subject_type VARCHAR(15) NOT NULL DEFAULT '',
allowed_cors_origins text NOT NULL,
audience text NOT NULL,
created_at timestamp NOT NULL DEFAULT now(),
updated_at timestamp NOT NULL DEFAULT now(),
UNIQUE (id)
);

-- +migrate Down
DROP TABLE hydra_client;
4 changes: 4 additions & 0 deletions client/migrations/sql/crdb/1_test.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- +migrate Up
INSERT INTO hydra_client (id, allowed_cors_origins, client_name, client_secret, redirect_uris, grant_types, response_types, scope, owner, policy_uri, tos_uri, client_uri, logo_uri, contacts, client_secret_expires_at, sector_identifier_uri, jwks, jwks_uri, token_endpoint_auth_method, request_uris, request_object_signing_alg, userinfo_signed_response_alg, subject_type, audience, created_at, updated_at) VALUES ('1-data', 'http://localhost|http://google', 'some-client', 'abcdef', 'http://localhost|http://google', 'authorize_code|implicit', 'token|id_token', 'foo|bar', 'aeneas', 'http://policy', 'http://tos', 'http://client', 'http://logo', 'aeneas|foo', 0, 'http://sector', '{"keys": []}', 'http://jwks', 'none', 'http://uri1|http://uri2', 'rs256', 'rs526', 'public', 'https://www.ory.sh/api', NOW(), NOW());

-- +migrate Down
Loading

1 comment on commit d7ce922

@lopezator
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @aeneasr I managed to pass all the tests in: ory#1348

We have a tricky game here because:

Maybe we could ask him to add it, so we could clean up this a little more.

I'll reason about why the use of postgres:// sometimes and why cockroach://. I think it happens the same to you when using a queryString prefix of mysql:// or not.

Anyway I have to clean up this, maybe by wrapping sqlx, lib/pq, SQLManager, RegistrySQL or whatever... Have to think about this.

Related code needed in x is here:

lopezator/x@1b7f14d

Please sign in to comment.