Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion operator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ On `apply` this reconciles (idempotently, reused by name):
1. **Cloud Map** private DNS namespace (`<cloudMapNamespace>-<vpc-id>`, 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-<vpc-id>`, shared per-VPC), waits until `AVAILABLE`
4. **API Gateway HTTP API** (`oab-webhook-<ns>-<name>`, one per bot) + `HTTP_PROXY` integration over the VPC Link
4. **API Gateway HTTP API** (`oab-webhook-<ns>-<name>`, 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`

Expand All @@ -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
Expand Down
65 changes: 64 additions & 1 deletion operator/src/ingress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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:
/// <https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-private.html>
fn has_stage_path_override(request_parameters: Option<&HashMap<String, String>>) -> 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:<region>:<account>:service/<id>`).
fn cloud_map_service_id_from_arn(arn: &str) -> Option<String> {
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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)
Expand All @@ -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")?;
Expand Down Expand Up @@ -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(&params)));
}

#[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(&params)));
}

#[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(&params)));
}

#[test]
fn cloud_map_service_id_parses_from_arn() {
assert_eq!(
Expand Down
Loading