Skip to content

Commit

Permalink
Add initial schema and storage provider for CockroachDB
Browse files Browse the repository at this point in the history
This creates an initial schema for CRDB by mirroring a lot of the
existing constructs from the MySQL storage provider. It also adds
scripts and test code to run unit and integration tests with CockRoachDB

While this lead to a lot of code duplication, the intention is to
address that in a separate PR in order to keep the work separated into
logical batches. Doing all the refactoring to create common constructs
for SQL backends would lead to a patch too big for folks to review in a
reasonable time and would create more risk.

The plan, thus, is as follows:

[x] Create CRDB storage provider (this PR).
[ ] Abstract SQL admin storage to its own package.
[ ] Abstract Statement cache into its own package.
[ ] Abstract (Log)Tree Transaction code into its own package.

This way, we could have re-usable parts for other SQL providers.

Signed-off-by: Juan Antonio Osorio <[email protected]>
  • Loading branch information
JAORMX committed Nov 8, 2022
1 parent fc29403 commit 0702e94
Show file tree
Hide file tree
Showing 25 changed files with 2,798 additions and 270 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@
/trillian_log_signer
/trillian_map_server
default.etcd
cockroach-data/
1 change: 1 addition & 0 deletions cmd/trillian_log_server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (

// Register supported storage providers.
_ "github.com/google/trillian/storage/cloudspanner"
_ "github.com/google/trillian/storage/crdb"
_ "github.com/google/trillian/storage/mysql"

// Load MySQL quota provider
Expand Down
1 change: 1 addition & 0 deletions cmd/trillian_log_signer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import (

// Register supported storage providers.
_ "github.com/google/trillian/storage/cloudspanner"
_ "github.com/google/trillian/storage/crdb"
_ "github.com/google/trillian/storage/mysql"

// Load MySQL quota provider
Expand Down
11 changes: 10 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
cloud.google.com/go/spanner v1.40.0
contrib.go.opencensus.io/exporter/stackdriver v0.13.12
github.com/apache/beam/sdks/v2 v2.0.0-20211012030016-ef4364519c94
github.com/cockroachdb/cockroach-go/v2 v2.2.16
github.com/fullstorydev/grpcurl v1.8.7
github.com/go-redis/redis v6.15.9+incompatible
github.com/go-sql-driver/mysql v1.6.0
Expand All @@ -16,6 +17,7 @@ require (
github.com/google/go-licenses v0.0.0-20210329231322-ce1d9163b77d
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
github.com/letsencrypt/pkcs11key/v4 v4.0.0
github.com/lib/pq v1.10.7
github.com/prometheus/client_golang v1.13.1
github.com/prometheus/client_model v0.3.0
github.com/pseudomuto/protoc-gen-doc v1.5.1
Expand Down Expand Up @@ -82,13 +84,20 @@ require (
github.com/huandu/xstrings v1.2.0 // indirect
github.com/imdario/mergo v0.3.9 // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/jackc/chunkreader/v2 v2.0.1 // indirect
github.com/jackc/pgconn v1.12.1 // indirect
github.com/jackc/pgio v1.0.0 // indirect
github.com/jackc/pgpassfile v1.0.0 // indirect
github.com/jackc/pgproto3/v2 v2.3.0 // indirect
github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b // indirect
github.com/jackc/pgtype v1.11.0 // indirect
github.com/jackc/pgx/v4 v4.16.1 // indirect
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
github.com/jhump/protoreflect v1.12.0 // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/jonboulle/clockwork v0.3.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/kevinburke/ssh_config v0.0.0-20190725054713-01f96b0aa0cd // indirect
github.com/mattn/go-colorable v0.1.4 // indirect
github.com/mattn/go-runewidth v0.0.13 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
github.com/miekg/pkcs11 v1.0.3 // indirect
Expand Down
367 changes: 119 additions & 248 deletions go.sum

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion integration/admin/admin_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,8 @@ func setupAdminServer(ctx context.Context, t *testing.T) (*testServer, error) {
return nil, err
}

registry, done, err := integration.NewRegistryForTests(ctx)
// TODO(jaosorior): Make this configurable for Cockroach or MySQL
registry, done, err := integration.NewRegistryForTests(ctx, testdb.DriverMySQL)
if err != nil {
ts.closeAll()
return nil, err
Expand Down
5 changes: 3 additions & 2 deletions integration/quota/quota_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ func TestEtcdRateLimiting(t *testing.T) {
testdb.SkipIfNoMySQL(t)
ctx := context.Background()

registry, done, err := integration.NewRegistryForTests(ctx)
// TODO(jaosorior): Make this configurable for Cockroach or MySQL
registry, done, err := integration.NewRegistryForTests(ctx, testdb.DriverMySQL)
if err != nil {
t.Fatalf("NewRegistryForTests() returned err = %v", err)
}
Expand Down Expand Up @@ -100,7 +101,7 @@ func TestEtcdRateLimiting(t *testing.T) {
func TestMySQLRateLimiting(t *testing.T) {
testdb.SkipIfNoMySQL(t)
ctx := context.Background()
db, done, err := testdb.NewTrillianDB(ctx)
db, done, err := testdb.NewTrillianDB(ctx, testdb.DriverMySQL)
if err != nil {
t.Fatalf("GetTestDB() returned err = %v", err)
}
Expand Down
6 changes: 3 additions & 3 deletions quota/mysqlqm/mysql_quota_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestQuotaManager_GetTokens(t *testing.T) {
testdb.SkipIfNoMySQL(t)
ctx := context.Background()

db, done, err := testdb.NewTrillianDB(ctx)
db, done, err := testdb.NewTrillianDB(ctx, testdb.DriverMySQL)
if err != nil {
t.Fatalf("GetTestDB() returned err = %v", err)
}
Expand Down Expand Up @@ -131,7 +131,7 @@ func TestQuotaManager_GetTokens_InformationSchema(t *testing.T) {
for _, test := range tests {
desc := fmt.Sprintf("useSelectCount = %v", test.useSelectCount)
t.Run(desc, func(t *testing.T) {
db, done, err := testdb.NewTrillianDB(ctx)
db, done, err := testdb.NewTrillianDB(ctx, testdb.DriverMySQL)
if err != nil {
t.Fatalf("NewTrillianDB() returned err = %v", err)
}
Expand Down Expand Up @@ -182,7 +182,7 @@ func TestQuotaManager_Noops(t *testing.T) {
testdb.SkipIfNoMySQL(t)
ctx := context.Background()

db, done, err := testdb.NewTrillianDB(ctx)
db, done, err := testdb.NewTrillianDB(ctx, testdb.DriverMySQL)
if err != nil {
t.Fatalf("GetTestDB() returned err = %v", err)
}
Expand Down
106 changes: 106 additions & 0 deletions scripts/resetcrdb.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
#!/bin/bash

set -e

usage() {
cat <<EOF
$(basename $0) [--force] [--verbose] ...
All unrecognised arguments will be passed through to the 'cockroach' command.
Accepts environment variables:
- CRDB_ROOT_USER: A user with sufficient rights to create/reset the Trillian
database (default: root).
- CRDB_ROOT_PASSWORD: The password for \$CRDB_ROOT_USER (default: none).
- CRDB_HOST: The hostname of the MySQL server (default: localhost).
- CRDB_PORT: The port the MySQL server is listening on (default: 3306).
- CRDB_DATABASE: The name to give to the new Trillian user and database
(default: test).
- CRDB_USER: The name to give to the new Trillian user (default: test).
- CRDB_PASSWORD: The password to use for the new Trillian user
(default: zaphod).
- CRDB_USER_HOST: The host that the Trillian user will connect from; use '%' as
a wildcard (default: localhost).
EOF
}

die() {
echo "$*" > /dev/stderr
exit 1
}

collect_vars() {
# set unset environment variables to defaults
[ -z ${CRDB_ROOT_USER+x} ] && CRDB_ROOT_USER="root"
[ -z ${CRDB_HOST+x} ] && CRDB_HOST="localhost"
[ -z ${CRDB_PORT+x} ] && CRDB_PORT="26257"
[ -z ${CRDB_DATABASE+x} ] && CRDB_DATABASE="defaultdb"
[ -z ${CRDB_USER+x} ] && CRDB_USER="test"
[ -z ${CRDB_PASSWORD+x} ] && CRDB_PASSWORD="zaphod"
[ -z ${CRDB_USER_HOST+x} ] && CRDB_USER_HOST="localhost"
[ -z ${CRDB_INSECURE+x} ] && CRDB_INSECURE="true"
FLAGS=()

# handle flags
FORCE=false
VERBOSE=false
while [[ $# -gt 0 ]]; do
case "$1" in
--force) FORCE=true ;;
--verbose) VERBOSE=true ;;
--help) usage; exit ;;
*) FLAGS+=("$1")
esac
shift 1
done

FLAGS+=(-u "${CRDB_ROOT_USER}")
FLAGS+=(--host "${CRDB_HOST}")
FLAGS+=(--port "${CRDB_PORT}")

# Useful for debugging
FLAGS+=(--echo-sql)

if [[ ${CRDB_INSECURE} = 'true' ]]; then
FLAGS+=(--insecure)
fi

# Optionally print flags (before appending password)
[[ ${VERBOSE} = 'true' ]] && echo "- Using MySQL Flags: ${FLAGS[@]}"

# append password if supplied
[ -z ${CRDB_ROOT_PASSWORD+x} ] || FLAGS+=(-p"${CRDB_ROOT_PASSWORD}")
}

main() {
collect_vars "$@"

readonly TRILLIAN_PATH=$(go list -f '{{.Dir}}' github.com/google/trillian)

echo "Warning: about to destroy and reset database '${CRDB_DATABASE}'"

[[ ${FORCE} = true ]] || read -p "Are you sure? [Y/N]: " -n 1 -r
echo # Print newline following the above prompt

if [ -z ${REPLY+x} ] || [[ $REPLY =~ ^[Yy]$ ]]
then
echo "Resetting DB..."
set -eux
cockroach sql "${FLAGS[@]}" -e "DROP DATABASE IF EXISTS ${CRDB_DATABASE};" || \
die "Error: Failed to drop database '${CRDB_DATABASE}'."
cockroach sql "${FLAGS[@]}" -e "CREATE DATABASE ${CRDB_DATABASE};" || \
die "Error: Failed to create database '${CRDB_DATABASE}'."
if [[ ${CRDB_INSECURE} = 'true' ]]; then
cockroach sql "${FLAGS[@]}" -e "CREATE USER IF NOT EXISTS ${CRDB_USER};" || \
die "Error: Failed to create user '${CRDB_USER}'."
else
cockroach sql "${FLAGS[@]}" -e "CREATE USER IF NOT EXISTS ${CRDB_USER} WITH PASSWORD '${CRDB_PASSWORD}';" || \
die "Error: Failed to create user '${CRDB_USER}'."
fi
cockroach sql "${FLAGS[@]}" -e "GRANT ALL PRIVILEGES ON DATABASE ${CRDB_DATABASE} TO ${CRDB_USER} WITH GRANT OPTION" || \
die "Error: Failed to grant '${CRDB_USER}' user all privileges on '${CRDB_DATABASE}'."
cockroach sql "${FLAGS[@]}" -d ${CRDB_DATABASE} < ${TRILLIAN_PATH}/storage/crdb/schema/storage.sql || \
die "Error: Failed to create tables in '${CRDB_DATABASE}' database."
echo "Reset Complete"
fi
}

main "$@"
91 changes: 91 additions & 0 deletions storage/crdb/common_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright 2022 <TBD>
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package crdb

import (
"context"
"database/sql"
"flag"
"os"
"sync"
"testing"

"k8s.io/klog/v2"

"github.com/google/trillian/storage/testdb"
)

// testDBs holds a set of test databases, one per test.
var testDBs sync.Map

type testDBHandle struct {
db *sql.DB
done func(context.Context)
}

func (db *testDBHandle) GetDB() *sql.DB {
return db.db
}

func TestMain(m *testing.M) {
flag.Parse()

if !testdb.CockroachDBAvailable() {
klog.Errorf("CockroachDB not available, skipping all CockroachDB storage tests")
return
}

status := m.Run()

// Clean up databases
testDBs.Range(func(key, value interface{}) bool {
testName := key.(string)
klog.Infof("Cleaning up database for test %s", testName)

db := value.(*testDBHandle)

// TODO(jaosorior): Set a timeout instead of using Background
db.done(context.Background())

return true
})

os.Exit(status)
}

// This is used to identify a database from the map
func getDBID(t *testing.T) string {
t.Helper()
return t.Name()
}

func openTestDBOrDie(t *testing.T) *testDBHandle {
t.Helper()

// TODO(jaosorior): Check for Cockroach
db, done, err := testdb.NewTrillianDB(context.TODO(), testdb.DriverCockroachDB)
if err != nil {
panic(err)
}

handle := &testDBHandle{
db: db,
done: done,
}

testDBs.Store(getDBID(t), handle)

return handle
}
9 changes: 9 additions & 0 deletions storage/crdb/drop_storage.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-- Caution - this removes all tables in our schema

DROP TABLE IF EXISTS Unsequenced;
DROP TABLE IF EXISTS Subtree;
DROP TABLE IF EXISTS SequencedLeafData;
DROP TABLE IF EXISTS TreeHead;
DROP TABLE IF EXISTS LeafData;
DROP TABLE IF EXISTS TreeControl;
DROP TABLE IF EXISTS Trees;
42 changes: 42 additions & 0 deletions storage/crdb/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright 2022 <TBD>
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package crdb

import (
"github.com/lib/pq"
)

var uniqueViolationErrorCode = pq.ErrorCode("23505")

// crdbToGRPC converts some types of CockroachDB errors to GRPC errors. This gives
// clients more signal when the operation can be retried.
func crdbToGRPC(err error) error {
_, ok := err.(*pq.Error)
if !ok {
return err
}
// TODO(jaosorior): Do we have a crdb equivalent for a deadlock
// error code?
return err
}

func isDuplicateErr(err error) bool {
switch err := err.(type) {
case *pq.Error:
return err.Code == uniqueViolationErrorCode
default:
return false
}
}
Loading

0 comments on commit 0702e94

Please sign in to comment.