From 7d16f050a0439517694089b1d8bd61c825e595ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carl=20F=C3=BCrstenberg?= <azatoth@gmail.com>
Date: Wed, 23 Nov 2022 00:24:16 +0100
Subject: [PATCH] fix(autoscaling): Allow adding AutoScalingGroup to multiple
 target groups

While this doesn't solve the underlying problem, which will require an
API breakage in `scaleOnRequestCount()` (see
https://github.com/aws/aws-cdk/issues/5667#issuecomment-636657482).

As `scaleOnRequestCount()` seems to be the only part that is affected by
this, we'll only issue a validation error if we have multiple
`targetGroupArns` and `scaleOnRequestCount()` has been called.

closes #5667
---
 .../aws-autoscaling/lib/auto-scaling-group.ts | 18 +++++++--
 .../test/auto-scaling-group.test.ts           | 38 +++++++++++++++++++
 2 files changed, 52 insertions(+), 4 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
index df442537b7232..acb3c8fe7ff76 100644
--- a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
+++ b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
@@ -968,6 +968,7 @@ abstract class AutoScalingGroupBase extends Resource implements IAutoScalingGrou
   public abstract readonly osType: ec2.OperatingSystemType;
   protected albTargetGroup?: elbv2.ApplicationTargetGroup;
   public readonly grantPrincipal: iam.IPrincipal = new iam.UnknownPrincipal({ resource: this });
+  protected hasSetScaleOnRequest: boolean = false;
 
   /**
    * Send a message to either an SQS queue or SNS topic when instances launch or terminate
@@ -1071,6 +1072,7 @@ abstract class AutoScalingGroupBase extends Resource implements IAutoScalingGrou
     });
 
     policy.node.addDependency(this.albTargetGroup.loadBalancerAttached);
+    this.hasSetScaleOnRequest = true;
     return policy;
   }
 
@@ -1370,6 +1372,8 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
     if (props.requireImdsv2) {
       Aspects.of(this).add(new AutoScalingGroupRequireImdsv2Aspect());
     }
+
+    this.node.addValidation({ validate: () => this.validateTargetGroup() });
   }
 
   /**
@@ -1397,10 +1401,6 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
    * Attach to ELBv2 Application Target Group
    */
   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);
     if (targetGroup instanceof elbv2.ApplicationTargetGroup) {
       // Copy onto self if it's a concrete type. We need this for autoscaling
@@ -1747,6 +1747,16 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
       };
     }
   }
+
+
+  private validateTargetGroup(): string[] {
+    const errors = new Array<string>();
+    if (this.hasSetScaleOnRequest && this.targetGroupArns.length > 1) {
+      errors.push('Cannon use multiple target groups if `setScaleOnRequest()` is being used.');
+    }
+
+    return errors;
+  }
 }
 
 /**
diff --git a/packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts b/packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts
index 48062024b9fb4..d73a332d4aab6 100644
--- a/packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts
+++ b/packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts
@@ -2,6 +2,7 @@ import { Annotations, Match, Template } from '@aws-cdk/assertions';
 import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
 import * as ec2 from '@aws-cdk/aws-ec2';
 import { AmazonLinuxCpuType, AmazonLinuxGeneration, AmazonLinuxImage, InstanceType, LaunchTemplate } from '@aws-cdk/aws-ec2';
+import { ApplicationLoadBalancer, ApplicationTargetGroup } from '@aws-cdk/aws-elasticloadbalancingv2';
 import * as iam from '@aws-cdk/aws-iam';
 import * as sns from '@aws-cdk/aws-sns';
 import { testDeprecated } from '@aws-cdk/cdk-build-tools';
@@ -1845,6 +1846,43 @@ describe('auto scaling group', () => {
     })).not.toThrow();
 
   });
+
+  test('Should validate multiple target groups i conjunction with `setScaleOnRequest()`', () => {
+    const stack = new cdk.Stack(undefined, 'MyStack', { env: { region: 'us-east-1', account: '1234' } });
+
+
+    const vpc = mockVpc(stack);
+
+    const alb = new ApplicationLoadBalancer(stack, 'alb', {
+      vpc,
+      internetFacing: true,
+    });
+
+    const listener = alb.addListener('Listener', {
+      port: 80,
+      open: true,
+    });
+    const asg = new autoscaling.AutoScalingGroup(stack, 'MyFleet', {
+      instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO),
+      machineImage: new ec2.AmazonLinuxImage(),
+      vpc,
+    });
+
+    const atg1 = new ApplicationTargetGroup(stack, 'ATG1', { port: 443 });
+    const atg2 = new ApplicationTargetGroup(stack, 'ATG2', { port: 443 });
+
+    listener.addTargetGroups('tgs', { targetGroups: [atg1, atg2] });
+
+    asg.attachToApplicationTargetGroup(atg1);
+    asg.attachToApplicationTargetGroup(atg2);
+
+    expect(asg.node.validate()).toEqual([]);
+
+    asg.scaleOnRequestCount('requests-per-minute', { targetRequestsPerMinute: 60 });
+
+    expect(asg.node.validate()).toContainEqual('Cannon use multiple target groups if `setScaleOnRequest()` is being used.');
+
+  });
 });
 
 function mockVpc(stack: cdk.Stack) {