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.
aws · Apr 23, 2019 · 6c73d8a · 6c73d8a
1 parent 9c2220c
commit 6c73d8a
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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();
   }
 };