Skip to content

Commit

Permalink
fix(aws-cloudwatch): remove workaround on optional DashboardName
Browse files Browse the repository at this point in the history
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 aws#213.
  • Loading branch information
gcacace authored and SanderKnape committed May 14, 2019
1 parent 314bea8 commit 565be04
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 29 deletions.
6 changes: 0 additions & 6 deletions packages/@aws-cdk/aws-cloudwatch/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 2 additions & 17 deletions packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
]
]
},
"DashboardName": "aws-cdk-cloudwatch-DashCCD7F836"
"DashboardName": "MyCustomDashboardName"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 19 additions & 5 deletions packages/@aws-cdk/aws-cloudwatch/test/test.dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
};
Expand Down

0 comments on commit 565be04

Please sign in to comment.