feat(chart): add serviceAccountName support at per-agent and global level#914
Conversation
…evel Per-agent value (agents.<name>.serviceAccountName) wins when set; otherwise falls back to chart-global $.Values.serviceAccountName. Both empty preserves current behaviour (no serviceAccountName rendered, Kubernetes uses cluster default SA). This is required to activate IRSA on EKS — without an explicit serviceAccountName, the pod-identity-webhook never injects AWS credentials and workloads silently fall back to the broad EC2 node role, breaking least-privilege. Scope: string reference to an existing SA only. The chart does NOT create a new SA or manage IRSA annotations (operators provision out-of-band via Terraform / IDP / kubectl), matching how PR openabdev#901 (existingSecret) and openabdev#910 (imagePullSecrets) reference existing K8s resources rather than creating them. Closes openabdev#913
OpenAB PR ScreeningThis is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Screening report## IntentPR #914 adds Helm chart support for setting an existing Kubernetes FeatFeature work. It adds a chart-level Who It ServesDeployers and agent runtime operators, especially teams running OpenAB on EKS or other Kubernetes clusters where pod identity is bound to named ServiceAccounts. Rewritten PromptImplement Helm chart support for referencing an existing Kubernetes ServiceAccount from OpenAB agent pods. Add Merge PitchThis is a narrow chart feature with clear operational value and low migration risk. It improves least-privilege deployments without requiring the chart to create or own ServiceAccounts, IAM annotations, or RBAC. The main reviewer concern should be template precedence and whether the README makes the scope explicit enough: this references an existing ServiceAccount only. Best-Practice ComparisonOpenClaw and Hermes Agent patterns around scheduling, durable jobs, isolated executions, routing, retry, locking, and run logs do not apply directly. This PR is Kubernetes deployment configuration, not runtime scheduling or delivery orchestration. The relevant best practice is closer to existing OpenAB chart conventions: reference externally managed Kubernetes resources by name, keep empty defaults backward-compatible, and test rendered manifests. This matches the direction of Implementation Options
Comparison Table
RecommendationMove forward with the conservative implementation if the tests pass on the current branch after rebasing against nearby chart changes. Keep this PR scoped to Comment: #914 (comment) |
Summary
serviceAccountNameat chart-global ($.Values.serviceAccountName) and per-agent (agents.<name>.serviceAccountName) levelserviceAccountNamerendered → Kubernetes uses clusterdefaultSA) — zero impact on existing usersserviceAccountName, thepod-identity-webhooknever injects AWS credentials and workloads silently fall back to the broad EC2 node role, breaking least-privilegeexistingSecret) and feat(chart): add imagePullSecrets support at per-agent and global level #910 (imagePullSecrets) reference existing K8s resourcesCloses #913
Discord discussion: https://discord.com/channels/1491295327620169908/1491365157010542652/1507736112913973268
Changes
charts/openab/values.yamlserviceAccountName: ""+ per-agentserviceAccountName: ""on thekiroagentcharts/openab/templates/deployment.yamlserviceAccountNameaftersecurityContextusingdefault $.Values.serviceAccountName $cfg.serviceAccountName;ifguards against empty value to preserve current "no field rendered" behaviourcharts/openab/README.mdcharts/openab/tests/serviceaccount_test.yamlTest plan
helm unittest charts/openab— all 113 tests pass (6 new + 107 existing)helm lint charts/openab— cleanhelm template testrelease charts/openab --set serviceAccountName=openab-sarendersserviceAccountName: openab-saat pod spec levelhelm templatewith no values set renders noserviceAccountNamefield (backwards-compat verified)Out of scope (possible follow-ups)
automountServiceAccountToken: falsefor agents that don't need K8s API accessNote on diff context
This branch is based on a slightly older fork main, so the diff context does not include PR #911 (
imagePullSecrets) which is still in review. Once #911 merges, this PR's newserviceAccountNameblock will sit immediately before theimagePullSecretsblock (both pod-level identity/credential fields grouped together) — no conflict expected.