[aarushitandon0]

What this PR does / why we need it

Problem

manifests/base/rbac/role.yaml is generated by controller-gen from the +kubebuilder:rbac markers. It deliberately splits the trainer.kubeflow.org resources into two rules:

• trainjobs receives create, because the OptimizationJob controller creates TrainJobs.
• clustertrainingruntimes, trainingruntimes, and optimizationjobs receive only get, list, patch, update, and watch.

The Helm chart's hand-maintained ClusterRole previously combined all four resources into a single rule that included create. Consequently, a Helm-installed controller manager could also create ClusterTrainingRuntimes, TrainingRuntimes, and OptimizationJobs cluster-wide, despite not requiring those permissions.

This caused an RBAC inconsistency between the Helm and Kustomize installation paths and violated the principle of least privilege.

Installing bundled runtimes does not depend on these permissions. The runtime installer and cleanup Jobs use their own ServiceAccount and ClusterRole (charts/kubeflow-trainer/templates/runtimes/rbac.yaml), while the controller-manager ClusterRole is bound only to the manager ServiceAccount.

Changes

This PR:

1. Corrects the Helm ClusterRole by splitting the combined trainer.kubeflow.org rule into two rules matching the generated RBAC manifest:

   • clustertrainingruntimes, trainingruntimes, and optimizationjobs: get, list, patch, update, watch.
   • trainjobs: create, get, list, patch, update, watch.

2. Updates the Helm unit tests to assert both corrected rules.

3. Adds a notContains assertion against the previous over-permissive combined rule, preventing the same regression from silently passing CI.

The result is consistent controller-manager permissions across Helm and Kustomize installations.

No functional behavior changes: this PR only removes unnecessary create permissions that the controller manager does not use.

Verification

• Helm unit tests: make helm-unittest — all 22 suites and 98 tests pass.
• Regression verification: Reverting only the ClusterRole template and rerunning the tests causes all three new assertions to fail, confirming that the tests detect the original regression.
• RBAC parity: Rendering the chart with helm template and comparing its permissions against manifests/base/rbac/role.yaml confirms:
  • 26 resource rules on each side.
  • No missing rules in either direction.
  • No verb mismatches (previously three).

Expected Outcome

• Helm and Kustomize installations grant identical controller-manager permissions.
• Only trainjobs receives the create permission among the affected resources.
• Existing runtime installation and controller functionality remain unchanged.
• Helm unit tests guard against reintroducing the over-permissive rule.

---

Which issue(s) this PR fixes

Fixes #4270

---

Checklist

• [Docs](https://trainer.kubeflow.org/en/latest/) included if any changes are user facing.

Documentation: Not applicable. This change only narrows unused RBAC permissions in the Helm chart to match the generated manifests. There are no user-facing API, CLI, or behavior changes requiring documentation.

[aarushitandon0]

/area deployment

[harshrajdebug]

I checked this against current master (4669ad8).

• No +kubebuilder:rbac marker grants create on clustertrainingruntimes, trainingruntimes or optimizationjobs, and the manager code never creates them. The runtime controllers only merge-patch existing runtimes, the OptimizationJob controller patches its own status and server-side applies its Deployment and Service, and the runtime snapshot is a ConfigMap. The bundled runtimes are installed by the installer and cleanup Jobs under their own ServiceAccount and ClusterRole, which keep their create.
• With helm v3.18.6 and helm-unittest 1.1.1, the versions pinned in the Makefile, the chart tests pass (22 suites, 98 tests). With only the old clusterrole.yaml template, exactly the three new assertions fail.
• Comparing the chart's rules with manifests/base/rbac/role.yaml by API group and resource gives 26 entries on each side, with 3 verb mismatches before this change and none after. The rules block has no template directives, so it renders as written.
• Merged into current master, the two still match. #4202, which adds optimizationjobs/finalizers to the same file, merges cleanly with this PR in either order, with 27 entries and no mismatches. #4254 and #4269, which also edit this ClusterRole, merge with it as well and leave no mismatches.

The new tests pin the two trainer.kubeflow.org rules. Apart from those and the tokenreviews and subjectaccessreviews rules, the test file asserts no rule, so drift in the others, such as the finalizers and status rule or the jobsets rule, would still pass. The check proposed in #4270, comparing the rendered ClusterRole with role.yaml in CI, would cover the whole file and could be a follow-up.

[andreyvelich]

I think, this is valid fix since controller doesn't need create RBAC for OptimizationJob or Runtimes.
/ok-to-test
/cc @aniket2405 @akshaychitneni @robert-bell
/lgtm

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment