From d8c00b7f529e22ec9e07e4f1fdc429f33b707f53 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 30 Jan 2020 09:29:38 -0500 Subject: [PATCH] Fix node authorizer index recomputation --- plugin/pkg/auth/authorizer/node/graph.go | 2 +- plugin/pkg/auth/authorizer/node/graph_test.go | 233 ++++++++++++++++++ 2 files changed, 234 insertions(+), 1 deletion(-) diff --git a/plugin/pkg/auth/authorizer/node/graph.go b/plugin/pkg/auth/authorizer/node/graph.go index 39e2cef4823..72f005080b9 100644 --- a/plugin/pkg/auth/authorizer/node/graph.go +++ b/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 }) diff --git a/plugin/pkg/auth/authorizer/node/graph_test.go b/plugin/pkg/auth/authorizer/node/graph_test.go index 02d196205d8..a87d6c0a7b8 100644 --- a/plugin/pkg/auth/authorizer/node/graph_test.go +++ b/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"}, + }) +}