From 6c73d8a459c87faa77ab2eff21c77fe4dc72ad0b Mon Sep 17 00:00:00 2001 From: Gianluca Date: Tue, 23 Apr 2019 15:11:02 +0100 Subject: [PATCH] fix(aws-cloudwatch): remove workaround on optional DashboardName There was a bug in CloudFormation which caused updates to `AWS::CloudWatch::Dashboard` to fail if `DashboardName` was not supplied. Now that the bug has been fixed we can revert the workaround in CDK which generates a name if the user doesn't supply one. Fixes #213. --- packages/@aws-cdk/aws-cloudwatch/README.md | 6 ----- .../@aws-cdk/aws-cloudwatch/lib/dashboard.ts | 19 ++------------- .../integ.alarm-and-dashboard.expected.json | 2 +- .../test/integ.alarm-and-dashboard.ts | 1 + .../aws-cloudwatch/test/test.dashboard.ts | 24 +++++++++++++++---- 5 files changed, 23 insertions(+), 29 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/README.md b/packages/@aws-cdk/aws-cloudwatch/README.md index 467a4ccab0b12..5f6e924e7274c 100644 --- a/packages/@aws-cdk/aws-cloudwatch/README.md +++ b/packages/@aws-cdk/aws-cloudwatch/README.md @@ -103,12 +103,6 @@ The following widgets are available: - `SingleValueWidget` -- shows the current value of a set of metrics. - `TextWidget` -- shows some static Markdown. -> Warning! Due to a bug in CloudFormation, you cannot update a Dashboard after -> initially creating it if you let its name automatically be generated. You -> must set `dashboardName` if you intend to update the dashboard after creation. -> -> (This note will be removed once the bug is fixed). - ### Graph widget A graph widget can display any number of metrics on either the `left` or diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts b/packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts index 0c876272ecf78..300cd69d49729 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts @@ -52,19 +52,12 @@ export interface DashboardProps { */ export class Dashboard extends Resource { private readonly rows: IWidget[] = []; - private readonly dashboard: CfnDashboard; constructor(scope: Construct, id: string, props?: DashboardProps) { super(scope, id); - // WORKAROUND -- Dashboard cannot be updated if the DashboardName is missing. - // This is a bug in CloudFormation, but we don't want CDK users to have a bad - // experience. We'll generate a name here if you did not supply one. - // See: https://github.com/awslabs/aws-cdk/issues/213 - const dashboardName = (props && props.dashboardName) || new Token(() => this.generateDashboardName()).toString(); - - this.dashboard = new CfnDashboard(this, 'Resource', { - dashboardName, + new CfnDashboard(this, 'Resource', { + dashboardName: (props && props.dashboardName) || undefined, dashboardBody: new Token(() => { const column = new Column(...this.rows); column.position(0, 0); @@ -95,12 +88,4 @@ export class Dashboard extends Resource { const w = widgets.length > 1 ? new Row(...widgets) : widgets[0]; this.rows.push(w); } - - /** - * Generate a unique dashboard name in case the user didn't supply one - */ - private generateDashboardName(): string { - // Combination of stack name and LogicalID, which are guaranteed to be unique. - return this.node.stack.name + '-' + this.dashboard.logicalId; - } } diff --git a/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.expected.json b/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.expected.json index 71b83bbda96f7..23cffe3022390 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.expected.json +++ b/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.expected.json @@ -71,7 +71,7 @@ ] ] }, - "DashboardName": "aws-cdk-cloudwatch-DashCCD7F836" + "DashboardName": "MyCustomDashboardName" } } } diff --git a/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.ts b/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.ts index 6e70689d86c38..79b53ac3392e7 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/integ.alarm-and-dashboard.ts @@ -27,6 +27,7 @@ const alarm = metric.newAlarm(stack, 'Alarm', { }); const dashboard = new cloudwatch.Dashboard(stack, 'Dash', { + dashboardName: 'MyCustomDashboardName', start: '-9H', end: '2018-12-17T06:00:00.000Z', periodOverride: PeriodOverride.Inherit diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.dashboard.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.dashboard.ts index 95ee4d33b1055..94c91e29955d4 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/test.dashboard.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/test.dashboard.ts @@ -120,21 +120,35 @@ export = { test.done(); }, - 'work around CloudFormation bug'(test: Test) { - // See: https://github.com/awslabs/aws-cdk/issues/213 - + 'DashboardName is set when provided'(test: Test) { // GIVEN const app = new App(); const stack = new Stack(app, 'MyStack'); // WHEN - new Dashboard(stack, 'MyDashboard'); + new Dashboard(stack, 'MyDashboard', { + dashboardName: 'MyCustomDashboardName' + }); // THEN expect(stack).to(haveResource('AWS::CloudWatch::Dashboard', { - DashboardName: 'MyStack-MyDashboardCD351363' + DashboardName: 'MyCustomDashboardName' })); + test.done(); + }, + + 'DashboardName is not generated if not provided'(test: Test) { + // GIVEN + const app = new App(); + const stack = new Stack(app, 'MyStack'); + + // WHEN + new Dashboard(stack, 'MyDashboard'); + + // THEN + expect(stack).to(haveResource('AWS::CloudWatch::Dashboard', {})); + test.done(); } };