removing the ability to configure role names. It adds a lot of complexity without adding much value. If somebody wants custom role names they can implement it at the display level

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

client/client_test.go

@@ -393,7 +393,7 @@ func requireRepoHasExpectedMetadata(t *testing.T, repo *NotaryRepository,
 		require.Len(t, decodedRoot.Roles, len(data.ValidRoles),
 			"wrong number of roles in root.json")
 
-		for role := range data.ValidRoles {
+		for _, role := range data.ValidRoles {
 			_, ok := decodedRoot.Roles[role]
 			require.True(t, ok, "Missing role %s in root.json", role)
 		}
@@ -1172,7 +1172,7 @@ func testPublishNoData(t *testing.T, rootType string, serverManagesSnapshot bool
 	require.NoError(t, err)
 	require.Empty(t, targets)
 
-	for role := range data.ValidRoles {
+	for _, role := range data.ValidRoles {
 		// we don't cache timstamp metadata
 		if role != data.CanonicalTimestampRole {
 			requireRepoHasExpectedMetadata(t, repo2, role, true)
@@ -2049,7 +2049,7 @@ func TestRotateKeyInvalidRole(t *testing.T) {
 
 	// the equivalent of: (root, true), (root, false), (timestamp, true),
 	// (timestamp, false), (targets, true)
-	for role := range data.ValidRoles {
+	for _, role := range data.ValidRoles {
 		if role == data.CanonicalSnapshotRole {
 			continue
 		}

server/handlers/default.go

@@ -119,7 +119,7 @@ func getHandler(ctx context.Context, w http.ResponseWriter, r *http.Request, var
 
 	logger := ctxu.GetLoggerWithFields(ctx, map[string]interface{}{"gun": gun, "tufRole": tufRole})
 
-	switch data.CanonicalRole(tufRole) {
+	switch tufRole {
 	case data.CanonicalTimestampRole:
 		return getTimestamp(ctx, w, logger, store, gun)
 	case data.CanonicalSnapshotRole:

server/handlers/validation.go

@@ -30,8 +30,8 @@ import (
 func validateUpdate(cs signed.CryptoService, gun string, updates []storage.MetaUpdate, store storage.MetaStore) ([]storage.MetaUpdate, error) {
 	kdb := keys.NewDB()
 	repo := tuf.NewRepo(kdb, cs)
-	rootRole := data.RoleName(data.CanonicalRootRole)
-	snapshotRole := data.RoleName(data.CanonicalSnapshotRole)
+	rootRole := data.CanonicalRootRole
+	snapshotRole := data.CanonicalSnapshotRole
 
 	// some delegated targets role may be invalid based on other updates
 	// that have been made by other clients. We'll rebuild the slice of
@@ -133,7 +133,7 @@ func validateUpdate(cs signed.CryptoService, gun string, updates []storage.MetaU
 func loadAndValidateTargets(gun string, repo *tuf.Repo, roles map[string]storage.MetaUpdate, kdb *keys.KeyDB, store storage.MetaStore) ([]storage.MetaUpdate, error) {
 	targetsRoles := make(utils.RoleList, 0)
 	for role := range roles {
-		if role == data.RoleName(data.CanonicalTargetsRole) || data.IsDelegation(role) {
+		if role == data.CanonicalTargetsRole || data.IsDelegation(role) {
 			targetsRoles = append(targetsRoles, role)
 		}
 	}
@@ -151,7 +151,7 @@ func loadAndValidateTargets(gun string, repo *tuf.Repo, roles map[string]storage
 		// don't load parent if current role is "targets" or if the parent has
 		// already been loaded
 		_, ok := repo.Targets[parentRole]
-		if role != data.RoleName(data.CanonicalTargetsRole) && !ok {
+		if role != data.CanonicalTargetsRole && !ok {
 			err := loadTargetsFromStore(gun, parentRole, repo, store)
 			if err != nil {
 				return nil, err
@@ -195,7 +195,7 @@ func loadTargetsFromStore(gun, role string, repo *tuf.Repo, store storage.MetaSt
 }
 
 func generateSnapshot(gun string, kdb *keys.KeyDB, repo *tuf.Repo, store storage.MetaStore) (*storage.MetaUpdate, error) {
-	role := kdb.GetRole(data.RoleName(data.CanonicalSnapshotRole))
+	role := kdb.GetRole(data.CanonicalSnapshotRole)
 	if role == nil {
 		return nil, validation.ErrBadRoot{Msg: "root did not include snapshot role"}
 	}
@@ -285,8 +285,8 @@ func validateSnapshot(role string, oldSnap *data.SignedSnapshot, snapUpdate stor
 }
 
 func checkSnapshotEntries(role string, oldSnap, snap *data.SignedSnapshot, roles map[string]storage.MetaUpdate) error {
-	snapshotRole := data.RoleName(data.CanonicalSnapshotRole)
-	timestampRole := data.RoleName(data.CanonicalTimestampRole) // just in case
+	snapshotRole := data.CanonicalSnapshotRole
+	timestampRole := data.CanonicalTimestampRole
 	for r, update := range roles {
 		if r == snapshotRole || r == timestampRole {
 			continue
@@ -386,10 +386,10 @@ func validateRoot(gun string, oldRoot, newRoot []byte, store storage.MetaStore)
 // threshold number of signatures is invalid, if there are an invalid
 // number of roles and keys, or if the timestamp keys are invalid
 func checkRoot(oldRoot, newRoot *data.SignedRoot, timestampKey data.PublicKey) error {
-	rootRole := data.RoleName(data.CanonicalRootRole)
-	targetsRole := data.RoleName(data.CanonicalTargetsRole)
-	snapshotRole := data.RoleName(data.CanonicalSnapshotRole)
-	timestampRole := data.RoleName(data.CanonicalTimestampRole)
+	rootRole := data.CanonicalRootRole
+	targetsRole := data.CanonicalTargetsRole
+	snapshotRole := data.CanonicalSnapshotRole
+	timestampRole := data.CanonicalTimestampRole
 
 	var oldRootRole *data.RootRole
 	newRootRole, ok := newRoot.Signed.Roles[rootRole]

server/handlers/validation_test.go

@@ -455,7 +455,7 @@ func TestValidateRootInvalidTimestampThreshold(t *testing.T) {
 
 // If any role has a threshold < 1, validation fails
 func TestValidateRootInvalidZeroThreshold(t *testing.T) {
-	for role := range data.ValidRoles {
+	for _, role := range data.ValidRoles {
 		kdb, oldRepo, cs := testutils.EmptyRepo()
 		tsRole, ok := oldRepo.Root.Signed.Roles[role]
 		assert.True(t, ok)

tuf/client/client.go

@@ -97,7 +97,7 @@ func (c *Client) update() error {
 // hash and size in snapshot are unchanged but the root file has expired,
 // there is little expectation that the situation can be remedied.
 func (c Client) checkRoot() error {
-	role := data.RoleName("root")
+	role := data.CanonicalRootRole
 	size := c.local.Snapshot.Signed.Meta[role].Length
 	hashSha256 := c.local.Snapshot.Signed.Meta[role].Hashes["sha256"]
 
@@ -129,7 +129,7 @@ func (c Client) checkRoot() error {
 
 // downloadRoot is responsible for downloading the root.json
 func (c *Client) downloadRoot() error {
-	role := data.RoleName("root")
+	role := data.CanonicalRootRole
 	size := maxSize
 	var expectedSha256 []byte
 	if c.local.Snapshot != nil {
@@ -241,7 +241,7 @@ func (c Client) verifyRoot(role string, s *data.Signed, minVersion int) error {
 // use cache if the download fails (and the cache is still valid).
 func (c *Client) downloadTimestamp() error {
 	logrus.Debug("downloadTimestamp")
-	role := data.RoleName("timestamp")
+	role := data.CanonicalTimestampRole
 
 	// We may not have a cached timestamp if this is the first time
 	// we're interacting with the repo. This will result in the
@@ -300,7 +300,7 @@ func (c *Client) downloadTimestamp() error {
 // downloadSnapshot is responsible for downloading the snapshot.json
 func (c *Client) downloadSnapshot() error {
 	logrus.Debug("downloadSnapshot")
-	role := data.RoleName("snapshot")
+	role := data.CanonicalSnapshotRole
 	if c.local.Timestamp == nil {
 		return ErrMissingMeta{role: "snapshot"}
 	}
@@ -379,7 +379,6 @@ func (c *Client) downloadTargets(role string) error {
 		if err != nil {
 			return err
 		}
-		role = data.RoleName(role) // this will really only do something for base targets role
 		if c.local.Snapshot == nil {
 			return ErrMissingMeta{role: role}
 		}

tuf/data/roles.go

@@ -15,15 +15,13 @@ const (
 	CanonicalTimestampRole = "timestamp"
 )
 
-// ValidRoles holds an overrideable mapping of canonical role names
-// to any custom roles names a user wants to make use of. This allows
-// us to be internally consistent while using different roles in the
-// public TUF files.
-var ValidRoles = map[string]string{
-	CanonicalRootRole:      CanonicalRootRole,
-	CanonicalTargetsRole:   CanonicalTargetsRole,
-	CanonicalSnapshotRole:  CanonicalSnapshotRole,
-	CanonicalTimestampRole: CanonicalTimestampRole,
+// ValidRoles is an easy to iterate list of the top level
+// roles.
+var ValidRoles = []string{
+	CanonicalRootRole,
+	CanonicalTargetsRole,
+	CanonicalSnapshotRole,
+	CanonicalTimestampRole,
 }
 
 // Regex for validating delegation names
@@ -53,57 +51,10 @@ func (e ErrInvalidRole) Error() string {
 	return fmt.Sprintf("tuf: invalid role %s.", e.Role)
 }
 
-// SetValidRoles is a utility function to override some or all of the roles
-func SetValidRoles(rs map[string]string) {
-	// iterate ValidRoles
-	for k := range ValidRoles {
-		if v, ok := rs[k]; ok {
-			ValidRoles[k] = v
-		}
-	}
-}
-
-// RoleName returns the (possibly overridden) role name for the provided
-// canonical role name
-func RoleName(canonicalRole string) string {
-	if r, ok := ValidRoles[canonicalRole]; ok {
-		return r
-	}
-	return canonicalRole
-}
-
-// CanonicalRole does a reverse lookup to get the canonical role name
-// from the (possibly overridden) role name
-func CanonicalRole(role string) string {
-	name := strings.ToLower(role)
-	if _, ok := ValidRoles[name]; ok {
-		// The canonical version is always lower case
-		// se ensure we return name, not role
-		return name
-	}
-	targetsBase := fmt.Sprintf("%s/", ValidRoles[CanonicalTargetsRole])
-	if strings.HasPrefix(name, targetsBase) {
-		role = strings.TrimPrefix(role, targetsBase)
-		role = fmt.Sprintf("%s/%s", CanonicalTargetsRole, role)
-		return role
-	}
-	for r, v := range ValidRoles {
-		if role == v {
-			return r
-		}
-	}
-	return ""
-}
-
 // ValidRole only determines the name is semantically
 // correct. For target delegated roles, it does NOT check
 // the the appropriate parent roles exist.
 func ValidRole(name string) bool {
-	name = strings.ToLower(name)
-	if v, ok := ValidRoles[name]; ok {
-		return name == v
-	}
-
 	if IsDelegation(name) {
 		return true
 	}
@@ -118,7 +69,7 @@ func ValidRole(name string) bool {
 
 // IsDelegation checks if the role is a delegation or a root role
 func IsDelegation(role string) bool {
-	targetsBase := ValidRoles[CanonicalTargetsRole] + "/"
+	targetsBase := CanonicalTargetsRole + "/"
 
 	whitelistedChars := delegationRegexp.MatchString(role)
 

tuf/data/roles_test.go

@@ -8,51 +8,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestCanonicalRole(t *testing.T) {
-
-	testRoles := map[string]string{
-		CanonicalRootRole:      "testRoot",
-		CanonicalTargetsRole:   "testTargets",
-		CanonicalSnapshotRole:  "testSnapshot",
-		CanonicalTimestampRole: "testTimestamp",
-		"garbageRole":          "testGarbageRole",
-	}
-
-	SetValidRoles(testRoles)
-
-	// make sure roles were set correctly
-	assert.Equal(t, "testRoot", ValidRoles[CanonicalRootRole])
-	assert.Equal(t, "testTargets", ValidRoles[CanonicalTargetsRole])
-	assert.Equal(t, "testSnapshot", ValidRoles[CanonicalSnapshotRole])
-	assert.Equal(t, "testTimestamp", ValidRoles[CanonicalTimestampRole])
-	// check SetValidRoles doesn't allow non-valid roles in
-	assert.Equal(t, "", ValidRoles["garbageRole"])
-
-	// check when looking up CanonicalRole from configured role
-	assert.Equal(t, CanonicalRootRole, CanonicalRole("testRoot"))
-	assert.Equal(t, CanonicalTargetsRole, CanonicalRole("testTargets"))
-	assert.Equal(t, CanonicalSnapshotRole, CanonicalRole("testSnapshot"))
-	assert.Equal(t, CanonicalTimestampRole, CanonicalRole("testTimestamp"))
-	assert.Equal(t, "", CanonicalRole("testGarbageRole"))
-
-	// check when looking up CanonicalRole with canonical role
-	assert.Equal(t, CanonicalRootRole, CanonicalRole(CanonicalRootRole))
-	assert.Equal(t, CanonicalTargetsRole, CanonicalRole(CanonicalTargetsRole))
-	assert.Equal(t, CanonicalSnapshotRole, CanonicalRole(CanonicalSnapshotRole))
-	assert.Equal(t, CanonicalTimestampRole, CanonicalRole(CanonicalTimestampRole))
-	assert.Equal(t, "", CanonicalRole("garbageRole"))
-
-	assert.Equal(t, "", CanonicalRole("not found"))
-
-	// reset ValidRoles so other tests aren't messed up
-	ValidRoles = map[string]string{
-		CanonicalRootRole:      CanonicalRootRole,
-		CanonicalTargetsRole:   CanonicalTargetsRole,
-		CanonicalSnapshotRole:  CanonicalSnapshotRole,
-		CanonicalTimestampRole: CanonicalTimestampRole,
-	}
-}
-
 func TestMergeStrSlicesExclusive(t *testing.T) {
 	orig := []string{"a"}
 	new := []string{"b"}

tuf/data/snapshot.go

@@ -52,8 +52,8 @@ func NewSnapshot(root *Signed, targets *Signed) (*SignedSnapshot, error) {
 			Version: 0,
 			Expires: DefaultExpires("snapshot"),
 			Meta: Files{
-				ValidRoles["root"]:    rootMeta,
-				ValidRoles["targets"]: targetsMeta,
+				CanonicalRootRole:    rootMeta,
+				CanonicalTargetsRole: targetsMeta,
 			},
 		},
 	}, nil

tuf/data/snapshot_test.go

@@ -14,8 +14,8 @@ func TestDeleteMeta(t *testing.T) {
 			Version: 0,
 			Expires: DefaultExpires("snapshot"),
 			Meta: Files{
-				ValidRoles["root"]:    FileMeta{},
-				ValidRoles["targets"]: FileMeta{},
+				CanonicalRootRole:    FileMeta{},
+				CanonicalTargetsRole: FileMeta{},
 			},
 		},
 	}

tuf/data/timestamp.go

@@ -39,7 +39,7 @@ func NewTimestamp(snapshot *Signed) (*SignedTimestamp, error) {
 			Version: 0,
 			Expires: DefaultExpires("timestamp"),
 			Meta: Files{
-				ValidRoles["snapshot"]: snapshotMeta,
+				CanonicalSnapshotRole: snapshotMeta,
 			},
 		},
 	}, nil

tuf/data/types.go

@@ -62,7 +62,6 @@ func ValidTUFType(typ, role string) bool {
 	if ValidRole(role) {
 		// All targets delegation roles must have
 		// the valid type is for targets.
-		role = CanonicalRole(role)
 		if role == "" {
 			// role is unknown and does not map to
 			// a type

tuf/tuf.go

@@ -353,14 +353,14 @@ func (tr *Repo) InitRoot(consistent bool) error {
 // InitTargets initializes an empty targets, and returns the new empty target
 func (tr *Repo) InitTargets(role string) (*data.SignedTargets, error) {
 	r := data.Role{Name: role}
-	if !r.IsDelegation() && data.CanonicalRole(role) != data.CanonicalTargetsRole {
+	if !r.IsDelegation() && role != data.CanonicalTargetsRole {
 		return nil, data.ErrInvalidRole{
 			Role:   role,
 			Reason: fmt.Sprintf("role is not a valid targets role name: %s", role),
 		}
 	}
 	targets := data.NewTargets()
-	tr.Targets[data.RoleName(role)] = targets
+	tr.Targets[role] = targets
 	return targets, nil
 }
 
@@ -374,10 +374,10 @@ func (tr *Repo) InitSnapshot() error {
 		return err
 	}
 
-	if _, ok := tr.Targets[data.RoleName(data.CanonicalTargetsRole)]; !ok {
+	if _, ok := tr.Targets[data.CanonicalTargetsRole]; !ok {
 		return ErrNotLoaded{role: "targets"}
 	}
-	targets, err := tr.Targets[data.RoleName(data.CanonicalTargetsRole)].ToSigned()
+	targets, err := tr.Targets[data.CanonicalTargetsRole].ToSigned()
 	if err != nil {
 		return err
 	}
@@ -574,7 +574,7 @@ func (tr *Repo) AddTargets(role string, targets data.Files) (data.Files, error)
 	for path, target := range targets {
 		pathDigest := sha256.Sum256([]byte(path))
 		pathHex := hex.EncodeToString(pathDigest[:])
-		if role == data.ValidRoles["targets"] || (r.CheckPaths(path) || r.CheckPrefixes(pathHex)) {
+		if role == data.CanonicalTargetsRole || (r.CheckPaths(path) || r.CheckPrefixes(pathHex)) {
 			t.Signed.Targets[path] = target
 		} else {
 			invalid[path] = target
@@ -640,7 +640,7 @@ func (tr *Repo) SignRoot(expires time.Time) (*data.Signed, error) {
 	logrus.Debug("signing root...")
 	tr.Root.Signed.Expires = expires
 	tr.Root.Signed.Version++
-	root := tr.keysDB.GetRole(data.ValidRoles["root"])
+	root := tr.keysDB.GetRole(data.CanonicalRootRole)
 	signed, err := tr.Root.ToSigned()
 	if err != nil {
 		return nil, err
@@ -708,7 +708,7 @@ func (tr *Repo) SignSnapshot(expires time.Time) (*data.Signed, error) {
 	if err != nil {
 		return nil, err
 	}
-	snapshot := tr.keysDB.GetRole(data.ValidRoles["snapshot"])
+	snapshot := tr.keysDB.GetRole(data.CanonicalSnapshotRole)
 	signed, err = tr.sign(signed, *snapshot)
 	if err != nil {
 		return nil, err
@@ -734,7 +734,7 @@ func (tr *Repo) SignTimestamp(expires time.Time) (*data.Signed, error) {
 	if err != nil {
 		return nil, err
 	}
-	timestamp := tr.keysDB.GetRole(data.ValidRoles["timestamp"])
+	timestamp := tr.keysDB.GetRole(data.CanonicalTimestampRole)
 	signed, err = tr.sign(signed, *timestamp)
 	if err != nil {
 		return nil, err

tuf/tuf_test.go

@@ -910,7 +910,7 @@ func TestRemoveTargetsNoSigningKeys(t *testing.T) {
 
 // adding a key to a role marks root as dirty as well as the role
 func TestAddBaseKeysToRoot(t *testing.T) {
-	for role := range data.ValidRoles {
+	for _, role := range data.ValidRoles {
 		ed25519 := signed.NewEd25519()
 		keyDB := keys.NewDB()
 		repo := initRepo(t, ed25519, keyDB)