Skip to content

Commit

Permalink
fix(appconfig): allow multiple environment monitor roles to be created (
Browse files Browse the repository at this point in the history
#27243)

Currently, when adding multiple monitors under an environment, users are seeing the following error

```
Error: There is already a Construct with name 'Role' in Environment [MyEnvironment]
```

Before the logical id for any monitor role that is created was just `Role`. In this change, we use the monitor index in the logical id so there aren't any repeats in logical id's. This is also why there is a change in the integ test snapshot, since the logical id was refactored.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
chenjane-dev authored Sep 26, 2023
1 parent 3196733 commit 9312c97
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 29 deletions.
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-appconfig-alpha/lib/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,10 @@ export class Environment extends EnvironmentBase {
applicationId: this.applicationId,
name: this.name,
description: this.description,
monitors: this.monitors?.map((monitor) => {
monitors: this.monitors?.map((monitor, index) => {
return {
alarmArn: monitor.alarm.alarmArn,
alarmRoleArn: monitor.alarmRole?.roleArn || this.createAlarmRole(monitor.alarm.alarmArn).roleArn,
alarmRoleArn: monitor.alarmRole?.roleArn || this.createAlarmRole(monitor.alarm.alarmArn, index).roleArn,
};
}),
});
Expand All @@ -256,7 +256,7 @@ export class Environment extends EnvironmentBase {
this.application.addExistingEnvironment(this);
}

private createAlarmRole(alarmArn: string): iam.IRole {
private createAlarmRole(alarmArn: string, index: number): iam.IRole {
const policy = new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
actions: ['cloudwatch:DescribeAlarms'],
Expand All @@ -265,7 +265,7 @@ export class Environment extends EnvironmentBase {
const document = new iam.PolicyDocument({
statements: [policy],
});
const role = new iam.Role(this, 'Role', {
const role = new iam.Role(this, `Role${index}`, {
roleName: PhysicalName.GENERATE_IF_NEEDED,
assumedBy: new iam.ServicePrincipal('appconfig.amazonaws.com'),
inlinePolicies: {
Expand Down
76 changes: 75 additions & 1 deletion packages/@aws-cdk/aws-appconfig-alpha/test/environment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ describe('environment', () => {
},
AlarmRoleArn: {
'Fn::GetAtt': [
'MyEnvironmentRoleC08961D3',
'MyEnvironmentRole01C8C013F',
'Arn',
],
},
Expand Down Expand Up @@ -177,6 +177,80 @@ describe('environment', () => {
});
});

test('environment with monitors with two alarms', () => {
const stack = new cdk.Stack();
const app = new Application(stack, 'MyAppConfig');
new Environment(stack, 'MyEnvironment', {
name: 'TestEnv',
application: app,
monitors: [
{
alarm: new Alarm(stack, 'Alarm1', {
threshold: 5,
evaluationPeriods: 5,
metric: new Metric(
{
namespace: 'aws',
metricName: 'myMetric',
},
),
}),
},
{
alarm: new Alarm(stack, 'Alarm2', {
threshold: 5,
evaluationPeriods: 5,
metric: new Metric(
{
namespace: 'aws',
metricName: 'myMetric',
},
),
}),
},
],
});

Template.fromStack(stack).resourceCountIs('AWS::CloudWatch::Alarm', 2);
Template.fromStack(stack).resourceCountIs('AWS::IAM::Role', 2);
Template.fromStack(stack).hasResourceProperties('AWS::AppConfig::Environment', {
Name: 'TestEnv',
ApplicationId: {
Ref: 'MyAppConfigB4B63E75',
},
Monitors: [
{
AlarmArn: {
'Fn::GetAtt': [
'Alarm1F9009D71',
'Arn',
],
},
AlarmRoleArn: {
'Fn::GetAtt': [
'MyEnvironmentRole01C8C013F',
'Arn',
],
},
},
{
AlarmArn: {
'Fn::GetAtt': [
'Alarm2A7122E13',
'Arn',
],
},
AlarmRoleArn: {
'Fn::GetAtt': [
'MyEnvironmentRole135A2CEE4',
'Arn',
],
},
},
],
});
});

test('from environment arn', () => {
const stack = new cdk.Stack();
const env = Environment.fromEnvironmentArn(stack, 'MyEnvironment',
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"Threshold": 10
}
},
"MyEnvironmentRoleC08961D3": {
"MyEnvironmentRole01C8C013F": {
"Type": "AWS::IAM::Role",
"Properties": {
"AssumeRolePolicyDocument": {
Expand Down Expand Up @@ -72,7 +72,7 @@
},
"AlarmRoleArn": {
"Fn::GetAtt": [
"MyEnvironmentRoleC08961D3",
"MyEnvironmentRole01C8C013F",
"Arn"
]
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 9312c97

Please sign in to comment.