fix: Fixed matching for ALB and API Gateway (v1 & v2) events for Lambda #2780
Conversation
Signed-off-by: mrickard <[email protected]>
lib/serverless/api-gateway.js
Outdated
| } | ||
| const albKeys = [ |
There was a problem hiding this comment.
| } | |
| const albKeys = [ | |
| } | |
| const albKeys = [ |
lib/serverless/api-gateway.js
Outdated
| } | ||
| const albKeys = [ |
There was a problem hiding this comment.
Please document this as our other event keys are documented.
lib/serverless/api-gateway.js
Outdated
| 'multiValueQueryStringParameters', | ||
| 'pathParameters' | ||
| ] | ||
| .sort() |
There was a problem hiding this comment.
I recommend listing them in sorted order to avoid the unnecessary sorting operation cost.
There was a problem hiding this comment.
I'm dispensing with the sort and string conversion, as we're no longer looking for an exact match of all keys.
| if (!event.multiValueQueryStringParameters) { | ||
| if ( | ||
| !event.multiValueQueryStringParameters || | ||
| Object.keys(event.multiValueQueryStringParameters).length === 0 |
There was a problem hiding this comment.
I don't understand. If we have multiValueQueryStringParameters defined at all, we should not have queryStringParameters defined. So why would we return queryStringParameters if the multi valued property is defined with zero elements?
There was a problem hiding this comment.
In the sample event captured by the customer, both queryStringParameters and multiValueQueryStringParameters were defined, event though each had zero properties. I could see changing this to check for properties in each. The definition of one, though, doesn't exclude the definition of the other, based on the input they've captured.
mrickard
changed the title
…hing against known-unique keys for each type. Also added event example fixtures from users. Signed-off-by: mrickard <[email protected]>
mrickard
force-pushed
the
NR-341636/restore-alb-lambda-proxy-properties
branch
from
a3dcdcc
to
ebdfb27
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2780 +/- ##
==========================================
- Coverage 97.26% 97.16% -0.11%
==========================================
Files 294 294
Lines 46315 46345 +30
==========================================
- Hits 45047 45029 -18
- Misses 1268 1316 +48
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description
Filtering of API Gateway v1 vs v2 did not allow for Lambda functions triggered by ALB/ELB. Prior to our most recent change to that filtering, ALB events were traced without explicit filtering. This adds explicit checking for an ALB event type, and allows ALB-triggered web requests to be treated as web transactions.
How to Test
This adds a unit test at
test/unit/server/ess/alb-event.test.jschecking for the expected request properties.Related Issues
Closes NR-341636
Closes #2779
Closes NR-342521
Closes #2753
Closes NR-341279