Add Recursion Detection middleware to all SDK requests #2105
Conversation
| "id": "d74f8a81-3ddb-431f-b600-6abefbdaba1b", | ||
| "type": "feature", | ||
| "description": "add recursion detection middleware to all SDK requests to avoid recursion invocation in Lambda", | ||
| "modules": [ |
There was a problem hiding this comment.
fix: I think we want this changelog entry to be a "rollup" to prevent duplicated changelogs for every service affected which can cause issues with release.
aws/middleware/middleware.go
Outdated
| @@ -166,3 +167,84 @@ func AddRawResponseToMetadata(stack *middleware.Stack) error { | |||
| func GetRawResponse(metadata middleware.Metadata) interface{} { | |||
| return metadata.Get(rawResponseKey{}) | |||
| } | |||
|
|
|||
| // AddRecursionDetection adds recursionDetection to the middleware stack | |||
| func AddRecursionDetection(stack *middleware.Stack) error { | |||
There was a problem hiding this comment.
fix: I can see we put several middleware here but let's move this to it's own file (e.g. recursion_detection.go) with it's own tests.
aws/middleware/middleware_test.go
Outdated
| t.Run(name, func(t *testing.T) { | ||
| // has to pre-check if current os has lambda function and trace ID environment variable | ||
| // if exists, need to restore them after each test case | ||
| initialLambdaFunc, hasInitialLambdaFunc := os.LookupEnv(lambdaFunc) |
There was a problem hiding this comment.
It's unfortunate that the testing utils that already exist for this depend on aws package.
I think we can simplify this by just clearing the environment and restoring it after the test such that each test gets a clean environment, example
Then you won't need the additional setEnvVar/recordEnvVar functions and we guarantee tests are isolated
aws/middleware/middleware_test.go
Outdated
| @@ -187,3 +188,98 @@ func TestAttemptClockSkewHandler(t *testing.T) { | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| func TestRecursionDetection(t *testing.T) { | |||
| const lambdaFunc = "AWS_LAMBDA_FUNCTION_NAME" | |||
There was a problem hiding this comment.
suggestion: You could define these as internal constants in the middleware and the test can just re-use them
e.g.
// file: recursion_detection.go
const envAwsLambdaFunctionName = "AWS_LAMBDA_FUNCTION_NAME"
const envAmznTraceId = "_X_AMZN_TRACE_ID"
const amznTraceIdHeader = "X-Amzn-Trace-Id"
Add Recursion Detection middleware to all SDK requests, which detects Lambda environment and sets its X-Ray trace ID to request header if absent to avoid recursion invocation in Lambda environment. Solves #2025