Upgrade to cdk version 0.26.0 #36
Conversation
| @@ -321,7 +321,6 @@ Object { | |||
| }, | |||
| "tagjanitorlambdatagjanitorlambdarate7days0B5F68379": Object { | |||
| "Properties": Object { | |||
| "Description": "Run tag-janitor every 7 days", | |||
There was a problem hiding this comment.
This has been dropped as the new pattern doesn't take a description. I think this is pretty clear from the rate(7 days) and the Targets block anyway.
| }, | ||
| ], | ||
| schedule: Schedule.rate(lambdaFrequency), | ||
| monitoringConfiguration: { noMonitoring: true }, |
There was a problem hiding this comment.
This PR is a simple upgrade, so I won't add new resources here; I will add an alert via monitoringConfiguration in a subsequent PR.
| "@guardian/eslint-config-typescript": "^0.4.1", | ||
| "@types/jest": "^26.0.20", | ||
| "@types/node": "10.17.27", | ||
| "@typescript-eslint/eslint-plugin": "^4.14.0", | ||
| "@typescript-eslint/parser": "^4.14.0", | ||
| "aws-cdk": "~1.74.0", | ||
| "aws-cdk": "1.86.0", |
There was a problem hiding this comment.
These version bumps are required to keep us in sync after guardian/cdk#191.
cdk/lib/cdk-stack.ts
Outdated
| @@ -52,16 +52,12 @@ export class CdkStack extends GuStack { | |||
| description: "Lambda to notify about instances with missing tags", | |||
| timeout: Duration.seconds(30), | |||
| memorySize: 512, | |||
| vpc: GuVpc.fromId(this, "vpc", parameters.vpc.valueAsString), | |||
| vpc: GuVpc.fromId(this, "vpc", { vpcId: parameters.vpc.valueAsString }), | |||
There was a problem hiding this comment.
This was changed here.
There was a problem hiding this comment.
There's also now the GuVpc.fromIdParameter and GuVpc.subnetsFromParameter methods which would save defining those parameters explicitly. It looks up the values from parameter store so they may need adding there first.
There was a problem hiding this comment.
Cool, I'll set this up now.
cdk/lib/cdk-stack.ts
Outdated
| timeout: Duration.seconds(30), | ||
| memorySize: 512, |
There was a problem hiding this comment.
Can these two props be omitted now as they match the default values as of guardian/cdk#188?
There was a problem hiding this comment.
Yep, good point, thanks for spotting this 👍
What does this change?
This PR updates the version of the
@guardian/cdklibrary and makes use of a new scheduled lambda pattern for the first time.This doesn't actually simplify our infrastructure definition much yet, but it allows us to configure an appropriate alert with minimal boilerplate in the future (I'll do this in a subsequent PR). It also means that we start conforming to the recommendation in the
@guardian/cdkREADME:Patterns should be your default entry point to this library.How to test
I think this check should suffice?
How can we measure success?
Moving to pattern (rather than construct) usage should help us to keep up to date with best practices more easily in the future.