DRA e2e: update VAP for a kubelet plugin

This fixes the message (node name and "cluster-scoped" were switched) and
simplifies the VAP:
- a single matchCondition short circuits completely unless they're a user
  we care about
- variables to extract the userNodeName and objectNodeName once
  (using optionals to gracefully turn missing claims and fields into empty strings)
- leaves very tiny concise validations

Co-authored-by: Jordan Liggitt <[email protected]>

Author: pohly

test/e2e/dra/dra.go

@@ -1184,7 +1184,16 @@ var _ = framework.SIGDescribe("node")("DRA", feature.DynamicResourceAllocation,
 			})
 
 			// Messages from test-driver/deploy/example/plugin-permissions.yaml
-			matchVAPDeniedError := gomega.MatchError(gomega.ContainSubstring("may only modify resourceslices that belong to the node the pod is running on"))
+			matchVAPDeniedError := func(nodeName string, slice *resourceapi.ResourceSlice) types.GomegaMatcher {
+				subStr := fmt.Sprintf("this user running on node '%s' may not modify ", nodeName)
+				switch {
+				case slice.Spec.NodeName != "":
+					subStr += fmt.Sprintf("resourceslices on node '%s'", slice.Spec.NodeName)
+				default:
+					subStr += "cluster resourceslices"
+				}
+				return gomega.MatchError(gomega.ContainSubstring(subStr))
+			}
 			mustCreate := func(clientSet kubernetes.Interface, clientName string, slice *resourceapi.ResourceSlice) *resourceapi.ResourceSlice {
 				ginkgo.GinkgoHelper()
 				slice, err := clientSet.ResourceV1alpha3().ResourceSlices().Create(ctx, slice, metav1.CreateOptions{})
@@ -1224,17 +1233,17 @@ var _ = framework.SIGDescribe("node")("DRA", feature.DynamicResourceAllocation,
 			}
 
 			// Create with different clients, keep it in the end.
-			mustFailToCreate(realNodeClient, "real plugin", fictionalNodeSlice, matchVAPDeniedError)
+			mustFailToCreate(realNodeClient, "real plugin", fictionalNodeSlice, matchVAPDeniedError(realNodeName, fictionalNodeSlice))
 			mustCreateAndDelete(fictionalNodeClient, "fictional plugin", fictionalNodeSlice)
 			createdFictionalNodeSlice := mustCreate(f.ClientSet, "admin", fictionalNodeSlice)
 
 			// Update with different clients.
-			mustFailToUpdate(realNodeClient, "real plugin", createdFictionalNodeSlice, matchVAPDeniedError)
+			mustFailToUpdate(realNodeClient, "real plugin", createdFictionalNodeSlice, matchVAPDeniedError(realNodeName, createdFictionalNodeSlice))
 			createdFictionalNodeSlice = mustUpdate(fictionalNodeClient, "fictional plugin", createdFictionalNodeSlice)
 			createdFictionalNodeSlice = mustUpdate(f.ClientSet, "admin", createdFictionalNodeSlice)
 
 			// Delete with different clients.
-			mustFailToDelete(realNodeClient, "real plugin", createdFictionalNodeSlice, matchVAPDeniedError)
+			mustFailToDelete(realNodeClient, "real plugin", createdFictionalNodeSlice, matchVAPDeniedError(realNodeName, createdFictionalNodeSlice))
 			mustDelete(fictionalNodeClient, "fictional plugin", createdFictionalNodeSlice)
 
 			// Now the same for a slice which is not associated with a node.
@@ -1259,18 +1268,18 @@ var _ = framework.SIGDescribe("node")("DRA", feature.DynamicResourceAllocation,
 			})
 
 			// Create with different clients, keep it in the end.
-			mustFailToCreate(realNodeClient, "real plugin", clusterSlice, matchVAPDeniedError)
-			mustFailToCreate(fictionalNodeClient, "fictional plugin", clusterSlice, matchVAPDeniedError)
+			mustFailToCreate(realNodeClient, "real plugin", clusterSlice, matchVAPDeniedError(realNodeName, clusterSlice))
+			mustFailToCreate(fictionalNodeClient, "fictional plugin", clusterSlice, matchVAPDeniedError(fictionalNodeName, clusterSlice))
 			createdClusterSlice := mustCreate(f.ClientSet, "admin", clusterSlice)
 
 			// Update with different clients.
-			mustFailToUpdate(realNodeClient, "real plugin", createdClusterSlice, matchVAPDeniedError)
-			mustFailToUpdate(fictionalNodeClient, "fictional plugin", createdClusterSlice, matchVAPDeniedError)
+			mustFailToUpdate(realNodeClient, "real plugin", createdClusterSlice, matchVAPDeniedError(realNodeName, createdClusterSlice))
+			mustFailToUpdate(fictionalNodeClient, "fictional plugin", createdClusterSlice, matchVAPDeniedError(fictionalNodeName, createdClusterSlice))
 			createdClusterSlice = mustUpdate(f.ClientSet, "admin", createdClusterSlice)
 
 			// Delete with different clients.
-			mustFailToDelete(realNodeClient, "real plugin", createdClusterSlice, matchVAPDeniedError)
-			mustFailToDelete(fictionalNodeClient, "fictional plugin", createdClusterSlice, matchVAPDeniedError)
+			mustFailToDelete(realNodeClient, "real plugin", createdClusterSlice, matchVAPDeniedError(realNodeName, createdClusterSlice))
+			mustFailToDelete(fictionalNodeClient, "fictional plugin", createdClusterSlice, matchVAPDeniedError(fictionalNodeName, createdClusterSlice))
 			mustDelete(f.ClientSet, "admin", createdClusterSlice)
 		})
 

test/e2e/dra/test-driver/deploy/example/plugin-permissions.yaml

@@ -50,29 +50,25 @@ spec:
       apiVersions: ["v1alpha3"]
       operations:  ["CREATE", "UPDATE", "DELETE"]
       resources:   ["resourceslices"]
-  variables:
-  - name: hasNodeName
-    expression: >-
-      "authentication.kubernetes.io/node-name" in request.userInfo.extra
-  - name: isKubeletPlugin
+  matchConditions:
+  - name: isRestrictedUser
     expression: >-
       request.userInfo.username == "system:serviceaccount:dra-kubelet-plugin-namespace:dra-kubelet-plugin-service-account"
+  variables:
+  - name: userNodeName
+    expression: >-
+      request.userInfo.extra[?'authentication.kubernetes.io/node-name'][0].orValue('')
   - name: objectNodeName
     expression: >-
       (request.operation == "DELETE" ? oldObject : object).spec.?nodeName.orValue("")
   validations:
-  - expression: >-
-      !variables.isKubeletPlugin || variables.hasNodeName
-    message: This user must have a "authentication.kubernetes.io/node-name" claim. ServiceAccountTokenNodeBindingValidation must be enabled in the cluster.
-  - expression: >-
-      !variables.isKubeletPlugin || !variables.hasNodeName ||
-      variables.objectNodeName == request.userInfo.extra["authentication.kubernetes.io/node-name"][0]
-    message: This DRA kubelet plugin may only modify resourceslices that belong to the node the pod is running on.
-    # This is useful for debugging. Can be dropped in a production deployment.
+  - expression: variables.userNodeName != ""
+    message: >-
+      no node association found for user, this user must run in a pod on a node and ServiceAccountTokenPodNodeInfo must be enabled
+  - expression: variables.userNodeName == variables.objectNodeName
     messageExpression: >-
-      "The DRA kubelet plugin on node " + request.userInfo.extra["authentication.kubernetes.io/node-name"][0] +
-      " may only modify resourceslices that belong to the node the pod is running on, not " +
-      (variables.objectNodeName == "" ? variables.objectNodeName : "a cluster-scoped slice") + "."
+      "this user running on node '"+variables.userNodeName+"' may not modify " +
+      (variables.objectNodeName == "" ?"cluster resourceslices" : "resourceslices on node '"+variables.objectNodeName+"'")
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingAdmissionPolicyBinding