Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(aws-autoscaling): Add hook ordering dependency #1218

Merged
merged 2 commits into from
Nov 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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