From 6e253f1a19825da97f0de5917d7ba7d61e263600 Mon Sep 17 00:00:00 2001 From: OksanaH Date: Sat, 24 Apr 2021 23:39:46 +0000 Subject: [PATCH 1/6] feat(cloudwatch): validate parameters for a Metric dimension (closes #3116) --- .../aws-cloudwatch/lib/metric-types.ts | 4 +- .../@aws-cdk/aws-cloudwatch/lib/metric.ts | 30 +++++++- .../aws-cloudwatch/test/test.metrics.ts | 72 +++++++++++++++++++ 3 files changed, 103 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts index eaa48446672f9..6c7e821d73bb4 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts @@ -12,14 +12,14 @@ export interface IMetric { /** * Turn this metric object into an alarm configuration * - * @deprecated Use `toMetricsConfig()` instead. + * @deprecated Use `toMetricConfig()` instead. */ toAlarmConfig(): MetricAlarmConfig; /** * Turn this metric object into a graph configuration * - * @deprecated Use `toMetricsConfig()` instead. + * @deprecated Use `toMetricConfig()` instead. */ toGraphConfig(): MetricGraphConfig; } diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts index 0590a592da502..0392f543dd629 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts @@ -1,3 +1,4 @@ +import { EOL } from 'os'; import * as iam from '@aws-cdk/aws-iam'; import * as cdk from '@aws-cdk/core'; import * as constructs from 'constructs'; @@ -216,7 +217,9 @@ export class Metric implements IMetric { if (periodSec !== 1 && periodSec !== 5 && periodSec !== 10 && periodSec !== 30 && periodSec % 60 !== 0) { throw new Error(`'period' must be 1, 5, 10, 30, or a multiple of 60 seconds, received ${periodSec}`); } - + if (props.dimensions) { + this.validateDimensions(props.dimensions); + } this.dimensions = props.dimensions; this.namespace = props.namespace; this.metricName = props.metricName; @@ -395,6 +398,31 @@ export class Metric implements IMetric { return list; } + + //rules from here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cw-dimension.html + private validateDimensions(dims: DimensionHash): void { + if (Object.keys(dims)?.length > 10) { + throw new Error('The maximum number of dimensions is 10'); + } + const errors: string[] = []; + + Object.keys(dims).sort().map(key => { + if (dims[key] === undefined || dims[key] === null) { + errors.push(`Dimension value of '${dims[key]}' is invalid`); + }; + if (key.length < 1 || key.length > 255) { + errors.push(`Dimension name must be at least 1 and no more than 255 characters; received ${key}`); + }; + + if (dims[key].length < 1 || dims[key].length > 255) { + errors.push(`Dimension value must be at least 1 and no more than 255 characters; received ${dims[key]}`); + }; + + if (errors.length > 0) { + throw new Error(`Invalid dimension properties: ${EOL}${errors.join(EOL)}`); + } + }); + } } function asString(x?: unknown): string | undefined { diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts index 7db77355f5d73..73403423e79d6 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts @@ -1,3 +1,4 @@ +import { EOL } from 'os'; import { expect, haveResource } from '@aws-cdk/assert'; import * as iam from '@aws-cdk/aws-iam'; import * as cdk from '@aws-cdk/core'; @@ -51,4 +52,75 @@ export = { test.done(); }, + + 'cannot use null or undefined dimension values'(test: Test) { + const expectedErrors = [ + 'Invalid dimension properties:', + 'Dimension value of \'undefined\' is invalid', + 'Dimension value of \'null\' is invalid', + ].join(EOL); + + test.throws(() => { + new Metric({ + namespace: 'Test', + metricName: 'ACount', + period: cdk.Duration.minutes(10), + dimensions: { + DimensionWithUndefined: undefined, + DimensionWithNull: null, + }, + }); + }, expectedErrors); + + test.done(); + }, + + 'cannot use long dimension values'(test: Test) { + const invalidDimensionValue = 'SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ + SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ + SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ + SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ + SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ + SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue'; + + test.throws(() => { + new Metric({ + namespace: 'Test', + metricName: 'ACount', + period: cdk.Duration.minutes(10), + dimensions: { + DimensionWithUndefined: undefined, + DimensionWithNull: null, + DimensionWithLongValue: invalidDimensionValue, + }, + }); + }, `Dimension value must be at least 1 and no more than 255 characters; received ${invalidDimensionValue}`); + + test.done(); + }, + + 'throws error when there are more than 10 dimensions'(test: Test) { + test.throws(() => { + new Metric({ + namespace: 'Test', + metricName: 'ACount', + period: cdk.Duration.minutes(10), + dimensions: { + dimensionA: 'value1', + dimensionB: 'value2', + dimensionC: 'value3', + dimensionD: 'value4', + dimensionE: 'value5', + dimensionF: 'value6', + dimensionG: 'value7', + dimensionH: 'value8', + dimensionI: 'value9', + dimensionJ: 'value10', + dimensionK: 'value11', + }, + } ); + }, /The maximum number of dimensions is 10/); + + test.done(); + }, }; From e988e936fe5f68b27703bc598b810bac0f914a93 Mon Sep 17 00:00:00 2001 From: OksanaH Date: Sun, 25 Apr 2021 00:21:40 +0000 Subject: [PATCH 2/6] feat(cloudwatch): rework error logic (closes #3116) --- .../@aws-cdk/aws-cloudwatch/lib/metric.ts | 12 +++------ .../aws-cloudwatch/test/test.metrics.ts | 25 ++++++++++++------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts index 0392f543dd629..88ebd36e9ffec 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts @@ -1,4 +1,3 @@ -import { EOL } from 'os'; import * as iam from '@aws-cdk/aws-iam'; import * as cdk from '@aws-cdk/core'; import * as constructs from 'constructs'; @@ -404,23 +403,18 @@ export class Metric implements IMetric { if (Object.keys(dims)?.length > 10) { throw new Error('The maximum number of dimensions is 10'); } - const errors: string[] = []; Object.keys(dims).sort().map(key => { if (dims[key] === undefined || dims[key] === null) { - errors.push(`Dimension value of '${dims[key]}' is invalid`); + throw new Error(`Dimension value of '${dims[key]}' is invalid`); }; if (key.length < 1 || key.length > 255) { - errors.push(`Dimension name must be at least 1 and no more than 255 characters; received ${key}`); + throw new Error(`Dimension name must be at least 1 and no more than 255 characters; received ${key}`); }; if (dims[key].length < 1 || dims[key].length > 255) { - errors.push(`Dimension value must be at least 1 and no more than 255 characters; received ${dims[key]}`); + throw new Error(`Dimension value must be at least 1 and no more than 255 characters; received ${dims[key]}`); }; - - if (errors.length > 0) { - throw new Error(`Invalid dimension properties: ${EOL}${errors.join(EOL)}`); - } }); } } diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts index 73403423e79d6..278b78e4bb499 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts @@ -1,4 +1,3 @@ -import { EOL } from 'os'; import { expect, haveResource } from '@aws-cdk/assert'; import * as iam from '@aws-cdk/aws-iam'; import * as cdk from '@aws-cdk/core'; @@ -53,13 +52,22 @@ export = { test.done(); }, - 'cannot use null or undefined dimension values'(test: Test) { - const expectedErrors = [ - 'Invalid dimension properties:', - 'Dimension value of \'undefined\' is invalid', - 'Dimension value of \'null\' is invalid', - ].join(EOL); + 'cannot use null dimension value'(test: Test) { + test.throws(() => { + new Metric({ + namespace: 'Test', + metricName: 'ACount', + period: cdk.Duration.minutes(10), + dimensions: { + DimensionWithNull: null, + }, + }); + }, /Dimension value of 'null' is invalid/); + + test.done(); + }, + 'cannot use undefined dimension value'(test: Test) { test.throws(() => { new Metric({ namespace: 'Test', @@ -67,10 +75,9 @@ export = { period: cdk.Duration.minutes(10), dimensions: { DimensionWithUndefined: undefined, - DimensionWithNull: null, }, }); - }, expectedErrors); + }, /Dimension value of 'undeffined' is invalid/); test.done(); }, From 8589b7c99102d00189422d37109f46c95de11f0a Mon Sep 17 00:00:00 2001 From: OksanaH Date: Sun, 25 Apr 2021 00:29:07 +0000 Subject: [PATCH 3/6] feat(cloudwatch): further corrections to unit tests closes #3116 --- packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts index 278b78e4bb499..328df24749c62 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts @@ -77,7 +77,7 @@ export = { DimensionWithUndefined: undefined, }, }); - }, /Dimension value of 'undeffined' is invalid/); + }, /Dimension value of 'undefined' is invalid/); test.done(); }, @@ -96,8 +96,6 @@ export = { metricName: 'ACount', period: cdk.Duration.minutes(10), dimensions: { - DimensionWithUndefined: undefined, - DimensionWithNull: null, DimensionWithLongValue: invalidDimensionValue, }, }); From 4ba576c70c2fec1616bb2f6b776a0b6353522c4e Mon Sep 17 00:00:00 2001 From: OksanaH Date: Sun, 25 Apr 2021 00:43:54 +0000 Subject: [PATCH 4/6] feat(cloudwatch): add information about metric dimensions to README --- packages/@aws-cdk/aws-cloudwatch/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/@aws-cdk/aws-cloudwatch/README.md b/packages/@aws-cdk/aws-cloudwatch/README.md index 943dc135fa775..c974456eb1c69 100644 --- a/packages/@aws-cdk/aws-cloudwatch/README.md +++ b/packages/@aws-cdk/aws-cloudwatch/README.md @@ -58,6 +58,10 @@ const metric = new Metric({ } }); ``` +### Metric dimensions + +The maximum number of dimensions a Metric can have is 10. The length of a dimension name and value must be +between 1-255 characters in length. ### Metric Math From 4c0b71c936b313eb06edeb1d9d288e76dc678433 Mon Sep 17 00:00:00 2001 From: OksanaH Date: Fri, 30 Apr 2021 13:52:54 +0000 Subject: [PATCH 5/6] feat(cloudwatch): address PR comments --- packages/@aws-cdk/aws-cloudwatch/README.md | 4 ---- packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts | 2 ++ packages/@aws-cdk/aws-cloudwatch/lib/metric.ts | 7 ++++--- packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/README.md b/packages/@aws-cdk/aws-cloudwatch/README.md index c974456eb1c69..943dc135fa775 100644 --- a/packages/@aws-cdk/aws-cloudwatch/README.md +++ b/packages/@aws-cdk/aws-cloudwatch/README.md @@ -58,10 +58,6 @@ const metric = new Metric({ } }); ``` -### Metric dimensions - -The maximum number of dimensions a Metric can have is 10. The length of a dimension name and value must be -between 1-255 characters in length. ### Metric Math diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts index 6c7e821d73bb4..296400ee7f910 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts @@ -27,6 +27,8 @@ export interface IMetric { /** * Metric dimension * + * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cw-dimension.html + * */ export interface Dimension { /** diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts index 88ebd36e9ffec..7f24db6591428 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts @@ -400,11 +400,12 @@ export class Metric implements IMetric { //rules from here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cw-dimension.html private validateDimensions(dims: DimensionHash): void { - if (Object.keys(dims)?.length > 10) { - throw new Error('The maximum number of dimensions is 10'); + var dimsArray = Object.keys(dims); + if (dimsArray?.length > 10) { + throw new Error(`The maximum number of dimensions is 10, received ${dimsArray.length}`); } - Object.keys(dims).sort().map(key => { + dimsArray.map(key => { if (dims[key] === undefined || dims[key] === null) { throw new Error(`Dimension value of '${dims[key]}' is invalid`); }; diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts index 328df24749c62..d6a8bb7cea87f 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts @@ -124,7 +124,7 @@ export = { dimensionK: 'value11', }, } ); - }, /The maximum number of dimensions is 10/); + }, /The maximum number of dimensions is 10, received 11/); test.done(); }, From 91bcd046cfab69c8cc1d60f963203dfa966478e4 Mon Sep 17 00:00:00 2001 From: OksanaH Date: Fri, 7 May 2021 18:39:48 +0000 Subject: [PATCH 6/6] feat(cloudwatch): make variable value creation more readable, remove commented out line --- packages/@aws-cdk/aws-cloudwatch/lib/metric.ts | 1 - packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts | 8 ++------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts index 7f24db6591428..3bb3d512c86d0 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts @@ -398,7 +398,6 @@ export class Metric implements IMetric { return list; } - //rules from here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cw-dimension.html private validateDimensions(dims: DimensionHash): void { var dimsArray = Object.keys(dims); if (dimsArray?.length > 10) { diff --git a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts index d6a8bb7cea87f..a08c248406bad 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/test.metrics.ts @@ -83,12 +83,8 @@ export = { }, 'cannot use long dimension values'(test: Test) { - const invalidDimensionValue = 'SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ - SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ - SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ - SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ - SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue\ - SomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValueSomeLongValue'; + const arr = new Array(256); + const invalidDimensionValue = arr.fill('A', 0).join(''); test.throws(() => { new Metric({