Skip to content

feat(evalhub):add secrets reader RBAC for API SA #652

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

Closed
scheruku-rh wants to merge 1 commit into from
Closed

feat(evalhub):add secrets reader RBAC for API SA #652

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
13 changes: 13 additions & 0 deletions config/rbac/evalhub/evalhub_secrets_reader_role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
# ClusterRole for EvalHub API secret existence checks.
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: evalhub-secrets-reader
labels:
app.kubernetes.io/name: trustyai-service-operator
app.kubernetes.io/component: evalhub
rules:
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get"]
Comment on lines +1 to +13
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the naming convention used by evalhub ClusterRole YAMLs
echo "=== ClusterRole names in evalhub YAML files ==="
fd 'evalhub.*role.yaml' config/rbac/evalhub/ --exec grep -H 'name:' {} \;

echo ""
echo "=== Go constants referencing ClusterRole names ==="
rg -n 'ClusterRoleName\s*=' config/controllers/evalhub/ --type=go -A 1

echo ""
echo "=== Full evalhub_secrets_reader_role.yaml content ==="
cat -n config/rbac/evalhub/evalhub_secrets_reader_role.yaml

echo ""
echo "=== Check service_accounts.go for secretsReaderClusterRoleName ==="
rg -n 'secretsReaderClusterRoleName' config/controllers/evalhub/ --type=go -A 2 -B 2

Repository: trustyai-explainability/trustyai-service-operator

Length of output: 2004

🏁 Script executed:

#!/bin/bash
# Find Go files related to evalhub and service accounts
echo "=== Finding evalhub-related Go files ==="
find . -type f -name '*.go' | grep -i evalhub | head -20

echo ""
echo "=== Searching for secretsReaderClusterRoleName in codebase ==="
rg -r 'secretsReaderClusterRoleName' --type=go

echo ""
echo "=== Searching for ClusterRole references in Go files ==="
rg -n 'evalhub-secrets-reader\|secretsReader' --type=go -A 2 -B 2

echo ""
echo "=== List all controller directories ==="
find . -type d -name 'controllers' -o -name 'controller'

echo ""
echo "=== Check kustomization files for the evalhub YAML ==="
find config -name 'kustomization.yaml' -o -name 'kustomization.yml' | xargs grep -l 'evalhub_secrets' 2>/dev/null || echo "No kustomization found with evalhub_secrets"

Repository: trustyai-explainability/trustyai-service-operator

Length of output: 1601

🏁 Script executed:

#!/bin/bash
# Read the service_accounts.go file to see actual constants
echo "=== service_accounts.go content ==="
cat -n ./controllers/evalhub/service_accounts.go

echo ""
echo "=== Check kustomization.yaml for name prefixes/transformations ==="
cat -n config/rbac/kustomization.yaml

echo ""
echo "=== Search for RoleBinding definitions related to secrets ==="
rg -n 'RoleBinding\|secrets.*Reader\|ClusterRole' ./controllers/evalhub/service_accounts.go -B 2 -A 2

Repository: trustyai-explainability/trustyai-service-operator

Length of output: 33488

Critical: ClusterRole name does not match the Go constant — RoleBinding will reference a non-existent ClusterRole.

The YAML defines the ClusterRole as evalhub-secrets-reader (line 6), but the Go controller constant secretsReaderClusterRoleName is set to "trustyai-service-operator-evalhub-secrets-reader" (in controllers/evalhub/service_accounts.go, line 236). The dynamically created RoleBinding will reference a ClusterRole that doesn't exist under that name, so the API SA will silently fail to get secrets read permissions.

All evalhub ClusterRoles in this directory follow the same pattern of missing the trustyai-service-operator- prefix. Update the name to match:

🐛 Proposed fix 
 metadata:
-  name: evalhub-secrets-reader
+  name: trustyai-service-operator-evalhub-secrets-reader
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
---
# ClusterRole for EvalHub API secret existence checks.
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: evalhub-secrets-reader
labels:
app.kubernetes.io/name: trustyai-service-operator
app.kubernetes.io/component: evalhub
rules:
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get"]
---
# ClusterRole for EvalHub API secret existence checks.
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: trustyai-service-operator-evalhub-secrets-reader
labels:
app.kubernetes.io/name: trustyai-service-operator
app.kubernetes.io/component: evalhub
rules:
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get"]
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@config/rbac/evalhub/evalhub_secrets_reader_role.yaml` around lines 1 - 13,
The ClusterRole metadata.name "evalhub-secrets-reader" does not match the Go
constant secretsReaderClusterRoleName (expected
"trustyai-service-operator-evalhub-secrets-reader"), causing RoleBinding to
reference a non-existent ClusterRole; update the ClusterRole metadata.name to
the exact value of secretsReaderClusterRoleName and apply the same prefix fix to
the other evalhub ClusterRole definitions in this directory so RoleBindings
created by controllers/evalhub/service_accounts.go resolve correctly.

Copy link
Collaborator Author

@scheruku-rh scheruku-rh Feb 25, 2026

Choose a reason for hiding this comment

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

RBAC is deployed through config/base, which sets namePrefix: trustyai-service-operator-, so the resource is created as trustyai-service-operator-evalhub-secrets-reader, which matches the Go constant. So I think no change is needed here?

Copy link
Contributor

@coderabbitai coderabbitai bot Feb 25, 2026

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if config/base/kustomization.yaml sets namePrefix
echo "=== config/base/kustomization.yaml content ==="
cat config/base/kustomization.yaml

echo ""
echo "=== Check if config/rbac is referenced in the kustomization chain ==="
rg -n "namePrefix.*trustyai-service-operator" config/ --type=yaml

echo ""
echo "=== Verify other evalhub ClusterRole names follow same pattern ==="
fd 'evalhub.*role.yaml' config/rbac/evalhub/ --exec sh -c 'echo "=== {} ===" && grep -A 1 "kind: ClusterRole" {} | grep "name:"'

Length of output: 1700

🏁 Script executed:

#!/bin/bash
# Get actual ClusterRole names from evalhub RBAC files
echo "=== ClusterRole names in evalhub directory ==="
for file in config/rbac/evalhub/evalhub_*_role.yaml; do
  echo "File: $(basename $file)"
  grep "name:" "$file" | head -1
  echo ""
done

echo "=== Verify kustomize applies prefix by checking generated manifests ==="
echo "Looking for how these ClusterRoles are referenced in Go code..."
rg -n "evalhub-secrets-reader|evalhub-auth-reviewer|evalhub-jobs-writer" controllers/evalhub/ --type=go -B 1 -A 1

Length of output: 1512

@scheruku-rh You're absolutely right! I apologize for the incorrect analysis.

The configuration is correct as-is. Kustomize applies the namePrefix: trustyai-service-operator- transformation (defined in config/base/kustomization.yaml) to all resources under ../rbac, which includes this ClusterRole. So:

  • YAML defines: evalhub-secrets-reader
  • Kustomize transforms to: trustyai-service-operator-evalhub-secrets-reader
  • Go constant expects: trustyai-service-operator-evalhub-secrets-reader ✅

This pattern is consistent across all evalhub ClusterRoles (jobs-writer, auth-reviewer, job-config, etc.). Thank you for the clarification!

✏️ Learnings added 
Learnt from: scheruku-rh
Repo: trustyai-explainability/trustyai-service-operator PR: 652
File: config/rbac/evalhub/evalhub_secrets_reader_role.yaml:1-13
Timestamp: 2026-02-25T12:57:09.782Z
Learning: In the trustyai-service-operator repository, RBAC resources in config/rbac/ are deployed through config/base/kustomization.yaml which applies namePrefix: trustyai-service-operator-. ClusterRole names in YAML manifests should use short names (e.g., evalhub-secrets-reader) as kustomize automatically prefixes them at deployment time to match Go constants (e.g., trustyai-service-operator-evalhub-secrets-reader).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: tarilabs
Repo: trustyai-explainability/trustyai-service-operator PR: 650
File: config/configmaps/evalhub/provider-garak.yaml:24-26
Timestamp: 2026-02-18T18:07:47.424Z
Learning: In the trustyai-service-operator EvalHub provider ConfigMaps (config/configmaps/evalhub/provider-*.yaml), the provider configurations are sourced from the upstream eval-hub repository via hack/sync-evalhub-providers.py script. The project policy is to preserve the upstream provider configs as-is, including any placeholder environment variables or template content, to maintain consistency with the upstream source.

1 change: 1 addition & 0 deletions config/rbac/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ resources:
- evalhub/evalhub_jobs_writer_binding.yaml
- evalhub/evalhub_job_config_role.yaml
- evalhub/evalhub_job_config_binding.yaml
- evalhub/evalhub_secrets_reader_role.yaml
- evalhub/evalhub_mlflow_access_role.yaml
- evalhub/evalhub_mlflow_access_binding.yaml
- evalhub/evalhub_mlflow_jobs_role.yaml
Expand Down
19 changes: 19 additions & 0 deletions controllers/evalhub/service_accounts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ func (r *EvalHubReconciler) createServiceAccount(ctx context.Context, instance *
return err
}

// Create RoleBinding for API SA secrets-reader
err = r.createSecretsReaderRoleBinding(ctx, instance, serviceAccountName)
if err != nil {
return err
}

// Create RoleBindings for split resource-manager roles (API SA only)
err = r.createJobsWriterRoleBinding(ctx, instance, serviceAccountName)
if err != nil {
Expand Down Expand Up @@ -226,6 +232,9 @@ const (
jobConfigClusterRoleName = "trustyai-service-operator-evalhub-job-config"
)

// secretsReaderClusterRoleName allows the API SA to validate referenced secrets.
const secretsReaderClusterRoleName = "trustyai-service-operator-evalhub-secrets-reader"

// MLFlow access uses custom ClusterRoles scoped to the "mlflow.kubeflow.org" API group.
// MLFlow's kubernetes-workspace-provider checks permissions via SelfSubjectAccessReview
// against this group (not core Kubernetes resources). The ClusterRoles are pre-created
Expand Down Expand Up @@ -527,6 +536,16 @@ func (r *EvalHubReconciler) createJobConfigRoleBinding(ctx context.Context, inst
})
}

// createSecretsReaderRoleBinding creates a RoleBinding for the API SA to the
// secrets-reader ClusterRole (secrets get).
func (r *EvalHubReconciler) createSecretsReaderRoleBinding(ctx context.Context, instance *evalhubv1alpha1.EvalHub, serviceAccountName string) error {
return r.createGenericRoleBinding(ctx, instance, instance.Name+"-secrets-reader-rb", serviceAccountName, rbacv1.RoleRef{
Kind: "ClusterRole",
Name: secretsReaderClusterRoleName,
APIGroup: rbacv1.GroupName,
})
}

// createGenericRoleBinding creates a namespace-scoped RoleBinding with the given
// name, subject SA, and role reference. Handles create-or-update logic including
// immutable RoleRef recreation.
Expand Down
54 changes: 54 additions & 0 deletions controllers/evalhub/unit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,60 @@ func TestEvalHubReconciler_createAPIAccessRoleBinding_RefersToRole(t *testing.T)
})
}

// TestEvalHubReconciler_createSecretsReaderRoleBinding verifies that the API SA
// is bound to the secrets-reader ClusterRole.
func TestEvalHubReconciler_createSecretsReaderRoleBinding(t *testing.T) {
scheme := runtime.NewScheme()
require.NoError(t, rbacv1.AddToScheme(scheme))
require.NoError(t, evalhubv1alpha1.AddToScheme(scheme))

ctx := context.Background()
testNamespace := "test-namespace"
evalHubName := "test-evalhub"

evalHub := &evalhubv1alpha1.EvalHub{
ObjectMeta: metav1.ObjectMeta{
Name: evalHubName,
Namespace: testNamespace,
UID: "test-uid-123",
},
}

fakeClient := fake.NewClientBuilder().
WithScheme(scheme).
WithObjects(evalHub).
Build()

reconciler := &EvalHubReconciler{
Client: fakeClient,
Scheme: scheme,
EventRecorder: record.NewFakeRecorder(10),
}

t.Run("should create RoleBinding to secrets-reader ClusterRole", func(t *testing.T) {
apiSAName := evalHubName + "-api"
err := reconciler.createSecretsReaderRoleBinding(ctx, evalHub, apiSAName)
require.NoError(t, err)

rbName := evalHubName + "-secrets-reader-rb"
rb := &rbacv1.RoleBinding{}
err = fakeClient.Get(ctx, types.NamespacedName{
Name: rbName,
Namespace: testNamespace,
}, rb)
require.NoError(t, err)

assert.Equal(t, "ClusterRole", rb.RoleRef.Kind)
assert.Equal(t, secretsReaderClusterRoleName, rb.RoleRef.Name)
assert.Equal(t, rbacv1.GroupName, rb.RoleRef.APIGroup)

require.Len(t, rb.Subjects, 1)
assert.Equal(t, "ServiceAccount", rb.Subjects[0].Kind)
assert.Equal(t, apiSAName, rb.Subjects[0].Name)
assert.Equal(t, testNamespace, rb.Subjects[0].Namespace)
})
}

// TestEvalHubReconciler_createMLFlowAccessRoleBinding_JobsRole verifies that the jobs
// MLflow RoleBinding uses the restricted jobs-specific ClusterRole.
func TestEvalHubReconciler_createMLFlowAccessRoleBinding_JobsRole(t *testing.T) {
Expand Down
Loading