From fca697aca08480667a7e42acbfab3ab63b910e29 Mon Sep 17 00:00:00 2001 From: Cordelia Link Date: Fri, 17 Jan 2025 02:49:36 +0000 Subject: [PATCH 1/2] Add logic to check for new ephemeral containers on pod update and update secret cache if they exist. --- .../auth/authorizer/node/graph_populator.go | 3 + .../authorizer/node/node_authorizer_test.go | 142 ++++++++++++++++++ 2 files changed, 145 insertions(+) diff --git a/plugin/pkg/auth/authorizer/node/graph_populator.go b/plugin/pkg/auth/authorizer/node/graph_populator.go index e1a514636a0..b5a14f016cf 100644 --- a/plugin/pkg/auth/authorizer/node/graph_populator.go +++ b/plugin/pkg/auth/authorizer/node/graph_populator.go @@ -94,7 +94,10 @@ func (g *graphPopulator) updatePod(oldObj, obj interface{}) { return } if oldPod, ok := oldObj.(*corev1.Pod); ok && oldPod != nil { + // Ephemeral containers can add new secret or config map references to the pod. + hasNewEphemeralContainers := len(pod.Spec.EphemeralContainers) > len(oldPod.Spec.EphemeralContainers) if (pod.Spec.NodeName == oldPod.Spec.NodeName) && (pod.UID == oldPod.UID) && + !hasNewEphemeralContainers && resourceclaim.PodStatusEqual(oldPod.Status.ResourceClaimStatuses, pod.Status.ResourceClaimStatuses) { // Node and uid are unchanged, all object references in the pod spec are immutable respectively unmodified (claim statuses). klog.V(5).Infof("updatePod %s/%s, node unchanged", pod.Namespace, pod.Name) diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go index c6378985531..0874ea1e2e9 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go @@ -856,6 +856,118 @@ func TestNodeAuthorizerSharedResources(t *testing.T) { } } +func TestNodeAuthorizerAddEphemeralContainers(t *testing.T) { + g := NewGraph() + g.destinationEdgeThreshold = 1 + identifier := nodeidentifier.NewDefaultNodeIdentifier() + authz := NewAuthorizer(g, identifier, bootstrappolicy.NodeRules()) + + node1 := &user.DefaultInfo{Name: "system:node:node1", Groups: []string{"system:nodes"}} + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "pod1-node1", Namespace: "ns1"}, + Spec: corev1.PodSpec{ + NodeName: "node1", + Volumes: []corev1.Volume{ + {VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "node1-only"}}}, + }, + }, + } + + ecNewSecret := corev1.EphemeralContainer{ + TargetContainerName: "targetContainerName", + EphemeralContainerCommon: corev1.EphemeralContainerCommon{ + Image: "imageURL", + Name: "eph", + Command: []string{"command"}, + EnvFrom: []corev1.EnvFromSource{ + { + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "new-secret", + }, + Optional: nil, + }, + }, + }, + SecurityContext: &corev1.SecurityContext{ + Privileged: &[]bool{true}[0], + }, + }, + } + + ecNewConfigMap := corev1.EphemeralContainer{ + TargetContainerName: "targetContainerName", + EphemeralContainerCommon: corev1.EphemeralContainerCommon{ + Image: "imageURL", + Name: "eph", + Command: []string{"command"}, + EnvFrom: []corev1.EnvFromSource{ + { + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "new-config-map", + }, + Optional: nil, + }, + }, + }, + SecurityContext: &corev1.SecurityContext{ + Privileged: &[]bool{true}[0], + }, + }, + } + p := &graphPopulator{} + p.graph = g + p.addPod(pod) + + testcases := []struct { + User user.Info + Secret string + ConfigMap string + Decision authorizer.Decision + EphCont *corev1.EphemeralContainer + }{ + {User: node1, Decision: authorizer.DecisionAllow, Secret: "node1-only"}, + {User: node1, Decision: authorizer.DecisionNoOpinion, Secret: "new-secret"}, + {User: node1, Decision: authorizer.DecisionAllow, Secret: "new-secret", EphCont: &ecNewSecret}, + {User: node1, Decision: authorizer.DecisionNoOpinion, ConfigMap: "new-config-map"}, + {User: node1, Decision: authorizer.DecisionAllow, ConfigMap: "new-config-map", EphCont: &ecNewConfigMap}, + } + + for i, tc := range testcases { + var ( + decision authorizer.Decision + err error + ) + if tc.EphCont != nil { + newPod := &corev1.Pod{} + pod.DeepCopyInto(newPod) + newPod.Spec.EphemeralContainers = append(newPod.Spec.EphemeralContainers, *tc.EphCont) + p.updatePod(pod, newPod) + } + + if len(tc.Secret) > 0 { + decision, _, err = authz.Authorize(context.Background(), authorizer.AttributesRecord{User: tc.User, ResourceRequest: true, Verb: "get", Resource: "secrets", Namespace: "ns1", Name: tc.Secret}) + if err != nil { + t.Errorf("%d: unexpected error: %v", i, err) + continue + } + } else if len(tc.ConfigMap) > 0 { + decision, _, err = authz.Authorize(context.Background(), authorizer.AttributesRecord{User: tc.User, ResourceRequest: true, Verb: "get", Resource: "configmaps", Namespace: "ns1", Name: tc.ConfigMap}) + if err != nil { + t.Errorf("%d: unexpected error: %v", i, err) + continue + } + } else { + t.Fatalf("test case must include a request for a Secret") + } + + if decision != tc.Decision { + t.Errorf("%d: expected %v, got %v", i, tc.Decision, decision) + } + } +} + type sampleDataOpts struct { nodes int namespaces int @@ -1226,6 +1338,36 @@ func populate(graph *Graph, nodes []*corev1.Node, pods []*corev1.Pod, pvs []*cor } } +func updateMoo(graph *Graph, oldPod *corev1.Pod) { + p := &graphPopulator{} + p.graph = graph + p.addPod(oldPod) + newPod := oldPod + newPod.Spec.EphemeralContainers = append(newPod.Spec.EphemeralContainers, corev1.EphemeralContainer{ + TargetContainerName: "targetContainerName", + EphemeralContainerCommon: corev1.EphemeralContainerCommon{ + Image: "imageURL", + Name: "eph", + Command: []string{"command"}, + EnvFrom: []corev1.EnvFromSource{ + { + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "new-secret", + }, + Optional: nil, + }, + }, + }, + SecurityContext: &corev1.SecurityContext{ + Privileged: &[]bool{true}[0], + }, + }, + }) + + p.updatePod(oldPod, newPod) +} + func randomSubset(a, b int) []int { if b < a { b = a From 9898bfdbdae86219a3d5f898c56d23253a8af135 Mon Sep 17 00:00:00 2001 From: Cordelia Link Date: Fri, 17 Jan 2025 02:52:41 +0000 Subject: [PATCH 2/2] Remove unused test code --- .../authorizer/node/node_authorizer_test.go | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go index 0874ea1e2e9..b9383ffddfd 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go @@ -1338,36 +1338,6 @@ func populate(graph *Graph, nodes []*corev1.Node, pods []*corev1.Pod, pvs []*cor } } -func updateMoo(graph *Graph, oldPod *corev1.Pod) { - p := &graphPopulator{} - p.graph = graph - p.addPod(oldPod) - newPod := oldPod - newPod.Spec.EphemeralContainers = append(newPod.Spec.EphemeralContainers, corev1.EphemeralContainer{ - TargetContainerName: "targetContainerName", - EphemeralContainerCommon: corev1.EphemeralContainerCommon{ - Image: "imageURL", - Name: "eph", - Command: []string{"command"}, - EnvFrom: []corev1.EnvFromSource{ - { - SecretRef: &corev1.SecretEnvSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "new-secret", - }, - Optional: nil, - }, - }, - }, - SecurityContext: &corev1.SecurityContext{ - Privileged: &[]bool{true}[0], - }, - }, - }) - - p.updatePod(oldPod, newPod) -} - func randomSubset(a, b int) []int { if b < a { b = a