Skip to content

Commit 55ecbf5

Browse files
author
Eugene Cheung
authored
fix(dashboards): enforce non-empty dashboardNamePrefix (#166)
We currently have this logic in the internal extension library, but this should be enforced at the core library level. --- _By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license_
1 parent 5fcd1f1 commit 55ecbf5

File tree

3 files changed

+89
-4
lines changed

3 files changed

+89
-4
lines changed

lib/dashboard/DefaultDashboardFactory.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,18 @@ export class DefaultDashboardFactory
9696
constructor(scope: Construct, id: string, props: MonitoringDashboardsProps) {
9797
super(scope, id);
9898

99+
const createDashboard = props.createDashboard ?? true;
100+
const createSummaryDashboard = props.createSummaryDashboard ?? false;
101+
const createAlarmDashboard = props.createAlarmDashboard ?? false;
102+
const shouldCreateDashboards =
103+
createDashboard || createAlarmDashboard || createSummaryDashboard;
104+
105+
if (shouldCreateDashboards && !props.dashboardNamePrefix) {
106+
throw Error(
107+
"A non-empty dashboardNamePrefix is required if dashboards are being created"
108+
);
109+
}
110+
99111
const renderingPreference =
100112
props.renderingPreference ??
101113
DashboardRenderingPreference.INTERACTIVE_ONLY;
@@ -105,7 +117,7 @@ export class DefaultDashboardFactory
105117
"-" + (props.summaryDashboardRange ?? Duration.days(14)).toIsoString();
106118
let anyDashboardCreated = false;
107119

108-
if (props.createDashboard ?? true) {
120+
if (createDashboard) {
109121
anyDashboardCreated = true;
110122
this.dashboard = this.createDashboard(renderingPreference, "Dashboard", {
111123
dashboardName: props.dashboardNamePrefix,
@@ -114,7 +126,7 @@ export class DefaultDashboardFactory
114126
props.detailDashboardPeriodOverride ?? PeriodOverride.INHERIT,
115127
});
116128
}
117-
if (props.createSummaryDashboard ?? false) {
129+
if (createSummaryDashboard) {
118130
anyDashboardCreated = true;
119131
this.summaryDashboard = this.createDashboard(
120132
renderingPreference,
@@ -127,7 +139,7 @@ export class DefaultDashboardFactory
127139
}
128140
);
129141
}
130-
if (props.createAlarmDashboard ?? false) {
142+
if (createAlarmDashboard) {
131143
anyDashboardCreated = true;
132144
this.alarmDashboard = this.createDashboard(
133145
renderingPreference,

test/dashboard/DefaultDashboardFactory.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,17 @@ import {
1111
test("default dashboards created", () => {
1212
const stack = new Stack();
1313

14+
new DefaultDashboardFactory(stack, "Dashboards", {
15+
dashboardNamePrefix: "DummyDashboard",
16+
renderingPreference: DashboardRenderingPreference.INTERACTIVE_ONLY,
17+
});
18+
19+
expect(Template.fromStack(stack)).toMatchSnapshot();
20+
});
21+
22+
test("all dashboards created", () => {
23+
const stack = new Stack();
24+
1425
new DefaultDashboardFactory(stack, "Dashboards", {
1526
dashboardNamePrefix: "DummyDashboard",
1627
createDashboard: true,
@@ -41,3 +52,17 @@ test("no dashboards created", () => {
4152

4253
expect(Template.fromStack(stack)).toMatchSnapshot();
4354
});
55+
56+
test("throws error if an empty dashboardNamePrefix is passed but dashboard are to be created", () => {
57+
const stack = new Stack();
58+
59+
expect(() => {
60+
new DefaultDashboardFactory(stack, "Dashboards", {
61+
dashboardNamePrefix: "",
62+
createDashboard: true,
63+
renderingPreference: DashboardRenderingPreference.INTERACTIVE_ONLY,
64+
});
65+
}).toThrow(
66+
"A non-empty dashboardNamePrefix is required if dashboards are being created"
67+
);
68+
});

test/dashboard/__snapshots__/DefaultDashboardFactory.test.ts.snap

Lines changed: 49 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)