From aeb0f4f726301b2ff4aa472740a6864a80bf4c29 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 14 Nov 2018 17:29:06 +0100 Subject: [PATCH] fix(aws-elasticloadbalancingv2): fix rule dependency (#1170) 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()`. --- .../@aws-cdk/aws-ecs/lib/base/base-service.ts | 4 +- .../aws-elasticloadbalancingv2/README.md | 6 +-- .../lib/alb/application-listener-rule.ts | 4 +- .../lib/alb/application-target-group.ts | 10 ++--- .../lib/nlb/network-target-group.ts | 4 +- .../lib/shared/base-target-group.ts | 12 +++--- .../test/alb/test.listener.ts | 39 ++++++++++++++++++- .../test/nlb/test.listener.ts | 2 +- 8 files changed, 58 insertions(+), 23 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts index 2883955def9f6..19704b50b0b65 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts @@ -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 }; @@ -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); \ No newline at end of file +const EPHEMERAL_PORT_RANGE = new ec2.TcpPortRange(32768, 65535); diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/README.md b/packages/@aws-cdk/aws-elasticloadbalancingv2/README.md index f199d83181da8..13d4029922c7e 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/README.md +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/README.md @@ -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()); -``` \ No newline at end of file +resourced.addDependency(targetGroup.loadBalancerDependency()); +``` diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.ts index 86985b5bb0f1c..c8c301bb11c74 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.ts @@ -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); } /** @@ -142,4 +142,4 @@ export class ApplicationListenerRule extends cdk.Construct implements cdk.IDepen } return ret; } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts index 2454f856af895..c0e419f83653b 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts @@ -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); } } @@ -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([]); } } diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-target-group.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-target-group.ts index 2d51b48855709..65060e9f8a647 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-target-group.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-target-group.ts @@ -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); } } @@ -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([]); } } diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts index 4037086bbdbdc..7a3aaf83df5fd 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts @@ -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(); + protected readonly loadBalancerAssociationDependencies = new Array(); /** * Attributes of this target group @@ -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); } /** @@ -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; } /** diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts index 964221b3112aa..7f148005cbb2e 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts @@ -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, @@ -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(); + }, }; diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/test.listener.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/test.listener.ts index 4091cf386323d..f96e0bca0b131 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/test.listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/test.listener.ts @@ -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({