feat(evalhub):add secrets reader RBAC for API SA#652
feat(evalhub):add secrets reader RBAC for API SA#652scheruku-rh wants to merge 1 commit intotrustyai-explainability:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughAdds RBAC configuration enabling the EvalHub API service account to read secrets. Introduces a new ClusterRole manifest, updates kustomization configuration, integrates a RoleBinding creation helper in the controller, and includes corresponding unit test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/rbac/evalhub/evalhub_secrets_reader_role.yaml`:
- Around line 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.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/rbac/evalhub/evalhub_secrets_reader_role.yamlconfig/rbac/kustomization.yamlcontrollers/evalhub/service_accounts.gocontrollers/evalhub/unit_test.go
| --- | ||
| # 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"] |
There was a problem hiding this comment.
🧩 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 2Repository: 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 2Repository: 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.
| --- | |
| # 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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
🧩 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.
|
@scheruku-rh: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
closing this PR based on discussion at https://redhat-internal.slack.com/archives/C09P3G66RGQ/p1772024438754589 |
2 participants
Adds RBAC so the EvalHub API ServiceAccount can read Kubernetes Secrets.
Summary by CodeRabbit
Release Notes
New Features
Tests