Skip to content

Commit b8da866

Browse files
committed
fix: address PR aws#8299 review feedback for nested stack changeset support
- Fix recursion bug: use is_parent=True for proper 3+ level nesting - Eliminate ~70 lines of duplicated inline logic via recursive call - Replace sys.stdout.write with click.echo() for consistency - Fix do_cli signature: align include_nested_stacks position with cli() call - Fix guided mode: propagate user's include_nested_stacks flag (not hardcoded) - Restore full assert_called_with in unit tests - Add 9 new unit tests for nested stack display and error handling - Improve integration test assertions for [Nested Stack:] header - Coverage improved from 93.86% to 94.07% (above 94% threshold)
1 parent c4dabcc commit b8da866

6 files changed

Lines changed: 336 additions & 119 deletions

File tree

samcli/commands/deploy/command.py

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -271,17 +271,17 @@ def do_cli(
271271
metadata,
272272
guided,
273273
confirm_changeset,
274-
include_nested_stacks=True,
275-
region=None,
276-
profile=None,
277-
signing_profiles=None,
278-
resolve_s3=False,
279-
config_file=None,
280-
config_env=None,
281-
resolve_image_repos=False,
282-
disable_rollback=False,
283-
on_failure=None,
284-
max_wait_duration=60,
274+
include_nested_stacks,
275+
region,
276+
profile,
277+
signing_profiles,
278+
resolve_s3,
279+
config_file,
280+
config_env,
281+
resolve_image_repos,
282+
disable_rollback,
283+
on_failure,
284+
max_wait_duration,
285285
):
286286
"""
287287
Implementation of the ``cli`` method
@@ -312,6 +312,7 @@ def do_cli(
312312
config_env=config_env,
313313
config_file=config_file,
314314
disable_rollback=disable_rollback,
315+
include_nested_stacks=include_nested_stacks,
315316
)
316317
guided_context.run()
317318
else:

samcli/commands/deploy/guided_context.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ def __init__(
6262
config_env=None,
6363
config_file=None,
6464
disable_rollback=None,
65+
include_nested_stacks=True,
6566
):
6667
self.template_file = template_file
6768
self.stack_name = stack_name
@@ -95,6 +96,7 @@ def __init__(
9596
self.color = Colored()
9697
self.function_provider = None
9798
self.disable_rollback = disable_rollback
99+
self.include_nested_stacks = include_nested_stacks
98100

99101
@property
100102
def guided_capabilities(self):
@@ -584,7 +586,7 @@ def run(self):
584586
region=self.guided_region,
585587
profile=self.guided_profile,
586588
confirm_changeset=self.confirm_changeset,
587-
include_nested_stacks=True,
589+
include_nested_stacks=self.include_nested_stacks,
588590
capabilities=self._capabilities,
589591
signing_profiles=self.signing_profiles,
590592
disable_rollback=self.disable_rollback,

samcli/lib/deploy/deployer.py

Lines changed: 12 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from typing import Dict, List, Optional
2626

2727
import botocore
28+
import click
2829

2930
from samcli.commands._utils.table_print import MIN_OFFSET, newline_per_item, pprint_column_names, pprint_columns
3031
from samcli.commands.deploy import exceptions as deploy_exceptions
@@ -336,95 +337,25 @@ def _display_changeset_changes(
336337
color=row_color,
337338
)
338339

339-
# Recursively display nested stack changes with pagination
340+
# Recursively display nested stack changes
340341
# Only process nested stacks when is_parent=True to avoid duplicates
341342
if is_parent:
342343
for nested in nested_changesets:
343344
try:
344345
# Display nested stack header
345-
sys.stdout.write(f"\n[Nested Stack: {nested['logical_id']}]\n")
346-
sys.stdout.flush()
347-
348-
# Use paginator for nested changesets to handle large changesets
349-
nested_paginator = self._client.get_paginator("describe_change_set")
350-
nested_iterator = nested_paginator.paginate(ChangeSetName=nested["changeset_id"])
351-
352-
nested_has_changes = False
353-
# Track nested-nested stacks for recursive processing
354-
deeply_nested_changesets = []
355-
356-
for nested_item in nested_iterator:
357-
nested_cf_changes = nested_item.get("Changes", [])
358-
for change in nested_cf_changes:
359-
nested_has_changes = True
360-
resource_props = change.get("ResourceChange", {})
361-
action = resource_props.get("Action")
362-
resource_type = resource_props.get("ResourceType")
363-
logical_id = resource_props.get("LogicalResourceId")
364-
replacement = resource_props.get("Replacement")
365-
366-
# Check for deeply nested stacks (3+ levels)
367-
deeply_nested_changeset_id = resource_props.get("ChangeSetId")
368-
if resource_type == "AWS::CloudFormation::Stack" and deeply_nested_changeset_id:
369-
deeply_nested_changesets.append(
370-
{"changeset_id": deeply_nested_changeset_id, "logical_id": logical_id}
371-
)
372-
373-
row_color = self.deploy_color.get_changeset_action_color(action=action)
374-
pprint_columns(
375-
columns=[
376-
changes_showcase.get(action, action),
377-
logical_id,
378-
resource_type,
379-
"N/A" if replacement is None else replacement,
380-
],
381-
width=kwargs["width"],
382-
margin=kwargs["margin"],
383-
format_string=DESCRIBE_CHANGESET_FORMAT_STRING,
384-
format_args=kwargs["format_args"],
385-
columns_dict=DESCRIBE_CHANGESET_DEFAULT_ARGS.copy(),
386-
color=row_color,
387-
)
388-
389-
if not nested_has_changes:
390-
pprint_columns(
391-
columns=["-", "-", "-", "-"],
392-
width=kwargs["width"],
393-
margin=kwargs["margin"],
394-
format_string=DESCRIBE_CHANGESET_FORMAT_STRING,
395-
format_args=kwargs["format_args"],
396-
columns_dict=DESCRIBE_CHANGESET_DEFAULT_ARGS.copy(),
346+
click.echo(f"\n[Nested Stack: {nested['logical_id']}]")
347+
348+
# Get the stack name from the changeset to support recursive call
349+
nested_response = self._client.describe_change_set(ChangeSetName=nested["changeset_id"])
350+
nested_stack_name = nested_response.get("StackName")
351+
if nested_stack_name:
352+
# Recursively call to display nested changes (supports arbitrary nesting depth)
353+
self._display_changeset_changes(
354+
nested["changeset_id"], nested_stack_name, is_parent=True, **kwargs
397355
)
398-
399-
# Recursively process deeply nested stacks (3+ levels)
400-
for deeply_nested in deeply_nested_changesets:
401-
# Get the stack name from the changeset to support recursive call
402-
try:
403-
deeply_nested_response = self._client.describe_change_set(
404-
ChangeSetName=deeply_nested["changeset_id"]
405-
)
406-
deeply_nested_stack_name = deeply_nested_response.get("StackName")
407-
if deeply_nested_stack_name:
408-
# Print header for deeply nested stack
409-
sys.stdout.write(f"\n[Nested Stack: {deeply_nested['logical_id']}]\n")
410-
sys.stdout.flush()
411-
# Recursively call to display deeply nested changes
412-
self._display_changeset_changes(
413-
deeply_nested["changeset_id"], deeply_nested_stack_name, is_parent=False, **kwargs
414-
)
415-
except Exception as e:
416-
LOG.debug(
417-
"Failed to describe deeply nested changeset %s: %s", deeply_nested["changeset_id"], e
418-
)
419-
sys.stdout.write(
420-
f"\n[Nested Stack: {deeply_nested['logical_id']}] - Unable to fetch changes: {str(e)}\n"
421-
)
422-
sys.stdout.flush()
423-
424356
except Exception as e:
425357
LOG.debug("Failed to describe nested changeset %s: %s", nested["changeset_id"], e)
426-
sys.stdout.write(f"Unable to fetch changes: {str(e)}\n")
427-
sys.stdout.flush()
358+
click.echo(f"Unable to fetch changes: {str(e)}")
428359

429360
return changes if changeset_found else None
430361

schema/samcli.json

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1233,7 +1233,7 @@
12331233
"properties": {
12341234
"parameters": {
12351235
"title": "Parameters for the deploy command",
1236-
"description": "Available parameters for the deploy command:\n* guided:\nSpecify this flag to allow SAM CLI to guide you through the deployment using guided prompts.\n* template_file:\nAWS SAM template which references built artifacts for resources in the template. (if applicable)\n* no_execute_changeset:\nIndicates whether to execute the change set. Specify this flag to view stack changes before executing the change set.\n* fail_on_empty_changeset:\nSpecify whether AWS SAM CLI should return a non-zero exit code if there are no changes to be made to the stack. Defaults to a non-zero exit code.\n* confirm_changeset:\nPrompt to confirm if the computed changeset is to be deployed by SAM CLI.\n* disable_rollback:\nPreserves the state of previously provisioned resources when an operation fails.\n* on_failure:\nProvide an action to determine what will happen when a stack fails to create. Three actions are available:\n\n- ROLLBACK: This will rollback a stack to a previous known good state.\n\n- DELETE: The stack will rollback to a previous state if one exists, otherwise the stack will be deleted.\n\n- DO_NOTHING: The stack will not rollback or delete, this is the same as disabling rollback.\n\nDefault behaviour is ROLLBACK.\n\n\n\nThis option is mutually exclusive with --disable-rollback/--no-disable-rollback. You can provide\n--on-failure or --disable-rollback/--no-disable-rollback but not both at the same time.\n* max_wait_duration:\nMaximum duration in minutes to wait for the deployment to complete.\n* stack_name:\nName of the AWS CloudFormation stack.\n* s3_bucket:\nAWS S3 bucket where artifacts referenced in the template are uploaded.\n* image_repository:\nAWS ECR repository URI where artifacts referenced in the template are uploaded.\n* image_repositories:\nMapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times.\n* force_upload:\nIndicates whether to override existing files in the S3 bucket. Specify this flag to upload artifacts even if they match existing artifacts in the S3 bucket.\n* s3_prefix:\nPrefix name that is added to the artifact's name when it is uploaded to the AWS S3 bucket.\n* kms_key_id:\nThe ID of an AWS KMS key that is used to encrypt artifacts that are at rest in the AWS S3 bucket.\n* role_arn:\nARN of an IAM role that AWS Cloudformation assumes when executing a deployment change set.\n* use_json:\nIndicates whether to use JSON as the format for the output AWS CloudFormation template. YAML is used by default.\n* resolve_s3:\nAutomatically resolve AWS S3 bucket for non-guided deployments. Enabling this option will also create a managed default AWS S3 bucket for you. If one does not provide a --s3-bucket value, the managed bucket will be used. Do not use --guided with this option.\n* resolve_image_repos:\nAutomatically create and delete ECR repositories for image-based functions in non-guided deployments. A companion stack containing ECR repos for each function will be deployed along with the template stack. Automatically created image repositories will be deleted if the corresponding functions are removed.\n* metadata:\nMap of metadata to attach to ALL the artifacts that are referenced in the template.\n* notification_arns:\nARNs of SNS topics that AWS Cloudformation associates with the stack.\n* tags:\nList of tags to associate with the stack.\n* parameter_overrides:\nString that contains AWS CloudFormation parameter overrides encoded as key=value pairs.\n* signing_profiles:\nA string that contains Code Sign configuration parameters as FunctionOrLayerNameToSign=SigningProfileName:SigningProfileOwner Since signing profile owner is optional, it could also be written as FunctionOrLayerNameToSign=SigningProfileName\n* no_progressbar:\nDoes not showcase a progress bar when uploading artifacts to S3 and pushing docker images to ECR\n* capabilities:\nList of capabilities that one must specify before AWS Cloudformation can create certain stacks.\n\nAccepted Values: CAPABILITY_IAM, CAPABILITY_NAMED_IAM, CAPABILITY_RESOURCE_POLICY, CAPABILITY_AUTO_EXPAND.\n\nLearn more at: https://docs.aws.amazon.com/serverlessrepo/latest/devguide/acknowledging-application-capabilities.html\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* save_params:\nSave the parameters provided via the command line to the configuration file.",
1236+
"description": "Available parameters for the deploy command:\n* guided:\nSpecify this flag to allow SAM CLI to guide you through the deployment using guided prompts.\n* template_file:\nAWS SAM template which references built artifacts for resources in the template. (if applicable)\n* no_execute_changeset:\nIndicates whether to execute the change set. Specify this flag to view stack changes before executing the change set.\n* fail_on_empty_changeset:\nSpecify whether AWS SAM CLI should return a non-zero exit code if there are no changes to be made to the stack. Defaults to a non-zero exit code.\n* confirm_changeset:\nPrompt to confirm if the computed changeset is to be deployed by SAM CLI.\n* include_nested_stacks:\nDisplay changes for nested stacks in the changeset. For large nested stack hierarchies, use --no-include-nested-stacks to reduce output verbosity. Defaults to displaying nested stack changes.\n* disable_rollback:\nPreserves the state of previously provisioned resources when an operation fails.\n* on_failure:\nProvide an action to determine what will happen when a stack fails to create. Three actions are available:\n\n- ROLLBACK: This will rollback a stack to a previous known good state.\n\n- DELETE: The stack will rollback to a previous state if one exists, otherwise the stack will be deleted.\n\n- DO_NOTHING: The stack will not rollback or delete, this is the same as disabling rollback.\n\nDefault behaviour is ROLLBACK.\n\n\n\nThis option is mutually exclusive with --disable-rollback/--no-disable-rollback. You can provide\n--on-failure or --disable-rollback/--no-disable-rollback but not both at the same time.\n* max_wait_duration:\nMaximum duration in minutes to wait for the deployment to complete.\n* stack_name:\nName of the AWS CloudFormation stack.\n* s3_bucket:\nAWS S3 bucket where artifacts referenced in the template are uploaded.\n* image_repository:\nAWS ECR repository URI where artifacts referenced in the template are uploaded.\n* image_repositories:\nMapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times.\n* force_upload:\nIndicates whether to override existing files in the S3 bucket. Specify this flag to upload artifacts even if they match existing artifacts in the S3 bucket.\n* s3_prefix:\nPrefix name that is added to the artifact's name when it is uploaded to the AWS S3 bucket.\n* kms_key_id:\nThe ID of an AWS KMS key that is used to encrypt artifacts that are at rest in the AWS S3 bucket.\n* role_arn:\nARN of an IAM role that AWS Cloudformation assumes when executing a deployment change set.\n* use_json:\nIndicates whether to use JSON as the format for the output AWS CloudFormation template. YAML is used by default.\n* resolve_s3:\nAutomatically resolve AWS S3 bucket for non-guided deployments. Enabling this option will also create a managed default AWS S3 bucket for you. If one does not provide a --s3-bucket value, the managed bucket will be used. Do not use --guided with this option.\n* resolve_image_repos:\nAutomatically create and delete ECR repositories for image-based functions in non-guided deployments. A companion stack containing ECR repos for each function will be deployed along with the template stack. Automatically created image repositories will be deleted if the corresponding functions are removed.\n* metadata:\nMap of metadata to attach to ALL the artifacts that are referenced in the template.\n* notification_arns:\nARNs of SNS topics that AWS Cloudformation associates with the stack.\n* tags:\nList of tags to associate with the stack.\n* parameter_overrides:\nString that contains AWS CloudFormation parameter overrides encoded as key=value pairs.\n* signing_profiles:\nA string that contains Code Sign configuration parameters as FunctionOrLayerNameToSign=SigningProfileName:SigningProfileOwner Since signing profile owner is optional, it could also be written as FunctionOrLayerNameToSign=SigningProfileName\n* no_progressbar:\nDoes not showcase a progress bar when uploading artifacts to S3 and pushing docker images to ECR\n* capabilities:\nList of capabilities that one must specify before AWS Cloudformation can create certain stacks.\n\nAccepted Values: CAPABILITY_IAM, CAPABILITY_NAMED_IAM, CAPABILITY_RESOURCE_POLICY, CAPABILITY_AUTO_EXPAND.\n\nLearn more at: https://docs.aws.amazon.com/serverlessrepo/latest/devguide/acknowledging-application-capabilities.html\n* profile:\nSelect a specific profile from your credential file to get AWS credentials.\n* region:\nSet the AWS Region of the service. (e.g. us-east-1)\n* beta_features:\nEnable/Disable beta features.\n* debug:\nTurn on debug logging to print debug message generated by AWS SAM CLI and display timestamps.\n* save_params:\nSave the parameters provided via the command line to the configuration file.",
12371237
"type": "object",
12381238
"properties": {
12391239
"guided": {
@@ -1263,6 +1263,12 @@
12631263
"type": "boolean",
12641264
"description": "Prompt to confirm if the computed changeset is to be deployed by SAM CLI."
12651265
},
1266+
"include_nested_stacks": {
1267+
"title": "include_nested_stacks",
1268+
"type": "boolean",
1269+
"description": "Display changes for nested stacks in the changeset. For large nested stack hierarchies, use --no-include-nested-stacks to reduce output verbosity. Defaults to displaying nested stack changes.",
1270+
"default": true
1271+
},
12661272
"disable_rollback": {
12671273
"title": "disable_rollback",
12681274
"type": "boolean",

tests/integration/deploy/test_nested_stack_changeset.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from unittest import skipIf
88

99
from tests.integration.deploy.deploy_integ_base import DeployIntegBase
10-
from tests.testing_utils import RUNNING_ON_CI, RUNNING_TEST_FOR_MASTER_ON_CI, RUN_BY_CANARY
10+
from tests.testing_utils import RUN_BY_CANARY, RUNNING_ON_CI, RUNNING_TEST_FOR_MASTER_ON_CI
1111

1212

1313
@skipIf(
@@ -58,9 +58,10 @@ def test_deploy_with_nested_stack_shows_nested_changes(self):
5858
# Should contain parent stack changes
5959
self.assertIn("CloudFormation stack changeset", stdout)
6060

61-
# For a stack with nested resources, verify the changes are shown
62-
# The actual nested stack display depends on the template structure
63-
# At minimum, verify no errors occurred and changeset was created
61+
# Verify nested stack header is displayed (validates nested stack feature)
62+
self.assertIn("[Nested Stack:", stdout)
63+
64+
# Verify no errors occurred
6465
self.assertNotIn("Error", stdout)
6566
self.assertNotIn("Failed", stdout)
6667

@@ -93,6 +94,9 @@ def test_deploy_nested_stack_with_parameters(self):
9394
# Verify changeset was created
9495
self.assertIn("CloudFormation stack changeset", stdout)
9596

97+
# Verify nested stack header is displayed (validates nested stack feature)
98+
self.assertIn("[Nested Stack:", stdout)
99+
96100
# Verify no errors
97101
self.assertNotIn("Error", stdout)
98102
self.assertNotIn("Failed", stdout)

0 commit comments

Comments
 (0)