fix(helm): add missing gateway env vars, deploy control, and sidecar support#693
Open
JARVIS-coding-Agent wants to merge 2 commits intoopenabdev:mainfrom
Open
fix(helm): add missing gateway env vars, deploy control, and sidecar support#693JARVIS-coding-Agent wants to merge 2 commits intoopenabdev:mainfrom
JARVIS-coding-Agent wants to merge 2 commits intoopenabdev:mainfrom
Conversation
…support - Add TELEGRAM_BOT_TOKEN, TELEGRAM_SECRET_TOKEN, TELEGRAM_WEBHOOK_PATH, LINE_CHANNEL_SECRET, LINE_CHANNEL_ACCESS_TOKEN env var injection - Add gateway.deploy flag (default: true) to decouple config from deployment - Add extraContainers, extraVolumes, extraVolumeMounts for sidecar support - Add nodeSelector, tolerations, affinity to gateway Deployment - Add telegram/line config sections to values.yaml - Expand gateway-secret.yaml to include telegram and LINE secrets - Add 18 new helm-unittest tests (28 total gateway tests, 64/64 passing) Closes openabdev#692
- Use toString pipe for deploy flag (default true handles false correctly) - Remove misleading single-arg 'and' on hasTelegram/hasLine assignments Co-authored-by: FRIDAY
Contributor
Author
Group Review Summary — PR #693Reviewers
Issues Fixed (Issue #692)
* Originally reported as P0 (startup panic), but latest Review RoundsRound 1 — FRIDAY
Round 2 — VISION
Round 3 — SHURI
Test Results
Files Changed
Suggested Follow-ups (out of scope for this hotfix)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Closes #692
The v0.8.2 Helm chart gateway templates (added in #677) are missing critical env var injection for Telegram and LINE adapters, have no sidecar support, and couple config generation with deployment creation.
Summary
This PR fixes all four issues reported in #692:
TELEGRAM_BOT_TOKEN,TELEGRAM_SECRET_TOKEN,TELEGRAM_WEBHOOK_PATH,LINE_CHANNEL_SECRET,LINE_CHANNEL_ACCESS_TOKENviasecretKeyRefgateway.deployflag (default:true). Set tofalsefor config-only mode (agent gets[gateway]config block without creating Gateway Deployment/Service)extraContainers,extraVolumes,extraVolumeMountsto gateway Deployment (e.g. for cloudflared tunnel sidecar)deploy: falseon all but one agent to share a single gateway instanceChanges
charts/openab/templates/gateway.yaml— Add telegram/LINE env vars,deployguard, extraContainers/extraVolumes/extraVolumeMounts, nodeSelector/affinity/tolerationscharts/openab/templates/gateway-secret.yaml— Add telegram-bot-token, telegram-secret-token, line-channel-secret, line-channel-access-token keyscharts/openab/values.yaml— Adddeploy,telegram.*,line.*,extraContainers,extraVolumeMounts,extraVolumes,nodeSelector,tolerations,affinityfields with documentation commentscharts/openab/tests/gateway_test.yaml— Add 18 new tests across 3 suites (config rendering, deployment rendering, secret rendering). 28 gateway tests total, 64/64 chart-wide passing.Design Decisions
gateway.deployinstead of top-level shared gateway: Minimal change that solves the coupling problem. Users who want a shared gateway setdeploy: falseon all agents except one (or deploy gateway externally). A top-level shared gateway abstraction is a larger architectural change better suited for a follow-up.GATEWAY_WS_TOKENremains in the agent Secret (shared between agent and gateway) — single source of truth preserved.requiredvalidation on telegram.botToken: The latest gateway binary uses.ok()(optional) forTELEGRAM_BOT_TOKEN, not.expect(). Missing token means the Telegram adapter is simply not enabled, not a crash. Addingrequiredwould block Teams-only or LINE-only deployments.Verification:
deploy: falsedoes not break agent SecretWhen
gateway.deploy: false:secret.yaml) which holdsgateway-ws-token— this is controlled bygateway.enabled+gateway.token, independent ofgateway.deploy[gateway]config block — still rendered whengateway.enabled: true[gateway]config andGATEWAY_WS_TOKENfrom its own Secret. No danglingsecretKeyRefreferences.Verified by helm-unittest:
does not render Deployment when deploy is falseanddoes not render when deploy is false(Secret) both pass while config rendering tests confirm[gateway]block is still generated.Note on P0 severity
Issue #692 reported
TELEGRAM_BOT_TOKENas causingpanicon startup. This was true in older gateway versions, but the currentmainbranch (gateway/src/main.rs) usesstd::env::var("TELEGRAM_BOT_TOKEN").ok()— the adapter is silently disabled rather than crashing. The fix is still necessary (users cannot configure Telegram via Helm without it), but severity is P1 (silent feature failure) rather than P0 (startup crash).Discord Discussion URL: https://discord.com/channels/1491295327620169908/1499911655142985888