fix(validator): harden Trainer controller toleration handling

validators/performance/consts.go

@@ -17,6 +17,7 @@ package main
 // Cross-file string constants for the performance validator.
 const (
 	apiGroupAPIExtensions    = "apiextensions.k8s.io"
+	apiGroupApps             = "apps"
 	resourceCRDs             = "customresourcedefinitions"
 	versionV1alpha1          = "v1alpha1"
 	versionV1beta1           = "v1beta1"

validators/performance/trainer_lifecycle.go

@@ -209,7 +209,7 @@ func cloneControllerTolerateAll() []any {
 // declares tolerations. Scoped to those two names so an unrelated Deployment
 // in the manifest set never gets a blanket toleration it didn't ask for.
 func applyControllerTolerations(obj *unstructured.Unstructured) error {
-	if gvk := obj.GroupVersionKind(); gvk.Kind != "Deployment" || gvk.Group != "apps" {
+	if gvk := obj.GroupVersionKind(); gvk.Kind != "Deployment" || gvk.Group != apiGroupApps {
 		return nil
 	}
 	switch obj.GetName() {
@@ -242,7 +242,7 @@ var (
 		Group: apiGroupAPIExtensions, Version: "v1", Resource: resourceCRDs,
 	}
 	trainerDeploymentGVR = schema.GroupVersionResource{
-		Group: "apps", Version: "v1", Resource: "deployments",
+		Group: apiGroupApps, Version: "v1", Resource: "deployments",
 	}
 	trainerServiceGVR = schema.GroupVersionResource{
 		Group: "", Version: "v1", Resource: "services",
@@ -888,7 +888,12 @@ func decodeTrainerObjects(resources []*resource.Resource) ([]*unstructured.Unstr
 		if tolErr := applyControllerTolerations(obj); tolErr != nil {
 			return nil, tolErr
 		}
-		if obj.GroupVersionKind().Kind == "Deployment" {
+		// Gated the same way as applyControllerTolerations above: a
+		// Deployment-kind resource in a non-apps group must not mark a
+		// controller name "seen" without actually receiving the toleration,
+		// or it would suppress the warning for the exact silent-miss below
+		// that the warning exists to catch.
+		if gvk := obj.GroupVersionKind(); gvk.Kind == "Deployment" && gvk.Group == apiGroupApps {
 			seenControllers[obj.GetName()] = true
 		}
 		objs = append(objs, obj)

validators/performance/trainer_lifecycle_test.go

@@ -105,8 +105,6 @@ func deploymentFixture(name string, existingTolerations []any) *unstructured.Uns
 	}}
 }
 
-// TestApplyControllerTolerations covers both controller names, the two
-// mutation-failure paths, and that an unrelated Deployment is left untouched.
 // TestApplyControllerTolerations_Isolation pins that the two controllers do
 // not share a live toleration slice: mutating one Deployment's stamped
 // tolerations in place must not affect the other, or the shared
@@ -122,12 +120,15 @@ func TestApplyControllerTolerations_Isolation(t *testing.T) {
 		t.Fatalf("applyControllerTolerations(jobset) error = %v", err)
 	}
 
-	trainerTols, _, _ := unstructured.NestedSlice(trainerObj.Object, "spec", "template", "spec", "tolerations")
-	trainerTol, _ := trainerTols[0].(map[string]any)
+	// NestedSlice deep-copies (runtime.DeepCopyJSONValue), so mutating its
+	// result can never observe aliasing back into the live object or the
+	// shared global. NestedFieldNoCopy returns the same map reference
+	// podSpec["tolerations"] actually holds, which is what this guard needs
+	// to exercise to be anything but a false green.
+	trainerTol := liveToleration(t, trainerObj)
 	trainerTol["key"] = "mutated-for-trainer-only"
 
-	jobSetTols, _, _ := unstructured.NestedSlice(jobSetObj.Object, "spec", "template", "spec", "tolerations")
-	jobSetTol, _ := jobSetTols[0].(map[string]any)
+	jobSetTol := liveToleration(t, jobSetObj)
 	if _, mutated := jobSetTol["key"]; mutated {
 		t.Errorf("mutating the Trainer Deployment's toleration leaked into the JobSet Deployment: %v", jobSetTol)
 	}
@@ -136,6 +137,28 @@ func TestApplyControllerTolerations_Isolation(t *testing.T) {
 	}
 }
 
+// liveToleration returns the live (non-deep-copied) first toleration map
+// stamped onto obj, so a caller can mutate it in place to test for aliasing.
+func liveToleration(t *testing.T, obj *unstructured.Unstructured) map[string]any {
+	t.Helper()
+
+	raw, found, err := unstructured.NestedFieldNoCopy(obj.Object, "spec", "template", "spec", "tolerations")
+	if err != nil || !found {
+		t.Fatalf("tolerations not found on %q: found=%v err=%v", obj.GetName(), found, err)
+	}
+	tols, ok := raw.([]any)
+	if !ok || len(tols) == 0 {
+		t.Fatalf("tolerations on %q have unexpected shape: %v", obj.GetName(), raw)
+	}
+	tol, ok := tols[0].(map[string]any)
+	if !ok {
+		t.Fatalf("toleration[0] on %q has unexpected shape: %v", obj.GetName(), tols[0])
+	}
+	return tol
+}
+
+// TestApplyControllerTolerations covers both controller names, the two
+// mutation-failure paths, and that an unrelated Deployment is left untouched.
 func TestApplyControllerTolerations(t *testing.T) {
 	tests := []struct {
 		name    string