From 1b8ab7c704d04893de4feef88c6b7ee036032eb3 Mon Sep 17 00:00:00 2001 From: JordanBrockopp Date: Wed, 31 May 2023 21:14:40 -0500 Subject: [PATCH 1/5] feat(database): add agnostic engine --- database/database.go | 58 +++++++++++++++++++++++++++- database/database_test.go | 79 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 134 insertions(+), 3 deletions(-) diff --git a/database/database.go b/database/database.go index ad26dd82a..d61cc5fc1 100644 --- a/database/database.go +++ b/database/database.go @@ -1,4 +1,4 @@ -// Copyright (c) 2022 Target Brands, Inc. All rights reserved. +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. // // Use of this source code is governed by the LICENSE file in this repository. @@ -6,10 +6,64 @@ package database import ( "fmt" + "time" + "github.com/go-vela/server/database/build" + "github.com/go-vela/server/database/hook" + "github.com/go-vela/server/database/log" + "github.com/go-vela/server/database/pipeline" + "github.com/go-vela/server/database/repo" + "github.com/go-vela/server/database/schedule" + "github.com/go-vela/server/database/secret" + "github.com/go-vela/server/database/service" + "github.com/go-vela/server/database/step" + "github.com/go-vela/server/database/user" + "github.com/go-vela/server/database/worker" "github.com/go-vela/types/constants" - "github.com/sirupsen/logrus" + + "gorm.io/gorm" +) + +type ( + // Config represents the settings required to create the engine that implements the Interface. + Config struct { + // specifies the address to use for the database client + Address string + // specifies the level of compression to use for the database client + CompressionLevel int + // specifies the connection duration to use for the database client + ConnectionLife time.Duration + // specifies the maximum idle connections for the database client + ConnectionIdle int + // specifies the maximum open connections for the database client + ConnectionOpen int + // specifies the driver to use for the database client + Driver string + // specifies the encryption key to use for the database client + EncryptionKey string + // specifies to skip creating tables and indexes for the database client + SkipCreation bool + } + + // engine represents the functionality that implements the Interface. + engine struct { + Config *Config + Database *gorm.DB + Logger *logrus.Entry + + build.BuildInterface + hook.HookInterface + log.LogInterface + pipeline.PipelineInterface + repo.RepoInterface + schedule.ScheduleInterface + secret.SecretInterface + service.ServiceInterface + step.StepInterface + user.UserInterface + worker.WorkerInterface + } ) // New creates and returns a Vela service capable of diff --git a/database/database_test.go b/database/database_test.go index 14081a382..77de76d5b 100644 --- a/database/database_test.go +++ b/database/database_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2022 Target Brands, Inc. All rights reserved. +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. // // Use of this source code is governed by the LICENSE file in this repository. @@ -7,6 +7,13 @@ package database import ( "testing" "time" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/sirupsen/logrus" + + "gorm.io/driver/postgres" + "gorm.io/driver/sqlite" + "gorm.io/gorm" ) func TestDatabase_New(t *testing.T) { @@ -86,3 +93,73 @@ func TestDatabase_New(t *testing.T) { } } } + +// testPostgres is a helper function to create a Postgres engine for testing. +func testPostgres(t *testing.T) (*engine, sqlmock.Sqlmock) { + // create the engine with test configuration + _engine := &engine{ + Config: &Config{ + CompressionLevel: 3, + ConnectionLife: 30 * time.Minute, + ConnectionIdle: 2, + ConnectionOpen: 0, + Driver: "postgres", + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", + SkipCreation: false, + }, + Logger: logrus.NewEntry(logrus.StandardLogger()), + } + + // create the new mock sql database + _sql, _mock, err := sqlmock.New( + sqlmock.MonitorPingsOption(true), + sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual), + ) + if err != nil { + t.Errorf("unable to create new SQL mock: %v", err) + } + // ensure the mock expects the ping + _mock.ExpectPing() + + // create the new mock Postgres database client + _engine.Database, err = gorm.Open( + postgres.New(postgres.Config{Conn: _sql}), + &gorm.Config{SkipDefaultTransaction: true}, + ) + if err != nil { + t.Errorf("unable to create new test postgres database: %v", err) + } + + return _engine, _mock +} + +// testSqlite is a helper function to create a Sqlite engine for testing. +func testSqlite(t *testing.T) *engine { + var err error + + // create the engine with test configuration + _engine := &engine{ + Config: &Config{ + Address: "file::memory:?cache=shared", + CompressionLevel: 3, + ConnectionLife: 30 * time.Minute, + ConnectionIdle: 2, + ConnectionOpen: 0, + Driver: "sqlite3", + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", + SkipCreation: false, + }, + Logger: logrus.NewEntry(logrus.StandardLogger()), + } + + // create the new mock Sqlite database client + _engine.Database, err = gorm.Open( + sqlite.Open(_engine.Config.Address), + &gorm.Config{SkipDefaultTransaction: true}, + ) + if err != nil { + t.Errorf("unable to create new test sqlite database: %v", err) + } + + return _engine +} From 19bf93bdc0ca1035bf4968b5d8c27bc4ba9325b0 Mon Sep 17 00:00:00 2001 From: JordanBrockopp Date: Wed, 31 May 2023 21:15:52 -0500 Subject: [PATCH 2/5] feat(database): add config.Validate() --- database/validate.go | 75 +++++++++++++++++ database/validate_test.go | 164 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 239 insertions(+) create mode 100644 database/validate.go create mode 100644 database/validate_test.go diff --git a/database/validate.go b/database/validate.go new file mode 100644 index 000000000..c16c07cd2 --- /dev/null +++ b/database/validate.go @@ -0,0 +1,75 @@ +// Copyright (c) 2023 Target Brands, Ine. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package database + +import ( + "fmt" + "strings" + + "github.com/go-vela/types/constants" + "github.com/sirupsen/logrus" +) + +// Validate verifies the required fields from the provided configuration are populated correctly. +func (c *Config) Validate() error { + logrus.Trace("validating database configuration for engine") + + // verify a database driver was provided + if len(c.Driver) == 0 { + return fmt.Errorf("no database driver provided") + } + + // verify a database address was provided + if len(c.Address) == 0 { + return fmt.Errorf("no database address provided") + } + + // check if the database address has a trailing slash + if strings.HasSuffix(c.Address, "/") { + return fmt.Errorf("invalid database address provided: address must not have trailing slash") + } + + // verify a database encryption key was provided + if len(c.EncryptionKey) == 0 { + return fmt.Errorf("no database encryption key provided") + } + + // check the database encryption key length - enforce AES-256 by forcing 32 characters in the key + if len(c.EncryptionKey) != 32 { + return fmt.Errorf("invalid database encryption key provided: key length (%d) must be 32 characters", len(c.EncryptionKey)) + } + + // verify the database compression level is valid + switch c.CompressionLevel { + case constants.CompressionNegOne: + fallthrough + case constants.CompressionZero: + fallthrough + case constants.CompressionOne: + fallthrough + case constants.CompressionTwo: + fallthrough + case constants.CompressionThree: + fallthrough + case constants.CompressionFour: + fallthrough + case constants.CompressionFive: + fallthrough + case constants.CompressionSix: + fallthrough + case constants.CompressionSeven: + fallthrough + case constants.CompressionEight: + fallthrough + case constants.CompressionNine: + break + default: + return fmt.Errorf("invalid database compression level provided: level (%d) must be between %d and %d", + c.CompressionLevel, constants.CompressionNegOne, constants.CompressionNine, + ) + } + + return nil +} diff --git a/database/validate_test.go b/database/validate_test.go new file mode 100644 index 000000000..11be746a8 --- /dev/null +++ b/database/validate_test.go @@ -0,0 +1,164 @@ +// Copyright (c) 2023 Target Brands, Ine. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package database + +import ( + "testing" + "time" +) + +func TestDatabase_Config_Validate(t *testing.T) { + tests := []struct { + failure bool + name string + config *Config + }{ + { + name: "success with postgres", + failure: false, + config: &Config{ + Driver: "postgres", + Address: "postgres://foo:bar@localhost:5432/vela", + CompressionLevel: 3, + ConnectionLife: 10 * time.Second, + ConnectionIdle: 5, + ConnectionOpen: 20, + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", + SkipCreation: false, + }, + }, + { + name: "success with sqlite3", + failure: false, + config: &Config{ + Driver: "sqlite3", + Address: "file::memory:?cache=shared", + CompressionLevel: 3, + ConnectionLife: 10 * time.Second, + ConnectionIdle: 5, + ConnectionOpen: 20, + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", + SkipCreation: false, + }, + }, + { + name: "success with negative compression level", + failure: false, + config: &Config{ + Driver: "postgres", + Address: "postgres://foo:bar@localhost:5432/vela", + CompressionLevel: -1, + ConnectionLife: 10 * time.Second, + ConnectionIdle: 5, + ConnectionOpen: 20, + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", + SkipCreation: false, + }, + }, + { + name: "failure with empty driver", + failure: true, + config: &Config{ + Driver: "", + Address: "postgres://foo:bar@localhost:5432/vela", + CompressionLevel: 3, + ConnectionLife: 10 * time.Second, + ConnectionIdle: 5, + ConnectionOpen: 20, + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", + SkipCreation: false, + }, + }, + { + name: "failure with empty address", + failure: true, + config: &Config{ + Driver: "postgres", + Address: "", + CompressionLevel: 3, + ConnectionLife: 10 * time.Second, + ConnectionIdle: 5, + ConnectionOpen: 20, + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", + SkipCreation: false, + }, + }, + { + name: "failure with invalid address", + failure: true, + config: &Config{ + Driver: "postgres", + Address: "postgres://foo:bar@localhost:5432/vela/", + CompressionLevel: 3, + ConnectionLife: 10 * time.Second, + ConnectionIdle: 5, + ConnectionOpen: 20, + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", + SkipCreation: false, + }, + }, + { + name: "failure with invalid compression level", + failure: true, + config: &Config{ + Driver: "postgres", + Address: "postgres://foo:bar@localhost:5432/vela", + CompressionLevel: 10, + ConnectionLife: 10 * time.Second, + ConnectionIdle: 5, + ConnectionOpen: 20, + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", + SkipCreation: false, + }, + }, + { + name: "failure with empty encryption key", + failure: true, + config: &Config{ + Driver: "postgres", + Address: "postgres://foo:bar@localhost:5432/vela", + CompressionLevel: 3, + ConnectionLife: 10 * time.Second, + ConnectionIdle: 5, + ConnectionOpen: 20, + EncryptionKey: "", + SkipCreation: false, + }, + }, + { + name: "failure with invalid encryption key", + failure: true, + config: &Config{ + Driver: "postgres", + Address: "postgres://foo:bar@localhost:5432/vela", + CompressionLevel: 3, + ConnectionLife: 10 * time.Second, + ConnectionIdle: 5, + ConnectionOpen: 20, + EncryptionKey: "A1B2C3D4E5G6H7I8J9K0", + SkipCreation: false, + }, + }, + } + + // run tests + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := test.config.Validate() + + if test.failure { + if err == nil { + t.Errorf("Validate for %s should have returned err", test.name) + } + + return + } + + if err != nil { + t.Errorf("Validate for %s returned err: %v", test.name, err) + } + }) + } +} From f820f6a797c9e9e6ac946a130ba26cb6dd151848 Mon Sep 17 00:00:00 2001 From: JordanBrockopp Date: Wed, 31 May 2023 21:26:30 -0500 Subject: [PATCH 3/5] feat(database): add engine.Close() --- database/close.go | 18 ++++++++++ database/close_test.go | 82 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 database/close.go create mode 100644 database/close_test.go diff --git a/database/close.go b/database/close.go new file mode 100644 index 000000000..0596b972e --- /dev/null +++ b/database/close.go @@ -0,0 +1,18 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package database + +// Close stops and terminates the connection to the database. +func (e *engine) Close() error { + e.Logger.Tracef("closing connection to the %s database", e.Driver()) + + // capture database/sql database from gorm.io/gorm database + _sql, err := e.Database.DB() + if err != nil { + return err + } + + return _sql.Close() +} diff --git a/database/close_test.go b/database/close_test.go new file mode 100644 index 000000000..bb9c6635b --- /dev/null +++ b/database/close_test.go @@ -0,0 +1,82 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package database + +import ( + "testing" + + "github.com/sirupsen/logrus" + "gorm.io/gorm" +) + +func TestDatabase_Engine_Close(t *testing.T) { + _postgres, _mock := testPostgres(t) + defer _postgres.Close() + // ensure the mock expects the close + _mock.ExpectClose() + + // create a test database without mocking the call + _unmocked, _ := testPostgres(t) + + _sqlite := testSqlite(t) + defer _sqlite.Close() + + // setup tests + tests := []struct { + failure bool + name string + database *engine + }{ + { + name: "success with postgres", + failure: false, + database: _postgres, + }, + { + name: "success with sqlite3", + failure: false, + database: _sqlite, + }, + { + name: "failure without mocked call", + failure: true, + database: _unmocked, + }, + { + name: "failure with invalid gorm database", + failure: true, + database: &engine{ + Config: &Config{ + Driver: "invalid", + }, + Database: &gorm.DB{ + Config: &gorm.Config{ + ConnPool: nil, + }, + }, + Logger: logrus.NewEntry(logrus.StandardLogger()), + }, + }, + } + + // run tests + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := test.database.Close() + + if test.failure { + if err == nil { + t.Errorf("Close for %s should have returned err", test.name) + } + + return + } + + if err != nil { + t.Errorf("Close for %s returned err: %v", test.name, err) + } + }) + } +} From 5d5490cce75e592e947df6b808713278c23b4576 Mon Sep 17 00:00:00 2001 From: JordanBrockopp Date: Wed, 31 May 2023 21:26:59 -0500 Subject: [PATCH 4/5] feat(database): add engine.Driver() --- database/driver.go | 10 +++++++++ database/driver_test.go | 47 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 database/driver.go create mode 100644 database/driver_test.go diff --git a/database/driver.go b/database/driver.go new file mode 100644 index 000000000..a81662846 --- /dev/null +++ b/database/driver.go @@ -0,0 +1,10 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package database + +// Driver outputs the configured database driver. +func (e *engine) Driver() string { + return e.Config.Driver +} diff --git a/database/driver_test.go b/database/driver_test.go new file mode 100644 index 000000000..6d382d8cd --- /dev/null +++ b/database/driver_test.go @@ -0,0 +1,47 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package database + +import ( + "strings" + "testing" +) + +func TestDatabase_Engine_Driver(t *testing.T) { + _postgres, _ := testPostgres(t) + defer _postgres.Close() + + _sqlite := testSqlite(t) + defer _sqlite.Close() + + // setup tests + tests := []struct { + name string + database *engine + want string + }{ + { + name: "success with postgres", + database: _postgres, + want: "postgres", + }, + { + name: "success with sqlite3", + database: _sqlite, + want: "sqlite3", + }, + } + + // run tests + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := test.database.Driver() + + if !strings.EqualFold(got, test.want) { + t.Errorf("Driver for %s is %v, want %v", test.name, got, test.want) + } + }) + } +} From ab04f2fba92464bd604be1c7f60ecf64a47412ba Mon Sep 17 00:00:00 2001 From: JordanBrockopp Date: Wed, 31 May 2023 21:37:17 -0500 Subject: [PATCH 5/5] chore: minor fixes --- database/close_test.go | 1 + database/database.go | 18 +++++++++--------- database/validate_test.go | 1 + 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/database/close_test.go b/database/close_test.go index bb9c6635b..b555e78d5 100644 --- a/database/close_test.go +++ b/database/close_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/sirupsen/logrus" + "gorm.io/gorm" ) diff --git a/database/database.go b/database/database.go index d61cc5fc1..bf0ca2724 100644 --- a/database/database.go +++ b/database/database.go @@ -28,21 +28,21 @@ import ( type ( // Config represents the settings required to create the engine that implements the Interface. Config struct { - // specifies the address to use for the database client + // specifies the address to use for the database engine Address string - // specifies the level of compression to use for the database client + // specifies the level of compression to use for the database engine CompressionLevel int - // specifies the connection duration to use for the database client - ConnectionLife time.Duration - // specifies the maximum idle connections for the database client + // specifies the maximum idle connections for the database engine ConnectionIdle int - // specifies the maximum open connections for the database client + // specifies the connection duration to use for the database engine + ConnectionLife time.Duration + // specifies the maximum open connections for the database engine ConnectionOpen int - // specifies the driver to use for the database client + // specifies the driver to use for the database engine Driver string - // specifies the encryption key to use for the database client + // specifies the encryption key to use for the database engine EncryptionKey string - // specifies to skip creating tables and indexes for the database client + // specifies to skip creating tables and indexes for the database engine SkipCreation bool } diff --git a/database/validate_test.go b/database/validate_test.go index 11be746a8..356705fe2 100644 --- a/database/validate_test.go +++ b/database/validate_test.go @@ -10,6 +10,7 @@ import ( ) func TestDatabase_Config_Validate(t *testing.T) { + // setup tests tests := []struct { failure bool name string