diff --git a/operator/README.md b/operator/README.md index 6ec280240..0e4fbb0d9 100644 --- a/operator/README.md +++ b/operator/README.md @@ -247,7 +247,7 @@ On `apply` this reconciles (idempotently, reused by name): 1. **Cloud Map** private DNS namespace (`-`, shared per-VPC) + a per-service **SRV** record (carries the container port; a plain A record does not work as a VPC-Link integration target) 2. **ECS service registry** wiring (attached at service creation) 3. **VPC Link** (`oab-vpc-link-`, shared per-VPC), waits until `AVAILABLE` -4. **API Gateway HTTP API** (`oab-webhook--`, one per bot) + `HTTP_PROXY` integration over the VPC Link +4. **API Gateway HTTP API** (`oab-webhook--`, one per bot) + `HTTP_PROXY` integration over the VPC Link, with an `overwrite:path` request parameter mapping so the backend receives the raw path (e.g. `/webhook/telegram`) instead of the stage-prefixed one (e.g. `/prod/webhook/telegram`) — see caveat below 5. One **route** per path + a `prod` auto-deploy **stage** 6. A self-referencing **security-group** inbound rule on `containerPort` @@ -269,6 +269,19 @@ LINE console: > `TELEGRAM_SECRET_TOKEN` when registering the webhook with BotFather to enable > that check. > +> **Stage prefix stripped before it reaches the backend:** for private +> (VPC_LINK) integrations, API Gateway forwards the *stage-prefixed* request +> path to the backend by default (e.g. `/prod/webhook/telegram`, not +> `/webhook/telegram`) — [documented AWS +> behavior](https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-private.html). +> OpenAB's webhook router matches the exact configured path, so without +> stripping the prefix every request 404s at the container despite the +> integration and Cloud Map wiring being otherwise correct. `apply` sets an +> `overwrite:path=$request.path` request parameter on the integration to strip +> it, and self-heals existing integrations created before this fix by patching +> the parameter in on the next `apply` — no manual intervention or recreate +> needed. +> > **Adding/fixing service discovery never requires recreating the service:** > if an existing ECS service has no Cloud Map registry, or has one pointing at a > different Cloud Map service than the one currently resolved for diff --git a/operator/src/ingress.rs b/operator/src/ingress.rs index 8de6b0430..3c33d7db9 100644 --- a/operator/src/ingress.rs +++ b/operator/src/ingress.rs @@ -28,6 +28,7 @@ use crate::manifest::{Ingress, OABServiceManifest}; use anyhow::{Context, Result}; use aws_sdk_apigatewayv2::types::{ConnectionType, IntegrationType, ProtocolType}; use aws_sdk_servicediscovery::types::{DnsConfig, DnsRecord, RecordType}; +use std::collections::HashMap; const STAGE_NAME: &str = "prod"; @@ -143,6 +144,19 @@ fn route_key(path: &str) -> String { format!("POST {path}") } +/// Whether an integration's request parameters already carry the +/// `overwrite:path` override needed to strip the stage prefix before it +/// reaches the backend. Without this, private (VPC_LINK) integrations +/// forward the stage-prefixed path (e.g. `/prod/webhook/telegram`) to the +/// container, and openab's exact-match router 404s on it. See: +/// +fn has_stage_path_override(request_parameters: Option<&HashMap>) -> bool { + request_parameters + .and_then(|p| p.get("overwrite:path")) + .map(|v| v == "$request.path") + .unwrap_or(false) +} + /// Extract the Cloud Map service ID from its ARN /// (`arn:aws:servicediscovery:::service/`). fn cloud_map_service_id_from_arn(arn: &str) -> Option { @@ -733,7 +747,23 @@ async fn ensure_integration( .integration_id() .context("integration missing id")? .to_string(); - eprintln!(" ✓ Integration exists → {integration_uri}"); + if has_stage_path_override(i.request_parameters()) { + eprintln!(" ✓ Integration exists → {integration_uri}"); + } else { + // Self-heal: existing integrations created before this + // fix forward the stage-prefixed path to the backend, + // causing every request to 404. Patch in the override. + eprintln!( + " ↻ Integration exists but missing path override → {integration_uri}, patching" + ); + api.update_integration() + .api_id(api_id) + .integration_id(&id) + .request_parameters("overwrite:path", "$request.path") + .send() + .await + .context("failed to patch integration path override")?; + } return Ok(id); } } @@ -743,6 +773,13 @@ async fn ensure_integration( } } eprintln!(" ⊕ Creating integration → {integration_uri}"); + // For private (VPC_LINK) integrations, API Gateway forwards the stage + // portion of the request path to the backend by default (e.g. + // `/prod/webhook/telegram` instead of `/webhook/telegram`), per AWS docs: + // https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-private.html + // openab's router matches the exact configured path, so without this + // override every request 404s at the backend. Overwrite the forwarded + // path with $request.path (stage-stripped) to match. let out = api .create_integration() .api_id(api_id) @@ -752,6 +789,7 @@ async fn ensure_integration( .connection_type(ConnectionType::VpcLink) .connection_id(vpc_link_id) .payload_format_version("1.0") + .request_parameters("overwrite:path", "$request.path") .send() .await .context("failed to create integration")?; @@ -882,6 +920,31 @@ mod tests { assert_eq!(route_key("/webhook/line"), "POST /webhook/line"); } + #[test] + fn stage_path_override_absent_when_no_request_parameters() { + // Integrations created before this fix have no RequestParameters at + // all, so they must be detected as needing the self-heal patch. + assert!(!has_stage_path_override(None)); + } + + #[test] + fn stage_path_override_absent_when_other_params_present() { + let params = HashMap::from([("someOtherKey".to_string(), "value".to_string())]); + assert!(!has_stage_path_override(Some(¶ms))); + } + + #[test] + fn stage_path_override_absent_when_value_wrong() { + let params = HashMap::from([("overwrite:path".to_string(), "/literal/path".to_string())]); + assert!(!has_stage_path_override(Some(¶ms))); + } + + #[test] + fn stage_path_override_present_when_correctly_set() { + let params = HashMap::from([("overwrite:path".to_string(), "$request.path".to_string())]); + assert!(has_stage_path_override(Some(¶ms))); + } + #[test] fn cloud_map_service_id_parses_from_arn() { assert_eq!(