From 0eb6c3bb5853194f8727fc2cd3b1c9acb6eea20f Mon Sep 17 00:00:00 2001 From: Kyle Laker Date: Fri, 13 May 2022 00:32:45 -0400 Subject: [PATCH] fix(cloudwatch-actions): stack partition is hardcoded 'aws' in action arn (#20224) This removes the hardcoded partition in the ARNs of Alarm Actions for EC2 and SSM. This ensures that these don't unnecessarily break in other non-standard partitions. This uses the ARN of the stack, as done for the region and account. This updates a regular expression in `@aws-cdk/aws-cloudwatch` as well to make sure that EC2 actions are still validated as-expected in GovCloud and other partitions that may support AlarmActions. Closes #19765 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-cloudwatch-actions/lib/ec2.ts | 2 +- .../@aws-cdk/aws-cloudwatch-actions/lib/ssm.ts | 4 ++-- .../aws-cloudwatch-actions/test/ec2.test.ts | 6 +++++- .../aws-cloudwatch-actions/test/ssm.test.ts | 12 ++++++++++-- packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts | 2 +- .../@aws-cdk/aws-cloudwatch/test/alarm.test.ts | 16 ++++++++++++++++ 6 files changed, 35 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudwatch-actions/lib/ec2.ts b/packages/@aws-cdk/aws-cloudwatch-actions/lib/ec2.ts index 57d5a4fb67501..568a03b30fbbe 100644 --- a/packages/@aws-cdk/aws-cloudwatch-actions/lib/ec2.ts +++ b/packages/@aws-cdk/aws-cloudwatch-actions/lib/ec2.ts @@ -41,7 +41,7 @@ export class Ec2Action implements cloudwatch.IAlarmAction { * Returns an alarm action configuration to use an EC2 action as an alarm action */ bind(_scope: Construct, _alarm: cloudwatch.IAlarm): cloudwatch.AlarmActionConfig { - return { alarmActionArn: `arn:aws:automate:${Stack.of(_scope).region}:ec2:${this.ec2Action}` }; + return { alarmActionArn: `arn:${Stack.of(_scope).partition}:automate:${Stack.of(_scope).region}:ec2:${this.ec2Action}` }; } } diff --git a/packages/@aws-cdk/aws-cloudwatch-actions/lib/ssm.ts b/packages/@aws-cdk/aws-cloudwatch-actions/lib/ssm.ts index 0d26c4258c0d5..db79030f15cde 100644 --- a/packages/@aws-cdk/aws-cloudwatch-actions/lib/ssm.ts +++ b/packages/@aws-cdk/aws-cloudwatch-actions/lib/ssm.ts @@ -70,9 +70,9 @@ export class SsmAction implements cloudwatch.IAlarmAction { */ bind(_scope: Construct, _alarm: cloudwatch.IAlarm): cloudwatch.AlarmActionConfig { if (this.category === undefined) { - return { alarmActionArn: `arn:aws:ssm:${Stack.of(_scope).region}:${Stack.of(_scope).account}:opsitem:${this.severity}` }; + return { alarmActionArn: `arn:${Stack.of(_scope).partition}:ssm:${Stack.of(_scope).region}:${Stack.of(_scope).account}:opsitem:${this.severity}` }; } else { - return { alarmActionArn: `arn:aws:ssm:${Stack.of(_scope).region}:${Stack.of(_scope).account}:opsitem:${this.severity}#CATEGORY=${this.category}` }; + return { alarmActionArn: `arn:${Stack.of(_scope).partition}:ssm:${Stack.of(_scope).region}:${Stack.of(_scope).account}:opsitem:${this.severity}#CATEGORY=${this.category}` }; } } } diff --git a/packages/@aws-cdk/aws-cloudwatch-actions/test/ec2.test.ts b/packages/@aws-cdk/aws-cloudwatch-actions/test/ec2.test.ts index ed4c4ba66bc85..0d4a31022b08b 100644 --- a/packages/@aws-cdk/aws-cloudwatch-actions/test/ec2.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch-actions/test/ec2.test.ts @@ -28,7 +28,11 @@ test('can use instance reboot as alarm action', () => { 'Fn::Join': [ '', [ - 'arn:aws:automate:', + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':automate:', { Ref: 'AWS::Region', }, diff --git a/packages/@aws-cdk/aws-cloudwatch-actions/test/ssm.test.ts b/packages/@aws-cdk/aws-cloudwatch-actions/test/ssm.test.ts index a4bb582005b22..771fe5ab65cbc 100644 --- a/packages/@aws-cdk/aws-cloudwatch-actions/test/ssm.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch-actions/test/ssm.test.ts @@ -25,7 +25,11 @@ test('can use ssm with critical severity and performance category as alarm actio 'Fn::Join': [ '', [ - 'arn:aws:ssm:', + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':ssm:', { Ref: 'AWS::Region', }, @@ -64,7 +68,11 @@ test('can use ssm with meduim severity and no category as alarm action', () => { 'Fn::Join': [ '', [ - 'arn:aws:ssm:', + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':ssm:', { Ref: 'AWS::Region', }, diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts index eacafe3d1c1ab..cecf0dd4f3476 100644 --- a/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts +++ b/packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts @@ -245,7 +245,7 @@ export class Alarm extends AlarmBase { } private validateActionArn(actionArn: string): string { - const ec2ActionsRegexp: RegExp = /arn:aws:automate:[a-z|\d|-]+:ec2:[a-z]+/; + const ec2ActionsRegexp: RegExp = /arn:aws[a-z0-9-]*:automate:[a-z|\d|-]+:ec2:[a-z]+/; if (ec2ActionsRegexp.test(actionArn)) { // Check per-instance metric const metricConfig = this.metric.toMetricConfig(); diff --git a/packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts b/packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts index 6a2a9eb19885b..d7c6a8a1411ad 100644 --- a/packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts +++ b/packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts @@ -49,6 +49,22 @@ describe('Alarm', () => { }).toThrow(/EC2 alarm actions requires an EC2 Per-Instance Metric. \(.+ does not have an 'InstanceId' dimension\)/); }); + test('non ec2 instance related alarm does not accept EC2 action in other partitions', () => { + const stack = new Stack(); + const alarm = new Alarm(stack, 'Alarm', { + metric: testMetric, + threshold: 1000, + evaluationPeriods: 2, + }); + + expect(() => { + alarm.addAlarmAction(new Ec2TestAlarmAction('arn:aws-us-gov:automate:us-east-1:ec2:reboot')); + }).toThrow(/EC2 alarm actions requires an EC2 Per-Instance Metric. \(.+ does not have an 'InstanceId' dimension\)/); + expect(() => { + alarm.addAlarmAction(new Ec2TestAlarmAction('arn:aws-cn:automate:us-east-1:ec2:reboot')); + }).toThrow(/EC2 alarm actions requires an EC2 Per-Instance Metric. \(.+ does not have an 'InstanceId' dimension\)/); + }); + test('can make simple alarm', () => { // GIVEN const stack = new Stack();