-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix(postgres-manager): failing unit test from cdk-nag
#128
Changes from all commits
5404d09
0031c98
772935b
442e79b
bbac3bf
a98c4d8
492396e
bfe2611
7e9f887
9f8e04f
8548733
9203c1c
d673b2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
/** | ||
* There are 2 ways of connecting from microservice to db | ||
*/ | ||
export enum DbAuthType { | ||
RDS_IAM, | ||
USERNAME_PASSWORD, | ||
} | ||
|
||
export type EventType = { | ||
microserviceName: string; | ||
}; | ||
|
||
export type MicroserviceConfig = { | ||
name: string; | ||
authType: DbAuthType; | ||
}[]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
{ | ||
"name": "lambda-with-rds", | ||
"name": "postgres-manager", | ||
"packageManager": "[email protected]", | ||
"dependencies": { | ||
"@aws-sdk/client-secrets-manager": "^3.515.0", | ||
Comment on lines
1
to
5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be in the top-level There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As part of the postgres-manager lambda, it will require the secret manager sdk part of the lambda asset to retrieve the master credentials and generating the random password. So I think putting at the inner |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,46 +1,94 @@ | ||
import { App, Aspects } from 'aws-cdk-lib'; | ||
import { App, Aspects, Stack } from 'aws-cdk-lib'; | ||
import { Annotations, Match } from 'aws-cdk-lib/assertions'; | ||
import { SynthesisMessage } from 'aws-cdk-lib/cx-api'; | ||
import { AwsSolutionsChecks } from 'cdk-nag'; | ||
import { AwsSolutionsChecks, NagSuppressions } from 'cdk-nag'; | ||
import { OrcaBusStatelessStack } from '../../lib/workload/orcabus-stateless-stack'; | ||
import { getEnvironmentConfig } from '../../config/constants'; | ||
|
||
function synthesisMessageToString(sm: SynthesisMessage): string { | ||
return `${sm.entry.data} [${sm.id}]`; | ||
} | ||
|
||
// Picking prod environment to test as it contain the sensitive data | ||
const config = getEnvironmentConfig('prod')!; | ||
|
||
describe('cdk-nag-stateless-stack', () => { | ||
let stack: OrcaBusStatelessStack; | ||
let app: App; | ||
|
||
beforeAll(() => { | ||
app = new App({}); | ||
stack = new OrcaBusStatelessStack(app, 'TestStack', { | ||
env: { | ||
account: '12345678', | ||
region: 'ap-southeast-2', | ||
}, | ||
...config.stackProps.orcaBusStatelessConfig, | ||
}); | ||
Aspects.of(stack).add(new AwsSolutionsChecks()); | ||
|
||
// Suppressions (if any) | ||
// ... | ||
const app: App = new App({}); | ||
const stack: OrcaBusStatelessStack = new OrcaBusStatelessStack(app, 'TestStack', { | ||
env: { | ||
account: '12345678', | ||
region: 'ap-southeast-2', | ||
}, | ||
...config.stackProps.orcaBusStatelessConfig, | ||
}); | ||
|
||
test('cdk-nag AwsSolutions Pack errors', () => { | ||
// stateless stack cdk-nag test | ||
Aspects.of(stack).add(new AwsSolutionsChecks()); | ||
test(`OrcaBusStatelessStack: cdk-nag AwsSolutions Pack errors`, () => { | ||
const errors = Annotations.fromStack(stack) | ||
.findError('*', Match.stringLikeRegexp('AwsSolutions-.*')) | ||
.map(synthesisMessageToString); | ||
expect(errors).toHaveLength(0); | ||
}); | ||
|
||
test('cdk-nag AwsSolutions Pack warnings', () => { | ||
test(`OrcaBusStatelessStack: cdk-nag AwsSolutions Pack warnings`, () => { | ||
const warnings = Annotations.fromStack(stack) | ||
.findWarning('*', Match.stringLikeRegexp('AwsSolutions-.*')) | ||
.map(synthesisMessageToString); | ||
expect(warnings).toHaveLength(0); | ||
}); | ||
|
||
// microservice cdk-nag test | ||
for (const ms_stack of stack.microserviceStackArray) { | ||
const stackId = ms_stack.node.id; | ||
|
||
Aspects.of(ms_stack).add(new AwsSolutionsChecks()); | ||
|
||
applyNagSuppression(stackId, ms_stack); | ||
|
||
test(`${stackId}: cdk-nag AwsSolutions Pack errors`, () => { | ||
const errors = Annotations.fromStack(ms_stack) | ||
.findError('*', Match.stringLikeRegexp('AwsSolutions-.*')) | ||
.map(synthesisMessageToString); | ||
expect(errors).toHaveLength(0); | ||
}); | ||
|
||
test(`${stackId}: cdk-nag AwsSolutions Pack warnings`, () => { | ||
const warnings = Annotations.fromStack(ms_stack) | ||
.findWarning('*', Match.stringLikeRegexp('AwsSolutions-.*')) | ||
.map(synthesisMessageToString); | ||
expect(warnings).toHaveLength(0); | ||
}); | ||
} | ||
}); | ||
|
||
/** | ||
* apply nag suppression according to the relevant stackId | ||
* @param stackId the stackId | ||
* @param stack | ||
*/ | ||
function applyNagSuppression(stackId: string, stack: Stack) { | ||
switch (stackId) { | ||
case 'PostgresManager': | ||
NagSuppressions.addStackSuppressions(stack, [ | ||
{ id: 'AwsSolutions-IAM4', reason: 'allow to use AWS managed policy' }, | ||
]); | ||
|
||
// suppress by resource | ||
NagSuppressions.addResourceSuppressionsByPath( | ||
stack, | ||
`/TestStack/PostgresManager/CreateUserPassPostgresLambda/ServiceRole/DefaultPolicy/Resource`, | ||
[ | ||
{ | ||
id: 'AwsSolutions-IAM5', | ||
reason: | ||
"'*' is required for secretsmanager:GetRandomPassword and new SM ARN will contain random character", | ||
}, | ||
] | ||
); | ||
break; | ||
|
||
default: | ||
break; | ||
} | ||
} |
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.
@mmalenic am thinking of making microservice to stack instead of construct. One of the thought is that we could have the control for each microservice stack (e.g. we could deploy
yarn cdk-stateless-pipeline deploy ${stateless-microservice-stack}
instead of the whole stateless stack). Do you have any thoughts or perhaps objection on this?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.
No objections, I think that's a good idea. What would be the process of deploying individual microservice stacks? Would there be a
individual_stacks.ts
file inbin
that lists all the microservices, or is there a way deploy stacks that are nested within other stacks?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.
Hmmm, perhaps adding in the stack construct at the
bin/orcabus.ts
could be it andyarn cdk-orcabus ls
could list all the stacks? I temporary deploy fromyarn cdk-stateless-pipeline deploy ${stateless-microservice-stack}
but maybe I should try defining atbin/orcabus.ts
(thou might be some namespace clashes). Not sure what is the best pattern/approach yet.