Feature/platform best practices#13
Conversation
…stom resource Lambda Apply Aspect at stack level to reach the singleton provider Lambda, set reservedConcurrentExecutions and DLQ, and skip VPC placement check.
- Add cdk-nag compliance guardrails (AwsSolutionsChecks + tag enforcement) - Add snapshot/synth tests to detect infrastructure drift - Define platform product contract with SLOs, personas, support model - Update CI pipeline with cdk-nag compliance checks - Add governance tag enforcement CDK Aspect - Integrate cdk-nag into CdkAppStack and OrdersServiceStack
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR introduces platform best practices including comprehensive compliance guardrails with cdk-nag, reusable platform constructs, and enhanced CI/CD with security scanning (Checkov and Trivy). The overall architecture follows secure-by-default patterns with encryption, VPC isolation, and IAM authorization.
Critical Issues Requiring Fix:
-
Logic Error: The Aspect in ApiLambdaDynamoService modifies ALL Lambda functions across the entire stack, overriding user-configured concurrency settings. Scope it to the construct only using
cdk.Aspects.of(this)instead ofcdk.Aspects.of(cdk.Stack.of(this)). -
Crash Risk: CORS validation in
validateProps()will throw a runtime error ifprops.cors?.allowOriginsis undefined. Add optional chaining to the.some()call.
These issues must be resolved before merge as they will cause runtime failures and unexpected behavior in production environments.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| cdk.Aspects.of(cdk.Stack.of(this)).add({ | ||
| visit(node: constructs.IConstruct): void { | ||
| if (node instanceof lambda.CfnFunction && !node.deadLetterConfig) { | ||
| node.deadLetterConfig = { targetArn: retryQueue.queueArn }; | ||
| node.reservedConcurrentExecutions = 1; | ||
| node.cfnOptions.metadata = { | ||
| ...node.cfnOptions.metadata, | ||
| checkov: { | ||
| skip: [ | ||
| { | ||
| id: 'CKV_AWS_117', | ||
| comment: 'Custom resource Lambda cannot be placed inside a VPC', | ||
| }, | ||
| ], | ||
| }, | ||
| }; | ||
| } | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛑 Logic Error: Aspect modifies ALL Lambda functions in the stack, potentially overriding user-configured concurrency. The aspect sets reservedConcurrentExecutions = 1 on every Lambda without a DLQ, which conflicts with the main Lambda's configured value of 10 and may break other functions in the stack.
| cdk.Aspects.of(cdk.Stack.of(this)).add({ | |
| visit(node: constructs.IConstruct): void { | |
| if (node instanceof lambda.CfnFunction && !node.deadLetterConfig) { | |
| node.deadLetterConfig = { targetArn: retryQueue.queueArn }; | |
| node.reservedConcurrentExecutions = 1; | |
| node.cfnOptions.metadata = { | |
| ...node.cfnOptions.metadata, | |
| checkov: { | |
| skip: [ | |
| { | |
| id: 'CKV_AWS_117', | |
| comment: 'Custom resource Lambda cannot be placed inside a VPC', | |
| }, | |
| ], | |
| }, | |
| }; | |
| } | |
| }, | |
| }); | |
| const retryQueue = this.retryQueue; | |
| cdk.Aspects.of(this).add({ | |
| visit(node: constructs.IConstruct): void { | |
| if (node instanceof lambda.CfnFunction && !node.deadLetterConfig) { | |
| node.deadLetterConfig = { targetArn: retryQueue.queueArn }; | |
| node.reservedConcurrentExecutions = 1; | |
| node.cfnOptions.metadata = { | |
| ...node.cfnOptions.metadata, | |
| checkov: { | |
| skip: [ | |
| { | |
| id: 'CKV_AWS_117', | |
| comment: 'Custom resource Lambda cannot be placed inside a VPC', | |
| }, | |
| ], | |
| }, | |
| }; | |
| } | |
| }, | |
| }); |
| if (props.cors?.allowOrigins.some((origin) => origin === '*')) { | ||
| throw new Error('ApiLambdaDynamoService cors.allowOrigins must not include wildcard origins.'); | ||
| } |
There was a problem hiding this comment.
🛑 Crash Risk: Missing validation for optional CORS array before calling .some(). If props.cors?.allowOrigins is undefined or null, this will throw a runtime error when validating CORS configuration.
| if (props.cors?.allowOrigins.some((origin) => origin === '*')) { | |
| throw new Error('ApiLambdaDynamoService cors.allowOrigins must not include wildcard origins.'); | |
| } | |
| if (props.cors?.allowOrigins?.some((origin) => origin === '*')) { | |
| throw new Error('ApiLambdaDynamoService cors.allowOrigins must not include wildcard origins.'); | |
| } |
- Scope DLQ aspect to construct only (cdk.Aspects.of(this)) instead of entire stack - Add optional chaining to prevent crash when allowOrigins is undefined
… validation covered by tests
…ties throws, not returns boolean)
1 participant
No description provided.