From 8162a597accfd3f95f923c2cb39e67b9d6575ddd Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Wed, 23 Oct 2019 17:16:58 +0100 Subject: [PATCH] Move storage providers (#1919) This change moves the individual storage providers to the corresponding implementation packages. This makes the server package agnostic of storage implementations, and makes it the responsibility of main.go files to import them. --- CHANGELOG.md | 5 ++- server/mysql_quota_provider.go | 3 +- server/trillian_log_server/main.go | 4 +++ server/trillian_log_signer/main.go | 4 +++ server/trillian_map_server/main.go | 4 +++ .../cloudspanner/storage_provider.go | 17 +++++----- .../memory/provider.go | 15 ++++----- .../memory/provider_test.go | 14 ++++---- .../mysql/provider.go | 32 ++++++++++--------- .../mysql/provider_test.go | 13 ++++---- .../postgres/provider.go | 13 ++++---- 11 files changed, 71 insertions(+), 53 deletions(-) rename server/cloudspanner_storage_provider.go => storage/cloudspanner/storage_provider.go (93%) rename server/memory_storage_provider.go => storage/memory/provider.go (76%) rename server/memory_storage_provider_test.go => storage/memory/provider_test.go (85%) rename server/mysql_storage_provider.go => storage/mysql/provider.go (82%) rename server/mysql_storage_provider_test.go => storage/mysql/provider_test.go (74%) rename server/postgres_storage_provider.go => storage/postgres/provider.go (84%) diff --git a/CHANGELOG.md b/CHANGELOG.md index befc11ac22..8dee600cd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,10 @@ SetAndVerifyMapLeaves method was deleted. The StorageProvider type and helpers have been moved from the server package to storage. Aliases for the old types/functions are created for backward compatibility, but the new code should not use them as we will remove them with -the next major version bump. +the next major version bump. The individual storage providers have been moved to +the corresponding packages, and are now required to be imported explicitly by +the main file in order to be registered. We are including only MySQL and +cloudspanner providers by default, since these are the ones that we support. ## v1.3.2 - Module fixes diff --git a/server/mysql_quota_provider.go b/server/mysql_quota_provider.go index 25e15f4c7d..fb363792e1 100644 --- a/server/mysql_quota_provider.go +++ b/server/mysql_quota_provider.go @@ -20,6 +20,7 @@ import ( "github.com/golang/glog" "github.com/google/trillian/quota" "github.com/google/trillian/quota/mysqlqm" + "github.com/google/trillian/storage/mysql" ) // QuotaMySQL represents the MySQL quota implementation. @@ -37,7 +38,7 @@ func init() { } func newMySQLQuotaManager() (quota.Manager, error) { - db, err := getMySQLDatabase() + db, err := mysql.GetDatabase() if err != nil { return nil, err } diff --git a/server/trillian_log_server/main.go b/server/trillian_log_server/main.go index 586f9dae36..ef32500ea2 100644 --- a/server/trillian_log_server/main.go +++ b/server/trillian_log_server/main.go @@ -48,6 +48,10 @@ import ( _ "github.com/google/trillian/crypto/keys/pem/proto" _ "github.com/google/trillian/crypto/keys/pkcs11/proto" + // Register supported storage providers. + _ "github.com/google/trillian/storage/cloudspanner" + _ "github.com/google/trillian/storage/mysql" + // Load hashers _ "github.com/google/trillian/merkle/rfc6962" ) diff --git a/server/trillian_log_signer/main.go b/server/trillian_log_signer/main.go index 973b8e70fb..c68702ca3e 100644 --- a/server/trillian_log_signer/main.go +++ b/server/trillian_log_signer/main.go @@ -48,6 +48,10 @@ import ( _ "github.com/google/trillian/crypto/keys/pem/proto" _ "github.com/google/trillian/crypto/keys/pkcs11/proto" + // Register supported storage providers. + _ "github.com/google/trillian/storage/cloudspanner" + _ "github.com/google/trillian/storage/mysql" + // Load hashers _ "github.com/google/trillian/merkle/rfc6962" ) diff --git a/server/trillian_map_server/main.go b/server/trillian_map_server/main.go index 14c22c4b49..153921db7d 100644 --- a/server/trillian_map_server/main.go +++ b/server/trillian_map_server/main.go @@ -45,6 +45,10 @@ import ( _ "github.com/google/trillian/crypto/keys/pem/proto" _ "github.com/google/trillian/crypto/keys/pkcs11/proto" + // Register supported storage providers. + _ "github.com/google/trillian/storage/cloudspanner" + _ "github.com/google/trillian/storage/mysql" + // Load hashers _ "github.com/google/trillian/merkle/coniks" _ "github.com/google/trillian/merkle/maphasher" diff --git a/server/cloudspanner_storage_provider.go b/storage/cloudspanner/storage_provider.go similarity index 93% rename from server/cloudspanner_storage_provider.go rename to storage/cloudspanner/storage_provider.go index c2fa336639..31a48fd137 100644 --- a/server/cloudspanner_storage_provider.go +++ b/storage/cloudspanner/storage_provider.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package server +package cloudspanner import ( "bytes" @@ -28,7 +28,6 @@ import ( "github.com/golang/glog" "github.com/google/trillian/monitoring" "github.com/google/trillian/storage" - "github.com/google/trillian/storage/cloudspanner" ) var ( @@ -50,7 +49,7 @@ var ( ) func init() { - if err := RegisterStorageProvider("cloud_spanner", newCloudSpannerStorageProvider); err != nil { + if err := storage.RegisterProvider("cloud_spanner", newCloudSpannerStorageProvider); err != nil { panic(err) } } @@ -88,7 +87,7 @@ func configFromFlags() spanner.ClientConfig { return r } -func newCloudSpannerStorageProvider(_ monitoring.MetricFactory) (StorageProvider, error) { +func newCloudSpannerStorageProvider(_ monitoring.MetricFactory) (storage.Provider, error) { csMu.Lock() defer csMu.Unlock() @@ -109,7 +108,7 @@ func newCloudSpannerStorageProvider(_ monitoring.MetricFactory) (StorageProvider // LogStorage builds and returns a new storage.LogStorage using CloudSpanner. func (s *cloudSpannerProvider) LogStorage() storage.LogStorage { warn() - opts := cloudspanner.LogStorageOptions{} + opts := LogStorageOptions{} frac := *csDequeueAcrossMerkleBucketsFraction if frac > 1.0 { frac = 1.0 @@ -121,23 +120,23 @@ func (s *cloudSpannerProvider) LogStorage() storage.LogStorage { if *csReadOnlyStaleness > 0 { opts.ReadOnlyStaleness = *csReadOnlyStaleness } - return cloudspanner.NewLogStorageWithOpts(s.client, opts) + return NewLogStorageWithOpts(s.client, opts) } // MapStorage builds and returns a new storage.MapStorage using CloudSpanner. func (s *cloudSpannerProvider) MapStorage() storage.MapStorage { warn() - opts := cloudspanner.MapStorageOptions{} + opts := MapStorageOptions{} if *csReadOnlyStaleness > 0 { opts.ReadOnlyStaleness = *csReadOnlyStaleness } - return cloudspanner.NewMapStorageWithOpts(s.client, opts) + return NewMapStorageWithOpts(s.client, opts) } // AdminStorage builds and returns a new storage.AdminStorage using CloudSpanner. func (s *cloudSpannerProvider) AdminStorage() storage.AdminStorage { warn() - return cloudspanner.NewAdminStorage(s.client) + return NewAdminStorage(s.client) } // Close shuts down this provider. Calls to the other methods will fail diff --git a/server/memory_storage_provider.go b/storage/memory/provider.go similarity index 76% rename from server/memory_storage_provider.go rename to storage/memory/provider.go index 75ac71a7f5..9efd249a66 100644 --- a/server/memory_storage_provider.go +++ b/storage/memory/provider.go @@ -12,35 +12,34 @@ // See the License for the specific language governing permissions and // limitations under the License. -package server +package memory import ( "github.com/golang/glog" "github.com/google/trillian/monitoring" "github.com/google/trillian/storage" - "github.com/google/trillian/storage/memory" ) func init() { - if err := RegisterStorageProvider("memory", newMemoryStorageProvider); err != nil { + if err := storage.RegisterProvider("memory", newMemoryStorageProvider); err != nil { glog.Fatalf("Failed to register storage provider memory: %v", err) } } type memProvider struct { mf monitoring.MetricFactory - ts *memory.TreeStorage + ts *TreeStorage } -func newMemoryStorageProvider(mf monitoring.MetricFactory) (StorageProvider, error) { +func newMemoryStorageProvider(mf monitoring.MetricFactory) (storage.Provider, error) { return &memProvider{ mf: mf, - ts: memory.NewTreeStorage(), + ts: NewTreeStorage(), }, nil } func (s *memProvider) LogStorage() storage.LogStorage { - return memory.NewLogStorage(s.ts, s.mf) + return NewLogStorage(s.ts, s.mf) } func (s *memProvider) MapStorage() storage.MapStorage { @@ -48,7 +47,7 @@ func (s *memProvider) MapStorage() storage.MapStorage { } func (s *memProvider) AdminStorage() storage.AdminStorage { - return memory.NewAdminStorage(s.ts) + return NewAdminStorage(s.ts) } func (s *memProvider) Close() error { diff --git a/server/memory_storage_provider_test.go b/storage/memory/provider_test.go similarity index 85% rename from server/memory_storage_provider_test.go rename to storage/memory/provider_test.go index c2015fdc2d..5ab4fd8223 100644 --- a/server/memory_storage_provider_test.go +++ b/storage/memory/provider_test.go @@ -12,14 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -package server +package memory import ( "testing" + + "github.com/google/trillian/storage" ) func TestMemoryStorageProvider(t *testing.T) { - sp, err := NewStorageProvider("memory", nil) + sp, err := storage.NewProvider("memory", nil) if err != nil { t.Fatalf("Got an unexpected error: %v", err) } @@ -29,7 +31,7 @@ func TestMemoryStorageProvider(t *testing.T) { } func TestMemoryStorageProviderLogStorage(t *testing.T) { - sp, err := NewStorageProvider("memory", nil) + sp, err := storage.NewProvider("memory", nil) if err != nil { t.Fatalf("Got an unexpected error: %v", err) } @@ -41,7 +43,7 @@ func TestMemoryStorageProviderLogStorage(t *testing.T) { } func TestMemoryStorageProviderMapStorage(t *testing.T) { - sp, err := NewStorageProvider("memory", nil) + sp, err := storage.NewProvider("memory", nil) if err != nil { t.Fatalf("Got an unexpected error: %v", err) } @@ -53,7 +55,7 @@ func TestMemoryStorageProviderMapStorage(t *testing.T) { } func TestMemoryStorageProviderAdminStorage(t *testing.T) { - sp, err := NewStorageProvider("memory", nil) + sp, err := storage.NewProvider("memory", nil) if err != nil { t.Fatalf("Got an unexpected error: %v", err) } @@ -65,7 +67,7 @@ func TestMemoryStorageProviderAdminStorage(t *testing.T) { } func TestMemoryStorageProviderClose(t *testing.T) { - sp, err := NewStorageProvider("memory", nil) + sp, err := storage.NewProvider("memory", nil) if err != nil { t.Fatalf("Got an unexpected error: %v", err) } diff --git a/server/mysql_storage_provider.go b/storage/mysql/provider.go similarity index 82% rename from server/mysql_storage_provider.go rename to storage/mysql/provider.go index dfce9771e1..8a56cb9ce7 100644 --- a/server/mysql_storage_provider.go +++ b/storage/mysql/provider.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package server +package mysql import ( "database/sql" @@ -22,7 +22,6 @@ import ( "github.com/golang/glog" "github.com/google/trillian/monitoring" "github.com/google/trillian/storage" - "github.com/google/trillian/storage/mysql" // Load MySQL driver _ "github.com/go-sql-driver/mysql" @@ -39,8 +38,18 @@ var ( mysqlStorageInstance *mysqlProvider ) +// GetDatabase returns an instance of MySQL database, or creates one. +// +// TODO(pavelkalinnikov): Make the dependency of MySQL quota provider from +// MySQL storage provider explicit. +func GetDatabase() (*sql.DB, error) { + mysqlMu.Lock() + defer mysqlMu.Unlock() + return getMySQLDatabaseLocked() +} + func init() { - if err := RegisterStorageProvider("mysql", newMySQLStorageProvider); err != nil { + if err := storage.RegisterProvider("mysql", newMySQLStorageProvider); err != nil { glog.Fatalf("Failed to register storage provider mysql: %v", err) } } @@ -50,7 +59,7 @@ type mysqlProvider struct { mf monitoring.MetricFactory } -func newMySQLStorageProvider(mf monitoring.MetricFactory) (StorageProvider, error) { +func newMySQLStorageProvider(mf monitoring.MetricFactory) (storage.Provider, error) { mysqlMu.Lock() defer mysqlMu.Unlock() if mysqlStorageInstance == nil { @@ -66,20 +75,13 @@ func newMySQLStorageProvider(mf monitoring.MetricFactory) (StorageProvider, erro return mysqlStorageInstance, nil } -// getMySQLDatabase returns an instance of MySQL database, or creates one. -func getMySQLDatabase() (*sql.DB, error) { - mysqlMu.Lock() - defer mysqlMu.Unlock() - return getMySQLDatabaseLocked() -} - // getMySQLDatabaseLocked returns an instance of MySQL database, or creates // one. Requires mysqlMu to be locked. func getMySQLDatabaseLocked() (*sql.DB, error) { if mysqlDB != nil || mysqlErr != nil { return mysqlDB, mysqlErr } - db, err := mysql.OpenDB(*mySQLURI) + db, err := OpenDB(*mySQLURI) if err != nil { mysqlErr = err return nil, err @@ -95,15 +97,15 @@ func getMySQLDatabaseLocked() (*sql.DB, error) { } func (s *mysqlProvider) LogStorage() storage.LogStorage { - return mysql.NewLogStorage(s.db, s.mf) + return NewLogStorage(s.db, s.mf) } func (s *mysqlProvider) MapStorage() storage.MapStorage { - return mysql.NewMapStorage(s.db) + return NewMapStorage(s.db) } func (s *mysqlProvider) AdminStorage() storage.AdminStorage { - return mysql.NewAdminStorage(s.db) + return NewAdminStorage(s.db) } func (s *mysqlProvider) Close() error { diff --git a/server/mysql_storage_provider_test.go b/storage/mysql/provider_test.go similarity index 74% rename from server/mysql_storage_provider_test.go rename to storage/mysql/provider_test.go index 6b4a3a182a..c0fafd3f33 100644 --- a/server/mysql_storage_provider_test.go +++ b/storage/mysql/provider_test.go @@ -12,12 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -package server +package mysql import ( "flag" "testing" + "github.com/google/trillian/storage" "github.com/google/trillian/testonly/flagsaver" ) @@ -28,18 +29,18 @@ func TestMySQLStorageProviderErrorPersistence(t *testing.T) { } // First call: This should fail due to the Database URL being garbage. - _, err1 := NewStorageProvider("mysql", nil) + _, err1 := storage.NewProvider("mysql", nil) if err1 == nil { - t.Fatalf("Expected 'server.NewStorageProvider' to fail") + t.Fatalf("Expected 'storage.NewProvider' to fail") } // Second call: This should fail with the same error. - _, err2 := NewStorageProvider("mysql", nil) + _, err2 := storage.NewProvider("mysql", nil) if err2 == nil { - t.Fatalf("Expected second call to 'server.NewStorageProvider' to fail") + t.Fatalf("Expected second call to 'storage.NewProvider' to fail") } if err2 != err1 { - t.Fatalf("Expected second call to 'server.NewStorageProvider' to fail with %q, instead got: %q", err1, err2) + t.Fatalf("Expected second call to 'storage.NewProvider' to fail with %q, instead got: %q", err1, err2) } } diff --git a/server/postgres_storage_provider.go b/storage/postgres/provider.go similarity index 84% rename from server/postgres_storage_provider.go rename to storage/postgres/provider.go index 8c045b11f9..1a3ab89eaa 100644 --- a/server/postgres_storage_provider.go +++ b/storage/postgres/provider.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package server +package postgres import ( "database/sql" @@ -22,7 +22,6 @@ import ( "github.com/golang/glog" "github.com/google/trillian/monitoring" "github.com/google/trillian/storage" - "github.com/google/trillian/storage/postgres" // Load PG driver _ "github.com/lib/pq" @@ -36,7 +35,7 @@ var ( ) func init() { - if err := RegisterStorageProvider("postgres", newPGProvider); err != nil { + if err := storage.RegisterProvider("postgres", newPGProvider); err != nil { glog.Fatalf("Failed to register storage provider postgres: %v", err) } } @@ -46,10 +45,10 @@ type pgProvider struct { mf monitoring.MetricFactory } -func newPGProvider(mf monitoring.MetricFactory) (StorageProvider, error) { +func newPGProvider(mf monitoring.MetricFactory) (storage.Provider, error) { pgOnce.Do(func() { var db *sql.DB - db, pgOnceErr = postgres.OpenDB(*pgConnStr) + db, pgOnceErr = OpenDB(*pgConnStr) if pgOnceErr != nil { return } @@ -68,7 +67,7 @@ func newPGProvider(mf monitoring.MetricFactory) (StorageProvider, error) { func (s *pgProvider) LogStorage() storage.LogStorage { glog.Warningf("Support for the PostgreSQL log is experimental. Please use at your own risk!!!") - return postgres.NewLogStorage(s.db, s.mf) + return NewLogStorage(s.db, s.mf) } func (s *pgProvider) MapStorage() storage.MapStorage { @@ -76,7 +75,7 @@ func (s *pgProvider) MapStorage() storage.MapStorage { } func (s *pgProvider) AdminStorage() storage.AdminStorage { - return postgres.NewAdminStorage(s.db) + return NewAdminStorage(s.db) } func (s *pgProvider) Close() error {