Skip to content

Commit 4e7a275

Browse files
authoredDec 11, 2021
fix(aws-autoscaling): notificationTargetArn should be optional in LifecycleHook (#16187)
This makes the notificationTargetArn optional in LifecycleHook. CloudFormation docs specify it as optional [here](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-as-lifecyclehook.html). Closes #14641. To achieve this, the `role` parameter was made optional. To avoid breaking users, a role is provided if users specify a `notificationTarget` (which they currently all do, as it is a required property) and is not provided otherwise. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 9f03dc4 commit 4e7a275

16 files changed

+1411
-90
lines changed
 

‎allowed-breaking-changes.txt

+7
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,10 @@ removed:@aws-cdk/aws-stepfunctions-tasks.EmrCreateClusterProps.autoTerminationPo
8686
# Changed property securityGroupId to optional because either securityGroupId or
8787
# securityGroupName is required. Therefore securityGroupId is no longer mandatory.
8888
weakened:@aws-cdk/cloud-assembly-schema.SecurityGroupContextQuery
89+
90+
# refactor autoscaling lifecycle hook target bind() methods to make role optional by
91+
# having bind() methods create the role if it isn't passed to them
92+
incompatible-argument:@aws-cdk/aws-autoscaling-hooktargets.FunctionHook.bind
93+
incompatible-argument:@aws-cdk/aws-autoscaling-hooktargets.QueueHook.bind
94+
incompatible-argument:@aws-cdk/aws-autoscaling-hooktargets.TopicHook.bind
95+
incompatible-argument:@aws-cdk/aws-autoscaling.ILifecycleHookTarget.bind
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// eslint-disable-next-line import/order
2+
import * as iam from '@aws-cdk/aws-iam';
3+
4+
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
5+
// eslint-disable-next-line no-duplicate-imports, import/order
6+
import * as constructs from 'constructs';
7+
8+
export function createRole(scope: constructs.Construct, _role?: iam.IRole) {
9+
let role = _role;
10+
if (!role) {
11+
role = new iam.Role(scope, 'Role', {
12+
assumedBy: new iam.ServicePrincipal('autoscaling.amazonaws.com'),
13+
});
14+
}
15+
16+
return role;
17+
}
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
export * from './common';
12
export * from './queue-hook';
23
export * from './topic-hook';
34
export * from './lambda-hook';

‎packages/@aws-cdk/aws-autoscaling-hooktargets/lib/lambda-hook.ts

+13-5
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ import * as lambda from '@aws-cdk/aws-lambda';
44
import * as sns from '@aws-cdk/aws-sns';
55
import * as subs from '@aws-cdk/aws-sns-subscriptions';
66

7+
import { createRole } from './common';
78
import { TopicHook } from './topic-hook';
89

910
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
1011
// eslint-disable-next-line no-duplicate-imports, import/order
11-
import { Construct } from '@aws-cdk/core';
12+
import { Construct } from 'constructs';
1213

1314
/**
1415
* Use a Lambda Function as a hook target
@@ -23,16 +24,23 @@ export class FunctionHook implements autoscaling.ILifecycleHookTarget {
2324
constructor(private readonly fn: lambda.IFunction, private readonly encryptionKey?: kms.IKey) {
2425
}
2526

26-
public bind(scope: Construct, lifecycleHook: autoscaling.ILifecycleHook): autoscaling.LifecycleHookTargetConfig {
27-
const topic = new sns.Topic(scope, 'Topic', {
27+
/**
28+
* If the `IRole` does not exist in `options`, will create an `IRole` and an SNS Topic and attach both to the lifecycle hook.
29+
* If the `IRole` does exist in `options`, will only create an SNS Topic and attach it to the lifecycle hook.
30+
*/
31+
public bind(_scope: Construct, options: autoscaling.BindHookTargetOptions): autoscaling.LifecycleHookTargetConfig {
32+
const topic = new sns.Topic(_scope, 'Topic', {
2833
masterKey: this.encryptionKey,
2934
});
35+
36+
const role = createRole(_scope, options.role);
37+
3038
// Per: https://docs.aws.amazon.com/sns/latest/dg/sns-key-management.html#sns-what-permissions-for-sse
3139
// Topic's grantPublish() is in a base class that does not know there is a kms key, and so does not
3240
// grant appropriate permissions to the kms key. We do that here to ensure the correct permissions
3341
// are in place.
34-
this.encryptionKey?.grant(lifecycleHook.role, 'kms:Decrypt', 'kms:GenerateDataKey');
42+
this.encryptionKey?.grant(role, 'kms:Decrypt', 'kms:GenerateDataKey');
3543
topic.addSubscription(new subs.LambdaSubscription(this.fn));
36-
return new TopicHook(topic).bind(scope, lifecycleHook);
44+
return new TopicHook(topic).bind(_scope, { lifecycleHook: options.lifecycleHook, role });
3745
}
3846
}
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import * as autoscaling from '@aws-cdk/aws-autoscaling';
22
import * as sqs from '@aws-cdk/aws-sqs';
3-
import { Construct } from '@aws-cdk/core';
3+
import { createRole } from './common';
4+
5+
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
6+
// eslint-disable-next-line no-duplicate-imports, import/order
7+
import { Construct } from 'constructs';
48

59
/**
610
* Use an SQS queue as a hook target
@@ -9,8 +13,19 @@ export class QueueHook implements autoscaling.ILifecycleHookTarget {
913
constructor(private readonly queue: sqs.IQueue) {
1014
}
1115

12-
public bind(_scope: Construct, lifecycleHook: autoscaling.ILifecycleHook): autoscaling.LifecycleHookTargetConfig {
13-
this.queue.grantSendMessages(lifecycleHook.role);
14-
return { notificationTargetArn: this.queue.queueArn };
16+
/**
17+
* If an `IRole` is found in `options`, grant it access to send messages.
18+
* Otherwise, create a new `IRole` and grant it access to send messages.
19+
*
20+
* @returns the `IRole` with access to send messages and the ARN of the queue it has access to send messages to.
21+
*/
22+
public bind(_scope: Construct, options: autoscaling.BindHookTargetOptions): autoscaling.LifecycleHookTargetConfig {
23+
const role = createRole(_scope, options.role);
24+
this.queue.grantSendMessages(role);
25+
26+
return {
27+
notificationTargetArn: this.queue.queueArn,
28+
createdRole: role,
29+
};
1530
}
1631
}
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import * as autoscaling from '@aws-cdk/aws-autoscaling';
22
import * as sns from '@aws-cdk/aws-sns';
3-
import { Construct } from '@aws-cdk/core';
3+
import { createRole } from './common';
4+
5+
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
6+
// eslint-disable-next-line no-duplicate-imports, import/order
7+
import { Construct } from 'constructs';
48

59
/**
610
* Use an SNS topic as a hook target
@@ -9,8 +13,19 @@ export class TopicHook implements autoscaling.ILifecycleHookTarget {
913
constructor(private readonly topic: sns.ITopic) {
1014
}
1115

12-
public bind(_scope: Construct, lifecycleHook: autoscaling.ILifecycleHook): autoscaling.LifecycleHookTargetConfig {
13-
this.topic.grantPublish(lifecycleHook.role);
14-
return { notificationTargetArn: this.topic.topicArn };
16+
/**
17+
* If an `IRole` is found in `options`, grant it topic publishing permissions.
18+
* Otherwise, create a new `IRole` and grant it topic publishing permissions.
19+
*
20+
* @returns the `IRole` with topic publishing permissions and the ARN of the topic it has publishing permission to.
21+
*/
22+
public bind(_scope: Construct, options: autoscaling.BindHookTargetOptions): autoscaling.LifecycleHookTargetConfig {
23+
const role = createRole(_scope, options.role);
24+
this.topic.grantPublish(role);
25+
26+
return {
27+
notificationTargetArn: this.topic.topicArn,
28+
createdRole: role,
29+
};
1530
}
1631
}

0 commit comments

Comments
 (0)
Please sign in to comment.