fix(cloudformation-include): parse MinActiveInstancesPercent in AutoScalingRollingUpdate policy

[scorbiere]

Issue #33810

Closes #33810.

Reason for this change

When using CfnInclude to import existing CloudFormation templates, the MinActiveInstancesPercent property in AutoScaling
group update policies was being silently dropped because it was missing from both the CfnAutoScalingRollingUpdate
interface in cfn-resource-policy.ts and the parser in the parseAutoScalingRollingUpdate function in cfn-parse.ts.

Description of changes

This fix adds:

1. The missing minActiveInstancesPercent property to the CfnAutoScalingRollingUpdate interface in cfn-resource-policy.ts
2. The corresponding property parser to the parseAutoScalingRollingUpdate function in cfn-parse.ts

These changes ensure that the MinActiveInstancesPercent property is correctly preserved when importing templates with
AutoScalingRollingUpdate policies.

Describe any new or updated permissions being added

N/A

Description of how you validated changes

Added a comprehensive test that verifies all properties of AutoScalingRollingUpdate are correctly parsed, including a new
test template with all properties and explicit assertions for each property.

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

(This review is outdated)

[scorbiere]

Exemption Request: this fix is focused on the correct parsing and preservation of a CloudFormation property during template import, which is fully testable with unit tests.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.98%. Comparing base (ea1436f) to head (2562871).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #33852      +/-   ##
==========================================
+ Coverage   82.43%   83.98%   +1.55%     
==========================================
  Files         120      120              
  Lines        6975     6976       +1     
  Branches     1178     1178              
==========================================
+ Hits         5750     5859     +109     
+ Misses       1120     1005     -115     
- Partials      105      112       +7     

Flag	Coverage Δ
suite.unit	83.98% <100.00%> (+1.55%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	∅ <ø> (∅)
packages/aws-cdk-lib/core	83.98% <100.00%> (+1.55%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[godwingrs22]

Thanks @scorbiere for your contribution. I have left some comments with some concerns. Please help to check once.

[godwingrs22]

I understand we are doing this only for CfnInclude construct. Not a blocker, but may be it would be good we support this in the rollingUpdate method in aws-autoscaling module which will be used when UpdatePolicy.rollingUpdate() is called.

We could have it as a separate PR. But just calling this out here.

[godwingrs22]

I have some concerns about potential breaking changes:

Based on the CFN doc as indicated below,

Setting MinActiveInstancesPercent in your UpdatePolicy will also affect instances launched when the DesiredCapacity property of the AWS::AutoScaling::AutoScalingGroup resource is set higher than the current desired capacity of that Auto Scaling group.

Currently, minActiveInstancesPercent is silently ignored when parsing templates in CfnInclude, which means customers are effectively getting the default behavior (100% active instances). By adding direct support for this property, we could potentially impact existing customers in the following ways:

1. Customers who have MinActiveInstancesPercent specified in their templates but it's currently being ignored would suddenly see different scaling behavior when their DesiredCapacity increases
2. This could affect application availability during scaling events and might break assumptions in their deployment processes

// Example scenario

// Template has:
UpdatePolicy: {
  AutoScalingRollingUpdate: {
    MinActiveInstancesPercent: 50
  }
}
// Current behavior: 100% instances stay active
// New behavior with this change: Only 50% instances stay active

I think we should handle this with feature flag since it will change existing behaviour. Please help to check.

[shikha372]

While I don’t believe this will cause an immediate traffic disruption for customers—since it only affects the update behavior—users will only notice the change in the number of active instances during an update operation, not during regular traffic on the auto-scaling group. However, the impact could be on operations where previously 100% of in-service instances were required for success, but now will be marked as successfully updated earlier with fewer instances. To be cautious, we can introduce a feature flag, but even then, the default behavior should be to consider the minActiveInstancesPresent field.

[godwingrs22]

While I don't anticipate availability issues during rolling updates with this change, I think it will be good to verify the behaviour when enforcing the MinActiveInstancesPercent property during rolling updates. This property specifies the percentage of instances that must be in InService state relative to the ASG's desired capacity for an update to succeed. Currently, since this property is silently ignored, it effectively defaults to 100%. I'm thinking if an existing customer has specified MinActiveInstancesPercent as 50% with a desired capacity of 4, will enforcing this property allow the rolling update to proceed with only 2 active instances (50% of desired capacity), whereas currently it might be maintaining more instances in service during the update process?

I feel it is good to compare the current rolling update behavior against the behavior with this property enforced, specifically focusing on the number of InService instances available during the update process. For example, if the current behavior maintains 3 active instances during a rolling update, but with MinActiveInstancesPercent: 50 only maintains 2 active instances, this would indicate reduced availability as fewer instances would be handling the application traffic during the rolling update period. To be clear, this concern is specifically about availability during the rolling update event, not about the general availability of the application after the update.

If we can verify there's no reduction in the number of available instances during rolling updates compared to current behavior, I'm comfortable proceeding without a feature flag. However, if we cannot conclusively verify the availability impact, it would be better to implement this with a feature flag as a safety measure. Given that rolling update behavior is also influenced by multiple other UpdatePolicy properties working in combination (MinInstancesInService, MaxBatchSize, etc.), it's challenging to predict all possible scenarios.

[scorbiere]

TL;DR I agree to stay on the safe side and implement a feature flag, because we cannot guarantee the potential drop in availability will not negatively impact existing services.

Here is the analysis summary.

Current Behavior (Before Fix)

Currently, when a customer imports a CloudFormation template with MinActiveInstancesPercent using CfnInclude, this property is silently dropped during parsing. When the CDK app synthesizes the template, this property is missing from the output, effectively using the default value of 100%.

New Behavior (After Fix)

After the fix, when a customer imports a CloudFormation template with MinActiveInstancesPercent using CfnInclude, this property will be correctly parsed and included in the synthesized template, using the value specified in the original template.

Potential Impact

According to the AWS CloudFormation documentation on UpdatePolicy, the MinActiveInstancesPercent property affects:

1. During Rolling Updates: It determines the percentage of instances that must remain in service during an update.
2. When Scaling Up: It affects instances launched when the desired capacity increases.

If a customer had templates with MinActiveInstancesPercent set to a value lower than 100%, but this property was being dropped by CDK, they were effectively getting 100% active instances during updates. After the fix, they will get the percentage they specified, which could be lower and potentially affect application availability during updates.

Actions That Trigger Rolling Updates

The following actions can trigger a rolling update for an Auto Scaling group:

1. Changes to the Launch Template or Launch Configuration
2. Changes to the Auto Scaling Group Properties
3. Stack Updates with an UpdatePolicy attribute
4. Desired Capacity Changes when increased above the current capacity

[scorbiere]

Update

There is no need for adding a new feature flag, because the property MinActiveInstancesPercent is automatically added when using CfnInclude since this PR: #32321

For the sake of clarity, I think it is good to keep the changes which include the explicit handling of this property in the CfnParse code and the tests (unit and integ).

[godwingrs22]

I would recommend we have a integ test for this here with a similar UpdatePolicy json for the EC2 autoscaling resource

[godwingrs22]

nit: I suggest we have a unit test for this parser here. Atleast for this parseAutoScalingRollingUpdate method

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 2562871
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[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.