helm: add first-class STT config to chart#228
Conversation
Add stt as a first-class config block in the Helm chart so users
can enable STT with a single helm upgrade command:
helm upgrade openab openab/openab \
--set agents.kiro.stt.enabled=true \
--set agents.kiro.stt.apiKey=gsk_xxx
- values.yaml: add stt defaults (enabled, apiKey, model, baseUrl)
- configmap.yaml: render [stt] section when enabled, using ${STT_API_KEY}
- secret.yaml: store apiKey in K8s Secret (same pattern as botToken)
- deployment.yaml: inject STT_API_KEY env var from Secret
API key stays out of the configmap — follows the existing
DISCORD_BOT_TOKEN pattern.
Closes openabdev#227
ecefce3 to
0968cc1
Compare
thepagent
left a comment
There was a problem hiding this comment.
Review
Clean PR that follows existing chart patterns well. A few observations:
👍 What looks good
- Follows the established
DISCORD_BOT_TOKENpattern — API key in K8s Secret, injected as env var, referenced as${STT_API_KEY}in configmap. No plaintext leakage. - All STT resources conditionally rendered only when
stt.enabled=true— zero impact on existing deployments. - Good defaults in
values.yaml(Groq endpoint, whisper-large-v3-turbo). - Docs updated in both
docs/stt.mdandREADME.md.
⚠️ Suggestion: add apiKey validation
In configmap.yaml, the [stt] block is rendered when stt.enabled is true, even if apiKey is empty. However deployment.yaml gates the env var on both enabled AND apiKey, so ${STT_API_KEY} would remain as a literal string at runtime.
Consider adding a validation check:
{{- if and ($cfg.stt).enabled (not ($cfg.stt).apiKey) }}
{{ fail "stt.apiKey is required when stt.enabled=true" }}
{{- end }}This gives users a clear error at helm upgrade time instead of a confusing runtime failure.
Minor
- Consider squashing the 3 commits into one on merge for cleaner history.
Overall looks good to merge once the apiKey validation is addressed. 👍
|
Thanks for the review! Addressed:
|
thepagent
left a comment
There was a problem hiding this comment.
LGTM! apiKey validation addressed — clean fail-fast at helm upgrade time with the agent name in the error message. Ship it. 🚀
* helm: add first-class STT config to chart
Add stt as a first-class config block in the Helm chart so users
can enable STT with a single helm upgrade command:
helm upgrade openab openab/openab \
--set agents.kiro.stt.enabled=true \
--set agents.kiro.stt.apiKey=gsk_xxx
- values.yaml: add stt defaults (enabled, apiKey, model, baseUrl)
- configmap.yaml: render [stt] section when enabled, using ${STT_API_KEY}
- secret.yaml: store apiKey in K8s Secret (same pattern as botToken)
- deployment.yaml: inject STT_API_KEY env var from Secret
API key stays out of the configmap — follows the existing
DISCORD_BOT_TOKEN pattern.
Closes openabdev#227
* docs: add Helm chart deployment section to stt.md
* docs: mention STT support in README with link to docs/stt.md
* fix(helm): fail fast when stt.enabled=true but apiKey is empty
---------
Co-authored-by: openab-bot <openab-bot@users.noreply.github.com>
Summary
Add
sttas a first-class config block in the Helm chart, eliminating the need to manually patch configmaps afterhelm upgrade.Closes #227
Before (manual patching required)
After (single command)
Changes
values.yamlsttdefaults (enabled, apiKey, model, baseUrl)configmap.yaml[stt]section when enabled, using${STT_API_KEY}env varsecret.yamlapiKeyin K8s Secret (same pattern asdiscord.botToken)deployment.yamlSTT_API_KEYenv var from SecretDesign
Follows the existing
DISCORD_BOT_TOKENpattern — API key is stored in a K8s Secret and injected as an env var, never in plaintext in the configmap:All STT fields are optional. When
stt.enabledis false (default), no STT resources are rendered.