refactor(elbv2): make load balancer targets bind to interfaces (#4132)

The `attachToApplicationTargetGroup()` (and similar for the NLB) methods of the load balancer target interfaces took a concrete TargetGroup class. This deviates from established CDK standards, and prevents their use with imported TargetGroups. Bring the code in line with expectations. Fixes #4121.
aws · Sep 25, 2019 · 1180c1b · 1180c1b · rob-gonz · Nov 27, 2019
1 parent 602bac2
commit 1180c1b
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 20 deletions.
diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
@@ -294,7 +294,7 @@ abstract class AutoScalingGroupBase extends Resource implements IAutoScalingGrou
    */
   public scaleOnRequestCount(id: string, props: RequestCountScalingProps): TargetTrackingScalingPolicy {
     if (this.albTargetGroup === undefined) {
-      throw new Error('Attach the AutoScalingGroup to an Application Load Balancer before calling scaleOnRequestCount()');
+      throw new Error('Attach the AutoScalingGroup to a non-imported Application Load Balancer before calling scaleOnRequestCount()');
     }
 
     const resourceLabel = `${this.albTargetGroup.firstLoadBalancerFullName}/${this.albTargetGroup.targetGroupFullName}`;
@@ -545,17 +545,26 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
   /**
    * Attach to ELBv2 Application Target Group
    */
-  public attachToApplicationTargetGroup(targetGroup: elbv2.ApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToApplicationTargetGroup(targetGroup: elbv2.IApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
+    if (this.albTargetGroup !== undefined) {
+      throw new Error('Cannot add AutoScalingGroup to 2nd Target Group');
+    }
+
     this.targetGroupArns.push(targetGroup.targetGroupArn);
-    this.albTargetGroup = targetGroup;
+    if (targetGroup instanceof elbv2.ApplicationTargetGroup) {
+      // Copy onto self if it's a concrete type. We need this for autoscaling
+      // based on request count, which we cannot do with an imported TargetGroup.
+      this.albTargetGroup = targetGroup;
+    }
+
     targetGroup.registerConnectable(this);
     return { targetType: elbv2.TargetType.INSTANCE };
   }
 
   /**
    * Attach to ELBv2 Application Target Group
    */
-  public attachToNetworkTargetGroup(targetGroup: elbv2.NetworkTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToNetworkTargetGroup(targetGroup: elbv2.INetworkTargetGroup): elbv2.LoadBalancerTargetProps {
     this.targetGroupArns.push(targetGroup.targetGroupArn);
     return { targetType: elbv2.TargetType.INSTANCE };
   }

diff --git a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
@@ -227,7 +227,7 @@ export abstract class BaseService extends Resource
    * Don't call this function directly. Instead, call `listener.addTargets()`
    * to add this service to a load balancer.
    */
-  public attachToApplicationTargetGroup(targetGroup: elbv2.ApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToApplicationTargetGroup(targetGroup: elbv2.IApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
     return this.defaultLoadBalancerTarget.attachToApplicationTargetGroup(targetGroup);
   }
 
@@ -282,7 +282,7 @@ export abstract class BaseService extends Resource
    * Don't call this function directly. Instead, call `listener.addTargets()`
    * to add this service to a load balancer.
    */
-  public attachToNetworkTargetGroup(targetGroup: elbv2.NetworkTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToNetworkTargetGroup(targetGroup: elbv2.INetworkTargetGroup): elbv2.LoadBalancerTargetProps {
     return this.defaultLoadBalancerTarget.attachToNetworkTargetGroup(targetGroup);
   }
 

diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2-targets/lib/instance-target.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2-targets/lib/instance-target.ts
@@ -23,7 +23,7 @@ export class InstanceIdTarget implements elbv2.IApplicationLoadBalancerTarget, e
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToApplicationTargetGroup(targetGroup: elbv2.ApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToApplicationTargetGroup(targetGroup: elbv2.IApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
     return this.attach(targetGroup);
   }
 
@@ -33,7 +33,7 @@ export class InstanceIdTarget implements elbv2.IApplicationLoadBalancerTarget, e
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToNetworkTargetGroup(targetGroup: elbv2.NetworkTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToNetworkTargetGroup(targetGroup: elbv2.INetworkTargetGroup): elbv2.LoadBalancerTargetProps {
     return this.attach(targetGroup);
   }
 

diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2-targets/lib/ip-target.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2-targets/lib/ip-target.ts
@@ -43,7 +43,7 @@ export class IpTarget implements elbv2.IApplicationLoadBalancerTarget, elbv2.INe
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToApplicationTargetGroup(targetGroup: elbv2.ApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToApplicationTargetGroup(targetGroup: elbv2.IApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
     return this.attach(targetGroup);
   }
 
@@ -53,7 +53,7 @@ export class IpTarget implements elbv2.IApplicationLoadBalancerTarget, elbv2.INe
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToNetworkTargetGroup(targetGroup: elbv2.NetworkTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToNetworkTargetGroup(targetGroup: elbv2.INetworkTargetGroup): elbv2.LoadBalancerTargetProps {
     return this.attach(targetGroup);
   }
 

diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2-targets/lib/lambda-target.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2-targets/lib/lambda-target.ts
@@ -17,7 +17,7 @@ export class LambdaTarget implements elbv2.IApplicationLoadBalancerTarget {
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToApplicationTargetGroup(targetGroup: elbv2.ApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToApplicationTargetGroup(targetGroup: elbv2.IApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
     this.fn.grantInvoke(new iam.ServicePrincipal('elasticloadbalancing.amazonaws.com'));
     return this.attach(targetGroup);
   }
@@ -28,7 +28,7 @@ export class LambdaTarget implements elbv2.IApplicationLoadBalancerTarget {
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToNetworkTargetGroup(targetGroup: elbv2.NetworkTargetGroup): elbv2.LoadBalancerTargetProps {
+  public attachToNetworkTargetGroup(targetGroup: elbv2.INetworkTargetGroup): elbv2.LoadBalancerTargetProps {
     this.fn.grantInvoke(new iam.ServicePrincipal('elasticloadbalancing.amazonaws.com'));
     return this.attach(targetGroup);
   }

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
@@ -338,6 +338,13 @@ export interface IApplicationTargetGroup extends ITargetGroup {
    * Don't call this directly. It will be called by listeners.
    */
   registerListener(listener: IApplicationListener, associatingConstruct?: IConstruct): void;
+
+  /**
+   * Register a connectable as a member of this target group.
+   *
+   * Don't call this directly. It will be called by load balancing targets.
+   */
+  registerConnectable(connectable: ec2.IConnectable, portRange?: ec2.Port): void;
 }
 
 /**
@@ -346,6 +353,11 @@ export interface IApplicationTargetGroup extends ITargetGroup {
 class ImportedApplicationTargetGroup extends ImportedTargetGroupBase implements IApplicationTargetGroup {
   public registerListener(_listener: IApplicationListener, _associatingConstruct?: IConstruct) {
     // Nothing to do, we know nothing of our members
+    this.node.addWarning(`Cannot register listener on imported target group -- security groups might need to be updated manually`);
+  }
+
+  public registerConnectable(_connectable: ec2.IConnectable, _portRange?: ec2.Port | undefined): void {
+    this.node.addWarning(`Cannot register connectable on imported target group -- security groups might need to be updated manually`);
   }
 }
 
@@ -359,5 +371,5 @@ export interface IApplicationLoadBalancerTarget {
    * May return JSON to directly add to the [Targets] list, or return undefined
    * if the target will register itself with the load balancer.
    */
-  attachToApplicationTargetGroup(targetGroup: ApplicationTargetGroup): LoadBalancerTargetProps;
+  attachToApplicationTargetGroup(targetGroup: IApplicationTargetGroup): LoadBalancerTargetProps;
 }
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
@@ -124,5 +124,5 @@ export interface INetworkLoadBalancerTarget {
    * May return JSON to directly add to the [Targets] list, or return undefined
    * if the target will register itself with the load balancer.
    */
-  attachToNetworkTargetGroup(targetGroup: NetworkTargetGroup): LoadBalancerTargetProps;
+  attachToNetworkTargetGroup(targetGroup: INetworkTargetGroup): LoadBalancerTargetProps;
 }
diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/load-balancer-targets.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/load-balancer-targets.ts
@@ -1,5 +1,5 @@
-import { ApplicationTargetGroup, IApplicationLoadBalancerTarget } from "../alb/application-target-group";
-import { INetworkLoadBalancerTarget, NetworkTargetGroup } from "../nlb/network-target-group";
+import { IApplicationLoadBalancerTarget, IApplicationTargetGroup } from "../alb/application-target-group";
+import { INetworkLoadBalancerTarget, INetworkTargetGroup } from "../nlb/network-target-group";
 import { ITargetGroup, LoadBalancerTargetProps } from "./base-target-group";
 import { TargetType } from "./enums";
 
@@ -27,7 +27,7 @@ export class InstanceTarget implements IApplicationLoadBalancerTarget, INetworkL
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToApplicationTargetGroup(targetGroup: ApplicationTargetGroup): LoadBalancerTargetProps {
+  public attachToApplicationTargetGroup(targetGroup: IApplicationTargetGroup): LoadBalancerTargetProps {
     return this.attach(targetGroup);
   }
 
@@ -37,7 +37,7 @@ export class InstanceTarget implements IApplicationLoadBalancerTarget, INetworkL
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToNetworkTargetGroup(targetGroup: NetworkTargetGroup): LoadBalancerTargetProps {
+  public attachToNetworkTargetGroup(targetGroup: INetworkTargetGroup): LoadBalancerTargetProps {
     return this.attach(targetGroup);
   }
 
@@ -94,7 +94,7 @@ export class IpTarget implements IApplicationLoadBalancerTarget, INetworkLoadBal
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToApplicationTargetGroup(targetGroup: ApplicationTargetGroup): LoadBalancerTargetProps {
+  public attachToApplicationTargetGroup(targetGroup: IApplicationTargetGroup): LoadBalancerTargetProps {
     return this.attach(targetGroup);
   }
 
@@ -104,7 +104,7 @@ export class IpTarget implements IApplicationLoadBalancerTarget, INetworkLoadBal
    * Don't call this, it is called automatically when you add the target to a
    * load balancer.
    */
-  public attachToNetworkTargetGroup(targetGroup: NetworkTargetGroup): LoadBalancerTargetProps {
+  public attachToNetworkTargetGroup(targetGroup: INetworkTargetGroup): LoadBalancerTargetProps {
     return this.attach(targetGroup);
   }