Skip to content

Commit d8c00b7

Browse files
liggittliggitt
committed
Fix node authorizer index recomputation
1 parent cad4460 commit d8c00b7

File tree

2 files changed

+234
-1
lines changed

2 files changed

+234
-1
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ func (g *Graph) deleteVertex_locked(vertexType vertexType, namespace, name strin
189189
neighborsToRemove = append(neighborsToRemove, neighbor)
190190
} else {
191191
// recompute the destination edge index on this neighbor
192-
neighborsToRecompute = append(neighborsToRemove, neighbor)
192+
neighborsToRecompute = append(neighborsToRecompute, neighbor)
193193
}
194194
return true
195195
})

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

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,17 @@ limitations under the License.
1717
package node
1818

1919
import (
20+
"encoding/json"
21+
"fmt"
22+
"reflect"
2023
"sort"
2124
"testing"
2225

2326
"github.com/stretchr/testify/assert"
27+
28+
corev1 "k8s.io/api/core/v1"
29+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
"k8s.io/apimachinery/pkg/types"
2431
)
2532

2633
func TestDeleteEdges_locked(t *testing.T) {
@@ -176,3 +183,229 @@ func TestDeleteEdges_locked(t *testing.T) {
176183
})
177184
}
178185
}
186+
187+
func TestIndex(t *testing.T) {
188+
g := NewGraph()
189+
g.destinationEdgeThreshold = 3
190+
191+
a := NewAuthorizer(g, nil, nil).(*NodeAuthorizer)
192+
193+
addPod := func(podNumber, nodeNumber int) {
194+
t.Helper()
195+
nodeName := fmt.Sprintf("node%d", nodeNumber)
196+
podName := fmt.Sprintf("pod%d", podNumber)
197+
pod := &corev1.Pod{
198+
ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: "ns", UID: types.UID(fmt.Sprintf("pod%duid1", podNumber))},
199+
Spec: corev1.PodSpec{
200+
NodeName: nodeName,
201+
ServiceAccountName: "sa1",
202+
DeprecatedServiceAccount: "sa1",
203+
Volumes: []corev1.Volume{
204+
{Name: "volume1", VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{LocalObjectReference: corev1.LocalObjectReference{Name: "cm1"}}}},
205+
{Name: "volume2", VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{LocalObjectReference: corev1.LocalObjectReference{Name: "cm2"}}}},
206+
{Name: "volume3", VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{LocalObjectReference: corev1.LocalObjectReference{Name: "cm3"}}}},
207+
},
208+
},
209+
}
210+
g.AddPod(pod)
211+
if ok, err := a.hasPathFrom(nodeName, configMapVertexType, "ns", "cm1"); err != nil || !ok {
212+
t.Errorf("expected path from %s to cm1, got %v, %v", nodeName, ok, err)
213+
}
214+
}
215+
216+
toString := func(id int) string {
217+
for _, namespaceName := range g.vertices {
218+
for _, nameVertex := range namespaceName {
219+
for _, vertex := range nameVertex {
220+
if vertex.id == id {
221+
return vertex.String()
222+
}
223+
}
224+
}
225+
}
226+
return ""
227+
}
228+
expectGraph := func(expect map[string][]string) {
229+
t.Helper()
230+
actual := map[string][]string{}
231+
for _, node := range g.graph.Nodes() {
232+
sortedTo := []string{}
233+
for _, to := range g.graph.From(node) {
234+
sortedTo = append(sortedTo, toString(to.ID()))
235+
}
236+
sort.Strings(sortedTo)
237+
actual[toString(node.ID())] = sortedTo
238+
}
239+
if !reflect.DeepEqual(expect, actual) {
240+
e, _ := json.MarshalIndent(expect, "", " ")
241+
a, _ := json.MarshalIndent(actual, "", " ")
242+
t.Errorf("expected graph:\n%s\ngot:\n%s", string(e), string(a))
243+
}
244+
}
245+
expectIndex := func(expect map[string][]string) {
246+
t.Helper()
247+
actual := map[string][]string{}
248+
for from, to := range g.destinationEdgeIndex {
249+
sortedValues := []string{}
250+
for member := range to.members {
251+
sortedValues = append(sortedValues, toString(member))
252+
}
253+
sort.Strings(sortedValues)
254+
actual[toString(from)] = sortedValues
255+
}
256+
if !reflect.DeepEqual(expect, actual) {
257+
e, _ := json.MarshalIndent(expect, "", " ")
258+
a, _ := json.MarshalIndent(actual, "", " ")
259+
t.Errorf("expected index:\n%s\ngot:\n%s", string(e), string(a))
260+
}
261+
}
262+
263+
for i := 1; i <= g.destinationEdgeThreshold; i++ {
264+
addPod(i, i)
265+
if i < g.destinationEdgeThreshold {
266+
// if we're under the threshold, no index expected
267+
expectIndex(map[string][]string{})
268+
}
269+
}
270+
expectGraph(map[string][]string{
271+
"node:node1": {},
272+
"node:node2": {},
273+
"node:node3": {},
274+
"pod:ns/pod1": {"node:node1"},
275+
"pod:ns/pod2": {"node:node2"},
276+
"pod:ns/pod3": {"node:node3"},
277+
"configmap:ns/cm1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"},
278+
"configmap:ns/cm2": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"},
279+
"configmap:ns/cm3": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"},
280+
"serviceAccount:ns/sa1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"},
281+
})
282+
expectIndex(map[string][]string{
283+
"configmap:ns/cm1": {"node:node1", "node:node2", "node:node3"},
284+
"configmap:ns/cm2": {"node:node1", "node:node2", "node:node3"},
285+
"configmap:ns/cm3": {"node:node1", "node:node2", "node:node3"},
286+
"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
287+
})
288+
289+
// delete one to drop below the threshold
290+
g.DeletePod("pod1", "ns")
291+
expectGraph(map[string][]string{
292+
"node:node2": {},
293+
"node:node3": {},
294+
"pod:ns/pod2": {"node:node2"},
295+
"pod:ns/pod3": {"node:node3"},
296+
"configmap:ns/cm1": {"pod:ns/pod2", "pod:ns/pod3"},
297+
"configmap:ns/cm2": {"pod:ns/pod2", "pod:ns/pod3"},
298+
"configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3"},
299+
"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3"},
300+
})
301+
expectIndex(map[string][]string{})
302+
303+
// add two to get above the threshold
304+
addPod(1, 1)
305+
addPod(4, 1)
306+
expectGraph(map[string][]string{
307+
"node:node1": {},
308+
"node:node2": {},
309+
"node:node3": {},
310+
"pod:ns/pod1": {"node:node1"},
311+
"pod:ns/pod2": {"node:node2"},
312+
"pod:ns/pod3": {"node:node3"},
313+
"pod:ns/pod4": {"node:node1"},
314+
"configmap:ns/cm1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
315+
"configmap:ns/cm2": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
316+
"configmap:ns/cm3": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
317+
"serviceAccount:ns/sa1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
318+
})
319+
expectIndex(map[string][]string{
320+
"configmap:ns/cm1": {"node:node1", "node:node2", "node:node3"},
321+
"configmap:ns/cm2": {"node:node1", "node:node2", "node:node3"},
322+
"configmap:ns/cm3": {"node:node1", "node:node2", "node:node3"},
323+
"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
324+
})
325+
326+
// delete one to remain above the threshold
327+
g.DeletePod("pod1", "ns")
328+
expectGraph(map[string][]string{
329+
"node:node1": {},
330+
"node:node2": {},
331+
"node:node3": {},
332+
"pod:ns/pod2": {"node:node2"},
333+
"pod:ns/pod3": {"node:node3"},
334+
"pod:ns/pod4": {"node:node1"},
335+
"configmap:ns/cm1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
336+
"configmap:ns/cm2": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
337+
"configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
338+
"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
339+
})
340+
expectIndex(map[string][]string{
341+
"configmap:ns/cm1": {"node:node1", "node:node2", "node:node3"},
342+
"configmap:ns/cm2": {"node:node1", "node:node2", "node:node3"},
343+
"configmap:ns/cm3": {"node:node1", "node:node2", "node:node3"},
344+
"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
345+
})
346+
347+
// Set node->configmap references
348+
g.SetNodeConfigMap("node1", "cm1", "ns")
349+
g.SetNodeConfigMap("node2", "cm1", "ns")
350+
g.SetNodeConfigMap("node3", "cm1", "ns")
351+
expectGraph(map[string][]string{
352+
"node:node1": {},
353+
"node:node2": {},
354+
"node:node3": {},
355+
"pod:ns/pod2": {"node:node2"},
356+
"pod:ns/pod3": {"node:node3"},
357+
"pod:ns/pod4": {"node:node1"},
358+
"configmap:ns/cm1": {"node:node1", "node:node2", "node:node3", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
359+
"configmap:ns/cm2": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
360+
"configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
361+
"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
362+
})
363+
expectIndex(map[string][]string{
364+
"configmap:ns/cm1": {"node:node1", "node:node2", "node:node3"},
365+
"configmap:ns/cm2": {"node:node1", "node:node2", "node:node3"},
366+
"configmap:ns/cm3": {"node:node1", "node:node2", "node:node3"},
367+
"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
368+
})
369+
370+
// Update node->configmap reference
371+
g.SetNodeConfigMap("node1", "cm2", "ns")
372+
expectGraph(map[string][]string{
373+
"node:node1": {},
374+
"node:node2": {},
375+
"node:node3": {},
376+
"pod:ns/pod2": {"node:node2"},
377+
"pod:ns/pod3": {"node:node3"},
378+
"pod:ns/pod4": {"node:node1"},
379+
"configmap:ns/cm1": {"node:node2", "node:node3", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
380+
"configmap:ns/cm2": {"node:node1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
381+
"configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
382+
"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
383+
})
384+
expectIndex(map[string][]string{
385+
"configmap:ns/cm1": {"node:node1", "node:node2", "node:node3"},
386+
"configmap:ns/cm2": {"node:node1", "node:node2", "node:node3"},
387+
"configmap:ns/cm3": {"node:node1", "node:node2", "node:node3"},
388+
"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
389+
})
390+
391+
// Remove node->configmap reference
392+
g.SetNodeConfigMap("node1", "", "")
393+
expectGraph(map[string][]string{
394+
"node:node1": {},
395+
"node:node2": {},
396+
"node:node3": {},
397+
"pod:ns/pod2": {"node:node2"},
398+
"pod:ns/pod3": {"node:node3"},
399+
"pod:ns/pod4": {"node:node1"},
400+
"configmap:ns/cm1": {"node:node2", "node:node3", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
401+
"configmap:ns/cm2": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
402+
"configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
403+
"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
404+
})
405+
expectIndex(map[string][]string{
406+
"configmap:ns/cm1": {"node:node1", "node:node2", "node:node3"},
407+
"configmap:ns/cm2": {"node:node1", "node:node2", "node:node3"},
408+
"configmap:ns/cm3": {"node:node1", "node:node2", "node:node3"},
409+
"serviceAccount:ns/sa1": {"node:node1", "node:node2", "node:node3"},
410+
})
411+
}

0 commit comments

Comments
 (0)