Some additional logging messages and assertions about the KeyDBStore …

…implementations.

Also wait to lock the keydb cache until after a key was retrieved successfully.

Signed-off-by: Ying Li <[email protected]>

signer/keydbstore/cachedkeystore.go

@@ -49,8 +49,6 @@ func (s *cachedKeyStore) AddKey(keyInfo trustmanager.KeyInfo, privKey data.Priva
 
 // GetKey returns the PrivateKey given a KeyID
 func (s *cachedKeyStore) GetKey(keyID string) (data.PrivateKey, string, error) {
-	s.lock.Lock()
-	defer s.lock.Unlock()
 	cachedKeyEntry, ok := s.cachedKeys[keyID]
 	if ok {
 		return cachedKeyEntry.key, cachedKeyEntry.role, nil
@@ -59,10 +57,13 @@ func (s *cachedKeyStore) GetKey(keyID string) (data.PrivateKey, string, error) {
 	// retrieve the key from the underlying store and put it into the cache
 	privKey, role, err := s.KeyStore.GetKey(keyID)
 	if err == nil {
+		s.lock.Lock()
+		defer s.lock.Unlock()
 		// Add the key to cache
 		s.cachedKeys[privKey.ID()] = &cachedKey{key: privKey, role: role}
+		return privKey, role, nil
 	}
-	return privKey, role, err
+	return nil, "", err
 }
 
 // RemoveKey removes the key from the keyfilestore

signer/keydbstore/mysql_test.go

@@ -46,6 +46,9 @@ func init() {
 		}
 		cleanup1()
 		dbStore := SetupSQLDB(t, "mysql", dburl)
+
+		require.Equal(t, "mysql", dbStore.Name())
+
 		return dbStore, func() {
 			dbStore.db.Close()
 			cleanup1()

signer/keydbstore/rethink_keydbstore.go

@@ -136,7 +136,7 @@ func (rdb *RethinkDBKeyStore) AddKey(keyInfo trustmanager.KeyInfo, privKey data.
 	// Add encrypted private key to the database
 	_, err = gorethink.DB(rdb.dbName).Table(rethinkPrivKey.TableName()).Insert(rethinkPrivKey).RunWrite(rdb.sess)
 	if err != nil {
-		return fmt.Errorf("failed to add private key to database: %s", privKey.ID())
+		return fmt.Errorf("failed to add private key %s to database: %s", privKey.ID(), err.Error())
 	}
 
 	return nil
@@ -206,7 +206,7 @@ func (rdb RethinkDBKeyStore) RemoveKey(keyID string) error {
 	dbPrivateKey := RDBPrivateKey{KeyID: keyID}
 	_, err := gorethink.DB(rdb.dbName).Table(dbPrivateKey.TableName()).Filter(gorethink.Row.Field("key_id").Eq(keyID)).Delete().RunWrite(rdb.sess)
 	if err != nil {
-		return fmt.Errorf("unable to delete private key from database: %s", err.Error())
+		return fmt.Errorf("unable to delete private key %s from database: %s", keyID, err.Error())
 	}
 
 	return nil

signer/keydbstore/rethink_realkeydbstore_test.go

@@ -38,7 +38,10 @@ func rethinkDBSetup(t *testing.T, dbName string) (*RethinkDBKeyStore, func()) {
 	err := rethinkdb.SetupDB(session, dbName, []rethinkdb.Table{PrivateKeysRethinkTable})
 	require.NoError(t, err)
 
-	return NewRethinkDBKeyStore(dbName, "", "", multiAliasRetriever, validAliases[0], session), cleanup
+	dbStore := NewRethinkDBKeyStore(dbName, "", "", multiAliasRetriever, validAliases[0], session)
+	require.Equal(t, "RethinkDB", dbStore.Name())
+
+	return dbStore, cleanup
 }
 
 func TestRethinkBootstrapSetsUsernamePassword(t *testing.T) {
@@ -90,8 +93,8 @@ func getRethinkDBRows(t *testing.T, dbStore *RethinkDBKeyStore) []RDBPrivateKey
 }
 
 func TestRethinkKeyCanOnlyBeAddedOnce(t *testing.T) {
-	dbStore, _ := rethinkDBSetup(t, "signerAddTests")
-	// defer cleanup()
+	dbStore, cleanup := rethinkDBSetup(t, "signerAddTests")
+	defer cleanup()
 	expectedKeys := testKeyCanOnlyBeAddedOnce(t, dbStore)
 
 	rows := getRethinkDBRows(t, dbStore)

signer/keydbstore/sqlite_test.go

@@ -23,6 +23,8 @@ func sqlite3Setup(t *testing.T) (*SQLKeyDBStore, func()) {
 		dbStore.db.Close()
 		os.RemoveAll(tempBaseDir)
 	}
+
+	require.Equal(t, "sqlite3", dbStore.Name())
 	return dbStore, cleanup
 }