-
Notifications
You must be signed in to change notification settings - Fork 4.4k
chore(lambda): fix integration tests #36938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Fix integration test failures for aws-lambda capacity provider, durable config, sourceKMSKeyArn, version scaling, and assets tests. Fixes include removing hardcoded physical names, correcting managed policy references, adding region constraints for CapacityProvider availability, fixing response size limits, resolving LogGroup race conditions, and recreating a corrupt test asset.
a74f4dd to
e1d0bd7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This review is outdated)
|
|
||||||||||||||
|
|
||||||||||||||
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
| }, | ||
| })); | ||
| // TODO: Uncomment when the Lambda runtime SDK is updated to >= 3.942.0. | ||
| // The nodejs22.x Lambda runtime currently ships SDK 3.895.0, which does not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it feasible to have the test setup work with node 20.x? Or are the assertion checks deployed on node 22.x and that is causing the limitations ?
I think if we dont want this code we can remove it completely as it's not useful to test that CFN template is passing the correct config to Lambda(outside of cdk scope)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The limitation is that the latest Node.js lambda (22) does not come the SDK with the new Lambda APIs. The integ test assertions lambda uses node js 22.
We can keep the assertions for future reference for when the SDK is updated.
| SecurityGroupIds: [securityGroup.securityGroupId], | ||
| }, | ||
| })); | ||
| // TODO: Uncomment when the Lambda runtime SDK is updated to >= 3.942.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as the above test case
| getFunction.assertAtPath('Configuration.State', integ.ExpectedResult.stringLikeRegexp('Active')); | ||
|
|
||
| // TODO: Uncomment when the Lambda runtime SDK is updated to >= 3.942.0. | ||
| // The nodejs22.x Lambda runtime currently ships SDK 3.895.0, which strips |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above test case
| getFunction.assertAtPath('Configuration.State', integ.ExpectedResult.stringLikeRegexp('Active')); | ||
|
|
||
| // TODO: Uncomment when the Lambda runtime SDK is updated to >= 3.942.0. | ||
| // The nodejs22.x Lambda runtime currently ships SDK 3.895.0, which strips |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above test case
|
|
||
| const version = fn.currentVersion; | ||
| // Access currentVersion to trigger Version resource creation with scaling config | ||
| fn.currentVersion; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we keep this line ?
If we are not assigning it or using it then we can totally remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a lazy initialiser, accessing it creates the version if none exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But isn't there a better way to do this, like asserting on the version ?
There seems to be an older version to the sdk command : https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/lambda/command/ListVersionsByFunctionCommand/
Maybe we can use that in our assertions?
| ], | ||
| })); | ||
| // TODO: Uncomment when the Lambda runtime SDK is updated to >= 3.942.0. | ||
| // The nodejs22.x Lambda runtime currently ships SDK 3.895.0, which does not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above test cases
b3ef1cc to
3c1dd06
Compare
3c1dd06 to
7102132
Compare
Remove commented assertions that were waiting for Lambda runtime SDK >= 3.942.0. These assertions cannot be tested with the current nodejs22.x runtime (SDK 3.895.0) and are not needed for the integration tests to pass.
7102132 to
556b46a
Compare
Issue # (if applicable)
N/A - Remediation of 8 failing aws-lambda integration tests.
Reason for this change
Eight aws-lambda integration tests were failing due to a combination of issues: hardcoded physical names causing collisions, a non-existent IAM managed policy reference, an unsupported EC2 instance type for Lambda CapacityProvider, regional availability constraints for
AWS::Lambda::CapacityProvider, API response size exceeding CloudFormation custom resource limits, a corrupt test asset (handler.zip), and the Lambda runtime SDK (3.895.0) not yet including capacity provider APIs (introduced in SDK 3.942.0).Description of changes
integ.capacity-provider-all-fields.ts:
capacityProviderNameto avoid name collisionsAmazonLambdaCapacityProviderOperatorRolePolicy(does not exist) toAWSLambdaManagedEC2ResourceOperatort2.microtom5.large(t2.micro not supported by Lambda CapacityProvider for x86_64)AWS::Lambda::CapacityProvideris available)GetCapacityProviderassertions (runtime SDK too old)integ.capacity-provider-defaults.ts:
GetCapacityProviderassertions (runtime SDK too old)integ.function-capacity-provider-all-fields.ts:
@aws-cdk/aws-lambda:useCdkManagedLogGroup: falseto avoid race condition where Lambda auto-creates the log group before CloudFormation can create the explicitAWS::Logs::LogGroupresourceoutputPathstoGetFunctionAPI call to stay within 4KB custom resource response limitinteg.function-capacity-provider-minimal.ts:
outputPathsfixinteg.version-scaling-config-all-fields.ts:
version.functionArntofn.functionNameforlistVersionsByFunction(API rejects version-qualified ARNs)FunctionScalingConfigassertions (runtime SDK too old)integ.lambda-sourceKMSKeyArn.ts:
bucketName: 's3sourcekmskeyarnbucket'to avoid global name collisionsinteg.assets.file.ts:
handler.zip(was truncated to 313 bytes)integ.durable-config.ts:
Why assertions are commented out:
The integ-test assertion Lambda runs on
nodejs22.x, which ships with AWS SDK 3.895.0. The Lambda capacity provider APIs (GetCapacityProviderCommand,CapacityProviderConfiginGetFunction,FunctionScalingConfiginlistVersionsByFunction) were introduced in SDK 3.942.0. The runtime SDK strips these fields from responses. Assertions are preserved as commented code with TODO markers and can be uncommented when the runtime SDK is updated. See runtime SDK version docs.Describe any new or updated permissions being added
No new permissions. The managed policy fix corrects
AmazonLambdaCapacityProviderOperatorRolePolicy(non-existent) toAWSLambdaManagedEC2ResourceOperator(the actual AWS managed policy for Lambda capacity provider operator roles).Description of how you validated changes
All 8 integration tests were deployed and validated via
integ-runnerwith--update-on-failedacross multiple parallel regions:All 8 tests pass. Snapshots updated via integ-runner only.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license