feat(operator): auto-register Telegram webhook, aws-sm:// secret refs#1285
Conversation
Two related additions to `spec.secrets`/ingress handling: 1. `apply` now calls Telegram's `setWebhook` API automatically when `/webhook/telegram` is one of `spec.ingress.paths` and `spec.secrets` has a `TELEGRAM_BOT_TOKEN` entry — no manual `curl setWebhook` step needed after deploying a Telegram bot. If `TELEGRAM_SECRET_TOKEN` is also set, it's passed through so Telegram signs webhook requests with it. Best-effort: a failure here (bad token, network blip) is printed as a warning but never fails `apply` — the AWS provisioning already succeeded by that point. LINE has no equivalent API, so its webhook URL still needs manual registration. 2. `spec.secrets` values now accept the same `aws-sm://<secret-id>#<json-key>` shorthand openab itself uses for in-app secret refs in config.toml, alongside the existing ECS-native `valueFrom` format (a full ARN, optionally with a `:<jsonKey>::` suffix). ECS has no knowledge of the `aws-sm://` scheme, so it's translated to the ECS-native form before being passed to `register_task_definition` — a bare secret name is resolved to its ARN via `DescribeSecret` first, since ECS's `valueFrom` requires a full ARN whenever a JSON-key suffix is present. The two are related: registering the webhook needs the actual plaintext token value (not just an ECS valueFrom reference), so a new `operator/src/secrets.rs` module centralizes both resolution paths (`resolve_value_from` for ECS, `resolve_string` for in-process use) on top of one shared `aws-sm://` URI parser, rather than duplicating it. Adds `reqwest` (rustls-tls, no default features) as a new dependency — the only way to call Telegram's HTTP API from oabctl. Testing: cargo build/clippy --all-targets -D warnings/test all clean on macmini; 26/26 tests pass (7 new: aws-sm:// URI parsing edge cases, find_telegram_webhook path/secret gating logic).
This comment has been minimized.
This comment has been minimized.
Both formats (ECS-native ARN valueFrom, or the aws-sm://<id>#<key> shorthand) are accepted per-value, not tied to a specific env var name - TELEGRAM_BOT_TOKEN (or any other key) can use either format. The previous example picked a different key for each format, which read as if the format choice depended on which secret it was.
This comment has been minimized.
This comment has been minimized.
…itly Previous revision buried this as a commented-out alternate line, which undersold the point — state directly, in active prose, that TELEGRAM_BOT_TOKEN (the example key actually used with the ARN format) supports aws-sm:// too, and vice versa for TELEGRAM_SECRET_TOKEN.
…ning Clarifies TELEGRAM_SECRET_TOKEN maps directly to Telegram's own setWebhook secret_token parameter (not an openab-specific invention) and links to core.telegram.org/bots/api#setwebhook for the canonical description.
This comment has been minimized.
This comment has been minimized.
…AM_BOT_TOKEN Immediately below the ARN example, so readers see at a glance that aws-sm:// is a drop-in alternative for that exact key, not just something explained separately further down.
This comment has been minimized.
This comment has been minimized.
… are for The spec.secrets format section only explained the two ARN/aws-sm:// value formats, never what the example keys themselves actually do. Add inline comments directly above each key stating its purpose (BotFather API token vs. optional setWebhook signing hardening) so a reader doesn't have to piece it together from other sections.
This comment has been minimized.
This comment has been minimized.
The existing ingress example is a partial spec: snippet illustrating the schema (both Telegram and LINE paths, non-default values). Add a separate, complete, copy-pasteable manifest showing only the fields actually required by the schema (apiVersion/kind/metadata, image, resources, configFrom, runtime.networking, ingress.paths) plus the one practically-required field for Telegram to work (TELEGRAM_BOT_TOKEN in spec.secrets, using the aws-sm:// shorthand), so a reader can go straight from README to a working oabctl apply without cross-referencing multiple examples.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…where public.ecr.aws/oablab/kiro was a stale pre-unification image reference - every other doc in the repo (image-tags.md, aws-ecs-fargate-spot.md, agentcore.md, etc.) already uses ghcr.io/openabdev/openab:<tag>. Fixes the 3 remaining occurrences in operator/README.md for consistency with the new minimal manifest example, which already used the correct registry.
Per docs/image-tags.md: there is no default agent and no bare stable/beta/latest tag — every tag must specify an agent (<version|floating>-<agent>). Fixes all 4 image references in this file to use stable-kiro or beta-kiro instead of the now-invalid bare tags.
This comment has been minimized.
This comment has been minimized.
|
LGTM ✅ — Clean, well-guarded addition of two related features with solid test coverage and best-effort semantics. What This PR DoesEliminates two manual steps after How It WorksA new Findings
What's Good (🟢)
Baseline Check
|
…#1288) * fix(operator): resolve_string mishandled ECS-native :jsonKey:: suffix Found via live E2E testing right after #1285 merged: applying a real manifest using the ECS-native valueFrom ARN format (the one the README itself documents as Format 1) failed with 'failed to resolve TELEGRAM_BOT_TOKEN', because resolve_string passed the full value - including the trailing :<jsonKey>:: suffix - straight to GetSecretValue's secret_id parameter. That suffix is purely an ECS valueFrom convention (resolved by ECS itself at container launch, via register_task_definition), not something the Secrets Manager API understands; AWS docs confirm SecretId only ever accepts a plain ARN or name. The previous doc comment's claim that 'GetSecretValue resolves natively' was simply wrong. Fix: add split_ecs_json_key_suffix to detect and strip the ECS-only suffix from an ARN-shaped value, then extract the JSON key manually - the same way the aws-sm://...#... path already did. resolve_value_from (used for the actual ECS task definition, where ECS does understand the suffix) is unaffected. Verified live: re-ran oabctl apply against the real manifest (TELEGRAM_BOT_TOKEN as an ECS-native ARN with the JSON-key suffix) - now prints '✓ Telegram webhook registered: Webhook was set' instead of the previous resolution failure. Testing: cargo build/clippy --all-targets -D warnings/test clean on macmini; 29/29 tests pass (3 new, including the exact real-world ARN value that surfaced the bug as a regression test). * fix(operator): address code review findings on split_ecs_json_key_suffix Two real gaps found in code review of the initial fix: 1. arn:...:secret:mysecret:: was misparsed as json-key='mysecret' when it's actually a full-secret-value reference where 'mysecret' is part of the secret name/base ARN, with all three optional ECS suffix fields empty. Fixed by locating the unambiguous ':secret:' marker and treating everything up to the next colon as the secret-name segment (which per AWS's own docs never contains a colon), rather than blindly rsplit'ing on the last colon. 2. The ECS valueFrom suffix actually has three positional fields, not one: <json-key>:<version-stage>:<version-id> (confirmed against AWS's ECS docs' worked examples for referencing a specific key/version). The previous implementation only handled the json-key-only case and silently fell through to 'no suffix' for any value with a non-empty version-stage or version-id, which would then fail at GetSecretValue with a confusing error. Now parses all three fields and fails closed with a clear, specific error when a version is pinned - oabctl's own use case (fetching a secret's plaintext value in-process) never needs to pin a rotation version, so this is an intentional, documented limitation rather than a silent misparse. Testing: cargo build/clippy --all-targets -D warnings/test clean on macmini; 33/33 tests pass (7 new, covering both findings using AWS's own documented example ARN shapes). Re-verified live against the real manifest from the original bug report - still resolves correctly ('Webhook is already set'), confirming no regression. --------- Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
Summary
Two related additions requested after the last E2E test of
oabctl applyagainst a real Telegram bot deployment (#1283): registering the webhook was
still a manual
curl setWebhookstep, and the only way to reference aSecrets Manager secret in
spec.secretswas the verbose ECS-nativevalueFromARN format.1. Auto-register the Telegram webhook
applynow calls Telegram'ssetWebhookAPI directly when/webhook/telegramis one ofspec.ingress.pathsandspec.secretshas aTELEGRAM_BOT_TOKENentry — no manualcurlstep needed after deploying abot. If
spec.secretsalso hasTELEGRAM_SECRET_TOKEN, it's passed throughso Telegram signs every webhook request with it.
This is best-effort by design: the AWS-side provisioning is already done and
correct by the time this runs, so a Telegram API hiccup (bad token, network
blip) shouldn't roll any of that back or fail the whole command. LINE has no
setWebhook-equivalent API, so its URL still needs manual registration inthe LINE Developers console.
2.
aws-sm://<secret-id>#<json-key>shorthand forspec.secretsspec.secretspreviously required the full ECS-nativevalueFromformat:Now also accepts the same
aws-sm://<secret-id>#<json-key>shorthand openabitself uses for in-app secret refs in
config.toml:ECS has no knowledge of the
aws-sm://scheme, sooabctltranslates it tothe ECS-native form before passing it to
register_task_definition. A baresecret name (not already an ARN) is resolved to its ARN via
DescribeSecretfirst, since ECS's
valueFromrequires the full ARN whenever a JSON-keysuffix is present — a bare name only works without the suffix.
The two features are related: registering the webhook needs the actual
plaintext token value in-process (not just an ECS
valueFromreference), soa new
operator/src/secrets.rsmodule centralizes both resolution paths(
resolve_value_fromfor ECS,resolve_stringfor in-process use) on oneshared
aws-sm://URI parser, instead of duplicating the parsing logic.Dependencies
Adds
reqwest(default-features = false,rustls-tls+jsononly —shares most of its TLS stack with what the AWS SDK crates already pull in
transitively) as the HTTP client for calling Telegram's API. No other new
dependencies.
Testing done
7 new unit tests:
aws-sm://URI parsing edge cases (missing#, emptysecret-id/json-key, non-
aws-sm://values pass through untouched) andfind_telegram_webhook's path/secret gating logic (fires only when both theTelegram path and the bot token secret are present).
All verified on macmini per the project's build-offloading convention — not
run locally. Could not exercise the live Telegram API call end-to-end in
this PR (would require registering against a real bot and secret); the
webhook-URL-matching and secret-gating logic is unit tested, and the
Secrets Manager resolution path reuses the same
GetSecretValue/DescribeSecretcalls already exercised live in #1283's E2E testing.