fix(aws-elasticloadbalancingv2): fix rule dependency

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.
aws · Nov 14, 2018 · 437584a · 437584a
1 parent 0cb1adf
commit 437584a
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 16 deletions.
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
@@ -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;
   }
-}
+}
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
@@ -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
@@ -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
@@ -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
@@ -250,9 +250,20 @@ export abstract class BaseTargetGroup extends cdk.Construct implements ITargetGr
 
   /**
    * Return an object to depend on the listeners added to this target group
+   *
+   * @deprecated Use 'loadBalancerDependency' instead.
    */
   public listenerDependency(): cdk.IDependable {
-    return new LazyDependable(this.dependableListeners);
+    return this.loadBalancerDependency();
+  }
+
+  /**
+   * Return an object to depend on this TargetGroup being attached to a load balancer
+   *
+   * @deprecated Use 'loadBalancerDependency' instead.
+   */
+  public loadBalancerDependency(): cdk.IDependable {
+    return new LazyDependable(this.loadBalancerAssociationDependencies);
   }
 
   /**
@@ -297,7 +308,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
@@ -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
@@ -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({