Skip to content

Commit

Permalink
fix(aws-elasticloadbalancingv2): fix rule dependency (#1170)
Browse files Browse the repository at this point in the history
When ALB listener rules are used, the ordering dependency for services
that require an attachment to a Load Balancer has to be on the
`AWS::ElasticLoadBalancingV2::ListenerRule` object, not on the
`Listener` object (as the current implementation does).

Fixes #1160.

BREAKING CHANGE: `targetGroup.listenerDependency()` has been renamed to
`targetGroup.loadBalancerDependency()`.
  • Loading branch information
rix0rrr committed Nov 14, 2018
1 parent 76a5cc7 commit aeb0f4f
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 23 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ export abstract class BaseService extends cdk.Construct
containerPort: this.taskDefinition.defaultContainer!.containerPort,
});

this.resource.addDependency(targetGroup.listenerDependency());
this.resource.addDependency(targetGroup.loadBalancerDependency());

const targetType = this.taskDefinition.networkMode === NetworkMode.AwsVpc ? elbv2.TargetType.Ip : elbv2.TargetType.Instance;
return { targetType };
Expand All @@ -239,4 +239,4 @@ export abstract class BaseService extends cdk.Construct
/**
* The port range to open up for dynamic port mapping
*/
const EPHEMERAL_PORT_RANGE = new ec2.TcpPortRange(32768, 65535);
const EPHEMERAL_PORT_RANGE = new ec2.TcpPortRange(32768, 65535);
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-elasticloadbalancingv2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,10 @@ group rules.
If your load balancer target requires that the TargetGroup has been
associated with a LoadBalancer before registration can happen (such as is the
case for ECS Services for example), take a resource dependency on
`targetGroup.listenerDependency()` as follows:
`targetGroup.loadBalancerDependency()` as follows:

```ts
// Make sure that the listener has been created, and so the TargetGroup
// has been associated with the LoadBalancer, before 'resource' is created.
resourced.addDependency(targetGroup.listenerDependency());
```
resourced.addDependency(targetGroup.loadBalancerDependency());
```
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export class ApplicationListenerRule extends cdk.Construct implements cdk.IDepen
targetGroupArn: targetGroup.targetGroupArn,
type: 'forward'
});
targetGroup.registerListener(this.listener);
targetGroup.registerListener(this.listener, this);
}

/**
Expand All @@ -142,4 +142,4 @@ export class ApplicationListenerRule extends cdk.Construct implements cdk.IDepen
}
return ret;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,14 @@ export class ApplicationTargetGroup extends BaseTargetGroup {
*
* Don't call this directly. It will be called by listeners.
*/
public registerListener(listener: IApplicationListener) {
public registerListener(listener: IApplicationListener, dependable?: cdk.IDependable) {
// Notify this listener of all connectables that we know about.
// Then remember for new connectables that might get added later.
for (const member of this.connectableMembers) {
listener.registerConnectable(member.connectable, member.portRange);
}
this.listeners.push(listener);
this.dependableListeners.push(listener);
this.loadBalancerAssociationDependencies.push(dependable || listener);
}
}

Expand Down Expand Up @@ -172,18 +172,18 @@ export interface IApplicationTargetGroup extends ITargetGroup {
*
* Don't call this directly. It will be called by listeners.
*/
registerListener(listener: IApplicationListener): void;
registerListener(listener: IApplicationListener, dependable?: cdk.IDependable): void;
}

/**
* An imported application target group
*/
class ImportedApplicationTargetGroup extends BaseImportedTargetGroup implements IApplicationTargetGroup {
public registerListener(_listener: IApplicationListener) {
public registerListener(_listener: IApplicationListener, _dependable?: cdk.IDependable) {
// Nothing to do, we know nothing of our members
}

public listenerDependency(): cdk.IDependable {
public loadBalancerDependency(): cdk.IDependable {
return new LazyDependable([]);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class NetworkTargetGroup extends BaseTargetGroup {
* Don't call this directly. It will be called by listeners.
*/
public registerListener(listener: INetworkListener) {
this.dependableListeners.push(listener);
this.loadBalancerAssociationDependencies.push(listener);
}
}

Expand All @@ -96,7 +96,7 @@ class ImportedNetworkTargetGroup extends BaseImportedTargetGroup implements INet
// Nothing to do, we know nothing of our members
}

public listenerDependency(): cdk.IDependable {
public loadBalancerDependency(): cdk.IDependable {
return new LazyDependable([]);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ export abstract class BaseTargetGroup extends cdk.Construct implements ITargetGr
protected readonly defaultPort: string;

/**
* List of listeners routing to this target group
* List of dependables that need to be depended on to ensure the TargetGroup is associated to a load balancer
*/
protected readonly dependableListeners = new Array<cdk.IDependable>();
protected readonly loadBalancerAssociationDependencies = new Array<cdk.IDependable>();

/**
* Attributes of this target group
Expand Down Expand Up @@ -249,10 +249,10 @@ export abstract class BaseTargetGroup extends cdk.Construct implements ITargetGr
}

/**
* Return an object to depend on the listeners added to this target group
* Return an object to depend on this TargetGroup being attached to a load balancer
*/
public listenerDependency(): cdk.IDependable {
return new LazyDependable(this.dependableListeners);
public loadBalancerDependency(): cdk.IDependable {
return new LazyDependable(this.loadBalancerAssociationDependencies);
}

/**
Expand Down Expand Up @@ -297,7 +297,7 @@ export interface ITargetGroup {
/**
* Return an object to depend on the listeners added to this target group
*/
listenerDependency(): cdk.IDependable;
loadBalancerDependency(): cdk.IDependable;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ export = {

// WHEN
const resource = new cdk.Resource(stack, 'SomeResource', { type: 'Test::Resource' });
resource.addDependency(group.listenerDependency());
resource.addDependency(group.loadBalancerDependency());

loadBalancer.addListener('Listener', {
port: 80,
Expand All @@ -363,5 +363,40 @@ export = {
}, MatchStyle.SUPERSET);

test.done();
}
},

'Can add dependency on ListenerRule via TargetGroup'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.VpcNetwork(stack, 'VPC');
const loadBalancer = new elbv2.ApplicationLoadBalancer(stack, 'LoadBalancer', { vpc });
const group1 = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup1', { vpc, port: 80 });
const group2 = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup2', { vpc, port: 80 });
const listener = loadBalancer.addListener('Listener', {
port: 80,
defaultTargetGroups: [group1]
});

// WHEN
const resource = new cdk.Resource(stack, 'SomeResource', { type: 'Test::Resource' });
resource.addDependency(group2.loadBalancerDependency());

listener.addTargetGroups('SecondGroup', {
pathPattern: '/bla',
priority: 10,
targetGroups: [group2]
});

// THEN
expect(stack).toMatch({
Resources: {
SomeResource: {
Type: "Test::Resource",
DependsOn: ["LoadBalancerListenerSecondGroupRuleF5FDC196"]
}
}
}, MatchStyle.SUPERSET);

test.done();
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export = {
const resource = new cdk.Resource(stack, 'MyResource', {
type: 'SomeResource',
});
resource.addDependency(group.listenerDependency());
resource.addDependency(group.loadBalancerDependency());

// THEN
expect(stack).toMatch({
Expand Down

0 comments on commit aeb0f4f

Please sign in to comment.