change to ListRoles, and GetAllLoadedRoles

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

client/client.go

@@ -577,20 +577,17 @@ type RoleWithSignatures struct {
 	data.Role
 }
 
-// GetRepoRoleMetaInfo returns a list of RoleWithSignatures objects for this repo
+// ListRoles returns a list of RoleWithSignatures objects for this repo
 // This represents the latest metadata for each role in this repo
-func (r *NotaryRepository) GetRepoRoleMetaInfo() ([]RoleWithSignatures, error) {
+func (r *NotaryRepository) ListRoles() ([]RoleWithSignatures, error) {
 	// Update to latest repo state
 	_, err := r.Update(false)
 	if err != nil {
 		return nil, err
 	}
 
-	// Get all role info from our updated keysDB
+	// Get all role info from our updated keysDB, can be empty
-	roles, err := r.tufRepo.GetAllRoles()
+	roles := r.tufRepo.GetAllLoadedRoles()
-	if err != nil {
-		return nil, err
-	}
 
 	var roleWithSigs []RoleWithSignatures
 

client/client_test.go

@@ -2950,8 +2950,8 @@ func TestDeleteRepoNoCerts(t *testing.T) {
 	assertRepoHasExpectedKeys(t, repo, rootKeyID, true)
 }
 
-// Test that we get a correct map of key IDs
+// Test that we get a correct list of roles with keys and signatures
-func TestGetRepoRoleMetaInfo(t *testing.T) {
+func TestListRoles(t *testing.T) {
 	ts := fullTestServer(t)
 	defer ts.Close()
 
@@ -2960,7 +2960,7 @@ func TestGetRepoRoleMetaInfo(t *testing.T) {
 
 	assert.NoError(t, repo.Publish())
 
-	rolesWithSigs, err := repo.GetRepoRoleMetaInfo()
+	rolesWithSigs, err := repo.ListRoles()
 	assert.NoError(t, err)
 
 	// Should only have base roles at this point
@@ -2980,7 +2980,7 @@ func TestGetRepoRoleMetaInfo(t *testing.T) {
 
 	assert.NoError(t, repo.Publish())
 
-	rolesWithSigs, err = repo.GetRepoRoleMetaInfo()
+	rolesWithSigs, err = repo.ListRoles()
 	assert.NoError(t, err)
 
 	assert.Len(t, rolesWithSigs, len(data.BaseRoles)+1)
@@ -2998,7 +2998,7 @@ func TestGetRepoRoleMetaInfo(t *testing.T) {
 	addTarget(t, repo, "current", "../fixtures/intermediate-ca.crt", "targets/a")
 	assert.NoError(t, repo.Publish())
 
-	rolesWithSigs, err = repo.GetRepoRoleMetaInfo()
+	rolesWithSigs, err = repo.ListRoles()
 	assert.NoError(t, err)
 
 	assert.Len(t, rolesWithSigs, len(data.BaseRoles)+1)
@@ -3017,7 +3017,7 @@ func TestGetRepoRoleMetaInfo(t *testing.T) {
 
 	assert.NoError(t, repo.Publish())
 
-	rolesWithSigs, err = repo.GetRepoRoleMetaInfo()
+	rolesWithSigs, err = repo.ListRoles()
 	assert.NoError(t, err)
 
 	assert.Len(t, rolesWithSigs, len(data.BaseRoles)+2)
@@ -3039,12 +3039,12 @@ func TestGetRepoRoleMetaInfo(t *testing.T) {
 	assert.NoError(t, repo2.Publish())
 
 	// repo2 only has the base roles
-	rolesWithSigs2, err := repo2.GetRepoRoleMetaInfo()
+	rolesWithSigs2, err := repo2.ListRoles()
 	assert.NoError(t, err)
 	assert.Len(t, rolesWithSigs2, len(data.BaseRoles))
 
 	// original repo stays in same state (base roles + 2 delegations)
-	rolesWithSigs, err = repo.GetRepoRoleMetaInfo()
+	rolesWithSigs, err = repo.ListRoles()
 	assert.NoError(t, err)
 	assert.Len(t, rolesWithSigs, len(data.BaseRoles)+2)
 }

tuf/data/roles.go

@@ -37,13 +37,6 @@ func (e ErrNoSuchRole) Error() string {
 	return fmt.Sprintf("role does not exist: %s", e.Role)
 }
 
-// ErrNoRoles indicates no roles exist for this repo
-type ErrNoRoles struct{}
-
-func (e ErrNoRoles) Error() string {
-	return fmt.Sprintf("no roles exist")
-}
-
 // ErrInvalidRole represents an error regarding a role. Typically
 // something like a role for which sone of the public keys were
 // not found in the TUF repo.

tuf/tuf.go

@@ -173,13 +173,9 @@ func (tr *Repo) RemoveBaseKeys(role string, keyIDs ...string) error {
 	return nil
 }
 
-// GetAllRoles returns a list of all role entries for this TUF repo
+// GetAllLoadedRoles returns a list of all role entries loaded in this TUF repo, could be empty
-func (tr *Repo) GetAllRoles() ([]*data.Role, error) {
+func (tr *Repo) GetAllLoadedRoles() []*data.Role {
-	roles := tr.keysDB.GetAllRoles()
+	return tr.keysDB.GetAllRoles()
-	if len(roles) == 0 {
-		return nil, data.ErrNoRoles{}
-	}
-	return roles, nil
 }
 
 // GetDelegation finds the role entry representing the provided

tuf/tuf_test.go

@@ -943,12 +943,11 @@ func TestGetAllRoles(t *testing.T) {
 	repo := initRepo(t, ed25519, keyDB)
 
 	// After we init, we get the base roles
-	roles, err := repo.GetAllRoles()
+	roles := repo.GetAllLoadedRoles()
 	assert.Len(t, roles, len(data.BaseRoles))
 
-	// Clear the keysDB, check that we error
+	// Clear the keysDB, check that we get an empty list
 	repo.keysDB = keys.NewDB()
-	roles, err = repo.GetAllRoles()
+	roles = repo.GetAllLoadedRoles()
-	assert.Error(t, err)
+	assert.Len(t, roles, 0)
-	assert.IsType(t, data.ErrNoRoles{}, err)
 }