Skip to content

refactor(evalhub): remove control-plane jobs SA (tenant-only) #657

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ruivieira merged 1 commit into from
Mar 3, 2026
+5 −394
Merged

refactor(evalhub): remove control-plane jobs SA (tenant-only) #657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 0 additions & 9 deletions controllers/evalhub/evalhub_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,6 @@ func (r *EvalHubReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
return RequeueWithError(err)
}

// Create ServiceAccount for jobs
err = r.createJobsServiceAccount(ctx, instance)
if err != nil {
log.Error(err, "Failed to create Jobs ServiceAccount")
instance.SetStatus("Ready", "Error", fmt.Sprintf("Failed to create Jobs ServiceAccount: %v", err), corev1.ConditionFalse)
r.Status().Update(ctx, instance)
return RequeueWithError(err)
}

// Reconcile tenant namespace RBAC (non-fatal)
if err := r.reconcileTenantNamespaces(ctx, instance); err != nil {
log.Error(err, "Failed to reconcile tenant namespaces")
Expand Down
27 changes: 0 additions & 27 deletions controllers/evalhub/proxy_rbac_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,33 +198,6 @@ var _ = Describe("EvalHub API RBAC", func() {
Expect(roleBinding.RoleRef.Name).To(Equal(generateAPIAccessRoleName(evalHub)))
})

It("should create namespace-scoped jobs API access RoleBinding referencing per-instance Role", func() {
By("Creating service account")
err := reconciler.createServiceAccount(ctx, evalHub)
Expect(err).NotTo(HaveOccurred())

By("Verifying jobs API access RoleBinding exists in namespace")
roleBinding := &rbacv1.RoleBinding{}
rbName := evalHubName + "-jobs-api-access-rb"
err = k8sClient.Get(ctx, types.NamespacedName{
Name: rbName,
Namespace: testNamespace,
}, roleBinding)
Expect(err).NotTo(HaveOccurred())

By("Checking RoleBinding is namespace-scoped")
Expect(roleBinding.Namespace).To(Equal(testNamespace))

By("Checking subjects use -jobs SA")
Expect(roleBinding.Subjects).To(HaveLen(1))
Expect(roleBinding.Subjects[0].Kind).To(Equal("ServiceAccount"))
Expect(roleBinding.Subjects[0].Name).To(Equal(evalHubName + "-jobs"))

By("Checking role reference points to per-instance Role (not ClusterRole)")
Expect(roleBinding.RoleRef.Kind).To(Equal("Role"))
Expect(roleBinding.RoleRef.Name).To(Equal(generateJobsAPIAccessRoleName(evalHub)))
})

It("should handle existing service account gracefully", func() {
By("Creating service account initially")
err := reconciler.createServiceAccount(ctx, evalHub)
Expand Down
132 changes: 2 additions & 130 deletions controllers/evalhub/service_accounts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ func generateAPIAccessRoleName(instance *evalhubv1alpha1.EvalHub) string {
return instance.Name + "-api-access-role"
}

// generateJobsAPIAccessRoleName returns the name for the per-instance jobs API access Role.
func generateJobsAPIAccessRoleName(instance *evalhubv1alpha1.EvalHub) string {
return instance.Name + "-jobs-api-access-role"
}

func generateAuthReviewerClusterRoleBindingName(instance *evalhubv1alpha1.EvalHub) string {
return instance.Name + "-" + instance.Namespace + "-auth-reviewer-crb"
}
Expand Down Expand Up @@ -172,11 +167,6 @@ func (r *EvalHubReconciler) createServiceAccount(ctx context.Context, instance *
return err
}

err = r.createJobsAPIAccessRole(ctx, instance)
if err != nil {
return err
}

// Create namespace-scoped RoleBinding for EvalHub API access (evalhubs/proxy)
err = r.createAPIAccessRoleBinding(ctx, instance, serviceAccountName)
if err != nil {
Expand All @@ -194,25 +184,14 @@ func (r *EvalHubReconciler) createServiceAccount(ctx context.Context, instance *
return err
}

// Create RoleBinding for jobs ServiceAccount to access evalhubs/proxy in this namespace
jobsServiceAccountName := generateJobsServiceAccountName(instance)
err = r.createJobsAPIAccessRoleBinding(ctx, instance, jobsServiceAccountName)
if err != nil {
return err
}

// Create MLFlow access RoleBindings for both ServiceAccounts.
// Create MLFlow access RoleBinding for the API ServiceAccount.
// MLFlow's kubernetes-auth plugin validates tokens via SubjectAccessReview against
// the workspace namespace. The custom "evalhub-mlflow-access" ClusterRole provides
// the required mlflow.kubeflow.org permissions for both the api and jobs SAs.
// the required mlflow.kubeflow.org permissions for the API SA.
err = r.createMLFlowAccessRoleBinding(ctx, instance, serviceAccountName, "api", mlflowAccessClusterRoleName)
if err != nil {
return err
}
err = r.createMLFlowAccessRoleBinding(ctx, instance, jobsServiceAccountName, "jobs", mlflowJobsAccessClusterRoleName)
if err != nil {
return err
}

return nil
}
Expand All @@ -235,10 +214,6 @@ const (
// config/rbac/evalhub_mlflow_jobs_role.yaml).
const mlflowAccessClusterRoleName = "trustyai-service-operator-evalhub-mlflow-access"

// mlflowJobsAccessClusterRoleName is a restricted MLflow ClusterRole for job pods.
// Jobs only need create, get, list -- not update or delete.
const mlflowJobsAccessClusterRoleName = "trustyai-service-operator-evalhub-mlflow-jobs-access"

// createAPIAccessRole creates a per-instance namespaced Role with resourceNames
// scoped to this specific EvalHub instance. This ensures the SA can only access
// its own instance's evalhubs/proxy subresource.
Expand Down Expand Up @@ -297,56 +272,6 @@ func (r *EvalHubReconciler) createAPIAccessRole(ctx context.Context, instance *e
return nil
}

// createJobsAPIAccessRole creates a per-instance namespaced Role for the jobs SA
// with resourceNames scoped to this specific EvalHub instance.
func (r *EvalHubReconciler) createJobsAPIAccessRole(ctx context.Context, instance *evalhubv1alpha1.EvalHub) error {
log := log.FromContext(ctx)

roleName := generateJobsAPIAccessRoleName(instance)
role := &rbacv1.Role{
ObjectMeta: metav1.ObjectMeta{
Name: roleName,
Namespace: instance.Namespace,
Labels: map[string]string{
"app": "eval-hub",
"app.kubernetes.io/name": roleName,
"app.kubernetes.io/instance": instance.Name,
"app.kubernetes.io/part-of": "eval-hub",
"app.kubernetes.io/version": constants.Version,
},
},
Rules: []rbacv1.PolicyRule{
{
APIGroups: []string{"trustyai.opendatahub.io"},
Resources: []string{"evalhubs/proxy"},
ResourceNames: []string{instance.Name},
Verbs: []string{"get", "create"},
},
},
}

if err := ctrl.SetControllerReference(instance, role, r.Scheme); err != nil {
return err
}

found := &rbacv1.Role{}
err := r.Get(ctx, types.NamespacedName{Name: role.Name, Namespace: role.Namespace}, found)
if err != nil && errors.IsNotFound(err) {
log.Info("Creating jobs API access Role", "Namespace", role.Namespace, "Name", role.Name)
return r.Create(ctx, role)
} else if err != nil {
return err
}

if !equalPolicyRules(found.Rules, role.Rules) {
found.Rules = role.Rules
log.Info("Updating jobs API access Role rules", "Name", role.Name)
return r.Update(ctx, found)
}

return nil
}

// createMLFlowAccessRoleBinding creates a RoleBinding for a ServiceAccount to
// the specified MLflow ClusterRole in the instance namespace. This allows the
// ServiceAccount to pass MLFlow's kubernetes-auth SubjectAccessReview checks
Expand Down Expand Up @@ -496,19 +421,6 @@ func (r *EvalHubReconciler) createAPIAccessRoleBinding(ctx context.Context, inst
})
}

// createJobsAPIAccessRoleBinding creates a namespace-scoped RoleBinding for the jobs
// ServiceAccount to the per-instance Role for evalhubs/proxy access.
func (r *EvalHubReconciler) createJobsAPIAccessRoleBinding(ctx context.Context, instance *evalhubv1alpha1.EvalHub, serviceAccountName string) error {
roleBindingName := instance.Name + "-jobs-api-access-rb"
roleName := generateJobsAPIAccessRoleName(instance)

return r.createGenericRoleBinding(ctx, instance, roleBindingName, serviceAccountName, rbacv1.RoleRef{
Kind: "Role",
Name: roleName,
APIGroup: rbacv1.GroupName,
})
}

// createJobsWriterRoleBinding creates a RoleBinding for the API SA to the
// jobs-writer ClusterRole (batch/jobs create,delete).
func (r *EvalHubReconciler) createJobsWriterRoleBinding(ctx context.Context, instance *evalhubv1alpha1.EvalHub, serviceAccountName string) error {
Expand Down Expand Up @@ -715,46 +627,6 @@ func generateJobsServiceAccountName(instance *evalhubv1alpha1.EvalHub) string {
return instance.Name + "-jobs"
}

// createJobsServiceAccount creates a service account for jobs created by this EvalHub instance
func (r *EvalHubReconciler) createJobsServiceAccount(ctx context.Context, instance *evalhubv1alpha1.EvalHub) error {
serviceAccountName := generateJobsServiceAccountName(instance)

sa := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: serviceAccountName,
Namespace: instance.Namespace,
Labels: map[string]string{
"app": "eval-hub",
"app.kubernetes.io/name": normalizeDNS1123LabelValue(serviceAccountName),
"app.kubernetes.io/instance": instance.Name,
"app.kubernetes.io/part-of": "eval-hub",
"app.kubernetes.io/component": "jobs",
"app.kubernetes.io/version": constants.Version,
},
},
}

// Set instance as the owner and controller
if err := ctrl.SetControllerReference(instance, sa, r.Scheme); err != nil {
return err
}

// Check if this ServiceAccount already exists
found := &corev1.ServiceAccount{}
err := r.Get(ctx, types.NamespacedName{Name: sa.Name, Namespace: sa.Namespace}, found)
if err != nil && errors.IsNotFound(err) {
log.FromContext(ctx).Info("Creating a new Jobs ServiceAccount", "Namespace", sa.Namespace, "Name", sa.Name)
err = r.Create(ctx, sa)
if err != nil {
return err
}
} else if err != nil {
return err
}

return nil
}

// reconcileTenantNamespaces discovers namespaces with the tenant annotation and
// provisions per-tenant RBAC (SA + RoleBindings) so the API SA can create jobs
// in tenant namespaces. It also cleans up resources in namespaces that lost the
Expand Down
Loading