diff --git a/cmd/notary-server/config.go b/cmd/notary-server/config.go index 336736f774..6e1e3a8f85 100644 --- a/cmd/notary-server/config.go +++ b/cmd/notary-server/config.go @@ -80,9 +80,12 @@ func getStore(configuration *viper.Viper, hRegister healthRegister) ( if err != nil { return nil, err } - s, err := storage.NewSQLStorage(storeConfig.Backend, storeConfig.Source) - store = *storage.NewTUFMetaStorage(s) - hRegister("DB operational", s.CheckHealth, time.Second*60) + var s *storage.SQLStorage + s, err = storage.NewSQLStorage(storeConfig.Backend, storeConfig.Source) + if err == nil { + store = *storage.NewTUFMetaStorage(s) + hRegister("DB operational", s.CheckHealth, time.Second*60) + } case notary.RethinkDBBackend: var sess *gorethink.Session storeConfig, err := utils.ParseRethinkDBStorage(configuration) diff --git a/cmd/notary-signer/main.go b/cmd/notary-signer/main.go index 07cff4a31f..ae4c9ba93b 100644 --- a/cmd/notary-signer/main.go +++ b/cmd/notary-signer/main.go @@ -93,39 +93,26 @@ func setUpCryptoservices(configuration *viper.Viper, allowedBackends []string) ( if err != nil { return nil, err } - sess, err = rethinkdb.Connection(storeConfig.CA, storeConfig.Source) - if err == nil { - defaultAlias := configuration.GetString("storage.default_alias") - if defaultAlias == "" { - // backwards compatibility - support this environment variable - defaultAlias = configuration.GetString(defaultAliasEnv) - } - - if defaultAlias == "" { - return nil, fmt.Errorf("must provide a default alias for the key DB") - } - logrus.Debug("Default Alias: ", defaultAlias) - s := keydbstore.NewRethinkDBKeyStore(passphraseRetriever, defaultAlias, sess) - keyStore = s - health.RegisterPeriodicFunc("DB operational", s.CheckHealth, time.Minute) + defaultAlias, err := getDefaultAlias(configuration) + if err != nil { + return nil, err } - + sess, err = rethinkdb.Connection(storeConfig.CA, storeConfig.Source) + if err != nil { + return nil, err + } + s := keydbstore.NewRethinkDBKeyStore(passphraseRetriever, defaultAlias, sess) + health.RegisterPeriodicFunc("DB operational", s.CheckHealth, time.Minute) + keyStore = s case notary.MySQLBackend, notary.SQLiteBackend: storeConfig, err := utils.ParseSQLStorage(configuration) if err != nil { return nil, err } - defaultAlias := configuration.GetString("storage.default_alias") - if defaultAlias == "" { - // backwards compatibility - support this environment variable - defaultAlias = configuration.GetString(defaultAliasEnv) + defaultAlias, err := getDefaultAlias(configuration) + if err != nil { + return nil, err } - - if defaultAlias == "" { - return nil, fmt.Errorf("must provide a default alias for the key DB") - } - logrus.Debug("Default Alias: ", defaultAlias) - dbStore, err := keydbstore.NewKeyDBStore( passphraseRetriever, defaultAlias, storeConfig.Backend, storeConfig.Source) if err != nil { @@ -152,6 +139,20 @@ func setUpCryptoservices(configuration *viper.Viper, allowedBackends []string) ( return cryptoServices, nil } +func getDefaultAlias(configuration *viper.Viper) (string, error) { + defaultAlias := configuration.GetString("storage.default_alias") + if defaultAlias == "" { + // backwards compatibility - support this environment variable + defaultAlias = configuration.GetString(defaultAliasEnv) + } + + if defaultAlias == "" { + return "", fmt.Errorf("must provide a default alias for the key DB") + } + logrus.Debug("Default Alias: ", defaultAlias) + return defaultAlias, nil +} + // set up the GRPC server func setupGRPCServer(grpcAddr string, tlsConfig *tls.Config, cryptoServices signer.CryptoServiceIndex) (*grpc.Server, net.Listener, error) { diff --git a/cmd/notary-signer/main_test.go b/cmd/notary-signer/main_test.go index 7c411d3667..f1e58edc9e 100644 --- a/cmd/notary-signer/main_test.go +++ b/cmd/notary-signer/main_test.go @@ -113,6 +113,23 @@ func TestSetupCryptoServicesDBStoreNoDefaultAlias(t *testing.T) { require.Contains(t, err.Error(), "must provide a default alias for the key DB") } +// If a default alias is not provided to a rethinkdb backend, an error is returned. +func TestSetupCryptoServicesRethinkDBStoreNoDefaultAlias(t *testing.T) { + _, err := setUpCryptoservices( + configure(fmt.Sprintf( + `{"storage": { + "backend": "%s", + "db_url": "host:port", + "tls_ca_file": "/tls/ca.pem", + "database": "rethinkdbtest" + } + }`, + notary.RethinkDBBackend)), + []string{notary.RethinkDBBackend}) + require.Error(t, err) + require.Contains(t, err.Error(), "must provide a default alias for the key DB") +} + // If a default alias *is* provided to a valid DB backend, a valid // CryptoService is returned. (This depends on ParseStorage, which is tested // separately, so this doesn't test all the possible cases of storage diff --git a/fixtures/server-config.rethink.json b/fixtures/server-config.rethink.json index 525ebbe9cd..c7dc6ecfa6 100644 --- a/fixtures/server-config.rethink.json +++ b/fixtures/server-config.rethink.json @@ -18,7 +18,6 @@ }, "storage": { "backend": "rethinkdb", - "auth_key": "devauthkey", "db_url": "rdb-proxy.rdb", "database": "notaryserver", "tls_ca_file": "/tls/ca.pem" diff --git a/fixtures/signer-config.rethink.json b/fixtures/signer-config.rethink.json index eb1c01fd82..d2cd8fa161 100644 --- a/fixtures/signer-config.rethink.json +++ b/fixtures/signer-config.rethink.json @@ -11,7 +11,6 @@ }, "storage": { "backend": "rethinkdb", - "auth_key": "devauthkey", "db_url": "rdb-proxy.rdb", "database": "notarysigner", "tls_ca_file": "/tls/ca.pem" diff --git a/server/storage/database_test.go b/server/storage/database_test.go index 090cfe8059..88bb51574d 100644 --- a/server/storage/database_test.go +++ b/server/storage/database_test.go @@ -19,16 +19,7 @@ import ( // SampleTUF returns a sample TUFFile with the given Version (ID will have // to be set independently) func SampleTUF(version int) TUFFile { - data := []byte("1") - checksum := sha256.Sum256(data) - hexChecksum := hex.EncodeToString(checksum[:]) - return TUFFile{ - Gun: "testGUN", - Role: "root", - Version: version, - Sha256: hexChecksum, - Data: []byte("1"), - } + return SampleCustomTUF(data.CanonicalRootRole, "testGUN", []byte("1"), version) } func SampleCustomTUF(role, gun string, data []byte, version int) TUFFile { diff --git a/utils/configuration.go b/utils/configuration.go index 9dd0a6c34a..78b6c31831 100644 --- a/utils/configuration.go +++ b/utils/configuration.go @@ -26,9 +26,8 @@ type Storage struct { // RethinkDBStorage is configuration about a RethinkDB backend service type RethinkDBStorage struct { Storage - CA string - AuthKey string - DBName string + CA string + DBName string } // GetPathRelativeToConfig gets a configuration key which is a path, and if @@ -117,9 +116,8 @@ func ParseRethinkDBStorage(configuration *viper.Viper) (*RethinkDBStorage, error Backend: configuration.GetString("storage.backend"), Source: configuration.GetString("storage.db_url"), }, - CA: configuration.GetString("storage.tls_ca_file"), - AuthKey: configuration.GetString("storage.auth_key"), - DBName: configuration.GetString("storage.database"), + CA: configuration.GetString("storage.tls_ca_file"), + DBName: configuration.GetString("storage.database"), } switch { @@ -138,11 +136,6 @@ func ParseRethinkDBStorage(configuration *viper.Viper) (*RethinkDBStorage, error "cowardly refusal to connect to %s without a CA cert", store.Backend, ) - case store.AuthKey == "": - return nil, fmt.Errorf( - "cowardly refusal to connect to %s without an AuthKey", - store.Backend, - ) case store.DBName == "": return nil, fmt.Errorf( "%s requires a specific database to connect to", diff --git a/utils/configuration_test.go b/utils/configuration_test.go index d7bca8402f..806d49ed5a 100644 --- a/utils/configuration_test.go +++ b/utils/configuration_test.go @@ -173,7 +173,7 @@ func TestParseInvalidStorageBackend(t *testing.T) { } // If there is no DB url for non-memory backends, an error is returned. -func TestParseInvalidStorageNoDBSource(t *testing.T) { +func TestParseInvalidSQLStorageNoDBSource(t *testing.T) { invalids := []string{ `{"storage": {"backend": "%s"}}`, `{"storage": {"backend": "%s", "db_url": ""}}`, @@ -208,6 +208,67 @@ func TestParseSQLStorageDBStore(t *testing.T) { require.Equal(t, expected, *store) } +// ParseRethinkDBStorage will reject non rethink databases +func TestParseRethinkStorageDBStoreInvalidBackend(t *testing.T) { + config := configure(`{ + "storage": { + "backend": "mysql", + "db_url": "username:password@tcp(hostname:1234)/dbname", + "tls_ca_file": "/tls/ca.pem", + "database": "rethinkdbtest" + } + }`) + + _, err := ParseRethinkDBStorage(config) + require.Error(t, err) + require.Contains(t, err.Error(), "not a supported RethinkDB backend") +} + +// ParseRethinkDBStorage will require a db_url for rethink databases +func TestParseRethinkStorageDBStoreEmptyDBUrl(t *testing.T) { + config := configure(`{ + "storage": { + "backend": "rethinkdb", + "tls_ca_file": "/tls/ca.pem", + "database": "rethinkdbtest" + } + }`) + + _, err := ParseRethinkDBStorage(config) + require.Error(t, err) + require.Contains(t, err.Error(), "must provide a non-empty host:port") +} + +// ParseRethinkDBStorage will require a dbname for rethink databases +func TestParseRethinkStorageDBStoreEmptyDBName(t *testing.T) { + config := configure(`{ + "storage": { + "backend": "rethinkdb", + "db_url": "username:password@tcp(hostname:1234)/dbname", + "tls_ca_file": "/tls/ca.pem" + } + }`) + + _, err := ParseRethinkDBStorage(config) + require.Error(t, err) + require.Contains(t, err.Error(), "requires a specific database to connect to") +} + +// ParseRethinkDBStorage will require a CA cert for rethink databases +func TestParseRethinkStorageDBStoreEmptyCA(t *testing.T) { + config := configure(`{ + "storage": { + "backend": "rethinkdb", + "db_url": "username:password@tcp(hostname:1234)/dbname", + "database": "rethinkdbtest" + } + }`) + + _, err := ParseRethinkDBStorage(config) + require.Error(t, err) + require.Contains(t, err.Error(), "cowardly refusal to connect to rethinkdb without a CA cert") +} + func TestParseSQLStorageWithEnvironmentVariables(t *testing.T) { config := configure(`{ "storage": {