(CfnInclude): CfnInclude omits aws_autoscaling. UpdatePolicy that dont exists in the CDK (MinActiveInstancesPercent)

[NetaNir]

Describe the bug

When CfnInclude references a template that has an UpdatePolicy for autoscaling groups it silently omits the MinActiveInstancesPercent property because the code in cfn-parse.ts does not include the property:

      return undefinedIfAllValuesAreEmpty({
        maxBatchSize: FromCloudFormation.getNumber(p.MaxBatchSize).value,
        minInstancesInService: FromCloudFormation.getNumber(p.MinInstancesInService).value,
        minSuccessfulInstancesPercent: FromCloudFormation.getNumber(p.MinSuccessfulInstancesPercent).value,
        pauseTime: FromCloudFormation.getString(p.PauseTime).value,
        suspendProcesses: FromCloudFormation.getStringArray(p.SuspendProcesses).value,
        waitOnResourceSignals: FromCloudFormation.getBoolean(p.WaitOnResourceSignals).value,
      });

source

The result is that the property is omitted completely and silently :(

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

MinActiveInstancesPercent should be included in the output template

Current Behavior

The result is that the property is omitted completely and silently :(

Reproduction Steps

Create a CFN template with an autoscaling group and an update policy which includes MinActiveInstancesPercent, reference the template file using CfnInclude, the MinActiveInstancesPercent will not exist in the template in cdk.out

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.1003.0

Framework Version

2.132.0

Node.js Version

18

OS

MacOs

Language

TypeScript

Language Version

No response

Other information

No response

[pahud]

Hi @NetaNir thank you for coming back for the issue report :-)

Root Cause

Your referenced source is at CDK 2.99.1. Looking at the main branch, the issue is in the AutoScaling UpdatePolicy parsing code in cfn-parse.ts. Here's the affected code:

aws-cdk/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts

Lines 452 to 464 in 7f3cc8f

	function parseAutoScalingRollingUpdate(p: any): FromCloudFormationResult<CfnAutoScalingRollingUpdate | undefined> {
	if (typeof p !== 'object') { return new FromCloudFormationResult(undefined); }
	self.throwIfIsIntrinsic(p, logicalId);
	const rollUp = new ObjectParser<CfnAutoScalingRollingUpdate>(p);
	rollUp.parseCase('maxBatchSize', FromCloudFormation.getNumber);
	rollUp.parseCase('minInstancesInService', FromCloudFormation.getNumber);
	rollUp.parseCase('minSuccessfulInstancesPercent', FromCloudFormation.getNumber);
	rollUp.parseCase('pauseTime', FromCloudFormation.getString);
	rollUp.parseCase('suspendProcesses', FromCloudFormation.getStringArray);
	rollUp.parseCase('waitOnResourceSignals', FromCloudFormation.getBoolean);
	return rollUp.toResult();
	}

The MinActiveInstancesPercent property from CloudFormation's UpdatePolicy is not included in the parsing code. This means when CfnInclude processes a template containing this property:

1. The property exists in the source CloudFormation template
2. During parsing, the parseAutoScalingRollingUpdate function doesn't have a case to handle MinActiveInstancesPercent
3. The property is silently dropped from the final template in cdk.out

Impact:

• This affects any CloudFormation template that uses the MinActiveInstancesPercent property in AutoScaling group update policies
• The property is completely omitted from the synthesized template without any warning
• This could lead to unexpected behavior during stack updates since the minimum percentage of active instances is not maintained as specified in the original template

Suggested Fix:

1. Adding a property parser in the parseAutoScalingRollingUpdate function

I'm making this a p1 and will discuss with the team.

[pahud]

Attaching a diagram to explain this issue

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.