From 594ed44d4f3532d829c3485778cccd1ef8754ebd Mon Sep 17 00:00:00 2001 From: Peter Woodworth <44349620+peterwoodworth@users.noreply.github.com> Date: Thu, 25 Aug 2022 09:28:34 -0700 Subject: [PATCH] fix(autoscaling): error not thrown when associatePublicIpAddress is set to false when specifying launchTemplate (#21714) Setting `associatePublicIpAddress` prop when also setting `launchTemplate` prop on a new ASG is supposed to throw an error, but doesn't due to `associatePublicIpAddress` being a boolean, and the error message not triggering if a falsy value is used for the prop. This PR will ensure an error is thrown when `associatePublicIpAddress` is set, even when the value passed is falsy. It also updates the documentation on all props which conflict with `launchTemplate` or `mixedInstancesPolicy` fixes: #21576 ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-autoscaling/lib/auto-scaling-group.ts | 22 ++++++++++++++----- .../test/auto-scaling-group.test.ts | 10 ++++++++- 2 files changed, 25 insertions(+), 7 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 3a5061fc0ff58..0c5006c29f514 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts @@ -80,6 +80,8 @@ export interface CommonAutoScalingGroupProps { /** * Name of SSH keypair to grant access to instances * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @default - No SSH access will be possible. */ readonly keyName?: string; @@ -190,6 +192,8 @@ export interface CommonAutoScalingGroupProps { * Whether instances in the Auto Scaling Group should have public * IP addresses associated with them. * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @default - Use subnet setting. */ readonly associatePublicIpAddress?: boolean; @@ -198,6 +202,8 @@ export interface CommonAutoScalingGroupProps { * The maximum hourly price (in USD) to be paid for any Spot Instance launched to fulfill the request. Spot Instances are * launched when the price you specify exceeds the current Spot market price. * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @default none */ readonly spotPrice?: string; @@ -217,6 +223,8 @@ export interface CommonAutoScalingGroupProps { * You can use block device mappings to specify additional EBS volumes or * instance store volumes to attach to an instance when it is launched. * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/block-device-mapping-concepts.html * * @default - Uses the block device mapping of the AMI @@ -243,6 +251,8 @@ export interface CommonAutoScalingGroupProps { * When detailed monitoring is enabled, Amazon CloudWatch generates metrics every minute and your account * is charged a fee. When you disable detailed monitoring, CloudWatch generates metrics every 5 minutes. * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @see https://docs.aws.amazon.com/autoscaling/latest/userguide/as-instance-monitoring.html#enable-as-instance-metrics * * @default - Monitoring.DETAILED @@ -543,7 +553,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { /** * Type of instance to launch * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @default - Do not provide any instance type */ @@ -552,7 +562,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { /** * AMI to launch * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @default - Do not provide any machine image */ @@ -561,7 +571,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { /** * Security group to launch the instances in. * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @default - A SecurityGroup will be created if none is specified. */ @@ -572,7 +582,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { * * The UserData may still be mutated after creation. * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @default - A UserData object appropriate for the MachineImage's * Operating System is created. @@ -584,7 +594,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { * * The role must be assumable by the service principal `ec2.amazonaws.com`: * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @example * @@ -1523,7 +1533,7 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements if (props.instanceMonitoring) { throw new Error('Setting \'instanceMonitoring\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); } - if (props.associatePublicIpAddress) { + if (props.associatePublicIpAddress !== undefined) { throw new Error('Setting \'associatePublicIpAddress\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); } if (props.spotPrice) { 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 cde7f73e86c31..0f836518ea204 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 @@ -1525,6 +1525,7 @@ describe('auto scaling group', () => { launchTemplateId: 'test-lt-id', versionNumber: '0', }); + const vpc = mockVpc(stack); // THEN expect(() => { @@ -1535,9 +1536,16 @@ describe('auto scaling group', () => { generation: AmazonLinuxGeneration.AMAZON_LINUX_2, cpuType: AmazonLinuxCpuType.X86_64, }), - vpc: mockVpc(stack), + vpc, }); }).toThrow('Setting \'machineImage\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); + expect(() => { + new autoscaling.AutoScalingGroup(stack, 'imported-lt-asg-2', { + launchTemplate: lt, + associatePublicIpAddress: true, + vpc, + }); + }).toThrow('Setting \'associatePublicIpAddress\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); }); test('Cannot specify Launch Template without instance type', () => {