Skip to content

feat(controller): scope MLflow RBAC to per-agent namespaced Role#366

Merged
pdettori merged 1 commit into
rossoctl:mainfrom
rh-dnagornuks:scoped-mlflow-rbac
May 21, 2026
Merged

feat(controller): scope MLflow RBAC to per-agent namespaced Role#366
pdettori merged 1 commit into
rossoctl:mainfrom
rh-dnagornuks:scoped-mlflow-rbac

Conversation

@rh-dnagornuks

@rh-dnagornuks rh-dnagornuks commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary

The MLflow controller previously bound each agent ServiceAccount to the broad mlflow-operator-mlflow-integration ClusterRole, granting access to all MLflow experiments, registered models, and gateway endpoints cluster-wide. This PR replaces that with a least-privilege namespaced Role per agent Deployment, scoped via resourceNames to only the agent's own experiment, and a corresponding RoleBinding — both owned by the Deployment so they are garbage-collected on deletion.

Changes

  • mlflow_controller.go — replace ensureRoleBinding (ClusterRole binding) with ensureScopedRBAC, which creates a namespaced Role granting only get+update on the specific mlflowexperiments resource name, plus a RoleBinding to the agent SA; remove the now-unused MLflowClusterRole field and clusterRoleName() helper
  • mlflow_controller.go — add Owns(&rbacv1.Role{}) to the controller watch so Role deletions trigger reconciliation
  • config/rbac/role.yaml and charts/.../rbac/role.yaml — add mlflowexperiments (get/list/watch/update) and roles (create/get/list/watch/update) RBAC rules, regenerated from kubebuilder markers
  • docs/mlflow-integration.md — update Authentication section to document the new two-resource scoped RBAC pattern
  • mlflow_controller_test.go — update tests to assert scoped Role creation with correct resourceNames, update RoleBinding assertions to check Kind: Role, remove obsolete clusterRoleName helper tests

Testing

  • Unit tests updated and passing (make test)
  • Linter passing (make lint)
  • Manually verified Role/RoleBinding created with correct resourceNames scoping on an OpenShift cluster

Related Issues

Resolves: RHAIENG-4446

Replace the broad ClusterRole binding approach with a least-privilege
scoped Role per agent Deployment. Instead of binding each agent SA to
the shared mlflow-operator-mlflow-integration ClusterRole, the
reconciler now creates:

- A namespaced Role granting only get+update on the agent's specific
  MLflow experiment (locked via resourceNames), preventing access to
  other experiments, registeredmodels, or gatewayendpoints.
- A RoleBinding that binds the agent SA to that scoped Role.

Both resources are owned by the Deployment and are garbage-collected
on deletion. The operator ClusterRoleBinding for the controller-manager
SA is unchanged.

RBAC markers and generated role.yaml files are updated accordingly.

Signed-off-by: Daniels Nagornuks <dnagornu@redhat.com>

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
@rh-dnagornuks
rh-dnagornuks marked this pull request as ready for review May 20, 2026 13:11
@rh-dnagornuks
rh-dnagornuks requested a review from a team as a code owner May 20, 2026 13:11

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well-structured least-privilege RBAC refactor. Replaces a broad ClusterRoleBinding (all agents access all MLflow resources cluster-wide) with namespace-scoped Roles locked to each agent's specific experiment via resourceNames. Both Role and RoleBinding are owned by the Deployment for automatic GC, and the controller watches Role deletions for re-reconciliation.

Areas reviewed: Go controller logic, Helm/K8s YAML (RBAC parity kustomize ↔ chart), tests, docs, security
Commits: 1 commit, signed-off ✓, proper Assisted-By trailer
CI: All 16 checks passing

@@ -40,9 +40,13 @@ import (
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The kubebuilder marker adds get;list;watch;update for mlflowexperiments on the operator's ClusterRole. Since the controller only verifies experiment existence for resourceNames scoping, list and watch may not be strictly necessary (just get might suffice). Not blocking — broader permissions are fine for controller reliability and future use.

@pdettori
pdettori merged commit 8cdcc59 into rossoctl:main May 21, 2026
16 checks passed
@rh-dnagornuks
rh-dnagornuks deleted the scoped-mlflow-rbac branch May 25, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants