Match spec for delegation traversal

client/delegations.go

@@ -149,22 +149,17 @@ type delegation struct {
 	child  data.DelegatedRole
 }
 
-type delegationID struct {
-	parent string
-	child  string
-}
-
 type delegationsIterator struct {
-	stack   []delegation
-	file    string
-	visited map[delegationID]struct{}
+	stack        []delegation
+	file         string
+	visitedRoles map[string]struct{}
 }
 
 func newDelegationsIterator(role data.DelegatedRole, parent string, file string) *delegationsIterator {
 	i := &delegationsIterator{
-		file:    file,
-		stack:   make([]delegation, 0, 1),
-		visited: make(map[delegationID]struct{}),
+		file:         file,
+		stack:        make([]delegation, 0, 1),
+		visitedRoles: make(map[string]struct{}),
 	}
 
 	i.add([]data.DelegatedRole{role}, parent)
@@ -179,12 +174,13 @@ func (d *delegationsIterator) next() (delegation, bool) {
 	delegation := d.stack[len(d.stack)-1]
 	d.stack = d.stack[:len(d.stack)-1]
 
-	// 5.6.7.1 cycles protection
-	id := delegationID{delegation.parent, delegation.child.Name}
-	if _, ok := d.visited[id]; ok {
+	// 5.6.7.1: If this role has been visited before, then skip this role (so
+	// that cycles in the delegation graph are avoided).
+	roleName := delegation.child.Name
+	if _, ok := d.visitedRoles[roleName]; ok {
 		return d.next()
 	}
-	d.visited[id] = struct{}{}
+	d.visitedRoles[roleName] = struct{}{}
 
 	// 5.6.7.2 trim delegations to visit, only the current role and its delegations
 	// will be considered

client/delegations_test.go

@@ -38,12 +38,23 @@ func TestDelegationsIterator(t *testing.T) {
 				},
 				"b": {
 					{Name: "c", Paths: defaultPathPatterns},
+				},
+				"c": {
 					{Name: "d", Paths: defaultPathPatterns},
 				},
+				"e": {
+					{Name: "f", Paths: defaultPathPatterns},
+					{Name: "g", Paths: defaultPathPatterns},
+				},
+				"g": {
+					{Name: "h", Paths: defaultPathPatterns},
+					{Name: "i", Paths: defaultPathPatterns},
+					{Name: "j", Paths: defaultPathPatterns},
+				},
 			},
 			rootDelegation: data.DelegatedRole{Name: "a", Paths: defaultPathPatterns},
 			file:           "",
-			resultOrder:    []string{"a", "b", "c", "d", "e"},
+			resultOrder:    []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"},
 		},
 		{
 			testName: "terminated in b",
@@ -78,7 +89,7 @@ func TestDelegationsIterator(t *testing.T) {
 			resultOrder:    []string{"a", "e"},
 		},
 		{
-			testName: "cycle avoided",
+			testName: "cycle avoided 1",
 			roles: map[string][]data.DelegatedRole{
 				"a": {
 					{Name: "b", Paths: defaultPathPatterns},
@@ -91,7 +102,28 @@ func TestDelegationsIterator(t *testing.T) {
 			},
 			rootDelegation: data.DelegatedRole{Name: "a", Paths: defaultPathPatterns},
 			file:           "",
-			resultOrder:    []string{"a", "b", "a", "e", "d"},
+			resultOrder:    []string{"a", "b", "d", "e"},
+		},
+
+		{
+			testName: "cycle avoided 2",
+			roles: map[string][]data.DelegatedRole{
+				"a": {
+					{Name: "a", Paths: defaultPathPatterns},
+					{Name: "b", Paths: defaultPathPatterns},
+				},
+				"b": {
+					{Name: "a", Paths: defaultPathPatterns},
+					{Name: "b", Paths: defaultPathPatterns},
+					{Name: "c", Paths: defaultPathPatterns},
+				},
+				"c": {
+					{Name: "c", Paths: defaultPathPatterns},
+				},
+			},
+			rootDelegation: data.DelegatedRole{Name: "a", Paths: defaultPathPatterns},
+			file:           "",
+			resultOrder:    []string{"a", "b", "c"},
 		},
 	}
 
@@ -111,10 +143,7 @@ func TestDelegationsIterator(t *testing.T) {
 				}
 				d.add(delegations, r.child.Name)
 			}
-			assert.Equal(t, len(tt.resultOrder), len(iterationOrder))
-			for i, role := range iterationOrder {
-				assert.Equal(t, tt.resultOrder[i], role)
-			}
+			assert.Equal(t, tt.resultOrder, iterationOrder)
 		})
 	}
 }