diff --git a/plugin/pkg/auth/authorizer/node/graph.go b/plugin/pkg/auth/authorizer/node/graph.go index 82800504d22..630dd3302c3 100644 --- a/plugin/pkg/auth/authorizer/node/graph.go +++ b/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 { diff --git a/plugin/pkg/auth/authorizer/node/graph_test.go b/plugin/pkg/auth/authorizer/node/graph_test.go index 77609e28d5d..7581343c63a 100644 --- a/plugin/pkg/auth/authorizer/node/graph_test.go +++ b/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"}, }) } diff --git a/plugin/pkg/auth/authorizer/node/intset.go b/plugin/pkg/auth/authorizer/node/intset.go index 812a5367d93..57b2305da60 100644 --- a/plugin/pkg/auth/authorizer/node/intset.go +++ b/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++ -} - -// mark indicates the specified int belongs to the current generation. -// it is not thread-safe. -func (s *intSet) mark(i int) { - s.members[i] = s.currentGeneration -} - -// sweep removes items not in the current generation. -// 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) reset() { + for k := range s.members { + delete(s.members, k) + } +} + +// increment adds one to the refcount of the specified id. +// it is not thread-safe. +func (s *intSet) increment(i int) { + s.members[i]++ +} + +// 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) decrement(i int) { + if s.members[i] <= 1 { + delete(s.members, i) + } else { + s.members[i]-- } } diff --git a/plugin/pkg/auth/authorizer/node/intset_test.go b/plugin/pkg/auth/authorizer/node/intset_test.go index 0fbc7a308e0..fa717462d2a 100644 --- a/plugin/pkg/auth/authorizer/node/intset_test.go +++ b/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)) }