feat(helm): add Gateway Deployment + Service templates#677
feat(helm): add Gateway Deployment + Service templates#677thepagent merged 6 commits intoopenabdev:mainfrom
Conversation
When gateway.enabled=true, the chart now creates: - Gateway Deployment (openab-gateway container) - Gateway Service (ClusterIP :8080) - Gateway Secret (GATEWAY_WS_TOKEN, TEAMS_APP_SECRET) TEAMS_* env vars are injected into the gateway container (not the agent container). Condition: appId AND appSecret both set (fail-closed). Also adds gateway.image, gateway.tag, and gateway.teams.* values. Closes openabdev#675
masami-agent
left a comment
There was a problem hiding this comment.
PR Review: #677
Summary
- Problem: Helm chart doesn't deploy Gateway — users must create K8s resources manually
- Approach: Add gateway.yaml (Deployment + Service) + gateway-secret.yaml
- Risk level: Low — new templates only, no changes to existing templates
Core Assessment
- Problem clearly stated: ✅ — Closes #675
- Approach appropriate: ✅ — follows existing chart patterns
- Best approach for now: ✅
Findings
✅ What looks good
$hasTeamsuses unified condition (appId AND appSecret) — fail-closed, no mismatch ✅- Secret uses
b64enccorrectly,helm.sh/resource-policy: keepmatches existing pattern ✅ - Gateway Service is ClusterIP :8080 — correct for internal cluster access ✅
- Health probes on
/health— matches gateway binary ✅ RUST_LOG=infodefault — sensible ✅- Optional fields only injected when set — no empty env vars ✅
allowedTenants | join ","— correct for gateway's comma-separated parsing ✅
🔧 Suggested Changes (non-blocking)
-
Missing
resourcesblock on gateway container — Deployment has hardcodedrequests: cpu 50m, memory 64Miandlimits: memory 128Mi. These should be configurable via values.yaml (e.g.gateway.resources) like the agent container. For now the hardcoded defaults are reasonable — can be a follow-up. -
No
checksum/configannotation — Agent deployment haschecksum/configto trigger rollout on config change. Gateway deployment doesn't. If Teams env vars change, pods won't restart automatically. Consider adding a checksum annotation on the gateway values. -
readOnlyRootFilesystem: true— The gateway container inheritscontainerSecurityContextwhich includesreadOnlyRootFilesystem: true. Verify the gateway binary doesn't write to the filesystem at runtime (it shouldn't — it's stateless).
⚪ Nit
- Gateway Deployment uses
strategy: Recreate— correct for simplicity, but gateway is stateless soRollingUpdatewould also work and avoid downtime. Not blocking.
Verdict
APPROVE — clean, follows existing patterns, all conditions fail-closed.
Note: Cannot submit binding approval on own PR.
obrutjack
left a comment
There was a problem hiding this comment.
Gateway Deployment + Service + Secret templates look clean. Proper conditional guards, health probes, security context inheritance, configurable image/tag. CI + helm-unittest green. LGTM.
Addressing review feedback1. TEAMS_APP_ID as plaintext — This is intentional. App ID is a public identifier (like Discord channel ID), not a secret. It appears in the Teams app manifest, Azure Portal, and JWT audience claims. Only 2-4. strategy override, resource limits, global image default — Noted for follow-up. These are good improvements but don't block the initial gateway template. Will track in #676. |
四法師 Triage Review — PR #677 (2026-05-01)LGTM ✅ — Clean Helm templates for Gateway deployment. Ready to merge. Review Details🟢 INFO
🟡 NIT (non-blocking, tracked in #676)
🔴 SUGGESTED CHANGES
Reviewed by 超渡法師 on behalf of the 四法師 triage team. <@1493128125402320996> <@1496097857940361326> <@1496553369442189472> — Gateway Helm templates,LGTM。有異議請補充。 |
|
All review feedback addressed. Ready for code owner review. |
- Add configurable resources block (gateway.resources in values.yaml) - Add checksum/config annotation for automatic rollout on config change - Make strategy configurable (gateway.strategy, default: Recreate)
|
All 3 NITs addressed in commit 4e2a037:
Ready for re-review. |
四法師 Triage Review — PR #677 (2026-05-01)Verdict: ✅ LGTM — Ready for maintainer merge 超渡法師 Review四問框架
Traffic Light🟢 INFO
🟡 NIT (non-blocking, follow-up level)
🔴 SUGGESTED CHANGES
Existing Reviews Summary
Reviewed by 超渡法師 on behalf of the 四法師 triage team. |
|
| 法師 | Verdict | Notes |
|---|---|---|
| 超渡法師 | 🔴 Blocker | nameOverride collision (confirmed) |
| 覺渡法師 | ✅ LGTM | (before blocker was found) |
| 擺渡法師 | 🔴 Blocker | nameOverride collision (found) |
| 普渡法師 | ⏳ Pending |
Board status: stays in chaodu Backlog. Ball is on contributor to fix the nameOverride collision.
Updated by 超渡法師 on behalf of the 四法師 triage team.
|
All 3 NITs from triage review have been addressed. Ready for re-triage and code owner review. |
|
When agents.<name>.nameOverride is set, openab.agentFullname returns the override verbatim, ignoring the -gateway suffix. This causes Gateway Deployment/Service/Secret to collide with agent resources. Fix: use omit to strip nameOverride from gateway config dict, so the helper always uses the else branch: <fullname>-<agent>-gateway. Repro before fix: agents.kiro.nameOverride: my-bot → both agent AND gateway = my-bot After fix: agent = my-bot, gateway = openab-kiro-gateway
|
🔴 Blocker fixed in commit 0eecedc: nameOverride collision — Gateway templates now use
Ready for re-triage. |
Add gateway.rustLog value (default: "info"). Allows operators to set debug logging without editing templates.
Four-Monk Triage Review — PR #677CHANGES REQUESTED — One blocking issue: 🔴 SUGGESTED CHANGES (blocking)
On Both values come from Suggested fix:
🟡 NIT (non-blocking)
🟢 INFO (looks good)
Reviewer breakdown
Consensus: 4/4 reviewers independently identified the same blocking issue. All non-blocking NITs also converged. |
Maintainer Review — Additional FindingsAfter deeper inspection, the following items were identified: 🟡 NIT (non-blocking)
🔴 SUGGESTED CHANGES (blocking)
The existing # secret.yaml (main)
{{- if $hasGateway }}
gateway-ws-token: {{ $cfg.gateway.token | b64enc | quote }}
{{- end }}The new # gateway-secret.yaml (this PR)
{{- if $hasToken }}
gateway-ws-token: {{ $cfg.gateway.token | b64enc | quote }}
{{- end }}This means the same token is stored in two different Secrets, which causes:
Suggested fix: Review by maintainer + Four-Monk Team (超渡・普渡・擺渡・覺渡) |
- gateway Deployment reads GATEWAY_WS_TOKEN from agent Secret instead of gateway Secret (eliminates token drift risk during rotation) - gateway-secret.yaml now only contains teams-app-secret - Fix strategy comment: Recreate prevents concurrent WS conflicts - Add --set-literal reminder for appSecret in values.yaml Co-authored-by: Four-Monk Review Team
Fix pushed —
|
GATEWAY_WS_TOKEN was stored in both agent Secret and gateway Secret. Fix: gateway reads GATEWAY_WS_TOKEN from agent's Secret (single source of truth). Gateway Secret now only contains teams-app-secret. Before: token in 2 Secrets → rotation risk After: token in 1 Secret → single source of truth
|
🔴 Blocker fixed — gateway-ws-token duplication eliminated. Before: After:
Ready for re-review. |
Four-Monk Re-review — LGTM ✅All four reviewers confirm the fix addresses the blocking issue. Ready to merge. What was fixed (commit
|
| Reviewer | Round 1 | Round 2 |
|---|---|---|
| 超渡 (Kiro) | 🔴 CHANGES REQUESTED | ✅ LGTM |
| 普渡 (Claude) | 🔴 CHANGES REQUESTED | ✅ LGTM |
| 擺渡 (Codex) | 🔴 CHANGES REQUESTED | ✅ LGTM |
| 覺渡 (Gemini) | 🔴 CHANGES REQUESTED | ✅ LGTM |
4/4 LGTM — recommend merge.
chaodu-agent
left a comment
There was a problem hiding this comment.
LGTM ✅ — Four-monk review complete. All blocking issues resolved. Approved on behalf of the review team.
🟢 LGTM — Post-merge reviewClean, well-structured Helm templates that follow existing chart conventions. 1. What problem does it solve?Deploying the OpenAB Gateway alongside agent pods required manual K8s manifests. This PR automates gateway provisioning via 2. How does it solve it?
Key: Gateway is a separate Deployment (not sidecar), 3-4. Considerations & Assessment
🟡 NIT — Shadowed Reviewed by 超渡法師 🪬 — post-merge review |
✅ Review — LGTM with one NITVerdict: 🟢 Clean Helm-only PR. Gateway Deployment + Service + Secret templates follow existing chart patterns. 1. What problem does it solve?When 2. How does it solve it?Three new/modified files:
3. What was considered?
4. Is this the best approach?Yes. Follows existing chart patterns (per-agent loop, safe accessors, conditional rendering). Zero gateway code changes. Detailed notes🟢 INFO — Good patterns
🟡 NIT — Redundant - {{- $agentD := dict "ctx" $ "agent" $name "cfg" $cfg }}
- name: GATEWAY_WS_TOKEN
valueFrom:
secretKeyRef:
name: {{ include "openab.agentFullname" $agentD }}No blocking issues found. |
Discord Discussion URL: https://discord.com/channels/1488041051187974246/1497258664090931280
Description
Add Helm chart templates to deploy the OpenAB Gateway alongside OAB agent pods. When
gateway.enabled=true, the chart now creates Gateway resources automatically.Closes #675
What
helm installcreates (after this PR)Changes
charts/openab/templates/gateway.yamlcharts/openab/templates/gateway-secret.yamlcharts/openab/values.yamlgateway.image,gateway.tag,gateway.teams.*valuesDesign Decisions
appId AND appSecretboth required (fail-closed, no condition mismatch)<release>-<agent>-gateway(e.g.openab-kiro-gateway)/healthghcr.io/openabdev/openab-gateway, tag defaults to Chart.AppVersionUsage
Notes