Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions charts/kagenti-operator/templates/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@ rules:
- get
- list
- watch
- apiGroups:
- mlflow.opendatahub.io
resources:
- mlflowexperiments
verbs:
- get
- list
- watch
- update
- apiGroups:
- networking.k8s.io
resources:
Expand All @@ -164,6 +173,7 @@ rules:
- apiGroups:
- rbac.authorization.k8s.io
resources:
- roles
- rolebindings
verbs:
- create
Expand Down
10 changes: 10 additions & 0 deletions kagenti-operator/config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ rules:
- get
- list
- watch
- apiGroups:
- mlflow.opendatahub.io
resources:
- mlflowexperiments
verbs:
- get
- list
- watch
- update
- apiGroups:
- networking.k8s.io
resources:
Expand All @@ -154,6 +163,7 @@ rules:
- rbac.authorization.k8s.io
resources:
- rolebindings
- roles
verbs:
- create
- get
Expand Down
14 changes: 9 additions & 5 deletions kagenti-operator/docs/mlflow-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,19 @@ The following annotations are set on the Deployment's PodTemplateSpec:
| `mlflow.kagenti.io/tracking-uri` | MLflow tracking URI |
| `mlflow.kagenti.io/tracking-auth` | `kubernetes-namespaced` |

## Authentication
## Authentication & RBAC

The MLflow controller uses Kubernetes namespace-scoped authentication:
The MLflow controller uses Kubernetes namespace-scoped authentication with least-privilege agent access:

1. The controller's own ServiceAccount token is used to call the MLflow REST API to create experiments. The `X-MLFLOW-WORKSPACE` header is set to the agent's namespace, scoping the experiment to that workspace.
1. **Operator access**: The controller-manager's ServiceAccount is bound (via a ClusterRoleBinding shipped in the Helm chart / kustomize) to the broad `mlflow-operator-mlflow-integration` ClusterRole. This lets the operator call the MLflow REST API to create experiments. The `X-MLFLOW-WORKSPACE` header is set to the agent's namespace, scoping the experiment to that workspace.

2. For agent-side access, the controller creates a **RoleBinding** named `kagenti-mlflow-<deployment-name>` in the agent's namespace. This binds the agent's ServiceAccount to the `mlflow-operator-mlflow-integration` ClusterRole (created by the RHOAI MLflow operator). The agent authenticates to MLflow using its projected SA token at `/var/run/secrets/kubernetes.io/serviceaccount/token`.
2. **Agent access (scoped)**: For each agent Deployment the controller creates:
- A **Role** named `kagenti-mlflow-<deployment-name>` with `get` and `update` on the `mlflowexperiments` resource, scoped to the agent's own experiment via `resourceNames`. This means agents **cannot** create/delete experiments or access `registeredmodels` or `gatewayendpoints`.
- A **RoleBinding** with the same name that binds the agent's ServiceAccount to the scoped Role.

3. The RoleBinding is owned by the Deployment — deleting the Deployment garbage-collects the RoleBinding automatically.
The agent authenticates to MLflow using its projected SA token at `/var/run/secrets/kubernetes.io/serviceaccount/token`.

3. Both the Role and RoleBinding are owned by the Deployment — deleting the Deployment garbage-collects them automatically.

## Verification

Expand Down
86 changes: 58 additions & 28 deletions kagenti-operator/internal/controller/mlflow_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.

const (
// DefaultMLflowClusterRole is the ClusterRole managed by the MLflow operator
// for agent access to MLflow resources (RHOAI 3.4+).
DefaultMLflowClusterRole = "mlflow-operator-mlflow-integration"
// MLflowExperimentsAPIGroup is the API group used by the MLflow Kubernetes
// authorization plugin for SubjectAccessReview checks.
MLflowExperimentsAPIGroup = "mlflow.opendatahub.io"

// MLflowExperimentsResource is the virtual resource the MLflow gateway checks
// when authorizing experiment-level operations.
MLflowExperimentsResource = "mlflowexperiments"

// MLflow annotation keys stored on the PodTemplateSpec.
AnnotationMLflowExperimentID = "mlflow.kagenti.io/experiment-id"
Expand All @@ -53,15 +57,16 @@ const (

// MLflowReconciler reconciles Deployments labelled kagenti.io/type=agent.
// It auto-discovers MLflow availability via the mlflows.mlflow.opendatahub.io CRD.
//
// For each agent Deployment the reconciler creates a scoped Role granting only
// get+update on the agent's specific experiment (via resourceNames), and a
// RoleBinding for the agent SA. This ensures agents cannot access other
// experiments, registered models, or gateway endpoints.
type MLflowReconciler struct {
client.Client
Scheme *runtime.Scheme
Recorder record.EventRecorder

// MLflowClusterRole is the ClusterRole to bind agent SAs to.
// Defaults to DefaultMLflowClusterRole if empty.
MLflowClusterRole string

// NewMLflowClient creates an MLflow client for the given base URL.
// If nil, a default client is used.
NewMLflowClient func(baseURL string) *mlflow.Client
Expand All @@ -72,6 +77,8 @@ type MLflowReconciler struct {
}

// +kubebuilder:rbac:groups=mlflow.opendatahub.io,resources=mlflows,verbs=get;list;watch
// +kubebuilder:rbac:groups=mlflow.opendatahub.io,resources=mlflowexperiments,verbs=get;list;watch;update
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=create;get;list;watch;update
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=create;get;list;watch;update
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;update;patch

Expand Down Expand Up @@ -129,8 +136,8 @@ func (r *MLflowReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
logger.Info("deployment has no explicit serviceAccountName, falling back to 'default'", "deployment", dep.Name)
}

if err := r.ensureRoleBinding(ctx, dep, saName); err != nil {
logger.Error(err, "Failed to ensure MLflow RoleBinding")
if err := r.ensureScopedRBAC(ctx, dep, saName, experimentName); err != nil {
logger.Error(err, "Failed to ensure scoped MLflow RBAC")
return ctrl.Result{}, err
}

Expand All @@ -148,13 +155,6 @@ func (r *MLflowReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, nil
}

func (r *MLflowReconciler) clusterRoleName() string {
if r.MLflowClusterRole != "" {
return r.MLflowClusterRole
}
return DefaultMLflowClusterRole
}

func (r *MLflowReconciler) mlflowClient(baseURL string) *mlflow.Client {
if r.NewMLflowClient != nil {
return r.NewMLflowClient(baseURL)
Expand Down Expand Up @@ -246,29 +246,55 @@ func (r *MLflowReconciler) configureDeployment(ctx context.Context, dep *appsv1.
})
}

// ensureRoleBinding creates or updates the RoleBinding for the agent SA.
func (r *MLflowReconciler) ensureRoleBinding(ctx context.Context, dep *appsv1.Deployment, saName string) error {
rbName := fmt.Sprintf("kagenti-mlflow-%s", dep.Name)
rb := &rbacv1.RoleBinding{
// ensureScopedRBAC creates a Role scoped to a single experiment (by resourceName)
// and a RoleBinding that grants the agent SA only get+update on that experiment.
// Both resources are owned by the Deployment so they are garbage-collected on deletion.
func (r *MLflowReconciler) ensureScopedRBAC(ctx context.Context, dep *appsv1.Deployment, saName, experimentName string) error {
roleName := fmt.Sprintf("kagenti-mlflow-%s", dep.Name)

role := &rbacv1.Role{
ObjectMeta: metav1.ObjectMeta{
Name: rbName,
Name: roleName,
Namespace: dep.Namespace,
},
}
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, role, func() error {
role.Labels = map[string]string{
LabelManagedBy: LabelManagedByValue,
}
if err := controllerutil.SetOwnerReference(dep, role, r.Scheme); err != nil {
return err
}
role.Rules = []rbacv1.PolicyRule{
{
APIGroups: []string{MLflowExperimentsAPIGroup},
Resources: []string{MLflowExperimentsResource},
ResourceNames: []string{experimentName},
Verbs: []string{"get", "update"},
},
}
return nil
}); err != nil {
return fmt.Errorf("ensuring scoped Role: %w", err)
}

_, err := controllerutil.CreateOrUpdate(ctx, r.Client, rb, func() error {
rb := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: roleName,
Namespace: dep.Namespace,
},
}
if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, rb, func() error {
rb.Labels = map[string]string{
LabelManagedBy: LabelManagedByValue,
}

if err := controllerutil.SetOwnerReference(dep, rb, r.Scheme); err != nil {
return err
}

rb.RoleRef = rbacv1.RoleRef{
APIGroup: rbacv1.GroupName,
Kind: "ClusterRole",
Name: r.clusterRoleName(),
Kind: "Role",
Name: roleName,
}
rb.Subjects = []rbacv1.Subject{
{
Expand All @@ -278,8 +304,11 @@ func (r *MLflowReconciler) ensureRoleBinding(ctx context.Context, dep *appsv1.De
},
}
return nil
})
return err
}); err != nil {
return fmt.Errorf("ensuring scoped RoleBinding: %w", err)
}

return nil
}

// setEnvVar sets an env var on a container, returning true if a change was made.
Expand All @@ -302,6 +331,7 @@ func setEnvVar(container *corev1.Container, name, value string) bool {
func (r *MLflowReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&appsv1.Deployment{}, builder.WithPredicates(agentLabelPredicate())).
Owns(&rbacv1.Role{}).
Owns(&rbacv1.RoleBinding{}).
Named("mlflow").
Complete(r)
Expand Down
34 changes: 20 additions & 14 deletions kagenti-operator/internal/controller/mlflow_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,21 +228,32 @@ var _ = Describe("MLflow Controller", func() {
cleanup()
})

It("should create RoleBinding and inject env vars", func() {
It("should create scoped Role, RoleBinding, and inject env vars", func() {
dep := newAgentDeployment("mlflow-full", namespace)
Expect(k8sClient.Create(ctx, dep)).To(Succeed())
defer func() { _ = k8sClient.Delete(ctx, dep) }()

r := newReconcilerWithServer(server.URL, tokenPath)
reconcileAndExpectNoOp(r, "mlflow-full", namespace)

role := &rbacv1.Role{}
Expect(k8sClient.Get(ctx, types.NamespacedName{
Name: "kagenti-mlflow-mlflow-full", Namespace: namespace,
}, role)).To(Succeed())
Expect(role.Rules).To(HaveLen(1))
Expect(role.Rules[0].APIGroups).To(Equal([]string{MLflowExperimentsAPIGroup}))
Expect(role.Rules[0].Resources).To(Equal([]string{MLflowExperimentsResource}))
Expect(role.Rules[0].ResourceNames).To(Equal([]string{"mlflow-full"}))
Expect(role.Rules[0].Verbs).To(Equal([]string{"get", "update"}))

rb := &rbacv1.RoleBinding{}
Expect(k8sClient.Get(ctx, types.NamespacedName{
Name: "kagenti-mlflow-mlflow-full", Namespace: namespace,
}, rb)).To(Succeed())
Expect(rb.Subjects).To(HaveLen(1))
Expect(rb.Subjects[0].Name).To(Equal("test-sa"))
Expect(rb.RoleRef.Name).To(Equal(DefaultMLflowClusterRole))
Expect(rb.RoleRef.Kind).To(Equal("Role"))
Expect(rb.RoleRef.Name).To(Equal("kagenti-mlflow-mlflow-full"))

updated := &appsv1.Deployment{}
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "mlflow-full", Namespace: namespace}, updated)).To(Succeed())
Expand Down Expand Up @@ -270,11 +281,18 @@ var _ = Describe("MLflow Controller", func() {
r := newReconcilerWithServer(server.URL, tokenPath)
reconcileAndExpectNoOp(r, "mlflow-default-sa", namespace)

role := &rbacv1.Role{}
Expect(k8sClient.Get(ctx, types.NamespacedName{
Name: "kagenti-mlflow-mlflow-default-sa", Namespace: namespace,
}, role)).To(Succeed())
Expect(role.Rules[0].ResourceNames).To(Equal([]string{"mlflow-default-sa"}))

rb := &rbacv1.RoleBinding{}
Expect(k8sClient.Get(ctx, types.NamespacedName{
Name: "kagenti-mlflow-mlflow-default-sa", Namespace: namespace,
}, rb)).To(Succeed())
Expect(rb.Subjects[0].Name).To(Equal("default"))
Expect(rb.RoleRef.Kind).To(Equal("Role"))
})
})

Expand Down Expand Up @@ -327,18 +345,6 @@ var _ = Describe("MLflow Controller", func() {
})

var _ = Describe("MLflow Controller helpers", func() {
Describe("clusterRoleName", func() {
It("should return the default when MLflowClusterRole is empty", func() {
r := &MLflowReconciler{}
Expect(r.clusterRoleName()).To(Equal(DefaultMLflowClusterRole))
})

It("should return the custom value when set", func() {
r := &MLflowReconciler{MLflowClusterRole: "custom-role"}
Expect(r.clusterRoleName()).To(Equal("custom-role"))
})
})

Describe("setEnvVar", func() {
It("should add a new env var", func() {
container := &corev1.Container{Name: "test"}
Expand Down
Loading