Skip to content

Commit

Permalink
fix(aws-autoscaling): Add hook ordering dependency (#1218)
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 21, 2018
1 parent d3b8448 commit 7e6ad84
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 3 deletions.
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));

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 dependableSource: cdk.IDependable) {
}

public get dependencyElements(): cdk.IDependable[] {
return this.dependableSource.dependencyElements;
}
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,11 @@
"Arn"
]
}
}
},
"DependsOn": [
"EcsClusterDefaultAutoScalingGroupLifecycleHookDrainHookRoleA38EC83B",
"EcsClusterDefaultAutoScalingGroupLifecycleHookDrainHookRoleDefaultPolicy75002F88"
]
},
"TaskDefTaskRole1EDB4A67": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,11 @@
"Arn"
]
}
}
},
"DependsOn": [
"EcsClusterDefaultAutoScalingGroupLifecycleHookDrainHookRoleA38EC83B",
"EcsClusterDefaultAutoScalingGroupLifecycleHookDrainHookRoleDefaultPolicy75002F88"
]
},
"TaskDefTaskRole1EDB4A67": {
"Type": "AWS::IAM::Role",
Expand Down

0 comments on commit 7e6ad84

Please sign in to comment.