More testing, cleanup

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

client/helpers_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"crypto/sha256"
 	"encoding/json"
+	"net/http"
 	"testing"
 	"time"
 
@@ -1019,3 +1020,15 @@ func TestAllNotNearExpiry(t *testing.T) {
 	require.NotContains(t, a.String(), "snapshot is nearing expiry, you should re-sign the role metadata", "Snapshot should not show near expiry")
 	require.NotContains(t, a.String(), "timestamp", "there should be no logrus warnings pertaining to timestamp")
 }
+
+func TestRotateRemoteKeyOffline(t *testing.T) {
+	// without a valid roundtripper, rotation should fail since we cannot initialize a HTTPStore
+	key, err := rotateRemoteKey("invalidURL", "gun", data.CanonicalSnapshotRole, nil)
+	require.Error(t, err)
+	require.Nil(t, key)
+
+	// if the underlying remote store is faulty and cannot rotate keys, we should get back the error
+	key, err = rotateRemoteKey("https://notary-server", "gun", data.CanonicalSnapshotRole, http.DefaultTransport)
+	require.Error(t, err)
+	require.Nil(t, key)
+}

server/handlers/default_test.go

@@ -193,8 +193,8 @@ func TestRotateKeyHandlerInvalidRole(t *testing.T) {
 	}
 }
 
-// Rotating the key fails if we don't pass a valid key algorithm, or if it isn't Ed25519
-func TestRotateKeyHandlerNonED25519(t *testing.T) {
+// Rotating the key fails if we don't pass a valid key algorithm
+func TestRotateKeyHandlerInvalidKeyAlgo(t *testing.T) {
 	roles := []string{data.CanonicalTimestampRole, data.CanonicalSnapshotRole}
 	req := &http.Request{Body: ioutil.NopCloser(bytes.NewBuffer(nil))}
 
@@ -205,9 +205,6 @@ func TestRotateKeyHandlerNonED25519(t *testing.T) {
 		invalidKeyAlgoState.keyAlgo = "notactuallyakeyalgorithm"
 		err := rotateKeyHandler(getContext(invalidKeyAlgoState), recorder, req, vars)
 		require.Error(t, err)
-		invalidKeyAlgoState.keyAlgo = data.ECDSAKey
-		err = rotateKeyHandler(getContext(invalidKeyAlgoState), recorder, req, vars)
-		require.Error(t, err)
 	}
 }
 

server/snapshot/snapshot_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"crypto/sha256"
 	"encoding/hex"
+	"fmt"
 	"testing"
 	"time"
 
@@ -51,6 +52,30 @@ func TestGetSnapshotKeyCreate(t *testing.T) {
 	require.NotNil(t, k2, "Key should not be nil")
 }
 
+type FailingStore struct {
+	*storage.MemStorage
+}
+
+func (f FailingStore) GetCurrent(role, gun string) (*time.Time, []byte, error) {
+	return nil, nil, fmt.Errorf("failing store failed")
+}
+
+func TestGetSnapshotKeyCreateWithFailingStore(t *testing.T) {
+	store := FailingStore{storage.NewMemStorage()}
+	crypto := signed.NewEd25519()
+	k, err := GetOrCreateSnapshotKey("gun", store, crypto, data.ED25519Key)
+	require.Error(t, err, "Expected error")
+	require.Nil(t, k, "Key should be nil")
+}
+
+func TestGetSnapshotKeyCreateWithInvalidAlgo(t *testing.T) {
+	store := storage.NewMemStorage()
+	crypto := signed.NewEd25519()
+	k, err := GetOrCreateSnapshotKey("gun", store, crypto, "notactuallyanalgorithm")
+	require.Error(t, err, "Expected error")
+	require.Nil(t, k, "Key should be nil")
+}
+
 func TestGetSnapshotKeyExisting(t *testing.T) {
 
 	repo, crypto, err := testutils.EmptyRepo("gun")

server/storage/rethinkdb.go

@@ -267,12 +267,10 @@ func (rdb RethinkDB) Bootstrap() error {
 
 // CheckHealth checks that all tables and databases exist and are query-able
 func (rdb RethinkDB) CheckHealth() error {
-	for _, table := range []string{TUFFilesRethinkTable.Name} {
-		res, err := gorethink.DB(rdb.dbName).Table(table).Info().Run(rdb.sess)
-		if err != nil {
-			return fmt.Errorf("%s is unavailable, or missing one or more tables, or permissions are incorrectly set", rdb.dbName)
-		}
-		defer res.Close()
+	res, err := gorethink.DB(rdb.dbName).Table(TUFFilesRethinkTable.Name).Info().Run(rdb.sess)
+	if err != nil {
+		return fmt.Errorf("%s is unavailable, or missing one or more tables, or permissions are incorrectly set", rdb.dbName)
 	}
+	defer res.Close()
 	return nil
 }

server/storage/sqldb.go

@@ -159,21 +159,15 @@ func (db *SQLStorage) Delete(gun string) error {
 	return db.Unscoped().Where(&TUFFile{Gun: gun}).Delete(TUFFile{}).Error
 }
 
-// CheckHealth asserts that both required tables are present
+// CheckHealth asserts that the tuf_files table is present
 func (db *SQLStorage) CheckHealth() error {
-	interfaces := []interface {
-		TableName() string
-	}{&TUFFile{}}
-
-	for _, model := range interfaces {
-		tableOk := db.HasTable(model)
-		if db.Error != nil {
-			return db.Error
-		}
-		if !tableOk {
-			return fmt.Errorf(
-				"Cannot access table: %s", model.TableName())
-		}
+	tableOk := db.HasTable(&TUFFile{})
+	if db.Error != nil {
+		return db.Error
+	}
+	if !tableOk {
+		return fmt.Errorf(
+			"Cannot access table: %s", TUFFile{}.TableName())
 	}
 	return nil
 }

server/storage/sqldb_test.go

@@ -24,11 +24,9 @@ func SetupSQLDB(t *testing.T, dbtype, dburl string) *SQLStorage {
 
 	// verify that the tables are empty
 	var count int
-	for _, model := range [1]interface{}{&TUFFile{}} {
-		query := dbStore.DB.Model(model).Count(&count)
-		require.NoError(t, query.Error)
-		require.Equal(t, 0, count)
-	}
+	query := dbStore.DB.Model(&TUFFile{}).Count(&count)
+	require.NoError(t, query.Error)
+	require.Equal(t, 0, count)
 	return dbStore
 }
 

server/timestamp/timestamp_test.go

@@ -2,16 +2,16 @@ package timestamp
 
 import (
 	"bytes"
+	"fmt"
 	"testing"
 	"time"
 
 	"github.com/docker/go/canonical/json"
+	"github.com/docker/notary/server/storage"
 	"github.com/docker/notary/tuf/data"
 	"github.com/docker/notary/tuf/signed"
 	"github.com/docker/notary/tuf/testutils"
 	"github.com/stretchr/testify/require"
-
-	"github.com/docker/notary/server/storage"
 )
 
 func TestTimestampExpired(t *testing.T) {
@@ -226,3 +226,27 @@ func TestCreateTimestampNoKeyInCrypto(t *testing.T) {
 	require.Error(t, err)
 	require.IsType(t, signed.ErrInsufficientSignatures{}, err)
 }
+
+type FailingStore struct {
+	*storage.MemStorage
+}
+
+func (f FailingStore) GetCurrent(role, gun string) (*time.Time, []byte, error) {
+	return nil, nil, fmt.Errorf("failing store failed")
+}
+
+func TestGetTimestampKeyCreateWithFailingStore(t *testing.T) {
+	store := FailingStore{storage.NewMemStorage()}
+	crypto := signed.NewEd25519()
+	k, err := GetOrCreateTimestampKey("gun", store, crypto, data.ED25519Key)
+	require.Error(t, err, "Expected error")
+	require.Nil(t, k, "Key should be nil")
+}
+
+func TestGetSnapshotKeyCreateWithInvalidAlgo(t *testing.T) {
+	store := storage.NewMemStorage()
+	crypto := signed.NewEd25519()
+	k, err := GetOrCreateTimestampKey("gun", store, crypto, "notactuallyanalgorithm")
+	require.Error(t, err, "Expected error")
+	require.Nil(t, k, "Key should be nil")
+}

signer/keydbstore/keydbstore_test.go

@@ -144,14 +144,20 @@ func testMarkKeyActive(t *testing.T, dbStore keyActivator) (data.PrivateKey, dat
 func testGetPendingKey(t *testing.T, dbStore keyActivator) (data.PrivateKey, data.PrivateKey) {
 	// Create a test key and add it to the db such that it will be pending (never marked active)
 	keyInfo := trustmanager.KeyInfo{Role: data.CanonicalSnapshotRole, Gun: "gun"}
+
+	// There should be no keys to start
+	retrievedKey, err := dbStore.GetPendingKey(keyInfo)
+	require.Error(t, err)
+	require.Nil(t, retrievedKey)
+
 	pendingTestKey, err := utils.GenerateECDSAKey(rand.Reader)
 	require.NoError(t, err)
 	requireGetKeyFailure(t, dbStore, pendingTestKey.ID())
 	err = dbStore.AddKey(keyInfo, pendingTestKey)
 	require.NoError(t, err)
 	requireGetKeySuccess(t, dbStore, data.CanonicalSnapshotRole, pendingTestKey)
 
-	retrievedKey, err := dbStore.GetPendingKey(keyInfo)
+	retrievedKey, err = dbStore.GetPendingKey(keyInfo)
 	require.NoError(t, err)
 	require.Equal(t, pendingTestKey.Public(), retrievedKey.Public())