Skip to content

Commit

Permalink
fix(aws-autoscaling): Add hook ordering dependency
Browse files Browse the repository at this point in the history
Add an ordering dependency between the LifecycleHook and the role policy
that grants permissions to perform the action (publish to a topic or
queue).

Without this ordering dependency, the template might fail to deploy.

Fixes #1212.
  • Loading branch information
rix0rrr committed Nov 20, 2018
1 parent d3b8448 commit 698e3a8
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 1 deletion.
9 changes: 9 additions & 0 deletions packages/@aws-cdk/aws-autoscaling/lib/lifecycle-hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import iam = require('@aws-cdk/aws-iam');
import cdk = require('@aws-cdk/cdk');
import { IAutoScalingGroup } from './auto-scaling-group';
import { cloudformation } from './autoscaling.generated';
import { LazyDependable } from './util';

/**
* Basic properties for a lifecycle hook
Expand Down Expand Up @@ -95,6 +96,14 @@ export class LifecycleHook extends cdk.Construct implements api.ILifecycleHook {
roleArn: this.role.roleArn,
});

// A LifecycleHook resource is going to do a permissions test upon creation,
// so we have to make sure the role has full permissions before creating the
// lifecycle hook.
// The LazyDependable makes sure we take a dependency on anything that gets
// added to the dependencyElements array, even if it happens after this
// point.
resource.addDependency(new LazyDependable(this.role.dependencyElements));

this.lifecycleHookName = resource.lifecycleHookName;
}
}
Expand Down
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-autoscaling/lib/util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import cdk = require('@aws-cdk/cdk');
/**
* Allow lazy evaluation of a list of dependables
*/
export class LazyDependable implements cdk.IDependable {
constructor(private readonly depList: cdk.IDependable[]) {
}

public get dependencyElements(): cdk.IDependable[] {
return this.depList;
}
}
10 changes: 9 additions & 1 deletion packages/@aws-cdk/aws-autoscaling/test/test.lifecyclehooks.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, haveResource } from '@aws-cdk/assert';
import { expect, haveResource, ResourcePart } from '@aws-cdk/assert';
import autoscaling_api = require('@aws-cdk/aws-autoscaling-api');
import ec2 = require('@aws-cdk/aws-ec2');
import iam = require('@aws-cdk/aws-iam');
Expand Down Expand Up @@ -31,6 +31,14 @@ export = {
NotificationTargetARN: "target:arn",
}));

// Lifecycle Hook has a dependency on the policy object
expect(stack).to(haveResource('AWS::AutoScaling::LifecycleHook', {
DependsOn: [
"ASGLifecycleHookTransitionRole3AAA6BB7",
"ASGLifecycleHookTransitionRoleDefaultPolicy2E50C7DB"
]
}, ResourcePart.CompleteDefinition));

expect(stack).to(haveResource('AWS::IAM::Role', {
AssumeRolePolicyDocument: {
Statement: [
Expand Down

0 comments on commit 698e3a8

Please sign in to comment.