Skip to content

docs: add refarch for running Telegram/LINE bots on AWS#1274

Merged
chaodu-agent merged 7 commits into
openabdev:mainfrom
smallgun01:docs/refarch-telegram-line-aws
Jul 7, 2026
Merged

docs: add refarch for running Telegram/LINE bots on AWS#1274
chaodu-agent merged 7 commits into
openabdev:mainfrom
smallgun01:docs/refarch-telegram-line-aws

Conversation

@smallgun01

@smallgun01 smallgun01 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Added docs/refarch/running-telegram-line-on-aws.md — a new reference architecture
    document for running OpenAB Telegram and LINE bots on AWS ECS Fargate.
  • Covers three ingress options with cost comparisons, step-by-step CLI commands,
    security group setup, webhook URL lifecycle, multi-platform guidance, and
    real-world pitfalls from production deployments.
  • Scope boundary: AWS infra layer only. Does not modify OpenAB code, config schema,
    or existing docs. Does not cover Kubernetes or non-AWS environments.

Prior Research

  • Existing issues/PRs checked: searched telegram aws refarch in openabdev/openab —
    found docs/refarch/telegram-cloudflare-tunnel.md (K3s-only), no AWS-native refarch.
  • Existing docs checked: docs/telegram.md, docs/refarch/telegram-cloudflare-tunnel.md,
    docs/refarch/aws-ecs-fargate-spot.md (Discord-only, outbound).
  • Discussion: https://discord.com/channels/1491295327620169908/1521849608525844594
    with @pahud, allenchen confirming the need.
  • Reviewed by OpenAB Bot prior to submission.

Testing

  • Not run (docs-only). All CLI commands are real production examples but not
    copy-paste replicable without an AWS account.

Add reference architecture document covering three AWS-based ingress
options for OpenAB Telegram and LINE bots:

- API Gateway HTTP API + VPC Link + Cloud Map (recommended, ~-10/mo)
- ALB + ECS Fargate (~8-21/mo)
- Cloudflare Tunnel sidecar (~-10/mo)

Includes cost breakdowns, security group setup, webhook URL lifecycle,
multi-platform guidance, and real-world pitfalls from production
deployments.

Co-authored-by: OpenAB Bot <bot@openab.dev>
@smallgun01 smallgun01 requested a review from thepagent as a code owner July 1, 2026 13:44
@openab-app openab-app Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Jul 1, 2026
@smallgun01

Copy link
Copy Markdown
Contributor Author

Added Discord thread link to Prior Research section. Thanks for the catch!

@openab-app openab-app Bot removed the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Jul 1, 2026
@chaodu-agent

This comment has been minimized.

F1: use secrets block instead of plaintext env vars in Option 3
F2: fix your-bot:latest to your-bot (ECS family name)
F3: soften VPC Link subnet claim to recommended not required
@smallgun01

Copy link
Copy Markdown
Contributor Author

All three findings resolved in 12653ab:

  • F1: Option 3 task def now uses secrets block (Secrets Manager) for all sensitive env vars, consistent with the TUNNEL_TOKEN pattern already used
  • F2: Changed --task-definition your-bot (ECS family name) + added note explaining ECS does not support Docker-style :latest
  • F3: Softened "must be private" → "should be private (recommended)" with explanation that public subnets also work

Ready for re-review.

@chaodu-agent

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

chaodu-agent added a commit that referenced this pull request Jul 1, 2026
…down retry

Round-8 review:
- F1 (blocking): README and code comments linked to
  docs/refarch/running-telegram-line-on-aws.md, which does not exist on
  main or this branch (its PR #1274 is still open). Repointed the README
  to the existing docs/refarch/telegram-cloudflare-tunnel.md as an
  interim cross-reference with a note that a dedicated AWS-native doc is
  tracked in #1274; ingress.rs/manifest.rs doc comments now point at
  operator/README.md instead of the nonexistent file.
- F2: VPC Link names are not unique to the apigatewayv2 API (confirmed:
  AWS does not error on a duplicate name), so two 'oabctl apply'
  processes racing to create the same per-VPC link in a brand-new VPC
  could end up with two links. Within a single apply run this can't
  happen (manifests are processed sequentially), but the reconciler
  hardens against the cross-process race anyway: ensure_vpc_link now
  collects ALL matching links, deterministically picks the
  lexicographically-first ID (stable regardless of list ordering) rather
  than an arbitrary one, and warns with cleanup commands if more than one
  exists.
- F3: Cloud Map service teardown gave up on the first 'still has
  registered instances' error, permanently orphaning the service if the
  caller didn't manually retry. ECS deregisters the instance
  asynchronously on scale-to-0/delete, so this is usually just a timing
  window, not a real conflict — teardown now retries delete_service up to
  6 times over ~25s before falling back to a warning with the exact
  manual cleanup command.

Verified: build, clippy --all-targets -D warnings, cargo test (13 passed).
Fixes based on chaodu-agent's live E2E finding in openabdev#1275:

- Cloud Map service must use SRV records (not A) — the port comes from SRV
- Integration URI must be the Cloud Map service ARN (not http:// URL)
- API Gateway HTTP API VPC_LINK rejects http:// URIs with:
  BadRequestException: integration uri should be a valid Cloud Map service ARN
- Capture SERVICE_ID from create-service for the service ARN lookup
- Also fix --task-definition your-bot:latest → your-bot (ECS family name)
- Update architecture diagram, path passthrough callout, and DNS record
  descriptions to match the corrected configuration
@smallgun01

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

thepagent pushed a commit that referenced this pull request Jul 2, 2026
…ink + Cloud Map) (#1275)

* feat(operator): add API Gateway ingress for webhook platforms

Adds an optional `spec.ingress` block to the oab.dev/v2 manifest so
`oabctl apply` can provision inbound HTTPS webhook ingress for
Telegram/LINE bots in one shot — instead of the ~7 manual `aws`
commands documented in the Telegram/LINE-on-AWS refarch (Option 1).

When `ingress` is present (ECS runtime only), apply reconciles, all
idempotently and reused by name:

- Cloud Map private DNS namespace + per-service A record
- ECS service registry wiring (attached at service *creation*)
- VPC Link (shared `oab-vpc-link`), waited until AVAILABLE
- API Gateway HTTP API (`oab-webhook`) + HTTP_PROXY integration
- one route per path + a `prod` auto-deploy stage
- a self-referencing security-group inbound rule on containerPort

It then prints the stable webhook URL(s) to register with the platform.

Backward compatible: `ingress` is optional and defaults to absent, so
existing outbound-only (Discord) deployments create no ingress
resources and behave exactly as before.

Because ECS service registries can only be set at creation time, apply
attaches the registry on create and, for a pre-existing service without
service discovery, provisions the ingress resources and prints how to
recreate the service (non-destructive — no automatic delete).

- manifest.rs: Ingress struct, validation, fleet passthrough, 7 unit tests
- ingress.rs: Cloud Map + VPC Link + API Gateway reconciliation
- apply.rs: two-phase wiring (Cloud Map before create, gateway after)
- schema/oabservice-v2.json: ingress def + refs
- README: ingress section
- Cargo.toml: aws-sdk-apigatewayv2, aws-sdk-servicediscovery

Refs #1274

* build(operator): declare standalone [workspace] so CI cargo check works

The ci.yml `operator` job runs `cargo check` with working-directory
operator, but the crate is not listed in the repo-root workspace
(members: openab-core, openab-gateway). Cargo then walks up to the root
Cargo.toml and errors ("believes it's in a workspace when it's not").
This job only runs when operator/** changes, so it was latent until now.

Adding an empty [workspace] table marks operator as its own workspace
root, matching how it's built and released standalone (own Cargo.lock).

* test(operator): unit-test ingress URL/route helpers; document shared VPC Link constraint

Addresses PR review feedback:
- Extract pure helpers (integration_uri, route_key, webhook_urls) from
  ensure_gateway and add 4 unit tests (F3 — ingress.rs had no coverage).
- Warn at apply time when reusing the shared oab-vpc-link and document
  in README that all ingress bots in a VPC must share subnets/SGs, since
  a VPC Link's subnets/SGs are fixed at creation (F2).

* fix(operator): correct recreate hint, add ingress teardown on delete, paginate API GW lists

Addresses PR review round 3:
- F1: recreate hint used `oabctl delete service` (invalid) and omitted
  --cluster; now prints `oabctl delete oabservice <name> --cluster oab
  --namespace <ns>` which matches delete.rs's contract and the hardcoded
  `oab` cluster.
- F2: oabctl delete oabservice now tears down the bot's per-bot ingress
  resources (Cloud Map service + API Gateway routes/integration),
  best-effort, leaving shared VPC Link / HTTP API / SG rule intact.
  No-op for bots that never had ingress. Documented in README.
- F3: apigatewayv2 GetApis/GetVpcLinks/GetIntegrations/GetRoutes/GetStages
  have no smithy paginator in this SDK, so paginate manually via
  next_token loops instead of reading only the first page.

Verified: cargo build, clippy --all-targets -D warnings, cargo test
(11 passed) in a nested layout mirroring the CI operator job.

* fix(operator): per-bot HTTP API to prevent cross-bot webhook path collisions

Round-4 review:
- F1 (critical): routes on the shared oab-webhook API were keyed by path
  only, so two bots both declaring /webhook/telegram would collide — the
  second apply saw the first's route, skipped, and silently misrouted
  traffic. Give each bot its own HTTP API (oab-webhook-<ns>-<name>) so
  webhook paths can never clash across bots; each bot gets a distinct
  {api-id} endpoint URL. Simplifies teardown to a single delete_api
  (cascades routes/integration/stage). VPC Link + SG rule stay shared.
- F2: removed duplicated comment line in ensure_vpc_link.
- Added api_name() helper + unit test (12 tests total).

Verified: build, clippy --all-targets -D warnings, cargo test (12 passed)
in a nested layout mirroring the CI operator job.

* fix(operator): match typed EC2 error code for duplicate SG rule

Round-5 review F1: ensure_sg_ingress classified the "rule already exists"
case by substring-matching the Debug-rendered error string, which breaks
if the SDK changes error formatting. Match the typed AWS error code
InvalidPermission.Duplicate via ProvideErrorMetadata::code() instead.

(F2 — CI green — the operator job now passes on the prior commit.)

Verified: build, clippy --all-targets -D warnings, cargo test (12 passed).

* fix(operator): use Cloud Map SRV + service-ARN integration (fixes live 503/BadRequest)

Live E2E against a real account revealed the documented Option-1 wiring
does not work: an HTTP API VPC_LINK integration rejects a raw
'http://<dns>:<port>' URI with

  BadRequestException: For VpcLink VPC_LINK, integration uri should be a
  valid ELB listener ARN or a valid Cloud Map service ARN.

Correct wiring (matches CDK's HttpServiceDiscoveryIntegration):
- Cloud Map service uses an SRV record (not A) so the container port is
  captured; ECS registers the task IP + port into it.
- ECS service registry sets containerName/containerPort and the task def
  exposes the container port, so ECS writes the SRV record.
- API Gateway integration URI is the Cloud Map *service ARN* with
  connectionType=VPC_LINK and integrationMethod=ANY; the port is resolved
  from SRV.

Verified end-to-end (POST through API Gateway → VPC Link → Cloud Map SRV
→ Fargate task returned HTTP 200 with the request echoed at
/prod/webhook/telegram), then torn down cleanly.

build + clippy -D warnings + cargo test (11 passed).

* feat(operator): consolidated apply summary for services needing ingress recreation

Review F2: the 'service exists without service discovery' warning was a
mid-run eprintln that's easy to miss in non-interactive CI, so a partial
apply could look like a clean success. apply_ecs now returns whether the
service needs recreation; apply::run collects these and prints a single
consolidated ⚠ summary block (with the exact recreate commands) after the
'N service(s) applied' line.

build + clippy -D warnings + test (11 passed).

* fix(operator): scope VPC Link/namespace per-VPC; teardown on ingress removal; prune stale routes

Round-7 review:
- F1 (critical): VPC Link was matched by a hardcoded name ('oab-vpc-link')
  with no VPC check. A VPC Link's ENIs live in one VPC and cannot route to
  another, so a second bot in a *different* VPC would silently reuse the
  first VPC's link and get unreachable integrations. Fixed by naming the
  link 'oab-vpc-link-<vpc-id>', scoped per-VPC.
- F2: same collision class for the Cloud Map namespace — matched by
  configured name only. Fixed by scoping the actual namespace name to
  '<cloudMapNamespace>-<vpc-id>' (the DnsConfig VPC association makes
  namespace-per-VPC the correct AWS-native mental model anyway; the
  private DNS only resolves inside that VPC).
- F3: apply only ever added ingress resources — editing a manifest to
  remove spec.ingress orphaned the per-bot HTTP API + Cloud Map service.
  apply now detects 'had ingress before, doesn't now' by comparing
  against the previously-stored S3 manifest and calls the same
  best-effort ingress::teardown used by .
- F4: ensure_route only ever added routes; renaming/removing a webhook
  path left a dead route on the bot's API forever. Added
  prune_stale_routes, which deletes any route on the bot's API whose key
  isn't in the current ingress.paths after ensuring the desired ones.

Added 3 unit tests (vpc_link_name, vpc_scoped_namespace) — 13 total.
Updated README + module doc to describe per-VPC scoping and the new
apply-time teardown/pruning behavior.

Verified: build, clippy --all-targets -D warnings, cargo test (13 passed).

* fix(operator): dead doc link, VPC Link race hardening, Cloud Map teardown retry

Round-8 review:
- F1 (blocking): README and code comments linked to
  docs/refarch/running-telegram-line-on-aws.md, which does not exist on
  main or this branch (its PR #1274 is still open). Repointed the README
  to the existing docs/refarch/telegram-cloudflare-tunnel.md as an
  interim cross-reference with a note that a dedicated AWS-native doc is
  tracked in #1274; ingress.rs/manifest.rs doc comments now point at
  operator/README.md instead of the nonexistent file.
- F2: VPC Link names are not unique to the apigatewayv2 API (confirmed:
  AWS does not error on a duplicate name), so two 'oabctl apply'
  processes racing to create the same per-VPC link in a brand-new VPC
  could end up with two links. Within a single apply run this can't
  happen (manifests are processed sequentially), but the reconciler
  hardens against the cross-process race anyway: ensure_vpc_link now
  collects ALL matching links, deterministically picks the
  lexicographically-first ID (stable regardless of list ordering) rather
  than an arbitrary one, and warns with cleanup commands if more than one
  exists.
- F3: Cloud Map service teardown gave up on the first 'still has
  registered instances' error, permanently orphaning the service if the
  caller didn't manually retry. ECS deregisters the instance
  asynchronously on scale-to-0/delete, so this is usually just a timing
  window, not a real conflict — teardown now retries delete_service up to
  6 times over ~25s before falling back to a warning with the exact
  manual cleanup command.

Verified: build, clippy --all-targets -D warnings, cargo test (13 passed).

* fix(operator): preserve webhook URL on recreate; exact Cloud Map targeting; namespace-change + SG mismatch detection

Round-10 review (full team pass on 838c220):

- F1 (blocking): the documented recreate path (oabctl delete && apply)
  deleted the bot's HTTP API resource itself via delete_api(), so the
  next apply's ensure_api() found nothing and created a NEW api-id,
  silently rotating the webhook URL hostname and breaking any Telegram/
  LINE registration made against the old URL. Fixed by splitting
  teardown into two functions:
    - ingress::teardown(): strips routes/integration/stage but keeps the
      HTTP API resource, so its api-id (and thus the public URL) survives
      an ECS-service recreate. Used by both apply's ingress-removed path
      and delete's ingress cleanup.
    - ingress::delete_api(): permanently deletes the HTTP API. Only
      called from oabctl delete's full-removal path, where there's no
      URL to keep stable since the bot itself is gone.

- F2: Cloud Map service teardown matched by name via an account-wide
  list_services() scan, so two bots with the same namespace/name in
  different VPCs/environments could collide. delete.rs now captures the
  ECS service's actual service_registries ARN BEFORE deleting it and
  passes it to teardown(), which resolves the exact Cloud Map service ID
  from that ARN instead of searching by name. Falls back to the by-name
  scan only when no ARN is known (apply's ingress-removed path, where the
  service may already be gone).

- F3: has_registries only checked whether SOME registry was attached,
  not whether it matched the currently-resolved registry for the
  manifest's ingress.cloudMapNamespace. Changing cloudMapNamespace on an
  existing service silently left it pointed at the old namespace (503s)
  without triggering the recreate warning. apply now compares the
  resolved registry ARN against the service's actual registry ARNs and
  flags a mismatch as needing recreate too, with a distinct message.

- F4: VPC Link reuse only printed a reminder about subnet/SG matching,
  never validated it. ensure_vpc_link now calls GetVpcLink on the reused
  link and compares its actual security groups against the manifest's,
  warning loudly on mismatch. (Subnets aren't exposed by GetVpcLink, so
  those remain a documented reminder only.)

- F5: VPC Link duplicate-name tie-breaker sorted candidates by ID only,
  ignoring status, so a Pending duplicate could be picked over an
  Available one (wasted wait time, not a correctness bug). Now sorts
  AVAILABLE first, ID as tiebreaker.

- F6: Cargo.lock was stale (missing the two new SDK deps entirely).
  Regenerated and committed.

Added 2 new unit tests (cloud_map_service_id_from_arn parsing) — 15
total. Rewrote the README ingress caveats section to describe the
stable-URL guarantee, exact-ARN teardown targeting, namespace-change
detection, and SG validation.

Verified: build, clippy --all-targets -D warnings, cargo test (15
passed).

* fix(operator): attach/fix service discovery via UpdateService, no recreate needed

Round-11 review (full team pass): the PR's core operational caveat —
"ECS service registries can only be set at creation time, so an existing
service needs delete-and-recreate to get ingress" — is outdated and
wrong. Confirmed directly against the AWS API_UpdateService reference:
serviceRegistries has been a documented UpdateService parameter since
March 2022. "When you add, update, or remove the service registries
configuration, Amazon ECS starts new tasks with the updated service
registries configuration, and then stops the old tasks when the new
tasks are running" — a normal rolling replacement, no downtime gap, no
delete needed. Requires the AWSServiceRoleForECS service-linked role,
which ECS creates automatically the first time any service in the
account uses service discovery.

Removed the entire recreate code path:
- apply_ecs's update_service branch now attaches or replaces the
  serviceRegistries directly when there's no registry or a mismatch
  (registry_mismatch detection from round-10 is kept — just resolved
  automatically instead of printed as a manual instruction).
- Deleted the "needs_recreate" plumbing through apply_ecs/run (dead code
  once the update path handles it directly).
- Corrected the "create-only" claim in ingress.rs's module doc and
  apply.rs's ensure_cloud_map-ordering comment.
- Rewrote README's "Recreate caveat" into "Adding/fixing service
  discovery never requires recreating the service", describing the
  actual UpdateService rolling-replacement behavior and the
  service-linked role note.

Also addressed the two non-blocking findings from the same review round:
- Added an "Additional permissions for spec.ingress" table to the
  Prerequisites section (Cloud Map, API Gateway, EC2, ECS UpdateService
  actions) — this is new AWS API surface the caller's credentials need
  that wasn't documented anywhere.
- Corrected the security note: Telegram validates the
  X-Telegram-Bot-Api-Secret-Token header + source-IP allowlist (not just
  "token in the path"), LINE does HMAC-SHA256 signature verification —
  verified against crates/openab-gateway/src/adapters/{telegram,line}.rs.

Verified live end-to-end against a real AWS account:
1. Applied a manifest WITHOUT ingress -> plain ECS service created.
2. Added ingress, re-applied -> printed "updated (service discovery
   attached; rolling replacement, no downtime)", NOT a recreate warning.
3. Confirmed via describe-services: same serviceArn, same createdAt
   timestamp (service was never deleted), serviceRegistries now
   populated with the correct Cloud Map ARN.
4. Confirmed the task rolled (old task ARN -> new task ARN) rather than
   the service being replaced.
5. curl through the printed webhook URL end-to-end -> HTTP 200.
6. Deleted and cleaned up all test resources (service, HTTP API, Cloud
   Map service/namespace, VPC Link, SG rule) - account left clean.

Verified: build, clippy --all-targets -D warnings, cargo test (15
passed, unchanged from round-10 since no new pure logic was added here).

* fix(operator): detach registry + exact-ARN teardown on ingress removal

Round-12 review (full team pass on 536e858): the ingress-removal path
(manifest edited to drop spec.ingress) had two gaps left over from
round-11's UpdateService redesign, both only reachable via that specific
transition:

- F1 (critical): the update_service branch only ever set
  `serviceRegistries` inside `if let Some(cm) = &cloud_map`. When ingress
  is removed, cloud_map is None, so the field was omitted from the
  request entirely. Per AWS's own UpdateService reference, an omitted
  serviceRegistries leaves the existing configuration unchanged — only an
  explicit empty list detaches it. Since ingress::teardown (running
  earlier in the same call) deletes the underlying Cloud Map service,
  the ECS service was left pointing at a registry ARN that no longer
  existed, which surfaces as registration failures on the next
  deployment. Fixed by tracking `needs_detach = cloud_map.is_none() &&
  has_registries` and calling `update_req.set_service_registries(Some(vec![]))`
  in that case.

- F2: the ingress-removal call site passed `None` for teardown's
  `known_registry_arn`, forcing a fallback to an account-wide
  `list_services()` scan matching only on `oab-{namespace}-{name}`. Two
  bots with the same namespace/name in different VPCs/environments
  sharing one account could collide there. Fixed by hoisting the
  `describe_services` call (previously done further down, purely for the
  service_active check) to the top of `apply_ecs`, before the
  ingress-removal branch, so the real registry ARN captured from the
  live ECS service can be threaded through instead of `None`. The
  service_active check later in the function now reuses this same
  response instead of issuing a second describe_services call.

No new tests added — this only changes AWS SDK request-building logic in
a code path (ingress removal) that isn't exercised by the existing unit
tests (which cover manifest/schema parsing, not live apply_ecs behavior).

Verified via `cargo fmt --check` (no new diffs beyond the changed lines)
and manual review of the full diff; `cargo build`/`clippy`/`test` could
not be run in this environment (no linker available), so CI is the
first real compile/test signal for this commit — flagged explicitly in
the accompanying PR comment.

---------

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
F1: VPC Link is NOT free — ENI-hour billing (~$0.01/hr, ~$7/month).
    Updated cost breakdown (Option 1: $5-10 → $12-17), architecture diagram,
    pros/cons, multi-platform total, and summary table.

F2: Add cross-reference to operator/README.md for oabctl ingress automation.
@smallgun01

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

@github-actions github-actions Bot added pending-contributor closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. and removed pending-maintainer labels Jul 2, 2026
@thepagent thepagent removed the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Jul 5, 2026
@github-actions github-actions Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Jul 5, 2026
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Caution

This PR has been waiting on the author for more than 2 days (labeled pending-contributor since 2026-07-02).
It will be automatically closed in 24 hours if there is no update.

Please push a commit, respond to outstanding review feedback, or comment here to keep this PR open. Feel free to reopen a new PR later if it gets closed and you want to pick it back up.

…(beta.4 version alignment)

F1: Add TELEGRAM_SECRET_TOKEN guidance to keep Telegram webhook signature
    verification intact when exposing :8080 to the public internet. New
    Security callout in Option 1 §7 explains that LINE auto-verifies via
    LINE_CHANNEL_SECRET (OpenAB built-in HMAC-SHA256), while Telegram
    requires TELEGRAM_SECRET_TOKEN env var matching the setWebhook
    secret_token parameter. Applied to:
      - Option 1 §6 setWebhook command (now uses `curl` + `openssl rand`)
      - Option 1 §7 env var block + new Security callout
      - Option 3 (Cloudflare Tunnel) task-def secrets block
      - Multi-Platform adapter table

F2: Align version note with docs/telegram.md (which states 'default since
    v0.9.0-beta.4'). Drop imprecise beta.2/beta.5/beta.6 references; keep
    the architecture advice version-agnostic.
Signed-off-by: smallgun01 <smallgun01@users.noreply.github.com>
@openab-app openab-app Bot removed the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Jul 6, 2026
@smallgun01

Copy link
Copy Markdown
Contributor Author

Both findings resolved in 4ec2bfc:

F1 — Missing webhook signature validation
Added a Security callout in Option 1 §7 explaining that LINE auto-verifies via LINE_CHANNEL_SECRET (OpenAB built-in HMAC-SHA256, no extra config), while Telegram requires TELEGRAM_SECRET_TOKEN (env var) matching the secret_token parameter passed to setWebhook. Concrete changes:

  • §6 setWebhook: command now generates a random secret via openssl rand -hex 32 and passes it as secret_token=
  • §7 env vars: TELEGRAM_SECRET_TOKEN added with comment pointing back to §6
  • Option 3 (Cloudflare Tunnel) task-def secrets block: added TELEGRAM_SECRET_TOKEN mapping
  • Multi-Platform adapter table: added row explaining the recommended env var and its relationship to the setWebhook call

F2 — beta.4 vs beta.5 version discrepancy
Confirmed against docs/telegram.md on main (which states "default since v0.9.0-beta.4") and aligned the version note callout accordingly. Dropped the imprecise beta.2/beta.5/beta.6 references and kept the architecture advice version-agnostic per your suggestion — the doc no longer makes load-bearing claims about specific minor versions.

Ready for re-review.

@chaodu-agent

This comment has been minimized.

@thepagent

Copy link
Copy Markdown
Collaborator

Hi @smallgun01

oabctl now supports ingress

https://github.com/openabdev/openab/blob/main/docs/oabctl.md#ingress--inbound-webhooks-telegram--line

can you try it out and see if it works for you. If yes, we should recommend oabctl in this doc over others.

@chaodu-agent

Copy link
Copy Markdown
Collaborator

LGTM ✅ — Production-tested AWS-native refarch for Telegram/LINE inbound webhooks on ECS Fargate. All prior findings addressed; latest commit is a maintainer merge-to-main with no content changes.

What This PR Does

Adds docs/refarch/running-telegram-line-on-aws.md — a reference architecture covering three ingress options for deploying OpenAB Telegram/LINE bots on AWS ECS Fargate with inbound webhooks, filling the gap between the existing K3s-only Cloudflare Tunnel doc and the Discord-only ECS Fargate Spot doc.

How It Works

Three production-grade ingress options compared with cost breakdowns, step-by-step AWS CLI commands, security group configuration, webhook signature validation, and real production pitfalls:

  1. API Gateway HTTP API + VPC Link + Cloud Map (~$12–17/mo, recommended)
  2. ALB + ECS Fargate (~$18–21/mo, enterprise)
  3. Cloudflare Tunnel sidecar (~$5–10/mo, simplest for Cloudflare users)

Also documents multi-platform (Telegram + LINE + Discord) on a single Fargate task with the unified binary.

Findings

# Severity Finding Location
1 🟢 Webhook signature validation documented for both platforms §6, §7, Option 3 task-def, Multi-Platform table
2 🟢 Version note correctly aligned to v0.9.0-beta.4+ Version note callout
3 🟢 VPC Link cost accurately stated (~$0.01/hr/ENI, ~$7/mo); totals consistent ($12–17/mo) Cost tables, summary
4 🟢 Cloud Map SRV record + service ARN integration URI matches verified AWS behavior Option 1 steps 1–4, DNS callout
5 🟢 All secrets use AWS Secrets Manager secrets block — no plaintext tokens Option 3 task-def
6 🟢 oabctl cross-reference present for one-shot automation Option 1 heading
Finding Details

🟢 F1: Webhook signature validation complete

Security gap from round 5 fully addressed:

  • §6 generates TELEGRAM_SECRET_TOKEN via openssl rand -hex 32 and passes as secret_token to setWebhook
  • §7 Security callout explains both Telegram (explicit env var) and LINE (automatic HMAC-SHA256 via LINE_CHANNEL_SECRET)
  • Option 3 task-def secrets block includes TELEGRAM_SECRET_TOKEN
  • Multi-platform adapter table documents the env var and its purpose

🟢 F2: Version note aligned

v0.9.0-beta.4+ matches canonical reference in docs/telegram.md on main.

🟢 F3: Cost accuracy verified

VPC Link ENI-hour billing (~$7/month) correctly documented. All totals consistent throughout the doc.

🟢 F4: SRV + Cloud Map ARN integration

Cloud Map uses Type="SRV" (not A), integration URI is the Cloud Map service ARN, DNS callout warns about BadRequestException for incorrect configuration. Confirmed via live E2E in #1275.

🟢 F5: Secrets Manager pattern consistent

All containers use secrets block for sensitive values. No inconsistency.

🟢 F6: Automation cross-reference

"Automation tip" callout under Option 1 points to operator/README.md for oabctl apply.

Addressing External Reviewer Feedback

@thepagent (Round 7)

oabctl now supports ingress — can you try it out and see if it works for you. If yes, we should recommend oabctl in this doc over others.

ℹ️ Acknowledged: This is a maintainer-to-contributor request for enhanced oabctl prominence. The doc already cross-references oabctl via the "Automation tip" callout under Option 1. Further integration is a follow-up discussion between maintainer and contributor — not a blocking review finding.

@github-actions[bot] (Stale Warning)

This PR has been waiting on the author for more than 2 days.

Resolved: Author pushed 4ec2bfc on 2026-07-06; maintainer merged main into branch (a466815) same day. PR is active.

Baseline Check
  • PR opened: 2026-07-01
  • Main already has: docs/refarch/telegram-cloudflare-tunnel.md (K3s-only), docs/refarch/aws-ecs-fargate-spot.md (Discord-only outbound), operator/README.md (oabctl ingress automation)
  • Net-new value: First AWS-native reference architecture for inbound Telegram/LINE webhooks on ECS Fargate — fills a confirmed documentation gap with production-tested content
  • Review rounds: 6 rounds total (5 content rounds + this merge-commit re-check), all findings addressed
What's Good (🟢)
  • Thorough security documentation: webhook signature validation for both Telegram and LINE
  • Real production pitfalls (QUIC instability on 256 CPU, ECS service-discovery create-only, SG self-reference, SRV-vs-A record)
  • Honest cost comparison with verified pricing
  • Clear outbound vs inbound mental model
  • Responsive contributor: all findings addressed promptly across 5 rounds
  • Docs-only change with zero code risk, CI green

Verified oabctl ingress on us-east-1 (2026-07-07):
- One  provisions the full stack in ~2.5min
- Replaces all 7 manual AWS CLI steps
- Two pitfalls documented: us-east-1e AZ unsupported, configFrom required

Switches from 'automation tip' to 'Recommended' callout at Option 1 top.
@smallgun01

Copy link
Copy Markdown
Contributor Author

Tested oabctl ingress on us-east-1 (2026-07-07) — works end-to-end.

One oabctl apply -f replaces all 7 manual CLI steps:

  • Cloud Map namespace + SRV record
  • VPC Link (provisioned + waited ~2 min)
  • API Gateway HTTP API + route + stage
  • Security group self-referencing rule
  • Clean teardown: oabctl delete -f removes everything

Two pitfalls found (documented in latest commit):

  1. us-east-1e (use1-az3) does not support API Gateway VPC Link — must avoid subnets in that AZ
  2. configFrom (S3 config path) is a required schema field, even for infra-only testing

Updated Option 1 section to recommend oabctl as the primary path, with manual CLI steps retained for reference.

Test manifest was minimal:

spec:
  ingress:
    type: apigateway
    paths:
      - /webhook/telegram

Full ingress reference is at docs/oabctl.md#ingress. 👍

@chaodu-agent

Copy link
Copy Markdown
Collaborator

LGTM ✅ — Comprehensive, production-tested AWS reference architecture for Telegram/LINE webhook ingress on ECS Fargate.

What This PR Does

Adds a new reference architecture document covering three production-grade options for getting inbound Telegram/LINE webhooks to OpenAB running on AWS ECS Fargate — filling a gap where only a K3s-based Cloudflare Tunnel guide existed.

How It Works

The document presents three architecture options (API Gateway + VPC Link + Cloud Map, ALB, Cloudflare Tunnel sidecar) with full CLI walkthroughs, cost breakdowns, security group configs, pitfalls from production, and a multi-platform section showing how to run all three platforms on a single task.

Findings

# Severity Finding Location
1 🟢 Excellent real-world production pitfalls section — saves readers from common mistakes Throughout
2 🟢 Clear cost comparison table helps readers make informed decisions quickly Line 18–22
3 🟢 Security section (webhook signature validation) is thorough and platform-specific §6–7
4 🟢 Good cross-reference to existing K3s doc and oabctl tooling Option 3 header
5 🟢 SRV vs A record DNS pitfall callout prevents a common misconfiguration Step 1 note
What's Good (🟢)
  • Production-battle-tested content: The pitfalls sections clearly come from real deployment experience (QUIC instability on 256 CPU, ECS service discovery create-only limitation, SG rules for VPC Link traffic).
  • oabctl fast-path: Calling out the one-command automated path upfront while preserving manual steps for learning is excellent documentation design.
  • Security-first approach: Webhook secret validation is prominently documented for both Telegram and LINE, with clear instructions on env var → setWebhook alignment.
  • Cost transparency: Honest cost breakdowns with real numbers (including the "Why We Didn't Choose It" section for ALB) help readers make pragmatic decisions.
  • Multi-platform consolidation: The section showing all three platforms on a single task with shared VPC Link is the kind of insight that saves significant cost.
  • Correct relative link: Cross-reference to telegram-cloudflare-tunnel.md uses proper relative path and the target file exists on main.
Baseline Check
  • PR opened: 2026-07-07
  • Main already has: docs/refarch/telegram-cloudflare-tunnel.md (K3s-only), docs/telegram.md (general), docs/oabctl.md (with ingress section)
  • Net-new value: First AWS-native reference architecture for Telegram/LINE webhook ingress on ECS Fargate. Fills a clear gap — no overlap with existing docs.

│ /webhook/telegram │
│ /webhook/line │
└───────────┬───────────┘
│ VPC Link (~$0.01/hr/ENI)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this should be free. Let me run a few days and check my billing to validate this.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's merge this for now.

@chaodu-agent chaodu-agent enabled auto-merge July 7, 2026 15:12
@chaodu-agent chaodu-agent merged commit fd1ce74 into openabdev:main Jul 7, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants