From 06cdaacbd3385df51e4632aa8d943ce647855e82 Mon Sep 17 00:00:00 2001 From: shinebleu Date: Mon, 9 Dec 2024 13:00:32 -0500 Subject: [PATCH] fix(autoscaling): `AutoScalingGroup` requireImdsv2 with launchTemplate or mixedInstancesPolicy throws unclear error (#32220) ### Issue #27586 Fixes #27586 The above issue is already marked as closed, but there is some discussion in it about an additional edge case that this pull request fixes. ### Reason for this change When defining an AutoScalingGroup with both the `requireImdsv2` prop and the `launchTemplate` prop, the error message is not clear about the problem. ``` // example asg definition const asg = new AutoScalingGroup(this, "myasg", { vpc, launchTemplate: lt, requireImdsv2: true, }); ``` ``` TypeError: Cannot read properties of undefined (reading 'node') at AutoScalingGroupRequireImdsv2Aspect.visit (~/git/cdkApp2/node_modules/aws-cdk-lib/aws-autoscaling/lib/aspects/require-imdsv2-aspect.js:1:715) at recurse (~/git/cdkApp2/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:2:2236) at recurse (~/git/cdkApp2/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:2:2600) at recurse (~/git/cdkApp2/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:2:2600) at invokeAspects (~/git/cdkApp2/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:2:1860) at synthesize (~/git/cdkApp2/node_modules/aws-cdk-lib/core/lib/private/synthesis.js:1:1473) at App.synth (~/git/cdkApp2/node_modules/aws-cdk-lib/core/lib/stage.js:1:2382) at process. (~/git/cdkApp2/node_modules/aws-cdk-lib/core/lib/app.js:1:1767) at Object.onceWrapper (node:events:632:26) at process.emit (node:events:517:28) ``` It is not clear from the error that `requireImdsv2` should be set in the provided launchTemplate itself rather than the AutoScalingGroup. The error occurs because setting `requireImdsv2` on the AutoScalingGroup adds the aspect AutoScalingGroupRequireImdsv2Aspect to it, which expects there to be a child node called either `'LaunchConfig'` or `'LaunchTemplate'` depending on a feature flag. This child node is only set in the AutoScalingGroup when neither `launchTemplate` nor `mixedInstancesPolicy` props are provided. ### Description of changes Add the `requireImdsv2` prop to the `verifyNoLaunchConfigPropIsGiven` method, which throws errors when certains props are set at the same time as `launchTemplate` or `mixedInstancesPolicy`. ### Description of how you validated changes - Added a unit test - Confirmed the new error message works in a sample cdk app ``` Error: Setting 'requireImdsv2' must not be set when 'launchTemplate' or 'mixedInstancesPolicy' is set at AutoScalingGroup.verifyNoLaunchConfigPropIsGiven (~/git/aws-cdk/packages/aws-cdk-lib/aws-autoscaling/lib/auto-scaling-group.ts:1762:13) at new AutoScalingGroup (~/git/aws-cdk/packages/aws-cdk-lib/aws-autoscaling/lib/auto-scaling-group.ts:1333:12) at new CdkAppStack (~/git/cdkApp/lib/cdk_app-stack.ts:31:17) at Object. (~/git/cdkApp/bin/cdk_app.ts:6:1) at Module._compile (node:internal/modules/cjs/loader:1256:14) at Module.m._compile (~/git/cdkApp/node_modules/ts-node/src/index.ts:1618:23) at Module._extensions..js (node:internal/modules/cjs/loader:1310:10) at Object.require.extensions. [as .ts] (~/git/cdkApp/node_modules/ts-node/src/index.ts:1621:12) at Module.load (node:internal/modules/cjs/loader:1119:32) at Function.Module._load (node:internal/modules/cjs/loader:960:12) ``` ### Checklist - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *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 | 3 +++ .../test/auto-scaling-group.test.ts | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/packages/aws-cdk-lib/aws-autoscaling/lib/auto-scaling-group.ts b/packages/aws-cdk-lib/aws-autoscaling/lib/auto-scaling-group.ts index e38c2456ebcb5..e0105d02ce4dd 100644 --- a/packages/aws-cdk-lib/aws-autoscaling/lib/auto-scaling-group.ts +++ b/packages/aws-cdk-lib/aws-autoscaling/lib/auto-scaling-group.ts @@ -1759,6 +1759,9 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements if (props.blockDevices) { throw new Error('Setting \'blockDevices\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); } + if (props.requireImdsv2) { + throw new Error('Setting \'requireImdsv2\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); + } } /** diff --git a/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts b/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts index 1ccbdd5f423ba..2b78904dd300c 100644 --- a/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts +++ b/packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts @@ -2947,6 +2947,26 @@ describe('InstanceMaintenancePolicy', () => { }); }).toThrow(/The difference between minHealthyPercentage and maxHealthyPercentage cannot be greater than 100, got 200/); }); + + test('throws if requireImdsv2 set when launchTemplate is set', () => { + // GIVEN + const stack = new cdk.Stack(); + stack.node.setContext(AUTOSCALING_GENERATE_LAUNCH_TEMPLATE, true); + const vpc = mockVpc(stack); + const lt = LaunchTemplate.fromLaunchTemplateAttributes(stack, 'imported-lt', { + launchTemplateId: 'test-lt-id', + versionNumber: '0', + }); + + // THEN + expect(() => { + new autoscaling.AutoScalingGroup(stack, 'MyFleet', { + vpc, + launchTemplate: lt, + requireImdsv2: true, + }); + }).toThrow(/Setting \'requireImdsv2\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set/); + }); }); function mockSecurityGroup(stack: cdk.Stack) {