Switch node authorizer index to refcounts

Author: liggitt

plugin/pkg/auth/authorizer/node/graph.go

@@ -259,7 +259,7 @@ func (g *Graph) addEdgeToDestinationIndex_locked(e graph.Edge) {
 	}
 	// fast-add the new edge to an existing index
 	if destinationEdge, ok := e.(*destinationEdge); ok {
-		index.mark(destinationEdge.DestinationID())
+		index.increment(destinationEdge.DestinationID())
 	}
 }
 
@@ -290,20 +290,17 @@ func (g *Graph) recomputeDestinationIndex_locked(n graph.Node) {
 	if index == nil {
 		index = newIntSet()
 	} else {
-		index.startNewGeneration()
+		index.reset()
 	}
 
 	// populate the index
 	g.graph.VisitFrom(n, func(dest graph.Node) bool {
 		if destinationEdge, ok := g.graph.EdgeBetween(n, dest).(*destinationEdge); ok {
-			index.mark(destinationEdge.DestinationID())
+			index.increment(destinationEdge.DestinationID())
 		}
 		return true
 	})
 
-	// remove existing items no longer in the list
-	index.sweep()
-
 	if len(index.members) < g.destinationEdgeThreshold {
 		delete(g.destinationEdgeIndex, n.ID())
 	} else {

plugin/pkg/auth/authorizer/node/graph_test.go

@@ -247,8 +247,8 @@ func TestIndex(t *testing.T) {
 		actual := map[string][]string{}
 		for from, to := range g.destinationEdgeIndex {
 			sortedValues := []string{}
-			for member := range to.members {
-				sortedValues = append(sortedValues, toString(member))
+			for member, count := range to.members {
+				sortedValues = append(sortedValues, fmt.Sprintf("%s=%d", toString(member), count))
 			}
 			sort.Strings(sortedValues)
 			actual[toString(from)] = sortedValues
@@ -280,10 +280,10 @@ func TestIndex(t *testing.T) {
 		"serviceAccount:ns/sa1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"},
 	})
 	expectIndex(map[string][]string{
-		"configmap:ns/cm1":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm2":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm3":      {"node:node1", "node:node2", "node:node3"},
-		"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
+		"configmap:ns/cm1":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"configmap:ns/cm2":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"configmap:ns/cm3":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"},
 	})
 
 	// delete one to drop below the threshold
@@ -317,10 +317,10 @@ func TestIndex(t *testing.T) {
 		"serviceAccount:ns/sa1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
 	})
 	expectIndex(map[string][]string{
-		"configmap:ns/cm1":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm2":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm3":      {"node:node1", "node:node2", "node:node3"},
-		"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
+		"configmap:ns/cm1":      {"node:node1=2", "node:node2=1", "node:node3=1"},
+		"configmap:ns/cm2":      {"node:node1=2", "node:node2=1", "node:node3=1"},
+		"configmap:ns/cm3":      {"node:node1=2", "node:node2=1", "node:node3=1"},
+		"serviceAccount:ns/sa1": {"node:node1=2", "node:node2=1", "node:node3=1"},
 	})
 
 	// delete one to remain above the threshold
@@ -338,10 +338,10 @@ func TestIndex(t *testing.T) {
 		"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
 	})
 	expectIndex(map[string][]string{
-		"configmap:ns/cm1":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm2":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm3":      {"node:node1", "node:node2", "node:node3"},
-		"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
+		"configmap:ns/cm1":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"configmap:ns/cm2":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"configmap:ns/cm3":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"},
 	})
 
 	// Set node->configmap references
@@ -363,10 +363,10 @@ func TestIndex(t *testing.T) {
 		"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
 	})
 	expectIndex(map[string][]string{
-		"configmap:ns/cm1":      {"node:node1", "node:node2", "node:node3", "node:node4"},
-		"configmap:ns/cm2":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm3":      {"node:node1", "node:node2", "node:node3"},
-		"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
+		"configmap:ns/cm1":      {"node:node1=2", "node:node2=2", "node:node3=2", "node:node4=1"},
+		"configmap:ns/cm2":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"configmap:ns/cm3":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"},
 	})
 
 	// Update node->configmap reference
@@ -385,10 +385,10 @@ func TestIndex(t *testing.T) {
 		"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
 	})
 	expectIndex(map[string][]string{
-		"configmap:ns/cm1":      {"node:node1", "node:node2", "node:node3", "node:node4"},
-		"configmap:ns/cm2":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm3":      {"node:node1", "node:node2", "node:node3"},
-		"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
+		"configmap:ns/cm1":      {"node:node1=1", "node:node2=2", "node:node3=2", "node:node4=1"},
+		"configmap:ns/cm2":      {"node:node1=2", "node:node2=1", "node:node3=1"},
+		"configmap:ns/cm3":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"},
 	})
 
 	// Remove node->configmap reference
@@ -408,9 +408,9 @@ func TestIndex(t *testing.T) {
 		"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
 	})
 	expectIndex(map[string][]string{
-		"configmap:ns/cm1":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm2":      {"node:node1", "node:node2", "node:node3"},
-		"configmap:ns/cm3":      {"node:node1", "node:node2", "node:node3"},
-		"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
+		"configmap:ns/cm1":      {"node:node1=1", "node:node2=2", "node:node3=2"},
+		"configmap:ns/cm2":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"configmap:ns/cm3":      {"node:node1=1", "node:node2=1", "node:node3=1"},
+		"serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"},
 	})
 }

plugin/pkg/auth/authorizer/node/intset.go

@@ -16,47 +16,47 @@ limitations under the License.
 
 package node
 
-// intSet maintains a set of ints, and supports promoting and culling the previous generation.
-// this allows tracking a large, mostly-stable set without constantly reallocating the entire set.
+// intSet maintains a map of id to refcounts
 type intSet struct {
-	currentGeneration byte
-	members           map[int]byte
+	// members is a map of id to refcounts
+	members map[int]int
 }
 
 func newIntSet() *intSet {
-	return &intSet{members: map[int]byte{}}
+	return &intSet{members: map[int]int{}}
 }
 
-// has returns true if the specified int is in the set.
+// has returns true if the specified id has a positive refcount.
 // it is safe to call concurrently, but must not be called concurrently with any of the other methods.
 func (s *intSet) has(i int) bool {
 	if s == nil {
 		return false
 	}
-	_, present := s.members[i]
-	return present
+	return s.members[i] > 0
 }
 
-// startNewGeneration begins a new generation.
-// it must be followed by a call to mark() for every member of the generation,
-// then a call to sweep() to remove members not present in the generation.
+// reset removes all ids, effectively setting their refcounts to 0.
 // it is not thread-safe.
-func (s *intSet) startNewGeneration() {
-	s.currentGeneration++
+func (s *intSet) reset() {
+	for k := range s.members {
+		delete(s.members, k)
+	}
 }
 
-// mark indicates the specified int belongs to the current generation.
+// increment adds one to the refcount of the specified id.
 // it is not thread-safe.
-func (s *intSet) mark(i int) {
-	s.members[i] = s.currentGeneration
+func (s *intSet) increment(i int) {
+	s.members[i]++
 }
 
-// sweep removes items not in the current generation.
+// decrement removes one from the refcount of the specified id,
+// and removes the id if the resulting refcount is <= 0.
+// it will not track refcounts lower than zero.
 // it is not thread-safe.
-func (s *intSet) sweep() {
-	for k, v := range s.members {
-		if v != s.currentGeneration {
-			delete(s.members, k)
-		}
+func (s *intSet) decrement(i int) {
+	if s.members[i] <= 1 {
+		delete(s.members, i)
+	} else {
+		s.members[i]--
 	}
 }

plugin/pkg/auth/authorizer/node/intset_test.go

@@ -30,33 +30,36 @@ func TestIntSet(t *testing.T) {
 	assert.False(t, i.has(3))
 	assert.False(t, i.has(4))
 
-	i.startNewGeneration()
-	i.mark(1)
-	i.mark(2)
-	i.sweep()
+	i.reset()
+	i.increment(1) // to 1
+	i.increment(2) // to 1
 
 	assert.True(t, i.has(1))
 	assert.True(t, i.has(2))
 	assert.False(t, i.has(3))
 	assert.False(t, i.has(4))
 
-	i.startNewGeneration()
-	i.mark(2)
-	i.mark(3)
-	i.sweep()
+	i.decrement(1) // to 0
+	i.increment(3) // to 1
 
-	assert.False(t, i.has(1))
-	assert.True(t, i.has(2))
-	assert.True(t, i.has(3))
-	assert.False(t, i.has(4))
+	assert.False(t, i.has(1)) // removed
+	assert.True(t, i.has(2))  // still present
+	assert.True(t, i.has(3))  // added
+	assert.False(t, i.has(4)) // not yet present
 
-	i.startNewGeneration()
-	i.mark(3)
-	i.mark(4)
-	i.sweep()
+	i.decrement(2) // to 0
+	i.increment(3) // to 2
+	i.decrement(3) // to 1
+	i.increment(4) // to 1
 
 	assert.False(t, i.has(1))
 	assert.False(t, i.has(2))
 	assert.True(t, i.has(3))
 	assert.True(t, i.has(4))
+
+	i.reset()
+	assert.False(t, i.has(1))
+	assert.False(t, i.has(2))
+	assert.False(t, i.has(3))
+	assert.False(t, i.has(4))
 }