Repository navigation
fix(triggers): use region-derived partition in Lambda Function URL ARN - #1431
Open
pedramsafaei wants to merge 2 commits into
Open
pedramsafaei wants to merge 2 commits into
pedramsafaei wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new test introduces unsafe environment mutations that are not isolated from concurrent native environment access.
1 open issue
What changed in this PR
Corrects Lambda Function URL inferred-span ARNs for GovCloud and China using the existing region-to-partition helper.
Changes:
- Derives the ARN partition from the region, preserving commercial-region output.
- Adds non-default partition regression tests and serializes both ARN tests.
| File | Description |
|---|---|
| bottlecap/src/lifecycle/invocation/triggers/lambda_function_url_event.rs | Updates partition selection and adds regression coverage. |
Review effort: Balanced
Comment on lines
+345
to
+365
| #[serial] // mutates the shared AWS_LAMBDA_FUNCTION_NAME env var | ||
| fn test_get_arn_non_default_partitions() { | ||
| unsafe { env::set_var("AWS_LAMBDA_FUNCTION_NAME", "mock-lambda") }; | ||
| let json = read_json_file("lambda_function_url_event.json"); | ||
| let payload = serde_json::from_str(&json).expect("Failed to deserialize into Value"); | ||
| let event = LambdaFunctionUrlEvent::new(payload) | ||
| .expect("Failed to deserialize LambdaFunctionUrlEvent"); | ||
|
|
||
| // GovCloud regions use the aws-us-gov partition. | ||
| assert_eq!( | ||
| event.get_arn("us-gov-west-1"), | ||
| "arn:aws-us-gov:lambda:us-gov-west-1:601427279990:url:mock-lambda" | ||
| ); | ||
|
|
||
| // China regions use the aws-cn partition. | ||
| assert_eq!( | ||
| event.get_arn("cn-north-1"), | ||
| "arn:aws-cn:lambda:cn-north-1:601427279990:url:mock-lambda" | ||
| ); | ||
|
|
||
| unsafe { env::remove_var("AWS_LAMBDA_FUNCTION_NAME") }; |
The inferred span ARN for Lambda Function URL events hardcoded the "aws" partition. In GovCloud (us-gov-*) and China (cn-*) regions this produced an incorrect ARN, since those regions use the aws-us-gov and aws-cn partitions respectively. Derive the partition from the region with get_aws_partition_by_region, matching the existing API Gateway and SQS trigger implementations. Adds a regression test covering the aws-us-gov and aws-cn partitions, and marks the two get_arn tests #[serial] since they mutate the shared AWS_LAMBDA_FUNCTION_NAME environment variable.
Derive the expected ARN function name from the current AWS_LAMBDA_FUNCTION_NAME value instead of calling set_var/remove_var, removing the unsafe environment mutation whose safety requirement could not be guaranteed against unsynchronized concurrent env readers. The serial attribute is retained so this test still coordinates with test_get_arn, which mutates the shared variable.
pedramsafaei
force-pushed
the
fix/lambda-function-url-arn-partition
branch
from
October 26, 2026 04:38
1ebe3b3 to
e8a3a3d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's broken
LambdaFunctionUrlEvent::get_arnhardcoded theawspartition when building the inferred span ARN for Lambda Function URL invocations:GovCloud (
us-gov-*) and China (cn-*) regions live in theaws-us-govandaws-cnpartitions, so this builds an ARN that cannot exist. The value is stored as the inferred span'sfunction_trigger.event_source_arn, so Function URL invocations in those regions report the wrong ARN.The fix
Derive the partition from the region with the existing
config::aws::get_aws_partition_by_regionhelper. The API Gateway REST/HTTP/WebSocket and SQS triggers already build their ARNs that way. Function URL was the last trigger inbottlecap/srcstill hardcoding a partition.Only the partition changes. Commercial regions are unchanged; the service, region, account,
urlresource marker and function name stay the same.sa-east-1arn:aws:lambda:sa-east-1:…:url:fnus-gov-west-1arn:aws:lambda:us-gov-west-1:…:url:fnarn:aws-us-gov:lambda:us-gov-west-1:…:url:fncn-north-1arn:aws:lambda:cn-north-1:…:url:fnarn:aws-cn:lambda:cn-north-1:…:url:fnChanges
get_arnnow resolves the partition viaget_aws_partition_by_region(region).test_get_arn_non_default_partitions, coveringus-gov-west-1→aws-us-govandcn-north-1→aws-cn.get_arntests#[serial]. They set the process-wideAWS_LAMBDA_FUNCTION_NAMEvariable, so running them at the same time is racy.serial_testis already a dev-dependency.Tests
The new test fails against the unmodified
get_arnand passes with the fix:With the fix applied:
cargo test --lib lifecycle::invocation::triggers::lambda_function_url_event— 9 passed, 0 failedcargo test --lib lifecycle::invocation::triggers— 167 passed, 0 failedcargo fmt --all -- --check— cleancargo clippy --workspace --all-targets --features default -- -D warnings— cleancargo clippy --workspace --all-targets --features default,test-mode -- -D warnings— cleancargo build --release --bin bottlecap— succeedsNo new test fixtures or identifiers were added; the test reuses the existing
lambda_function_url_event.jsonpayload.Local end-to-end run
The unit test only checks what
get_arnreturns, so I also checked the field in the payload the process actually sends. The releasebottlecapbinary built frommainand the one built from this branch were each run as a child process against a local Lambda Extensions API / Telemetry API simulator and a local trace intake, and the protobuf payload each process sent over the socket was decompressed and decoded. Six runs: two binaries ×us-east-1,us-gov-west-1,cn-north-1. Same fixture, same simulator, same build flags from a shared target directory — only the binary andAWS_DEFAULT_REGIONdiffer.This is local only. No real Lambda invocation in any partition, no GovCloud or China account, and no Datadog backend ingestion (the intake is a local stub). No AWS API, SDK or CLI call was made: the child process environment carries no credential variables at all, and
DD_API_KEYis a literal dummy, so the secrets path never reaches KMS/Secrets Manager/SSM/STS. CI'scargo nextestwas not available on the machine used, socargo teststands in for it.Loopback ports, per-run request/extension/span IDs and the fixture's
sourceIpare replaced with placeholders below. Everything else is unedited. The account ID601427279990and thelambda-url.eu-south-1.amazonaws.comhost are the repository fixture's own public values.Input
The unmodified
bottlecap/tests/payloads/lambda_function_url_event.json, POSTed to/lambda/start-invocationin all six runs. Only the fields that drive the ARN are shown:{ "version": "2.0", "routeKey": "$default", "rawPath": "/", "headers": { "...": "elided — 20 browser headers, verbatim upstream fixture" }, "requestContext": { "accountId": "601427279990", "apiId": "a8hyhsshac", "domainName": "a8hyhsshac.lambda-url.eu-south-1.amazonaws.com", "http": { "method": "GET", "path": "/", "protocol": "HTTP/1.1", "sourceIp": "<source-ip>", "userAgent": "Mozilla/5.0 (Macintosh; …) Chrome/95.0.4638.69 Safari/537.36" }, "requestId": "<fixture-request-id>", "stage": "$default", "timeEpoch": 1637169449721 }, "isBase64Encoded": false }requestContext.accountIdis the only account source for the URL ARN, and the fixture carries no region of its own — the region comes from the process environment, which is exactly the input this PR changes. So the same bytes exercise all three partitions.As received by the extension:
Extensions API and Telemetry API requests and responses
Captured on both sides — request bodies as the simulator received them, responses as it returned them. This excerpt is the
us-gov-west-1run of this branch's binary:Five telemetry records were delivered to the extension's own listener between those calls —
platform.initStart,platform.initReport,platform.start,platform.runtimeDone,platform.report— each answered200and each echoed back in the extension's debug log as a parsed record, so delivery is confirmed on both sides. End of invocation and flush:Every run did the same nine things: registered, subscribed to telemetry, served the invoke, brought up both listeners, answered start-invocation with
200 {}, answered end-invocation with200 {}, served the shutdown, and exited the process. Each run made exactly 2/nextcalls, sent exactly 1 trace payload, and exited 0.Decoded protobuf, before and after
Each run produced one payload:
Content-Type: application/x-protobuf,Content-Encoding: zstd. Bodies were zstd-decompressed and read asAgentPayload → TracerPayload → TraceChunk → Span, withSpan.meta(field 10) read as amap<string,string>.Span.meta["function_trigger.event_source_arn"], on the invocation span parented by the inferredaws.lambda.urlspan:mainbinaryus-east-1arn:aws:lambda:us-east-1:601427279990:url:mock-lambdaarn:aws:lambda:us-east-1:601427279990:url:mock-lambdaus-gov-west-1arn:aws:lambda:us-gov-west-1:601427279990:url:mock-lambda← bugarn:aws-us-gov:lambda:us-gov-west-1:601427279990:url:mock-lambdacn-north-1arn:aws:lambda:cn-north-1:601427279990:url:mock-lambda← bugarn:aws-cn:lambda:cn-north-1:601427279990:url:mock-lambdaCommercial is the same string on both sides, and the decoded payload is the same size (3308 B both). GovCloud went 3404 → 3410 B and China 3337 → 3342 B, which is the longer partition string.
Everything else in the payload is identical. GovCloud, both sides:
China looks the same, with the same single change (
arn:aws:…:url:…→arn:aws-cn:…:url:…) andmeta.function_arn = arn:aws-cn:…:function:mock-lambdaon both sides.One thing worth noting about the
mainpayloads on their own: the same GovCloud payload that carriesfunction_trigger.event_source_arn = arn:aws:…also carriesfunction_arn = arn:aws-us-gov:…, so one function ends up with two different partitions, because the function-ARN path already derives the partition from the region with the same helper. China is split the same way. This PR makes the two agree rather than introducing a new convention.Exact tests and commands
Run from
bottlecap/in each worktree, matching.github/workflows/rs_ci.yml:The one extra trigger test is the test this PR adds:
grep -c test_get_arn_non_default_partitionsreturns 1 against the branch log and 0 against themainlog, which confirms the two binaries above are the two sides of this PR. The six runs used one script and one fixture, changing only--binaryand--region.Limits of the local run
Four things about the local environment, none of them in the diff:
cargo test --release --workspaceunder the machine's inherited environment fails 2–3 tests inbottlecap/src/tags/lambda/tags.rs, a file this PR does not touch. They are off-by-one tag-count assertions (left: 14, right: 13) — strayDD_*/AWS_*variables in the ambient environment add an extra tag to the map. The same failures appear onmain, and with the environment scrubbed both sides are green (687/687 and 688/688).ERROR | Error shipping data … 0 series and 1 sketchesfrom the metrics path, because the local stub intake does not implement the sketches contract. Identical on both sides; the trace send in the same flush reportsSuccessfully sent trace (1 attempts, …).meta.runtimereflects the local machine rather than a Lambda runtime, since the proc-runtime search ran off-Lambda. Identical on both sides.LIBCLANG_PATHplusBINDGEN_EXTRA_CLANG_ARGSfor thelibddwaf-sysbindgen step, andRUSTFLAGS="-C link-arg=-lgcc"because the prebuilt libddwaf archive references the libgcc__cpu_model/__cpu_indicator_inithelpers.Note
get_aws_partition_by_regionmapsus-gov-*andcn-*and falls back toawsfor everything else, so the ISO partitions are not covered. That is existing behavior of the shared helper used by all the other triggers, and this change deliberately does not alter it. Happy to extend it in a separate PR if you'd like.