Merge pull request kubernetes#87693 from liggitt/node-authz-index

Fix node authorizer index recomputation

Author: k8s-ci-robot

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

@@ -189,7 +189,7 @@ func (g *Graph) deleteVertex_locked(vertexType vertexType, namespace, name strin
 			neighborsToRemove = append(neighborsToRemove, neighbor)
 		} else {
 			// recompute the destination edge index on this neighbor
-			neighborsToRecompute = append(neighborsToRemove, neighbor)
+			neighborsToRecompute = append(neighborsToRecompute, neighbor)
 		}
 		return true
 	})

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

@@ -17,10 +17,17 @@ limitations under the License.
 package node
 
 import (
+	"encoding/json"
+	"fmt"
+	"reflect"
 	"sort"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 func TestDeleteEdges_locked(t *testing.T) {
@@ -176,3 +183,229 @@ func TestDeleteEdges_locked(t *testing.T) {
 		})
 	}
 }
+
+func TestIndex(t *testing.T) {
+	g := NewGraph()
+	g.destinationEdgeThreshold = 3
+
+	a := NewAuthorizer(g, nil, nil).(*NodeAuthorizer)
+
+	addPod := func(podNumber, nodeNumber int) {
+		t.Helper()
+		nodeName := fmt.Sprintf("node%d", nodeNumber)
+		podName := fmt.Sprintf("pod%d", podNumber)
+		pod := &corev1.Pod{
+			ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: "ns", UID: types.UID(fmt.Sprintf("pod%duid1", podNumber))},
+			Spec: corev1.PodSpec{
+				NodeName:                 nodeName,
+				ServiceAccountName:       "sa1",
+				DeprecatedServiceAccount: "sa1",
+				Volumes: []corev1.Volume{
+					{Name: "volume1", VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{LocalObjectReference: corev1.LocalObjectReference{Name: "cm1"}}}},
+					{Name: "volume2", VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{LocalObjectReference: corev1.LocalObjectReference{Name: "cm2"}}}},
+					{Name: "volume3", VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{LocalObjectReference: corev1.LocalObjectReference{Name: "cm3"}}}},
+				},
+			},
+		}
+		g.AddPod(pod)
+		if ok, err := a.hasPathFrom(nodeName, configMapVertexType, "ns", "cm1"); err != nil || !ok {
+			t.Errorf("expected path from %s to cm1, got %v, %v", nodeName, ok, err)
+		}
+	}
+
+	toString := func(id int) string {
+		for _, namespaceName := range g.vertices {
+			for _, nameVertex := range namespaceName {
+				for _, vertex := range nameVertex {
+					if vertex.id == id {
+						return vertex.String()
+					}
+				}
+			}
+		}
+		return ""
+	}
+	expectGraph := func(expect map[string][]string) {
+		t.Helper()
+		actual := map[string][]string{}
+		for _, node := range g.graph.Nodes() {
+			sortedTo := []string{}
+			for _, to := range g.graph.From(node) {
+				sortedTo = append(sortedTo, toString(to.ID()))
+			}
+			sort.Strings(sortedTo)
+			actual[toString(node.ID())] = sortedTo
+		}
+		if !reflect.DeepEqual(expect, actual) {
+			e, _ := json.MarshalIndent(expect, "", "  ")
+			a, _ := json.MarshalIndent(actual, "", "  ")
+			t.Errorf("expected graph:\n%s\ngot:\n%s", string(e), string(a))
+		}
+	}
+	expectIndex := func(expect map[string][]string) {
+		t.Helper()
+		actual := map[string][]string{}
+		for from, to := range g.destinationEdgeIndex {
+			sortedValues := []string{}
+			for member := range to.members {
+				sortedValues = append(sortedValues, toString(member))
+			}
+			sort.Strings(sortedValues)
+			actual[toString(from)] = sortedValues
+		}
+		if !reflect.DeepEqual(expect, actual) {
+			e, _ := json.MarshalIndent(expect, "", "  ")
+			a, _ := json.MarshalIndent(actual, "", "  ")
+			t.Errorf("expected index:\n%s\ngot:\n%s", string(e), string(a))
+		}
+	}
+
+	for i := 1; i <= g.destinationEdgeThreshold; i++ {
+		addPod(i, i)
+		if i < g.destinationEdgeThreshold {
+			// if we're under the threshold, no index expected
+			expectIndex(map[string][]string{})
+		}
+	}
+	expectGraph(map[string][]string{
+		"node:node1":            {},
+		"node:node2":            {},
+		"node:node3":            {},
+		"pod:ns/pod1":           {"node:node1"},
+		"pod:ns/pod2":           {"node:node2"},
+		"pod:ns/pod3":           {"node:node3"},
+		"configmap:ns/cm1":      {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"},
+		"configmap:ns/cm2":      {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"},
+		"configmap:ns/cm3":      {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"},
+		"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"},
+	})
+
+	// delete one to drop below the threshold
+	g.DeletePod("pod1", "ns")
+	expectGraph(map[string][]string{
+		"node:node2":            {},
+		"node:node3":            {},
+		"pod:ns/pod2":           {"node:node2"},
+		"pod:ns/pod3":           {"node:node3"},
+		"configmap:ns/cm1":      {"pod:ns/pod2", "pod:ns/pod3"},
+		"configmap:ns/cm2":      {"pod:ns/pod2", "pod:ns/pod3"},
+		"configmap:ns/cm3":      {"pod:ns/pod2", "pod:ns/pod3"},
+		"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3"},
+	})
+	expectIndex(map[string][]string{})
+
+	// add two to get above the threshold
+	addPod(1, 1)
+	addPod(4, 1)
+	expectGraph(map[string][]string{
+		"node:node1":            {},
+		"node:node2":            {},
+		"node:node3":            {},
+		"pod:ns/pod1":           {"node:node1"},
+		"pod:ns/pod2":           {"node:node2"},
+		"pod:ns/pod3":           {"node:node3"},
+		"pod:ns/pod4":           {"node:node1"},
+		"configmap:ns/cm1":      {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm2":      {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm3":      {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"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"},
+	})
+
+	// delete one to remain above the threshold
+	g.DeletePod("pod1", "ns")
+	expectGraph(map[string][]string{
+		"node:node1":            {},
+		"node:node2":            {},
+		"node:node3":            {},
+		"pod:ns/pod2":           {"node:node2"},
+		"pod:ns/pod3":           {"node:node3"},
+		"pod:ns/pod4":           {"node:node1"},
+		"configmap:ns/cm1":      {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm2":      {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm3":      {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"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"},
+	})
+
+	// Set node->configmap references
+	g.SetNodeConfigMap("node1", "cm1", "ns")
+	g.SetNodeConfigMap("node2", "cm1", "ns")
+	g.SetNodeConfigMap("node3", "cm1", "ns")
+	expectGraph(map[string][]string{
+		"node:node1":            {},
+		"node:node2":            {},
+		"node:node3":            {},
+		"pod:ns/pod2":           {"node:node2"},
+		"pod:ns/pod3":           {"node:node3"},
+		"pod:ns/pod4":           {"node:node1"},
+		"configmap:ns/cm1":      {"node:node1", "node:node2", "node:node3", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm2":      {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm3":      {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"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"},
+	})
+
+	// Update node->configmap reference
+	g.SetNodeConfigMap("node1", "cm2", "ns")
+	expectGraph(map[string][]string{
+		"node:node1":            {},
+		"node:node2":            {},
+		"node:node3":            {},
+		"pod:ns/pod2":           {"node:node2"},
+		"pod:ns/pod3":           {"node:node3"},
+		"pod:ns/pod4":           {"node:node1"},
+		"configmap:ns/cm1":      {"node:node2", "node:node3", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm2":      {"node:node1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm3":      {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"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"},
+	})
+
+	// Remove node->configmap reference
+	g.SetNodeConfigMap("node1", "", "")
+	expectGraph(map[string][]string{
+		"node:node1":            {},
+		"node:node2":            {},
+		"node:node3":            {},
+		"pod:ns/pod2":           {"node:node2"},
+		"pod:ns/pod3":           {"node:node3"},
+		"pod:ns/pod4":           {"node:node1"},
+		"configmap:ns/cm1":      {"node:node2", "node:node3", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm2":      {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"configmap:ns/cm3":      {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
+		"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"},
+	})
+}