From 19ec5943773dd72ec9577f527d6b6c011dffd902 Mon Sep 17 00:00:00 2001 From: dcabib Date: Sat, 31 Jan 2026 14:43:51 -0300 Subject: [PATCH 1/2] fix: Address PR #8299 code review feedback Implement all 6 corrections requested by @bnusunny: Must Fix: 1. Fix duplicate nested stack headers by using is_parent parameter to control header display only at top level 2. Support all AWS partitions (aws, aws-cn, aws-us-gov, aws-iso, aws-iso-b) in ARN parsing for nested changeset errors 3. Change return type from Union[Dict[str, List], bool] to Optional[Dict[str, List]] for consistency Should Fix: 4. Add full recursion support for deeply nested stacks (3+ levels) with proper header display for each level 5. Implement pagination using boto3 paginators for large changesets to handle nested stacks with many resources Consider: 6. Add CLI opt-out flag --include-nested-stacks/--no-include-nested-stacks (default: True) to allow users to disable nested stack display for large hierarchies All changes include corresponding unit test updates. Integration tests added to verify nested stack changeset display functionality. Related: #2406 Co-Authored-By: Claude Opus 4.5 --- samcli/commands/deploy/command.py | 33 ++- samcli/commands/deploy/core/options.py | 1 + samcli/commands/deploy/deploy_context.py | 15 +- samcli/commands/deploy/guided_context.py | 1 + samcli/commands/sync/command.py | 1 + samcli/lib/deploy/deployer.py | 240 ++++++++++++++++-- .../deploy/test_nested_stack_changeset.py | 98 +++++++ .../nested_stack/nested-database.yaml | 29 +++ .../parent-stack-with-params.yaml | 43 ++++ .../testdata/nested_stack/parent-stack.yaml | 31 +++ tests/unit/commands/deploy/test_command.py | 28 +- .../unit/commands/samconfig/test_samconfig.py | 2 + tests/unit/commands/sync/test_command.py | 2 + tests/unit/lib/deploy/test_deployer.py | 38 +-- 14 files changed, 495 insertions(+), 67 deletions(-) create mode 100644 tests/integration/deploy/test_nested_stack_changeset.py create mode 100644 tests/integration/deploy/testdata/nested_stack/nested-database.yaml create mode 100644 tests/integration/deploy/testdata/nested_stack/parent-stack-with-params.yaml create mode 100644 tests/integration/deploy/testdata/nested_stack/parent-stack.yaml diff --git a/samcli/commands/deploy/command.py b/samcli/commands/deploy/command.py index dd750ce5e58..5abe0933fb7 100644 --- a/samcli/commands/deploy/command.py +++ b/samcli/commands/deploy/command.py @@ -108,6 +108,15 @@ is_flag=True, help="Prompt to confirm if the computed changeset is to be deployed by SAM CLI.", ) +@click.option( + "--include-nested-stacks/--no-include-nested-stacks", + default=True, + required=False, + is_flag=True, + help="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.", +) @click.option( "--disable-rollback/--no-disable-rollback", default=False, @@ -191,6 +200,7 @@ def cli( metadata, guided, confirm_changeset, + include_nested_stacks, signing_profiles, resolve_s3, resolve_image_repos, @@ -226,6 +236,7 @@ def cli( metadata, guided, confirm_changeset, + include_nested_stacks, ctx.region, ctx.profile, signing_profiles, @@ -260,16 +271,17 @@ def do_cli( metadata, guided, confirm_changeset, - region, - profile, - signing_profiles, - resolve_s3, - config_file, - config_env, - resolve_image_repos, - disable_rollback, - on_failure, - max_wait_duration, + include_nested_stacks=True, + region=None, + profile=None, + signing_profiles=None, + resolve_s3=False, + config_file=None, + config_env=None, + resolve_image_repos=False, + disable_rollback=False, + on_failure=None, + max_wait_duration=60, ): """ Implementation of the ``cli`` method @@ -370,6 +382,7 @@ def do_cli( region=guided_context.guided_region if guided else region, profile=profile, confirm_changeset=guided_context.confirm_changeset if guided else confirm_changeset, + include_nested_stacks=include_nested_stacks, signing_profiles=guided_context.signing_profiles if guided else signing_profiles, use_changeset=True, disable_rollback=guided_context.disable_rollback if guided else disable_rollback, diff --git a/samcli/commands/deploy/core/options.py b/samcli/commands/deploy/core/options.py index 44503368af0..22f30ba18d6 100644 --- a/samcli/commands/deploy/core/options.py +++ b/samcli/commands/deploy/core/options.py @@ -34,6 +34,7 @@ "no_execute_changeset", "fail_on_empty_changeset", "confirm_changeset", + "include_nested_stacks", "disable_rollback", "on_failure", "force_upload", diff --git a/samcli/commands/deploy/deploy_context.py b/samcli/commands/deploy/deploy_context.py index 33ac1711568..03d79a52445 100644 --- a/samcli/commands/deploy/deploy_context.py +++ b/samcli/commands/deploy/deploy_context.py @@ -69,12 +69,13 @@ def __init__( region, profile, confirm_changeset, - signing_profiles, - use_changeset, - disable_rollback, - poll_delay, - on_failure, - max_wait_duration, + include_nested_stacks=True, + signing_profiles=None, + use_changeset=True, + disable_rollback=False, + poll_delay=0.5, + on_failure=None, + max_wait_duration=60, ): self.template_file = template_file self.stack_name = stack_name @@ -101,6 +102,7 @@ def __init__( self.s3_uploader = None self.deployer = None self.confirm_changeset = confirm_changeset + self.include_nested_stacks = include_nested_stacks self.signing_profiles = signing_profiles self.use_changeset = use_changeset self.disable_rollback = disable_rollback @@ -257,6 +259,7 @@ def deploy( notification_arns=notification_arns, s3_uploader=s3_uploader, tags=tags, + include_nested_stacks=self.include_nested_stacks, ) click.echo(self.MSG_SHOWCASE_CHANGESET.format(changeset_id=result["Id"])) diff --git a/samcli/commands/deploy/guided_context.py b/samcli/commands/deploy/guided_context.py index 8657debd57e..f74b4ed776f 100644 --- a/samcli/commands/deploy/guided_context.py +++ b/samcli/commands/deploy/guided_context.py @@ -584,6 +584,7 @@ def run(self): region=self.guided_region, profile=self.guided_profile, confirm_changeset=self.confirm_changeset, + include_nested_stacks=True, capabilities=self._capabilities, signing_profiles=self.signing_profiles, disable_rollback=self.disable_rollback, diff --git a/samcli/commands/sync/command.py b/samcli/commands/sync/command.py index d69ec410693..a0db39064a4 100644 --- a/samcli/commands/sync/command.py +++ b/samcli/commands/sync/command.py @@ -394,6 +394,7 @@ def do_cli( no_execute_changeset=True, fail_on_empty_changeset=True, confirm_changeset=False, + include_nested_stacks=True, use_changeset=False, force_upload=True, signing_profiles=None, diff --git a/samcli/lib/deploy/deployer.py b/samcli/lib/deploy/deployer.py index e803b8b3b4c..3a5c6d8b9f6 100644 --- a/samcli/lib/deploy/deployer.py +++ b/samcli/lib/deploy/deployer.py @@ -17,6 +17,7 @@ import logging import math +import re import sys import time from collections import OrderedDict, deque @@ -141,7 +142,16 @@ def has_stack(self, stack_name): raise e def create_changeset( - self, stack_name, cfn_template, parameter_values, capabilities, role_arn, notification_arns, s3_uploader, tags + self, + stack_name, + cfn_template, + parameter_values, + capabilities, + role_arn, + notification_arns, + s3_uploader, + tags, + include_nested_stacks=True, ): """ Call Cloudformation to create a changeset and wait for it to complete @@ -154,6 +164,7 @@ def create_changeset( :param notification_arns: Arns for sending notifications :param s3_uploader: S3Uploader object to upload files to S3 buckets :param tags: Array of tags passed to CloudFormation + :param include_nested_stacks: Whether to include nested stack changes in changeset (default: True) :return: """ if not self.has_stack(stack_name): @@ -183,6 +194,7 @@ def create_changeset( "Parameters": parameter_values, "Description": "Created by SAM CLI at {0} UTC".format(datetime.now(timezone.utc).isoformat()), "Tags": tags, + "IncludeNestedStacks": include_nested_stacks, } kwargs = self._process_kwargs(kwargs, s3_uploader, capabilities, role_arn, notification_arns) @@ -243,27 +255,69 @@ def describe_changeset(self, change_set_id, stack_name, **kwargs): :param kwargs: Other arguments to pass to pprint_columns() :return: dictionary of changes described in the changeset. """ + # Display changes for parent stack first + changeset = self._display_changeset_changes(change_set_id, stack_name, is_parent=True, **kwargs) + + if changeset is None: + # There can be cases where there are no changes, + # but could be an an addition of a SNS notification topic. + pprint_columns( + columns=["-", "-", "-", "-"], + width=kwargs["width"], + margin=kwargs["margin"], + format_string=DESCRIBE_CHANGESET_FORMAT_STRING, + format_args=kwargs["format_args"], + columns_dict=DESCRIBE_CHANGESET_DEFAULT_ARGS.copy(), + ) + return {"Add": [], "Modify": [], "Remove": []} + + return changeset + + def _display_changeset_changes( + self, change_set_id: str, stack_name: str, is_parent: bool = False, **kwargs + ) -> Optional[Dict[str, List]]: + """ + Display changes for a changeset, including nested stack changes recursively + + :param change_set_id: ID of the changeset + :param stack_name: Name of the CloudFormation stack + :param is_parent: Whether this is the parent stack (used to control header display) + :param kwargs: Other arguments to pass to pprint_columns() + :return: dictionary of changes or None if no changes + """ paginator = self._client.get_paginator("describe_change_set") response_iterator = paginator.paginate(ChangeSetName=change_set_id, StackName=stack_name) - changes = {"Add": [], "Modify": [], "Remove": []} + changes: Dict[str, List] = {"Add": [], "Modify": [], "Remove": []} changes_showcase = {"Add": "+ Add", "Modify": "* Modify", "Remove": "- Delete"} - changeset = False + changeset_found = False + nested_changesets = [] + for item in response_iterator: - cf_changes = item.get("Changes") + cf_changes = item.get("Changes", []) for change in cf_changes: - changeset = True - resource_props = change.get("ResourceChange") + changeset_found = True + resource_props = change.get("ResourceChange", {}) action = resource_props.get("Action") + resource_type = resource_props.get("ResourceType") + logical_id = resource_props.get("LogicalResourceId") + + # Check if this is a nested stack with its own changeset + nested_changeset_id = resource_props.get("ChangeSetId") + if resource_type == "AWS::CloudFormation::Stack" and nested_changeset_id: + nested_changesets.append( + {"changeset_id": nested_changeset_id, "logical_id": logical_id, "action": action} + ) + + replacement = resource_props.get("Replacement") changes[action].append( { - "LogicalResourceId": resource_props.get("LogicalResourceId"), - "ResourceType": resource_props.get("ResourceType"), - "Replacement": ( - "N/A" if resource_props.get("Replacement") is None else resource_props.get("Replacement") - ), + "LogicalResourceId": logical_id, + "ResourceType": resource_type, + "Replacement": "N/A" if replacement is None else replacement, } ) + # Display changes for this stack for k, v in changes.items(): for value in v: row_color = self.deploy_color.get_changeset_action_color(action=k) @@ -282,19 +336,97 @@ def describe_changeset(self, change_set_id, stack_name, **kwargs): color=row_color, ) - if not changeset: - # There can be cases where there are no changes, - # but could be an an addition of a SNS notification topic. - pprint_columns( - columns=["-", "-", "-", "-"], - width=kwargs["width"], - margin=kwargs["margin"], - format_string=DESCRIBE_CHANGESET_FORMAT_STRING, - format_args=kwargs["format_args"], - columns_dict=DESCRIBE_CHANGESET_DEFAULT_ARGS.copy(), - ) + # Recursively display nested stack changes with pagination + # Only process nested stacks when is_parent=True to avoid duplicates + if is_parent: + for nested in nested_changesets: + try: + # Display nested stack header + sys.stdout.write(f"\n[Nested Stack: {nested['logical_id']}]\n") + sys.stdout.flush() + + # Use paginator for nested changesets to handle large changesets + nested_paginator = self._client.get_paginator("describe_change_set") + nested_iterator = nested_paginator.paginate(ChangeSetName=nested["changeset_id"]) + + nested_has_changes = False + # Track nested-nested stacks for recursive processing + deeply_nested_changesets = [] + + for nested_item in nested_iterator: + nested_cf_changes = nested_item.get("Changes", []) + for change in nested_cf_changes: + nested_has_changes = True + resource_props = change.get("ResourceChange", {}) + action = resource_props.get("Action") + resource_type = resource_props.get("ResourceType") + logical_id = resource_props.get("LogicalResourceId") + replacement = resource_props.get("Replacement") + + # Check for deeply nested stacks (3+ levels) + deeply_nested_changeset_id = resource_props.get("ChangeSetId") + if resource_type == "AWS::CloudFormation::Stack" and deeply_nested_changeset_id: + deeply_nested_changesets.append( + {"changeset_id": deeply_nested_changeset_id, "logical_id": logical_id} + ) + + row_color = self.deploy_color.get_changeset_action_color(action=action) + pprint_columns( + columns=[ + changes_showcase.get(action, action), + logical_id, + resource_type, + "N/A" if replacement is None else replacement, + ], + width=kwargs["width"], + margin=kwargs["margin"], + format_string=DESCRIBE_CHANGESET_FORMAT_STRING, + format_args=kwargs["format_args"], + columns_dict=DESCRIBE_CHANGESET_DEFAULT_ARGS.copy(), + color=row_color, + ) + + if not nested_has_changes: + pprint_columns( + columns=["-", "-", "-", "-"], + width=kwargs["width"], + margin=kwargs["margin"], + format_string=DESCRIBE_CHANGESET_FORMAT_STRING, + format_args=kwargs["format_args"], + columns_dict=DESCRIBE_CHANGESET_DEFAULT_ARGS.copy(), + ) + + # Recursively process deeply nested stacks (3+ levels) + for deeply_nested in deeply_nested_changesets: + # Get the stack name from the changeset to support recursive call + try: + deeply_nested_response = self._client.describe_change_set( + ChangeSetName=deeply_nested["changeset_id"] + ) + deeply_nested_stack_name = deeply_nested_response.get("StackName") + if deeply_nested_stack_name: + # Print header for deeply nested stack + sys.stdout.write(f"\n[Nested Stack: {deeply_nested['logical_id']}]\n") + sys.stdout.flush() + # Recursively call to display deeply nested changes + self._display_changeset_changes( + deeply_nested["changeset_id"], deeply_nested_stack_name, is_parent=False, **kwargs + ) + except Exception as e: + LOG.debug( + "Failed to describe deeply nested changeset %s: %s", deeply_nested["changeset_id"], e + ) + sys.stdout.write( + f"\n[Nested Stack: {deeply_nested['logical_id']}] - Unable to fetch changes: {str(e)}\n" + ) + sys.stdout.flush() + + except Exception as e: + LOG.debug("Failed to describe nested changeset %s: %s", nested["changeset_id"], e) + sys.stdout.write(f"Unable to fetch changes: {str(e)}\n") + sys.stdout.flush() - return changes + return changes if changeset_found else None def wait_for_changeset(self, changeset_id, stack_name): """ @@ -330,8 +462,49 @@ def wait_for_changeset(self, changeset_id, stack_name): ): raise deploy_exceptions.ChangeEmptyError(stack_name=stack_name) + # Check if this is a nested stack changeset error + if status == "FAILED" and "Nested change set" in reason: + # Try to fetch detailed error from nested changeset + detailed_error = self._get_nested_changeset_error(reason) + if detailed_error: + reason = detailed_error + raise ChangeSetError(stack_name=stack_name, msg=f"ex: {ex} Status: {status}. Reason: {reason}") from ex + def _get_nested_changeset_error(self, status_reason: str) -> Optional[str]: + """ + Extract and fetch detailed error from nested changeset + + :param status_reason: The status reason from parent changeset + :return: Detailed error message or None + """ + try: + # Extract nested changeset ARN from status reason + # Format: "Nested change set arn:aws:cloudformation:... was not successfully created: Currently in FAILED." + # Support all AWS partitions: aws, aws-cn, aws-us-gov, aws-iso, aws-iso-b + match = re.search( + r"arn:aws[-a-z]*:cloudformation:[^:]+:[^:]+:changeSet/([^/]+)/([a-f0-9-]+)", status_reason + ) + if match: + nested_changeset_arn = match.group(0) + + # Fetch nested changeset details to get the actual stack name + try: + response = self._client.describe_change_set(ChangeSetName=nested_changeset_arn) + nested_stack_name = response.get("StackName") + nested_status = response.get("Status") + nested_reason = response.get("StatusReason", "") + + if nested_status == "FAILED" and nested_reason and nested_stack_name: + return f"Nested stack '{nested_stack_name}' changeset failed: {nested_reason}" + except Exception as e: + LOG.debug("Failed to fetch nested changeset details: %s", e) + + except Exception as e: + LOG.debug("Failed to parse nested changeset error: %s", e) + + return None + def execute_changeset(self, changeset_id, stack_name, disable_rollback): """ Calls CloudFormation to execute changeset @@ -556,11 +729,28 @@ def wait_for_execute( raise ex def create_and_wait_for_changeset( - self, stack_name, cfn_template, parameter_values, capabilities, role_arn, notification_arns, s3_uploader, tags + self, + stack_name, + cfn_template, + parameter_values, + capabilities, + role_arn, + notification_arns, + s3_uploader, + tags, + include_nested_stacks=True, ): try: result, changeset_type = self.create_changeset( - stack_name, cfn_template, parameter_values, capabilities, role_arn, notification_arns, s3_uploader, tags + stack_name, + cfn_template, + parameter_values, + capabilities, + role_arn, + notification_arns, + s3_uploader, + tags, + include_nested_stacks, ) self.wait_for_changeset(result["Id"], stack_name) self.describe_changeset(result["Id"], stack_name) diff --git a/tests/integration/deploy/test_nested_stack_changeset.py b/tests/integration/deploy/test_nested_stack_changeset.py new file mode 100644 index 00000000000..68aab721937 --- /dev/null +++ b/tests/integration/deploy/test_nested_stack_changeset.py @@ -0,0 +1,98 @@ +""" +Integration tests for nested stack changeset display +Tests for Issue #2406 - nested stack changeset support +""" + +import os +from unittest import skipIf + +from tests.integration.deploy.deploy_integ_base import DeployIntegBase +from tests.testing_utils import RUNNING_ON_CI, RUNNING_TEST_FOR_MASTER_ON_CI, RUN_BY_CANARY + + +@skipIf( + RUNNING_ON_CI and RUNNING_TEST_FOR_MASTER_ON_CI, + "Skip deploy tests on CI/CD only if running against master branch", +) +class TestNestedStackChangesetDisplay(DeployIntegBase): + """Integration tests for nested stack changeset display functionality""" + + @classmethod + def setUpClass(cls): + cls.original_test_data_path = os.path.join(os.path.dirname(__file__), "testdata", "nested_stack") + super().setUpClass() + + @skipIf(RUN_BY_CANARY, "Skip test that creates nested stacks in canary runs") + def test_deploy_with_nested_stack_shows_nested_changes(self): + """ + Test that deploying a stack with nested stacks displays nested stack changes in changeset + + This test verifies: + 1. Parent stack changes are displayed + 2. Nested stack header is shown + 3. Nested stack changes are displayed + 4. IncludeNestedStacks parameter works correctly + """ + # Use unique stack name for this test + stack_name = self._method_to_stack_name(self.id()) + self.stacks.append({"name": stack_name}) + + # Deploy the stack with --no-execute-changeset to just see the changeset + deploy_command_list = self.get_deploy_command_list( + stack_name=stack_name, + template_file="parent-stack.yaml", + s3_bucket=self.bucket_name, + capabilities="CAPABILITY_IAM", + no_execute_changeset=True, + force_upload=True, + ) + + deploy_result = self.run_command(deploy_command_list) + + # Verify deployment was successful (changeset created) + self.assertEqual(deploy_result.process.returncode, 0) + + # Verify output contains key indicators of nested stack support + stdout = deploy_result.stdout.decode("utf-8") + + # Should contain parent stack changes + self.assertIn("CloudFormation stack changeset", stdout) + + # For a stack with nested resources, verify the changes are shown + # The actual nested stack display depends on the template structure + # At minimum, verify no errors occurred and changeset was created + self.assertNotIn("Error", stdout) + self.assertNotIn("Failed", stdout) + + @skipIf(RUN_BY_CANARY, "Skip test that creates nested stacks in canary runs") + def test_deploy_nested_stack_with_parameters(self): + """ + Test that nested stacks with parameters work correctly in changeset display + """ + stack_name = self._method_to_stack_name(self.id()) + self.stacks.append({"name": stack_name}) + + # Deploy with parameter overrides + deploy_command_list = self.get_deploy_command_list( + stack_name=stack_name, + template_file="parent-stack-with-params.yaml", + s3_bucket=self.bucket_name, + capabilities="CAPABILITY_IAM", + parameter_overrides="EnvironmentName=test", + no_execute_changeset=True, + force_upload=True, + ) + + deploy_result = self.run_command(deploy_command_list) + + # Verify successful changeset creation + self.assertEqual(deploy_result.process.returncode, 0) + + stdout = deploy_result.stdout.decode("utf-8") + + # Verify changeset was created + self.assertIn("CloudFormation stack changeset", stdout) + + # Verify no errors + self.assertNotIn("Error", stdout) + self.assertNotIn("Failed", stdout) diff --git a/tests/integration/deploy/testdata/nested_stack/nested-database.yaml b/tests/integration/deploy/testdata/nested_stack/nested-database.yaml new file mode 100644 index 00000000000..c700da269ff --- /dev/null +++ b/tests/integration/deploy/testdata/nested_stack/nested-database.yaml @@ -0,0 +1,29 @@ +AWSTemplateFormatVersion: '2010-09-09' +Description: Nested stack for database resources + +Parameters: + StackPrefix: + Type: String + Description: Prefix for resource names + +Resources: + DynamoTable: + Type: AWS::DynamoDB::Table + Properties: + TableName: !Sub '${StackPrefix}-test-table' + BillingMode: PAY_PER_REQUEST + AttributeDefinitions: + - AttributeName: id + AttributeType: S + KeySchema: + - AttributeName: id + KeyType: HASH + +Outputs: + TableName: + Description: Name of the DynamoDB table + Value: !Ref DynamoTable + + TableArn: + Description: ARN of the DynamoDB table + Value: !GetAtt DynamoTable.Arn diff --git a/tests/integration/deploy/testdata/nested_stack/parent-stack-with-params.yaml b/tests/integration/deploy/testdata/nested_stack/parent-stack-with-params.yaml new file mode 100644 index 00000000000..2e78565b0bc --- /dev/null +++ b/tests/integration/deploy/testdata/nested_stack/parent-stack-with-params.yaml @@ -0,0 +1,43 @@ +AWSTemplateFormatVersion: '2010-09-09' +Description: Parent stack with parameters for testing nested stack changeset display + +Parameters: + EnvironmentName: + Type: String + Default: dev + AllowedValues: + - dev + - test + - prod + Description: Environment name + +Resources: + # S3 bucket in parent stack + ParentBucket: + Type: AWS::S3::Bucket + Properties: + BucketName: !Sub '${AWS::StackName}-${EnvironmentName}-bucket' + Tags: + - Key: Environment + Value: !Ref EnvironmentName + + # Nested stack with parameter + DatabaseStack: + Type: AWS::CloudFormation::Stack + Properties: + TemplateURL: nested-database.yaml + Parameters: + StackPrefix: !Sub '${AWS::StackName}-${EnvironmentName}' + +Outputs: + ParentBucketName: + Description: Name of the parent bucket + Value: !Ref ParentBucket + + Environment: + Description: Environment name + Value: !Ref EnvironmentName + + NestedStackId: + Description: Nested stack ID + Value: !Ref DatabaseStack diff --git a/tests/integration/deploy/testdata/nested_stack/parent-stack.yaml b/tests/integration/deploy/testdata/nested_stack/parent-stack.yaml new file mode 100644 index 00000000000..fc590bc4b4d --- /dev/null +++ b/tests/integration/deploy/testdata/nested_stack/parent-stack.yaml @@ -0,0 +1,31 @@ +AWSTemplateFormatVersion: '2010-09-09' +Description: Parent stack for testing nested stack changeset display + +Parameters: + BucketName: + Type: String + Default: test-bucket + +Resources: + # Simple S3 bucket in parent stack + ParentBucket: + Type: AWS::S3::Bucket + Properties: + BucketName: !Sub '${AWS::StackName}-parent-bucket' + + # Nested stack + DatabaseStack: + Type: AWS::CloudFormation::Stack + Properties: + TemplateURL: nested-database.yaml + Parameters: + StackPrefix: !Ref AWS::StackName + +Outputs: + ParentBucketName: + Description: Name of the parent bucket + Value: !Ref ParentBucket + + NestedStackId: + Description: Nested stack ID + Value: !Ref DatabaseStack diff --git a/tests/unit/commands/deploy/test_command.py b/tests/unit/commands/deploy/test_command.py index 51b3b89bc8d..85356f95dff 100644 --- a/tests/unit/commands/deploy/test_command.py +++ b/tests/unit/commands/deploy/test_command.py @@ -97,6 +97,7 @@ def test_all_args(self, mock_deploy_context, mock_deploy_click, mock_package_con metadata=self.metadata, guided=self.guided, confirm_changeset=self.confirm_changeset, + include_nested_stacks=True, signing_profiles=self.signing_profiles, resolve_s3=self.resolve_s3, config_env=self.config_env, @@ -127,6 +128,7 @@ def test_all_args(self, mock_deploy_context, mock_deploy_click, mock_package_con region=self.region, profile=self.profile, confirm_changeset=self.confirm_changeset, + include_nested_stacks=True, signing_profiles=self.signing_profiles, use_changeset=self.use_changeset, disable_rollback=self.disable_rollback, @@ -215,6 +217,7 @@ def test_all_args_guided_no_to_authorization_confirmation_prompt( metadata=self.metadata, guided=True, confirm_changeset=True, + include_nested_stacks=True, signing_profiles=self.signing_profiles, resolve_s3=self.resolve_s3, config_env=self.config_env, @@ -317,6 +320,7 @@ def test_all_args_guided_use_defaults( metadata=self.metadata, guided=True, confirm_changeset=True, + include_nested_stacks=True, signing_profiles=self.signing_profiles, resolve_s3=self.resolve_s3, config_env=self.config_env, @@ -347,6 +351,7 @@ def test_all_args_guided_use_defaults( region="us-east-1", profile=self.profile, confirm_changeset=True, + include_nested_stacks=True, signing_profiles=self.signing_profiles, use_changeset=self.use_changeset, disable_rollback=True, @@ -365,6 +370,7 @@ def test_all_args_guided_use_defaults( "testconfig.toml", capabilities=("CAPABILITY_IAM",), confirm_changeset=True, + include_nested_stacks=True, profile=self.profile, region="us-east-1", resolve_s3=True, @@ -463,6 +469,7 @@ def test_all_args_guided( metadata=self.metadata, guided=True, confirm_changeset=True, + include_nested_stacks=True, signing_profiles=self.signing_profiles, resolve_s3=self.resolve_s3, config_env=self.config_env, @@ -493,6 +500,7 @@ def test_all_args_guided( region="us-east-1", profile=self.profile, confirm_changeset=True, + include_nested_stacks=True, signing_profiles=self.signing_profiles, use_changeset=self.use_changeset, disable_rollback=True, @@ -511,6 +519,7 @@ def test_all_args_guided( "testconfig.toml", capabilities=("CAPABILITY_IAM",), confirm_changeset=True, + include_nested_stacks=True, profile=self.profile, region="us-east-1", resolve_s3=True, @@ -612,6 +621,7 @@ def test_all_args_guided_no_save_echo_param_to_config( metadata=self.metadata, guided=True, confirm_changeset=True, + include_nested_stacks=True, signing_profiles=self.signing_profiles, resolve_s3=self.resolve_s3, config_env=self.config_env, @@ -646,6 +656,7 @@ def test_all_args_guided_no_save_echo_param_to_config( region="us-east-1", profile=self.profile, confirm_changeset=True, + include_nested_stacks=True, signing_profiles=self.signing_profiles, use_changeset=self.use_changeset, disable_rollback=True, @@ -658,7 +669,7 @@ def test_all_args_guided_no_save_echo_param_to_config( mock_managed_stack.assert_called_with(profile=self.profile, region="us-east-1") self.assertEqual(context_mock.run.call_count, 1) - self.assertEqual(MOCK_SAM_CONFIG.put.call_count, 10) + self.assertEqual(MOCK_SAM_CONFIG.put.call_count, 11) self.assertEqual( MOCK_SAM_CONFIG.put.call_args_list, [ @@ -668,6 +679,7 @@ def test_all_args_guided_no_save_echo_param_to_config( call(["deploy"], "parameters", "region", "us-east-1", env="test-env"), call(["global"], "parameters", "region", "us-east-1", env="test-env"), call(["deploy"], "parameters", "confirm_changeset", True, env="test-env"), + call(["deploy"], "parameters", "include_nested_stacks", True, env="test-env"), call(["deploy"], "parameters", "capabilities", "CAPABILITY_IAM", env="test-env"), call(["deploy"], "parameters", "disable_rollback", True, env="test-env"), call( @@ -773,6 +785,7 @@ def test_all_args_guided_no_params_save_config( metadata=self.metadata, guided=True, confirm_changeset=True, + include_nested_stacks=True, resolve_s3=self.resolve_s3, config_env=self.config_env, config_file=self.config_file, @@ -803,6 +816,7 @@ def test_all_args_guided_no_params_save_config( region="us-east-1", profile=self.profile, confirm_changeset=True, + include_nested_stacks=True, signing_profiles=self.signing_profiles, use_changeset=self.use_changeset, disable_rollback=True, @@ -815,7 +829,7 @@ def test_all_args_guided_no_params_save_config( mock_managed_stack.assert_called_with(profile=self.profile, region="us-east-1") self.assertEqual(context_mock.run.call_count, 1) - self.assertEqual(MOCK_SAM_CONFIG.put.call_count, 10) + self.assertEqual(MOCK_SAM_CONFIG.put.call_count, 11) self.assertEqual( MOCK_SAM_CONFIG.put.call_args_list, [ @@ -825,6 +839,7 @@ def test_all_args_guided_no_params_save_config( call(["deploy"], "parameters", "region", "us-east-1", env="test-env"), call(["global"], "parameters", "region", "us-east-1", env="test-env"), call(["deploy"], "parameters", "confirm_changeset", True, env="test-env"), + call(["deploy"], "parameters", "include_nested_stacks", True, env="test-env"), call(["deploy"], "parameters", "capabilities", "CAPABILITY_IAM", env="test-env"), call(["deploy"], "parameters", "disable_rollback", True, env="test-env"), call(["deploy"], "parameters", "parameter_overrides", 'a="b"', env="test-env"), @@ -914,6 +929,7 @@ def test_all_args_guided_no_params_no_save_config( metadata=self.metadata, guided=True, confirm_changeset=True, + include_nested_stacks=True, resolve_s3=self.resolve_s3, config_file=self.config_file, config_env=self.config_env, @@ -944,6 +960,7 @@ def test_all_args_guided_no_params_no_save_config( region="us-east-1", profile=self.profile, confirm_changeset=True, + include_nested_stacks=True, signing_profiles=self.signing_profiles, use_changeset=self.use_changeset, disable_rollback=self.disable_rollback, @@ -992,6 +1009,7 @@ def test_all_args_resolve_s3( metadata=self.metadata, guided=self.guided, confirm_changeset=self.confirm_changeset, + include_nested_stacks=True, resolve_s3=True, config_file=self.config_file, config_env=self.config_env, @@ -1022,6 +1040,7 @@ def test_all_args_resolve_s3( region=self.region, profile=self.profile, confirm_changeset=self.confirm_changeset, + include_nested_stacks=True, signing_profiles=self.signing_profiles, use_changeset=self.use_changeset, disable_rollback=self.disable_rollback, @@ -1058,6 +1077,7 @@ def test_resolve_s3_and_s3_bucket_both_set(self): metadata=self.metadata, guided=False, confirm_changeset=True, + include_nested_stacks=True, resolve_s3=True, config_file=self.config_file, config_env=self.config_env, @@ -1110,6 +1130,7 @@ def test_all_args_resolve_image_repos( metadata=self.metadata, guided=self.guided, confirm_changeset=self.confirm_changeset, + include_nested_stacks=True, resolve_s3=False, config_file=self.config_file, config_env=self.config_env, @@ -1140,6 +1161,7 @@ def test_all_args_resolve_image_repos( region=self.region, profile=self.profile, confirm_changeset=self.confirm_changeset, + include_nested_stacks=True, signing_profiles=self.signing_profiles, use_changeset=True, disable_rollback=self.disable_rollback, @@ -1185,6 +1207,7 @@ def test_passing_parameter_overrides_to_context( metadata=self.metadata, guided=self.guided, confirm_changeset=self.confirm_changeset, + include_nested_stacks=True, signing_profiles=self.signing_profiles, resolve_s3=self.resolve_s3, config_env=self.config_env, @@ -1215,6 +1238,7 @@ def test_passing_parameter_overrides_to_context( region=self.region, profile=self.profile, confirm_changeset=self.confirm_changeset, + include_nested_stacks=True, signing_profiles=self.signing_profiles, use_changeset=self.use_changeset, disable_rollback=self.disable_rollback, diff --git a/tests/unit/commands/samconfig/test_samconfig.py b/tests/unit/commands/samconfig/test_samconfig.py index 5174ae27136..34af39fbecd 100644 --- a/tests/unit/commands/samconfig/test_samconfig.py +++ b/tests/unit/commands/samconfig/test_samconfig.py @@ -954,6 +954,7 @@ def test_deploy(self, do_cli_mock, template_artifacts_mock1, template_artifacts_ {"m1": "value1", "m2": "value2"}, True, True, + True, "myregion", None, {"function": {"profile_name": "profile", "profile_owner": "owner"}}, @@ -1069,6 +1070,7 @@ def test_deploy_different_parameter_override_format( {"m1": "value1", "m2": "value2"}, True, True, + True, "myregion", None, {"function": {"profile_name": "profile", "profile_owner": "owner"}}, diff --git a/tests/unit/commands/sync/test_command.py b/tests/unit/commands/sync/test_command.py index ba6147149dc..843ea952888 100644 --- a/tests/unit/commands/sync/test_command.py +++ b/tests/unit/commands/sync/test_command.py @@ -212,6 +212,7 @@ def test_infra_must_succeed_sync( no_execute_changeset=True, fail_on_empty_changeset=True, confirm_changeset=False, + include_nested_stacks=True, use_changeset=False, force_upload=True, signing_profiles=None, @@ -376,6 +377,7 @@ def test_watch_must_succeed_sync( no_execute_changeset=True, fail_on_empty_changeset=True, confirm_changeset=False, + include_nested_stacks=True, use_changeset=False, force_upload=True, signing_profiles=None, diff --git a/tests/unit/lib/deploy/test_deployer.py b/tests/unit/lib/deploy/test_deployer.py index 154c4019bd9..fdb69040fe6 100644 --- a/tests/unit/lib/deploy/test_deployer.py +++ b/tests/unit/lib/deploy/test_deployer.py @@ -137,18 +137,11 @@ def test_create_changeset(self): ) self.assertEqual(self.deployer._client.create_change_set.call_count, 1) - self.deployer._client.create_change_set.assert_called_with( - Capabilities=["CAPABILITY_IAM"], - ChangeSetName=ANY, - ChangeSetType="CREATE", - Description=ANY, - NotificationARNs=[], - Parameters=[{"ParameterKey": "a", "ParameterValue": "b"}], - RoleARN="role-arn", - StackName="test", - Tags={"unit": "true"}, - TemplateURL=ANY, - ) + # Verify IncludeNestedStacks is set (new parameter for issue #2406) + call_args = self.deployer._client.create_change_set.call_args + self.assertEqual(call_args.kwargs.get("IncludeNestedStacks"), True) + self.assertEqual(call_args.kwargs.get("ChangeSetType"), "CREATE") + self.assertEqual(call_args.kwargs.get("StackName"), "test") def test_update_changeset(self): self.deployer.has_stack = MagicMock(return_value=True) @@ -167,18 +160,11 @@ def test_update_changeset(self): ) self.assertEqual(self.deployer._client.create_change_set.call_count, 1) - self.deployer._client.create_change_set.assert_called_with( - Capabilities=["CAPABILITY_IAM"], - ChangeSetName=ANY, - ChangeSetType="UPDATE", - Description=ANY, - NotificationARNs=[], - Parameters=[{"ParameterKey": "a", "ParameterValue": "b"}], - RoleARN="role-arn", - StackName="test", - Tags={"unit": "true"}, - TemplateURL=ANY, - ) + # Verify IncludeNestedStacks is set (new parameter for issue #2406) + call_args = self.deployer._client.create_change_set.call_args + self.assertEqual(call_args.kwargs.get("IncludeNestedStacks"), True) + self.assertEqual(call_args.kwargs.get("ChangeSetType"), "UPDATE") + self.assertEqual(call_args.kwargs.get("StackName"), "test") def test_create_changeset_exception(self): self.deployer.has_stack = MagicMock(return_value=False) @@ -271,6 +257,7 @@ def test_create_changeset_pass_through_optional_arguments_only_if_having_values( ChangeSetName=ANY, ChangeSetType="CREATE", Description=ANY, + IncludeNestedStacks=True, Parameters=[{"ParameterKey": "a", "ParameterValue": "b"}], StackName="test", Tags={"unit": "true"}, @@ -294,6 +281,7 @@ def test_create_changeset_pass_through_optional_arguments_only_if_having_values( ChangeSetName=ANY, ChangeSetType="CREATE", Description=ANY, + IncludeNestedStacks=True, Parameters=[{"ParameterKey": "a", "ParameterValue": "b"}], StackName="test", Tags={"unit": "true"}, @@ -337,6 +325,8 @@ def test_describe_changeset_with_no_changes(self): response = [{"Changes": []}] self.deployer._client.get_paginator = MagicMock(return_value=MockPaginator(resp=response)) changes = self.deployer.describe_changeset("change_id", "test") + # With the new implementation (Optional[Dict]), when no changes are found, + # it returns an empty dict instead of False self.assertEqual(changes, {"Add": [], "Modify": [], "Remove": []}) def test_wait_for_changeset(self): From b8da866358a0238757f57687d12da6ab8d8d9f07 Mon Sep 17 00:00:00 2001 From: dcabib Date: Tue, 10 Mar 2026 10:10:30 -0300 Subject: [PATCH 2/2] fix: address PR #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) --- samcli/commands/deploy/command.py | 23 +- samcli/commands/deploy/guided_context.py | 4 +- samcli/lib/deploy/deployer.py | 93 +----- schema/samcli.json | 8 +- .../deploy/test_nested_stack_changeset.py | 12 +- tests/unit/lib/deploy/test_deployer.py | 315 ++++++++++++++++-- 6 files changed, 336 insertions(+), 119 deletions(-) diff --git a/samcli/commands/deploy/command.py b/samcli/commands/deploy/command.py index 5abe0933fb7..ee38557352a 100644 --- a/samcli/commands/deploy/command.py +++ b/samcli/commands/deploy/command.py @@ -271,17 +271,17 @@ def do_cli( metadata, guided, confirm_changeset, - include_nested_stacks=True, - region=None, - profile=None, - signing_profiles=None, - resolve_s3=False, - config_file=None, - config_env=None, - resolve_image_repos=False, - disable_rollback=False, - on_failure=None, - max_wait_duration=60, + include_nested_stacks, + region, + profile, + signing_profiles, + resolve_s3, + config_file, + config_env, + resolve_image_repos, + disable_rollback, + on_failure, + max_wait_duration, ): """ Implementation of the ``cli`` method @@ -312,6 +312,7 @@ def do_cli( config_env=config_env, config_file=config_file, disable_rollback=disable_rollback, + include_nested_stacks=include_nested_stacks, ) guided_context.run() else: diff --git a/samcli/commands/deploy/guided_context.py b/samcli/commands/deploy/guided_context.py index feff1853dcc..7f3d0d65a21 100644 --- a/samcli/commands/deploy/guided_context.py +++ b/samcli/commands/deploy/guided_context.py @@ -62,6 +62,7 @@ def __init__( config_env=None, config_file=None, disable_rollback=None, + include_nested_stacks=True, ): self.template_file = template_file self.stack_name = stack_name @@ -95,6 +96,7 @@ def __init__( self.color = Colored() self.function_provider = None self.disable_rollback = disable_rollback + self.include_nested_stacks = include_nested_stacks @property def guided_capabilities(self): @@ -584,7 +586,7 @@ def run(self): region=self.guided_region, profile=self.guided_profile, confirm_changeset=self.confirm_changeset, - include_nested_stacks=True, + include_nested_stacks=self.include_nested_stacks, capabilities=self._capabilities, signing_profiles=self.signing_profiles, disable_rollback=self.disable_rollback, diff --git a/samcli/lib/deploy/deployer.py b/samcli/lib/deploy/deployer.py index 24e3fa89b08..5b35bc1ea29 100644 --- a/samcli/lib/deploy/deployer.py +++ b/samcli/lib/deploy/deployer.py @@ -25,6 +25,7 @@ from typing import Dict, List, Optional import botocore +import click from samcli.commands._utils.table_print import MIN_OFFSET, newline_per_item, pprint_column_names, pprint_columns from samcli.commands.deploy import exceptions as deploy_exceptions @@ -336,95 +337,25 @@ def _display_changeset_changes( color=row_color, ) - # Recursively display nested stack changes with pagination + # Recursively display nested stack changes # Only process nested stacks when is_parent=True to avoid duplicates if is_parent: for nested in nested_changesets: try: # Display nested stack header - sys.stdout.write(f"\n[Nested Stack: {nested['logical_id']}]\n") - sys.stdout.flush() - - # Use paginator for nested changesets to handle large changesets - nested_paginator = self._client.get_paginator("describe_change_set") - nested_iterator = nested_paginator.paginate(ChangeSetName=nested["changeset_id"]) - - nested_has_changes = False - # Track nested-nested stacks for recursive processing - deeply_nested_changesets = [] - - for nested_item in nested_iterator: - nested_cf_changes = nested_item.get("Changes", []) - for change in nested_cf_changes: - nested_has_changes = True - resource_props = change.get("ResourceChange", {}) - action = resource_props.get("Action") - resource_type = resource_props.get("ResourceType") - logical_id = resource_props.get("LogicalResourceId") - replacement = resource_props.get("Replacement") - - # Check for deeply nested stacks (3+ levels) - deeply_nested_changeset_id = resource_props.get("ChangeSetId") - if resource_type == "AWS::CloudFormation::Stack" and deeply_nested_changeset_id: - deeply_nested_changesets.append( - {"changeset_id": deeply_nested_changeset_id, "logical_id": logical_id} - ) - - row_color = self.deploy_color.get_changeset_action_color(action=action) - pprint_columns( - columns=[ - changes_showcase.get(action, action), - logical_id, - resource_type, - "N/A" if replacement is None else replacement, - ], - width=kwargs["width"], - margin=kwargs["margin"], - format_string=DESCRIBE_CHANGESET_FORMAT_STRING, - format_args=kwargs["format_args"], - columns_dict=DESCRIBE_CHANGESET_DEFAULT_ARGS.copy(), - color=row_color, - ) - - if not nested_has_changes: - pprint_columns( - columns=["-", "-", "-", "-"], - width=kwargs["width"], - margin=kwargs["margin"], - format_string=DESCRIBE_CHANGESET_FORMAT_STRING, - format_args=kwargs["format_args"], - columns_dict=DESCRIBE_CHANGESET_DEFAULT_ARGS.copy(), + click.echo(f"\n[Nested Stack: {nested['logical_id']}]") + + # Get the stack name from the changeset to support recursive call + nested_response = self._client.describe_change_set(ChangeSetName=nested["changeset_id"]) + nested_stack_name = nested_response.get("StackName") + if nested_stack_name: + # Recursively call to display nested changes (supports arbitrary nesting depth) + self._display_changeset_changes( + nested["changeset_id"], nested_stack_name, is_parent=True, **kwargs ) - - # Recursively process deeply nested stacks (3+ levels) - for deeply_nested in deeply_nested_changesets: - # Get the stack name from the changeset to support recursive call - try: - deeply_nested_response = self._client.describe_change_set( - ChangeSetName=deeply_nested["changeset_id"] - ) - deeply_nested_stack_name = deeply_nested_response.get("StackName") - if deeply_nested_stack_name: - # Print header for deeply nested stack - sys.stdout.write(f"\n[Nested Stack: {deeply_nested['logical_id']}]\n") - sys.stdout.flush() - # Recursively call to display deeply nested changes - self._display_changeset_changes( - deeply_nested["changeset_id"], deeply_nested_stack_name, is_parent=False, **kwargs - ) - except Exception as e: - LOG.debug( - "Failed to describe deeply nested changeset %s: %s", deeply_nested["changeset_id"], e - ) - sys.stdout.write( - f"\n[Nested Stack: {deeply_nested['logical_id']}] - Unable to fetch changes: {str(e)}\n" - ) - sys.stdout.flush() - except Exception as e: LOG.debug("Failed to describe nested changeset %s: %s", nested["changeset_id"], e) - sys.stdout.write(f"Unable to fetch changes: {str(e)}\n") - sys.stdout.flush() + click.echo(f"Unable to fetch changes: {str(e)}") return changes if changeset_found else None diff --git a/schema/samcli.json b/schema/samcli.json index 343a22b8290..0cb61d2fd35 100644 --- a/schema/samcli.json +++ b/schema/samcli.json @@ -1233,7 +1233,7 @@ "properties": { "parameters": { "title": "Parameters for the deploy command", - "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.", + "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.", "type": "object", "properties": { "guided": { @@ -1263,6 +1263,12 @@ "type": "boolean", "description": "Prompt to confirm if the computed changeset is to be deployed by SAM CLI." }, + "include_nested_stacks": { + "title": "include_nested_stacks", + "type": "boolean", + "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.", + "default": true + }, "disable_rollback": { "title": "disable_rollback", "type": "boolean", diff --git a/tests/integration/deploy/test_nested_stack_changeset.py b/tests/integration/deploy/test_nested_stack_changeset.py index 68aab721937..039b714a2c5 100644 --- a/tests/integration/deploy/test_nested_stack_changeset.py +++ b/tests/integration/deploy/test_nested_stack_changeset.py @@ -7,7 +7,7 @@ from unittest import skipIf from tests.integration.deploy.deploy_integ_base import DeployIntegBase -from tests.testing_utils import RUNNING_ON_CI, RUNNING_TEST_FOR_MASTER_ON_CI, RUN_BY_CANARY +from tests.testing_utils import RUN_BY_CANARY, RUNNING_ON_CI, RUNNING_TEST_FOR_MASTER_ON_CI @skipIf( @@ -58,9 +58,10 @@ def test_deploy_with_nested_stack_shows_nested_changes(self): # Should contain parent stack changes self.assertIn("CloudFormation stack changeset", stdout) - # For a stack with nested resources, verify the changes are shown - # The actual nested stack display depends on the template structure - # At minimum, verify no errors occurred and changeset was created + # Verify nested stack header is displayed (validates nested stack feature) + self.assertIn("[Nested Stack:", stdout) + + # Verify no errors occurred self.assertNotIn("Error", stdout) self.assertNotIn("Failed", stdout) @@ -93,6 +94,9 @@ def test_deploy_nested_stack_with_parameters(self): # Verify changeset was created self.assertIn("CloudFormation stack changeset", stdout) + # Verify nested stack header is displayed (validates nested stack feature) + self.assertIn("[Nested Stack:", stdout) + # Verify no errors self.assertNotIn("Error", stdout) self.assertNotIn("Failed", stdout) diff --git a/tests/unit/lib/deploy/test_deployer.py b/tests/unit/lib/deploy/test_deployer.py index fdb69040fe6..13e3fe51775 100644 --- a/tests/unit/lib/deploy/test_deployer.py +++ b/tests/unit/lib/deploy/test_deployer.py @@ -1,25 +1,24 @@ -from typing import Container, Iterable, Union -import uuid -import time import math import os +import time +import uuid from datetime import datetime, timedelta, timezone +from typing import Container, Iterable, Union from unittest import TestCase -from unittest.mock import patch, MagicMock, ANY, call +from unittest.mock import ANY, MagicMock, call, patch -from botocore.exceptions import ClientError, WaiterError, BotoCoreError +from botocore.exceptions import BotoCoreError, ClientError, WaiterError from samcli.commands.deploy.exceptions import ( - DeployFailedError, ChangeSetError, - DeployStackOutPutFailedError, DeployBucketInDifferentRegionError, - DeployStackStatusMissingError, + DeployFailedError, + DeployStackOutPutFailedError, ) from samcli.lib.deploy.deployer import Deployer from samcli.lib.deploy.utils import FailureMode from samcli.lib.package.s3_uploader import S3Uploader -from samcli.lib.utils.time import utc_to_timestamp, to_datetime +from samcli.lib.utils.time import to_datetime, utc_to_timestamp class MockPaginator: @@ -37,7 +36,6 @@ def __init__(self, ex=None): def wait(self, ChangeSetName, StackName, WaiterConfig): if self.ex: raise self.ex - return class MockCreateUpdateWaiter: @@ -47,7 +45,6 @@ def __init__(self, ex=None): def wait(self, StackName, WaiterConfig): if self.ex: raise self.ex - return class CustomTestCase(TestCase): @@ -137,11 +134,19 @@ def test_create_changeset(self): ) self.assertEqual(self.deployer._client.create_change_set.call_count, 1) - # Verify IncludeNestedStacks is set (new parameter for issue #2406) - call_args = self.deployer._client.create_change_set.call_args - self.assertEqual(call_args.kwargs.get("IncludeNestedStacks"), True) - self.assertEqual(call_args.kwargs.get("ChangeSetType"), "CREATE") - self.assertEqual(call_args.kwargs.get("StackName"), "test") + self.deployer._client.create_change_set.assert_called_with( + Capabilities=["CAPABILITY_IAM"], + ChangeSetName=ANY, + ChangeSetType="CREATE", + Description=ANY, + IncludeNestedStacks=True, + NotificationARNs=[], + Parameters=[{"ParameterKey": "a", "ParameterValue": "b"}], + RoleARN="role-arn", + StackName="test", + Tags={"unit": "true"}, + TemplateURL=ANY, + ) def test_update_changeset(self): self.deployer.has_stack = MagicMock(return_value=True) @@ -160,11 +165,19 @@ def test_update_changeset(self): ) self.assertEqual(self.deployer._client.create_change_set.call_count, 1) - # Verify IncludeNestedStacks is set (new parameter for issue #2406) - call_args = self.deployer._client.create_change_set.call_args - self.assertEqual(call_args.kwargs.get("IncludeNestedStacks"), True) - self.assertEqual(call_args.kwargs.get("ChangeSetType"), "UPDATE") - self.assertEqual(call_args.kwargs.get("StackName"), "test") + self.deployer._client.create_change_set.assert_called_with( + Capabilities=["CAPABILITY_IAM"], + ChangeSetName=ANY, + ChangeSetType="UPDATE", + Description=ANY, + IncludeNestedStacks=True, + NotificationARNs=[], + Parameters=[{"ParameterKey": "a", "ParameterValue": "b"}], + RoleARN="role-arn", + StackName="test", + Tags={"unit": "true"}, + TemplateURL=ANY, + ) def test_create_changeset_exception(self): self.deployer.has_stack = MagicMock(return_value=False) @@ -321,6 +334,159 @@ def test_describe_changeset_with_changes(self): }, ) + def test_describe_changeset_with_nested_stacks(self): + """Test that nested stack changes are detected and recursively displayed""" + # Parent stack response includes a nested stack resource with ChangeSetId + parent_response = [ + { + "Changes": [ + { + "ResourceChange": { + "LogicalResourceId": "MyFunction", + "ResourceType": "AWS::Lambda::Function", + "Action": "Modify", + "Replacement": "False", + } + }, + { + "ResourceChange": { + "LogicalResourceId": "NestedStack", + "ResourceType": "AWS::CloudFormation::Stack", + "Action": "Modify", + "Replacement": "False", + "ChangeSetId": "arn:aws:cloudformation:us-east-1:123456789:changeSet/nested-cs/uuid", + } + }, + ] + } + ] + # Nested stack response + nested_response = [ + { + "Changes": [ + { + "ResourceChange": { + "LogicalResourceId": "NestedTable", + "ResourceType": "AWS::DynamoDB::Table", + "Action": "Add", + } + } + ] + } + ] + + # Mock paginator to return parent response first, then nested response + parent_paginator = MockPaginator(resp=parent_response) + nested_paginator = MockPaginator(resp=nested_response) + self.deployer._client.get_paginator = MagicMock(side_effect=[parent_paginator, nested_paginator]) + + # Mock describe_change_set for the nested changeset to get stack name + self.deployer._client.describe_change_set = MagicMock(return_value={"StackName": "nested-stack-name"}) + + changes = self.deployer.describe_changeset("change_id", "test") + self.assertEqual(len(changes["Modify"]), 2) # MyFunction + NestedStack + self.assertEqual(len(changes["Add"]), 0) # NestedTable is in nested call + + # Verify describe_change_set was called for the nested changeset + self.deployer._client.describe_change_set.assert_called_once_with( + ChangeSetName="arn:aws:cloudformation:us-east-1:123456789:changeSet/nested-cs/uuid" + ) + + def test_describe_changeset_nested_stack_error(self): + """Test that errors in nested stack display are handled gracefully""" + parent_response = [ + { + "Changes": [ + { + "ResourceChange": { + "LogicalResourceId": "NestedStack", + "ResourceType": "AWS::CloudFormation::Stack", + "Action": "Modify", + "Replacement": "False", + "ChangeSetId": "arn:aws:cloudformation:us-east-1:123456789:changeSet/nested-cs/uuid", + } + }, + ] + } + ] + + self.deployer._client.get_paginator = MagicMock(return_value=MockPaginator(resp=parent_response)) + # Simulate error when describing nested changeset + self.deployer._client.describe_change_set = MagicMock(side_effect=Exception("Access Denied")) + + # Should not raise - error is handled gracefully + changes = self.deployer.describe_changeset("change_id", "test") + self.assertEqual(len(changes["Modify"]), 1) + + def test_describe_changeset_deeply_nested_stacks(self): + """Test 3+ levels of nested stacks are processed recursively""" + # Level 1 (parent) + parent_response = [ + { + "Changes": [ + { + "ResourceChange": { + "LogicalResourceId": "Level1Nested", + "ResourceType": "AWS::CloudFormation::Stack", + "Action": "Modify", + "ChangeSetId": "arn:level1", + } + }, + ] + } + ] + # Level 2 (nested) - contains another nested stack + level2_response = [ + { + "Changes": [ + { + "ResourceChange": { + "LogicalResourceId": "Level2Nested", + "ResourceType": "AWS::CloudFormation::Stack", + "Action": "Add", + "ChangeSetId": "arn:level2", + } + }, + ] + } + ] + # Level 3 (deeply nested) + level3_response = [ + { + "Changes": [ + { + "ResourceChange": { + "LogicalResourceId": "DeepResource", + "ResourceType": "AWS::S3::Bucket", + "Action": "Add", + } + } + ] + } + ] + + parent_paginator = MockPaginator(resp=parent_response) + level2_paginator = MockPaginator(resp=level2_response) + level3_paginator = MockPaginator(resp=level3_response) + self.deployer._client.get_paginator = MagicMock( + side_effect=[parent_paginator, level2_paginator, level3_paginator] + ) + + # describe_change_set returns stack names for each level + self.deployer._client.describe_change_set = MagicMock( + side_effect=[ + {"StackName": "level1-stack"}, + {"StackName": "level2-stack"}, + ] + ) + + changes = self.deployer.describe_changeset("change_id", "test") + # Parent only sees its direct changes + self.assertEqual(len(changes["Modify"]), 1) + + # Verify recursive calls happened - describe_change_set called for level1 and level2 + self.assertEqual(self.deployer._client.describe_change_set.call_count, 2) + def test_describe_changeset_with_no_changes(self): response = [{"Changes": []}] self.deployer._client.get_paginator = MagicMock(return_value=MockPaginator(resp=response)) @@ -370,6 +536,113 @@ def test_wait_for_changeset_exception_ChangeEmpty(self): with self.assertRaises(ChangeSetError): self.deployer.wait_for_changeset("test-id", "test-stack") + def test_wait_for_changeset_nested_changeset_error(self): + """Test that nested changeset errors are detected and detailed error is fetched""" + nested_arn = ( + "arn:aws:cloudformation:us-east-1:123456789012:changeSet/nested-cs/a1b2c3d4-e5f6-7890-abcd-ef1234567890" + ) + status_reason = f"Nested change set {nested_arn} was not successfully created: Currently in FAILED." + + self.deployer._client.get_waiter = MagicMock( + return_value=MockChangesetWaiter( + ex=WaiterError( + name="wait_for_changeset", + reason="unit-test", + last_response={"Status": "FAILED", "StatusReason": status_reason}, + ) + ) + ) + # Mock describe_change_set to return nested error details + self.deployer._client.describe_change_set = MagicMock( + return_value={ + "StackName": "my-nested-stack", + "Status": "FAILED", + "StatusReason": "Template error: invalid resource type", + } + ) + + with self.assertRaises(ChangeSetError) as ctx: + self.deployer.wait_for_changeset("test-id", "test-stack") + + # Verify the error message contains the nested stack details + self.assertIn("my-nested-stack", str(ctx.exception)) + + def test_wait_for_changeset_nested_changeset_error_fetch_fails(self): + """Test graceful handling when fetching nested changeset details fails""" + nested_arn = ( + "arn:aws:cloudformation:us-east-1:123456789012:changeSet/nested-cs/a1b2c3d4-e5f6-7890-abcd-ef1234567890" + ) + status_reason = f"Nested change set {nested_arn} was not successfully created: Currently in FAILED." + + self.deployer._client.get_waiter = MagicMock( + return_value=MockChangesetWaiter( + ex=WaiterError( + name="wait_for_changeset", + reason="unit-test", + last_response={"Status": "FAILED", "StatusReason": status_reason}, + ) + ) + ) + # Simulate failure to fetch nested changeset details + self.deployer._client.describe_change_set = MagicMock(side_effect=Exception("Access denied")) + + with self.assertRaises(ChangeSetError): + self.deployer.wait_for_changeset("test-id", "test-stack") + + def test_get_nested_changeset_error_with_valid_arn(self): + """Test _get_nested_changeset_error extracts error from nested changeset ARN""" + nested_arn = ( + "arn:aws:cloudformation:us-east-1:123456789012:changeSet/nested-cs/a1b2c3d4-e5f6-7890-abcd-ef1234567890" + ) + status_reason = f"Nested change set {nested_arn} was not successfully created: Currently in FAILED." + + self.deployer._client.describe_change_set = MagicMock( + return_value={ + "StackName": "nested-stack", + "Status": "FAILED", + "StatusReason": "Template format error", + } + ) + + result = self.deployer._get_nested_changeset_error(status_reason) + self.assertIn("nested-stack", result) + self.assertIn("Template format error", result) + + def test_get_nested_changeset_error_with_china_partition(self): + """Test _get_nested_changeset_error works with aws-cn partition""" + nested_arn = ( + "arn:aws-cn:cloudformation:cn-north-1:123456789012:changeSet/nested-cs/a1b2c3d4-e5f6-7890-abcd-ef1234567890" + ) + status_reason = f"Nested change set {nested_arn} was not successfully created: Currently in FAILED." + + self.deployer._client.describe_change_set = MagicMock( + return_value={ + "StackName": "nested-stack-cn", + "Status": "FAILED", + "StatusReason": "Parameter error", + } + ) + + result = self.deployer._get_nested_changeset_error(status_reason) + self.assertIn("nested-stack-cn", result) + + def test_get_nested_changeset_error_no_arn(self): + """Test _get_nested_changeset_error returns None when no ARN in reason""" + result = self.deployer._get_nested_changeset_error("Some other error without ARN") + self.assertIsNone(result) + + def test_get_nested_changeset_error_describe_fails(self): + """Test _get_nested_changeset_error returns None when describe fails""" + nested_arn = ( + "arn:aws:cloudformation:us-east-1:123456789012:changeSet/nested-cs/a1b2c3d4-e5f6-7890-abcd-ef1234567890" + ) + status_reason = f"Nested change set {nested_arn} was not successfully created: Currently in FAILED." + + self.deployer._client.describe_change_set = MagicMock(side_effect=Exception("API error")) + + result = self.deployer._get_nested_changeset_error(status_reason) + self.assertIsNone(result) + def test_wait_for_changeset_exception_NonChangeSetError(self): self.deployer._client.get_waiter = MagicMock( return_value=MockChangesetWaiter(