fix(aws-elasticloadbalancingv2): target group metrics (#1226)

rix0rrr · web-flow · commit de488df5406a · 2018-11-21T11:39:16.000+01:00
TargetGroup metrics used to use ${TargetGroup.LoadBalancerArns} to find the load balancer's full name, but that introduces a deployment-time ordering dependency on the creation of the Listener object. Instead, use the Listener ARN to get the load balancer name. We now have an ordering requirement in the CDK code but that can be detected early and solved by the user. Fixes #1213.
diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json
@@ -495,15 +495,7 @@
                         "Fn::Split": [
                           "/",
                           {
-                            "Fn::Select": [
-                              0,
-                              {
-                                "Fn::GetAtt": [
-                                  "LBListenerTargetGroupF04FCF6D",
-                                  "LoadBalancerArns"
-                                ]
-                              }
-                            ]
+                            "Ref": "LBListener49E825B4"
                           }
                         ]
                       }
@@ -517,15 +509,7 @@
                         "Fn::Split": [
                           "/",
                           {
-                            "Fn::Select": [
-                              0,
-                              {
-                                "Fn::GetAtt": [
-                                  "LBListenerTargetGroupF04FCF6D",
-                                  "LoadBalancerArns"
-                                ]
-                              }
-                            ]
+                            "Ref": "LBListener49E825B4"
                           }
                         ]
                       }
@@ -539,15 +523,7 @@
                         "Fn::Split": [
                           "/",
                           {
-                            "Fn::Select": [
-                              0,
-                              {
-                                "Fn::GetAtt": [
-                                  "LBListenerTargetGroupF04FCF6D",
-                                  "LoadBalancerArns"
-                                ]
-                              }
-                            ]
+                            "Ref": "LBListener49E825B4"
                           }
                         ]
                       }
diff --git a/packages/@aws-cdk/aws-autoscaling/test/test.scaling.ts b/packages/@aws-cdk/aws-autoscaling/test/test.scaling.ts
@@ -94,10 +94,7 @@ export = {
       const arnParts = {
         "Fn::Split": [
           "/",
-          { "Fn::Select": [
-            0,
-            { "Fn::GetAtt": [ "ALBListenerTargetsGroup01D7716A", "LoadBalancerArns" ] }
-          ]}
+          { Ref: "ALBListener3B99FF85" }
         ]};
 
       expect(stack).to(haveResource('AWS::AutoScaling::ScalingPolicy', {
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
@@ -1,7 +1,8 @@
 import cloudwatch = require('@aws-cdk/aws-cloudwatch');
 import ec2 = require('@aws-cdk/aws-ec2');
 import cdk = require('@aws-cdk/cdk');
-import { BaseTargetGroup, BaseTargetGroupProps, ITargetGroup, LoadBalancerTargetProps, TargetGroupRefProps } from '../shared/base-target-group';
+import { BaseTargetGroup, BaseTargetGroupProps, ITargetGroup, loadBalancerNameFromListenerArn,
+         LoadBalancerTargetProps, TargetGroupRefProps } from '../shared/base-target-group';
 import { ApplicationProtocol } from '../shared/enums';
 import { BaseImportedTargetGroup } from '../shared/imported';
 import { determineProtocolAndPort, LazyDependable } from '../shared/util';
@@ -149,6 +150,16 @@ export class ApplicationTargetGroup extends BaseTargetGroup {
     this.loadBalancerAssociationDependencies.push(dependable || listener);
   }
 
+  /**
+   * Full name of first load balancer
+   */
+  public get firstLoadBalancerFullName(): string {
+    if (this.listeners.length === 0) {
+      throw new Error('The TargetGroup needs to be attached to a LoadBalancer before you can call this method');
+    }
+    return loadBalancerNameFromListenerArn(this.listeners[0].listenerArn);
+  }
+
   /**
    * Return the given named metric for this Application Load Balancer Target Group
    *
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
@@ -1,5 +1,6 @@
 import cdk = require('@aws-cdk/cdk');
-import { BaseTargetGroup, BaseTargetGroupProps, ITargetGroup, LoadBalancerTargetProps, TargetGroupRefProps } from '../shared/base-target-group';
+import { BaseTargetGroup, BaseTargetGroupProps, ITargetGroup, loadBalancerNameFromListenerArn,
+         LoadBalancerTargetProps, TargetGroupRefProps } from '../shared/base-target-group';
 import { Protocol } from '../shared/enums';
 import { BaseImportedTargetGroup } from '../shared/imported';
 import { LazyDependable } from '../shared/util';
@@ -42,12 +43,16 @@ export class NetworkTargetGroup extends BaseTargetGroup {
     return new ImportedNetworkTargetGroup(parent, id, props);
   }
 
+  private readonly listeners: INetworkListener[];
+
   constructor(parent: cdk.Construct, id: string, props: NetworkTargetGroupProps) {
     super(parent, id, props, {
       protocol: Protocol.Tcp,
       port: props.port,
     });
 
+    this.listeners = [];
+
     if (props.proxyProtocolV2) {
       this.setAttribute('proxy_protocol_v2.enabled', 'true');
     }
@@ -72,6 +77,17 @@ export class NetworkTargetGroup extends BaseTargetGroup {
    */
   public registerListener(listener: INetworkListener) {
     this.loadBalancerAssociationDependencies.push(listener);
+    this.listeners.push(listener);
+  }
+
+  /**
+   * Full name of first load balancer
+   */
+  public get firstLoadBalancerFullName(): string {
+    if (this.listeners.length === 0) {
+      throw new Error('The TargetGroup needs to be attached to a LoadBalancer before you can call this method');
+    }
+    return loadBalancerNameFromListenerArn(this.listeners[0].listenerArn);
   }
 }
 
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
@@ -160,7 +160,7 @@ export abstract class BaseTargetGroup extends cdk.Construct implements ITargetGr
    *
    * @example app/my-load-balancer/123456789
    */
-  public readonly firstLoadBalancerFullName: string;
+  public abstract readonly firstLoadBalancerFullName: string;
 
   /**
    * Health check for the members of this target group
@@ -240,11 +240,6 @@ export abstract class BaseTargetGroup extends cdk.Construct implements ITargetGr
     this.loadBalancerArns = this.resource.targetGroupLoadBalancerArns.toString();
     this.targetGroupName = this.resource.targetGroupName;
     this.defaultPort = `${additionalProps.port}`;
-
-    const firstLoadBalancerArn = new cdk.FnSelect(0, this.targetGroupLoadBalancerArns);
-    // arn:aws:elasticloadbalancing:us-west-2:123456789012:loadbalancer/app/my-internal-load-balancer/50dc6c495c0c9188
-    const arnParts = new cdk.FnSplit('/', firstLoadBalancerArn);
-    this.firstLoadBalancerFullName = `${new cdk.FnSelect(1, arnParts)}/${new cdk.FnSelect(2, arnParts)}/${new cdk.FnSelect(3, arnParts)}`;
   }
 
   /**
@@ -365,3 +360,19 @@ export interface LoadBalancerTargetProps {
    */
   targetJson?: any;
 }
+
+/**
+ * Extract the full load balancer name (used for metrics) from the listener ARN:
+ *
+ * Turns
+ *
+ *     arn:aws:elasticloadbalancing:us-west-2:123456789012:listener/app/my-load-balancer/50dc6c495c0c9188/f2f7dc8efc522ab2
+ *
+ * Into
+ *
+ *     app/my-load-balancer/50dc6c495c0c9188
+ */
+export function loadBalancerNameFromListenerArn(listenerArn: string) {
+    const arnParts = new cdk.FnSplit('/', listenerArn);
+    return `${new cdk.FnSelect(1, arnParts)}/${new cdk.FnSelect(2, arnParts)}/${new cdk.FnSelect(3, arnParts)}`;
+}
diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts
@@ -389,7 +389,12 @@ export = {
     // GIVEN
     const stack = new cdk.Stack();
     const vpc = new ec2.VpcNetwork(stack, 'VPC');
+    const lb = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc });
     const group = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', { vpc, port: 80 });
+    lb.addListener('SomeListener', {
+      port: 80,
+      defaultTargetGroups: [group]
+    });
 
     // WHEN
     const metrics = [];
@@ -404,16 +409,17 @@ export = {
 
     for (const metric of metrics) {
       test.equal('AWS/ApplicationELB', metric.namespace);
-      const firstArn = { "Fn::Select": [0, { "Fn::GetAtt": ["TargetGroup3D7CD9B8", "LoadBalancerArns"] }] };
+      const loadBalancerArn = { Ref: "LBSomeListenerCA01F1A0" };
+
       test.deepEqual(cdk.resolve(metric.dimensions), {
          TargetGroup: { 'Fn::GetAtt': [ 'TargetGroup3D7CD9B8', 'TargetGroupFullName' ] },
          LoadBalancer: { 'Fn::Join':
             [ '',
-              [ { 'Fn::Select': [ 1, { 'Fn::Split': [ '/', firstArn ] } ] },
+              [ { 'Fn::Select': [ 1, { 'Fn::Split': [ '/', loadBalancerArn ] } ] },
                 '/',
-                { 'Fn::Select': [ 2, { 'Fn::Split': [ '/', firstArn ] } ] },
+                { 'Fn::Select': [ 2, { 'Fn::Split': [ '/', loadBalancerArn ] } ] },
                 '/',
-                { 'Fn::Select': [ 3, { 'Fn::Split': [ '/', firstArn ] } ] }
+                { 'Fn::Select': [ 3, { 'Fn::Split': [ '/', loadBalancerArn ] } ] }
               ]
             ]
          }
diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json
diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.ts

Original file line number	Diff line number	Diff line change
`@@ -495,15 +495,7 @@`
`495`	`495`	`"Fn::Split": [`
`496`	`496`	`"/",`
`497`	`497`	`{`
`498`		`- "Fn::Select": [`
`499`		`- 0,`
`500`		`- {`
`501`		`- "Fn::GetAtt": [`
`502`		`- "LBListenerTargetGroupF04FCF6D",`
`503`		`- "LoadBalancerArns"`
`504`		`- ]`
`505`		`- }`
`506`		`- ]`
	`498`	`+ "Ref": "LBListener49E825B4"`
`507`	`499`	`}`
`508`	`500`	`]`
`509`	`501`	`}`
`@@ -517,15 +509,7 @@`
`517`	`509`	`"Fn::Split": [`
`518`	`510`	`"/",`
`519`	`511`	`{`
`520`		`- "Fn::Select": [`
`521`		`- 0,`
`522`		`- {`
`523`		`- "Fn::GetAtt": [`
`524`		`- "LBListenerTargetGroupF04FCF6D",`
`525`		`- "LoadBalancerArns"`
`526`		`- ]`
`527`		`- }`
`528`		`- ]`
	`512`	`+ "Ref": "LBListener49E825B4"`
`529`	`513`	`}`
`530`	`514`	`]`
`531`	`515`	`}`
`@@ -539,15 +523,7 @@`
`539`	`523`	`"Fn::Split": [`
`540`	`524`	`"/",`
`541`	`525`	`{`
`542`		`- "Fn::Select": [`
`543`		`- 0,`
`544`		`- {`
`545`		`- "Fn::GetAtt": [`
`546`		`- "LBListenerTargetGroupF04FCF6D",`
`547`		`- "LoadBalancerArns"`
`548`		`- ]`
`549`		`- }`
`550`		`- ]`
	`526`	`+ "Ref": "LBListener49E825B4"`
`551`	`527`	`}`
`552`	`528`	`]`
`553`	`529`	`}`