diff --git a/appveyor-linux-binary.yml b/appveyor-linux-binary.yml index f4a69d548f..5e4e849dce 100644 --- a/appveyor-linux-binary.yml +++ b/appveyor-linux-binary.yml @@ -71,9 +71,11 @@ install: - sh: "PATH=$(echo $PWD'/aws_cli/bin'):$PATH" # Install pytest - - sh: "pip3 install -r requirements/pre-dev.txt" - - sh: "pip3 install -r requirements/dev.txt" - - sh: "pip3 install -r requirements/base.txt" + - sh: "virtualenv pytest" + - sh: "./pytest/bin/python -m pip install -r requirements/pre-dev.txt" + - sh: "./pytest/bin/python -m pip install -r requirements/dev.txt" + - sh: "./pytest/bin/python -m pip install -r requirements/base.txt" + - sh: "PATH=$(echo $PWD'/pytest/bin'):$PATH" - sh: "pytest --version" - sh: "PATH=$PATH:$HOME/venv3.7/bin:$HOME/venv3.8/bin:$HOME/venv3.9/bin:$HOME/venv3.10/bin" diff --git a/samcli/commands/delete/delete_context.py b/samcli/commands/delete/delete_context.py index 1424c87f4a..2de317d46e 100644 --- a/samcli/commands/delete/delete_context.py +++ b/samcli/commands/delete/delete_context.py @@ -1,7 +1,6 @@ """ Delete a SAM stack """ -import json import logging from typing import Optional @@ -132,6 +131,11 @@ def init_clients(self): self.uploaders = Uploaders(self.s3_uploader, self.ecr_uploader) self.cf_utils = CfnUtils(cloudformation_client) + # Set region, this is purely for logging purposes + # the cloudformation client is able to read from + # the configuration file to get the region + self.region = self.region or cloudformation_client.meta.config.region_name + def s3_prompts(self): """ Guided prompts asking user to delete s3 artifacts @@ -218,16 +222,16 @@ def delete_ecr_companion_stack(self): """ delete_ecr_companion_stack_prompt = self.ecr_companion_stack_prompts() if delete_ecr_companion_stack_prompt or self.no_prompts: - cf_ecr_companion_stack = self.cf_utils.get_stack_template(self.companion_stack_name, TEMPLATE_STAGE) - ecr_stack_template_str = cf_ecr_companion_stack.get("TemplateBody", None) - ecr_stack_template_str = json.dumps(ecr_stack_template_str, indent=4, ensure_ascii=False) + cf_ecr_companion_stack_template = self.cf_utils.get_stack_template( + self.companion_stack_name, TEMPLATE_STAGE + ) ecr_companion_stack_template = Template( template_path=None, parent_dir=None, uploaders=self.uploaders, code_signer=None, - template_str=ecr_stack_template_str, + template_str=cf_ecr_companion_stack_template, ) retain_repos = self.ecr_repos_prompts(ecr_companion_stack_template) @@ -253,20 +257,16 @@ def delete(self): """ # Fetch the template using the stack-name cf_template = self.cf_utils.get_stack_template(self.stack_name, TEMPLATE_STAGE) - template_str = cf_template.get("TemplateBody", None) - - if isinstance(template_str, dict): - template_str = json.dumps(template_str, indent=4, ensure_ascii=False) # Get the cloudformation template name using template_str - self.cf_template_file_name = get_uploaded_s3_object_name(file_content=template_str, extension="template") + self.cf_template_file_name = get_uploaded_s3_object_name(file_content=cf_template, extension="template") template = Template( template_path=None, parent_dir=None, uploaders=self.uploaders, code_signer=None, - template_str=template_str, + template_str=cf_template, ) # If s3 info is not available, try to obtain it from CF @@ -286,7 +286,7 @@ def delete(self): # ECR companion stack delete prompts, if it exists companion_stack = CompanionStack(self.stack_name) - ecr_companion_stack_exists = self.cf_utils.has_stack(stack_name=companion_stack.stack_name) + ecr_companion_stack_exists = self.cf_utils.can_delete_stack(stack_name=companion_stack.stack_name) if ecr_companion_stack_exists: LOG.debug("ECR Companion stack found for the input stack") self.companion_stack_name = companion_stack.stack_name @@ -340,7 +340,7 @@ def run(self): ) if self.no_prompts or delete_stack: - is_deployed = self.cf_utils.has_stack(stack_name=self.stack_name) + is_deployed = self.cf_utils.can_delete_stack(stack_name=self.stack_name) # Check if the provided stack-name exists if is_deployed: LOG.debug("Input stack is deployed, continue deleting") diff --git a/samcli/commands/delete/exceptions.py b/samcli/commands/delete/exceptions.py index 5486189cf0..1b6a21f8bc 100644 --- a/samcli/commands/delete/exceptions.py +++ b/samcli/commands/delete/exceptions.py @@ -38,3 +38,39 @@ def __init__(self, stack_name, msg): message = f"Failed to fetch the template for the stack: {stack_name}, {msg}" super().__init__(message=message) + + +class FetchChangeSetError(UserException): + def __init__(self, stack_name, msg): + self.stack_name = stack_name + self.msg = msg + + message = f"Failed to fetch change sets for stack: {stack_name}, {msg}" + + super().__init__(message=message) + + +class NoChangeSetFoundError(UserException): + def __init__(self, stack_name): + self.stack_name = stack_name + + message = f"Stack {stack_name} does not contain any change sets" + + super().__init__(message=message) + + +class StackFetchError(UserException): + def __init__(self, stack_name, msg): + self.stack_name = stack_name + self.msg = msg + + message = f"Failed to complete an API call to fetch stack information for {stack_name}: {msg}" + super().__init__(message=message) + + +class StackProtectionEnabledError(UserException): + def __init__(self, stack_name): + self.stack_name = stack_name + + message = f"Stack {stack_name} cannot be deleted while TerminationProtection is enabled." + super().__init__(message=message) diff --git a/samcli/lib/delete/cfn_utils.py b/samcli/lib/delete/cfn_utils.py index 2a4389e026..40c2614bc9 100644 --- a/samcli/lib/delete/cfn_utils.py +++ b/samcli/lib/delete/cfn_utils.py @@ -3,11 +3,19 @@ """ import logging -from typing import Dict, List, Optional +from typing import List, Optional from botocore.exceptions import BotoCoreError, ClientError, WaiterError -from samcli.commands.delete.exceptions import CfDeleteFailedStatusError, DeleteFailedError, FetchTemplateFailedError +from samcli.commands.delete.exceptions import ( + CfDeleteFailedStatusError, + DeleteFailedError, + FetchChangeSetError, + FetchTemplateFailedError, + NoChangeSetFoundError, + StackFetchError, + StackProtectionEnabledError, +) LOG = logging.getLogger(__name__) @@ -16,12 +24,26 @@ class CfnUtils: def __init__(self, cloudformation_client): self._client = cloudformation_client - def has_stack(self, stack_name: str) -> bool: + def can_delete_stack(self, stack_name: str) -> bool: """ Checks if a CloudFormation stack with given name exists - :param stack_name: Name or ID of the stack - :return: True if stack exists. False otherwise + Parameters + ---------- + stack_name: str + Name or ID of the stack + + Returns + ------- + bool + True if stack exists. False otherwise + + Raises + ------ + StackFetchError + Raised when the boto call fails to get stack information + StackProtectionEnabledError + Raised when the stack is protected from deletions """ try: resp = self._client.describe_stacks(StackName=stack_name) @@ -30,14 +52,9 @@ def has_stack(self, stack_name: str) -> bool: stack = resp["Stacks"][0] if stack["EnableTerminationProtection"]: - message = "Stack cannot be deleted while TerminationProtection is enabled." - raise DeleteFailedError(stack_name=stack_name, msg=message) + raise StackProtectionEnabledError(stack_name=stack_name) - # Note: Stacks with REVIEW_IN_PROGRESS can be deleted - # using delete_stack but get_template does not return - # the template_str for this stack restricting deletion of - # artifacts. - return bool(stack["StackStatus"] != "REVIEW_IN_PROGRESS") + return True except ClientError as e: # If a stack does not exist, describe_stacks will throw an @@ -48,27 +65,52 @@ def has_stack(self, stack_name: str) -> bool: LOG.debug("Stack with id %s does not exist", stack_name) return False LOG.error("ClientError Exception : %s", str(e)) - raise DeleteFailedError(stack_name=stack_name, msg=str(e)) from e + raise StackFetchError(stack_name=stack_name, msg=str(e)) from e except BotoCoreError as e: # If there are credentials, environment errors, # catch that and throw a delete failed error. LOG.error("Botocore Exception : %s", str(e)) - raise DeleteFailedError(stack_name=stack_name, msg=str(e)) from e + raise StackFetchError(stack_name=stack_name, msg=str(e)) from e - def get_stack_template(self, stack_name: str, stage: str) -> Dict: + def get_stack_template(self, stack_name: str, stage: str) -> str: """ Return the Cloudformation template of the given stack_name - :param stack_name: Name or ID of the stack - :param stage: The Stage of the template Original or Processed - :return: Template body of the stack + Parameters + ---------- + + stack_name: str + Name or ID of the stack + stage: str + The Stage of the template Original or Processed + + Returns + ------- + str + Template body of the stack + + Raises + ------ + FetchTemplateFailedError + Raised when boto calls or parsing fails to fetch template """ try: resp = self._client.get_template(StackName=stack_name, TemplateStage=stage) - if not resp["TemplateBody"]: - return {} - return dict(resp) + template = resp.get("TemplateBody", "") + + # stack may not have template, check the change set + if not template: + change_set_name = self._get_change_set_name(stack_name) + + if change_set_name: + # the stack has a change set, use the template from this + resp = self._client.get_template( + StackName=stack_name, TemplateStage=stage, ChangeSetName=change_set_name + ) + template = resp.get("TemplateBody", "") + + return str(template) except (ClientError, BotoCoreError) as e: # If there are credentials, environment errors, @@ -76,7 +118,11 @@ def get_stack_template(self, stack_name: str, stage: str) -> Dict: LOG.error("Failed to fetch template for the stack : %s", str(e)) raise FetchTemplateFailedError(stack_name=stack_name, msg=str(e)) from e - + except FetchChangeSetError as ex: + raise FetchTemplateFailedError(stack_name=stack_name, msg=str(ex)) from ex + except NoChangeSetFoundError as ex: + msg = "Failed to find a change set to fetch the template" + raise FetchTemplateFailedError(stack_name=stack_name, msg=msg) from ex except Exception as e: # We don't know anything about this exception. Don't handle LOG.error("Unable to get stack details.", exc_info=e) @@ -86,8 +132,17 @@ def delete_stack(self, stack_name: str, retain_resources: Optional[List] = None) """ Delete the Cloudformation stack with the given stack_name - :param stack_name: Name or ID of the stack - :param retain_resources: List of repositories to retain if the stack has DELETE_FAILED status. + Parameters + ---------- + stack_name: + str Name or ID of the stack + retain_resources: Optional[List] + List of repositories to retain if the stack has DELETE_FAILED status. + + Raises + ------ + DeleteFailedError + Raised when the boto delete_stack call fails """ if not retain_resources: retain_resources = [] @@ -106,11 +161,21 @@ def delete_stack(self, stack_name: str, retain_resources: Optional[List] = None) LOG.error("Failed to delete stack. ", exc_info=e) raise e - def wait_for_delete(self, stack_name): + def wait_for_delete(self, stack_name: str): """ Waits until the delete stack completes - :param stack_name: Stack name + Parameter + --------- + stack_name: str + The name of the stack to watch when deleting + + Raises + ------ + CfDeleteFailedStatusError + Raised when the stack fails to delete + DeleteFailedError + Raised when the stack fails to wait when polling for status """ # Wait for Delete to Finish @@ -121,7 +186,7 @@ def wait_for_delete(self, stack_name): try: waiter.wait(StackName=stack_name, WaiterConfig=waiter_config) except WaiterError as ex: - stack_status = ex.last_response.get("Stacks", [{}])[0].get("StackStatusReason", "") + stack_status = ex.last_response.get("Stacks", [{}])[0].get("StackStatusReason", "") # type: ignore if "DELETE_FAILED" in str(ex): raise CfDeleteFailedStatusError( @@ -129,3 +194,48 @@ def wait_for_delete(self, stack_name): ) from ex raise DeleteFailedError(stack_name=stack_name, stack_status=stack_status, msg="ex: {0}".format(ex)) from ex + + def _get_change_set_name(self, stack_name: str) -> str: + """ + Returns the name of the change set for a stack + + Parameters + ---------- + stack_name: str + The name of the stack to find a change set + + Returns + ------- + str + The name of a change set + + Raises + ------ + FetchChangeSetError + Raised if there are boto call errors or parsing errors + NoChangeSetFoundError + Raised if a stack does not have any change sets + """ + try: + change_sets: dict = self._client.list_change_sets(StackName=stack_name) + except (ClientError, BotoCoreError) as ex: + LOG.debug("Failed to perform boto call to fetch change sets") + raise FetchChangeSetError(stack_name=stack_name, msg=str(ex)) from ex + + change_sets = change_sets.get("Summaries", []) + + if len(change_sets) > 1: + LOG.info( + "More than one change set was found, please clean up any " + "lingering template files that may exist in the S3 bucket." + ) + + if len(change_sets) > 0: + change_set = change_sets[0] + change_set_name = str(change_set.get("ChangeSetName", "")) + + LOG.debug(f"Returning change set: {change_set}") + return change_set_name + + LOG.debug("Stack contains no change sets") + raise NoChangeSetFoundError(stack_name=stack_name) diff --git a/tests/integration/delete/test_delete_command.py b/tests/integration/delete/test_delete_command.py index eb7e5e7a50..955916feea 100644 --- a/tests/integration/delete/test_delete_command.py +++ b/tests/integration/delete/test_delete_command.py @@ -8,10 +8,14 @@ from parameterized import parameterized from tests.integration.delete.delete_integ_base import DeleteIntegBase -from tests.integration.deploy.deploy_integ_base import DeployIntegBase -from tests.integration.package.package_integ_base import PackageIntegBase from tests.testing_utils import RUNNING_ON_CI, RUNNING_TEST_FOR_MASTER_ON_CI, RUN_BY_CANARY, CommandResult -from tests.testing_utils import run_command, run_command_with_input +from tests.testing_utils import ( + run_command, + run_command_with_input, + start_persistent_process, + read_until_string, + kill_process, +) # Delete tests require credentials and CI/CD will only add credentials to the env if the PR is from the same repo. # This is to restrict package tests to run outside of CI/CD, when the branch is not master or tests are not run by Canary @@ -78,10 +82,7 @@ def test_s3_options(self, template_file): self.validate_delete_process(delete_process_execute) # Check if the stack was deleted - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) # Check for zero objects in bucket s3_objects_resp = self.s3_client.list_objects_v2(Bucket=self.bucket_name, Prefix=self.s3_prefix) @@ -138,11 +139,7 @@ def test_delete_no_prompts_with_s3_prefix_present_zip(self, template_file): delete_process_execute = run_command(delete_command_list) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) # Remove the local config file created if os.path.isfile(config_file_path): @@ -173,11 +170,7 @@ def test_delete_no_prompts_with_s3_prefix_present_image(self, template_file): delete_process_execute = run_command(delete_command_list) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) # Remove the local config file created if os.path.isfile(config_file_path): @@ -204,11 +197,7 @@ def test_delete_guided_config_file_present(self, template_file): delete_process_execute = run_command_with_input(delete_command_list, "y\nn\ny\n".encode()) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) # Remove the local config file created if os.path.isfile(config_file_path): @@ -232,11 +221,7 @@ def test_delete_no_config_file_zip(self, template_file): ) delete_process_execute = run_command(delete_command_list) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) @parameterized.expand( [ @@ -270,11 +255,7 @@ def test_delete_no_prompts_no_s3_prefix_zip(self, template_file): ) delete_process_execute = run_command(delete_command_list) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) @parameterized.expand( [ @@ -310,11 +291,7 @@ def test_delete_no_prompts_no_s3_prefix_image(self, template_file): ) delete_process_execute = run_command(delete_command_list) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) @parameterized.expand( [os.path.join("deep-nested", "template.yaml"), os.path.join("deep-nested-image", "template.yaml")] @@ -349,11 +326,7 @@ def test_delete_nested_stacks(self, template_file): ) delete_process_execute = run_command(delete_command_list) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) def test_delete_stack_termination_protection_enabled(self): template_str = """ @@ -389,11 +362,7 @@ def test_delete_stack_termination_protection_enabled(self): delete_process_execute = run_command(delete_command_list) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) def test_no_prompts_no_stack_name(self): delete_command_list = self.get_delete_command_list(no_prompts=True) @@ -430,11 +399,7 @@ def test_delete_guided_ecr_repository_present(self, template_file): delete_process_execute = run_command_with_input(delete_command_list, "y\ny\ny\n".encode()) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) @parameterized.expand( [ @@ -469,11 +434,7 @@ def test_delete_guided_no_s3_prefix_image(self, template_file): delete_command_list = self.get_delete_command_list(stack_name=stack_name, region=self._session.region_name) delete_process_execute = run_command_with_input(delete_command_list, "y\n".encode()) self.validate_delete_process(delete_process_execute) - - try: - _ = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + self._validate_stack_deleted(stack_name=stack_name) @parameterized.expand( [ @@ -507,15 +468,55 @@ def test_delete_guided_retain_s3_artifact(self, template_file): self.validate_delete_process(delete_process_execute) - try: - resp = self.cf_client.describe_stacks(StackName=stack_name) - except ClientError as ex: - self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + def test_delete_stack_review_in_progress(self): + template_path = self.test_data_path.joinpath("aws-serverless-function.yaml") + stack_name = self._method_to_stack_name(self.id()) + + deploy_command = self.get_deploy_command_list( + template_file=template_path, + stack_name=stack_name, + capabilities="CAPABILITY_IAM", + s3_bucket=self.bucket_name, + s3_prefix=self.s3_prefix, + force_upload=True, + no_execute_changeset=False, + confirm_changeset=True, + region=self._session.region_name, + ) + + # run deploy command and wait for it to ask about change set + deploy_process = start_persistent_process(deploy_command) + read_until_string(deploy_process, "Deploy this changeset? [y/N]:") + + # kill the deploy process so that the stack is stuck in REVIEW_IN_PROGRESS + kill_process(deploy_process) + + delete_command = self.get_delete_command_list( + stack_name=stack_name, region=self._session.region_name, no_prompts=True + ) + delete_result = run_command(delete_command) + + self.validate_delete_process(delete_result) + self._validate_stack_deleted(stack_name=stack_name) def validate_delete_process(self, command_result: CommandResult): self.assertEqual(command_result.process.returncode, 0) self.assertNotIn(b"Could not find and delete the S3 object with the key", command_result.stderr) + def _validate_stack_deleted(self, stack_name: str) -> None: + """ + Validates that the stack is deleted from Cloudformation + + Parameters + ---------- + stack_name: str + The name of the stack to check if it exists in Cloudformation + """ + try: + self.cf_client.describe_stacks(StackName=stack_name) + except ClientError as ex: + self.assertIn(f"Stack with id {stack_name} does not exist", str(ex)) + # TODO: Add 3 more tests after Auto ECR is merged to develop # 1. Create a stack using guided deploy of type image and delete # 2. Delete the ECR Companion Stack as input stack. diff --git a/tests/unit/commands/delete/test_delete_context.py b/tests/unit/commands/delete/test_delete_context.py index 92e2aa2c6a..d710f30ad5 100644 --- a/tests/unit/commands/delete/test_delete_context.py +++ b/tests/unit/commands/delete/test_delete_context.py @@ -1,3 +1,4 @@ +import json from samcli.lib.bootstrap.companion_stack.data_types import CompanionStack from unittest import TestCase from unittest.mock import patch, call, MagicMock, Mock @@ -19,7 +20,7 @@ class TestDeleteContext(TestCase): @patch("samcli.commands.delete.delete_context.click.echo") @patch("samcli.commands.delete.delete_context.click.get_current_context") - @patch.object(CfnUtils, "has_stack", MagicMock(return_value=(False))) + @patch.object(CfnUtils, "can_delete_stack", MagicMock(return_value=(False))) @patch("samcli.commands.delete.delete_context.get_boto_client_provider_with_config") def test_delete_context_stack_does_not_exist( self, get_boto_client_provider_mock, patched_click_get_current_context, patched_click_echo @@ -95,7 +96,7 @@ def test_delete_context_parse_config_file(self, get_boto_client_provider_mock, p @patch("samcli.commands.delete.delete_context.prompt") @patch("samcli.commands.delete.delete_context.confirm") @patch("samcli.commands.delete.delete_context.click.get_current_context") - @patch.object(CfnUtils, "has_stack", MagicMock(return_value=(False))) + @patch.object(CfnUtils, "can_delete_stack", MagicMock(return_value=(False))) @patch("samcli.commands.delete.delete_context.get_boto_client_provider_with_config") def test_delete_no_user_input( self, get_boto_client_provider_mock, patched_click_get_current_context, patched_confirm, patched_prompt @@ -137,8 +138,8 @@ def test_delete_no_user_input( ) ), ) - @patch.object(CfnUtils, "has_stack", MagicMock(return_value=(True))) - @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value=({"TemplateBody": "Hello World"}))) + @patch.object(CfnUtils, "can_delete_stack", MagicMock(return_value=(True))) + @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value="Hello World")) @patch.object(CfnUtils, "delete_stack", MagicMock()) @patch.object(CfnUtils, "wait_for_delete", MagicMock()) @patch.object(Template, "get_ecr_repos", MagicMock(return_value=({"logical_id": {"Repository": "test_id"}}))) @@ -159,7 +160,7 @@ def test_delete_context_valid_execute_run(self, get_boto_client_provider_mock, p ) as delete_context: delete_context.run() - self.assertEqual(CfnUtils.has_stack.call_count, 2) + self.assertEqual(CfnUtils.can_delete_stack.call_count, 2) self.assertEqual(CfnUtils.get_stack_template.call_count, 2) self.assertEqual(CfnUtils.delete_stack.call_count, 2) self.assertEqual(CfnUtils.wait_for_delete.call_count, 2) @@ -169,8 +170,8 @@ def test_delete_context_valid_execute_run(self, get_boto_client_provider_mock, p @patch("samcli.commands.delete.delete_context.click.echo") @patch("samcli.commands.deploy.guided_context.click.secho") @patch("samcli.commands.delete.delete_context.click.get_current_context") - @patch.object(CfnUtils, "has_stack", MagicMock(side_effect=(True, False))) - @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value=({"TemplateBody": "Hello World"}))) + @patch.object(CfnUtils, "can_delete_stack", MagicMock(side_effect=(True, False))) + @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value="Hello World")) @patch.object(CfnUtils, "delete_stack", MagicMock()) @patch.object(CfnUtils, "wait_for_delete", MagicMock()) @patch("samcli.commands.delete.delete_context.get_boto_client_provider_with_config") @@ -208,8 +209,8 @@ def test_delete_context_no_s3_bucket( @patch("samcli.commands.delete.delete_context.get_uploaded_s3_object_name") @patch("samcli.commands.delete.delete_context.confirm") @patch("samcli.commands.delete.delete_context.click.get_current_context") - @patch.object(CfnUtils, "has_stack", MagicMock(side_effect=(True, False))) - @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value=({"TemplateBody": "Hello World"}))) + @patch.object(CfnUtils, "can_delete_stack", MagicMock(side_effect=(True, False))) + @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value="Hello World")) @patch.object(CfnUtils, "delete_stack", MagicMock()) @patch.object(CfnUtils, "wait_for_delete", MagicMock()) @patch.object(S3Uploader, "delete_artifact", MagicMock()) @@ -270,8 +271,8 @@ def test_guided_prompts_s3_bucket_prefix_present_execute_run( @patch("samcli.commands.delete.delete_context.get_uploaded_s3_object_name") @patch("samcli.commands.delete.delete_context.confirm") @patch("samcli.commands.delete.delete_context.click.get_current_context") - @patch.object(CfnUtils, "has_stack", MagicMock(side_effect=(True, False))) - @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value=({"TemplateBody": "Hello World"}))) + @patch.object(CfnUtils, "can_delete_stack", MagicMock(side_effect=(True, False))) + @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value="Hello World")) @patch.object(CfnUtils, "delete_stack", MagicMock()) @patch.object(CfnUtils, "wait_for_delete", MagicMock()) @patch.object(S3Uploader, "delete_artifact", MagicMock()) @@ -323,8 +324,8 @@ def test_guided_prompts_s3_bucket_present_no_prefix_execute_run( @patch("samcli.commands.delete.delete_context.get_uploaded_s3_object_name") @patch("samcli.commands.delete.delete_context.confirm") @patch("samcli.commands.delete.delete_context.click.get_current_context") - @patch.object(CfnUtils, "has_stack", MagicMock(side_effect=(True, True))) - @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value=({"TemplateBody": "Hello World"}))) + @patch.object(CfnUtils, "can_delete_stack", MagicMock(side_effect=(True, True))) + @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value="Hello World")) @patch.object(CfnUtils, "delete_stack", MagicMock()) @patch.object(CfnUtils, "wait_for_delete", MagicMock()) @patch.object(S3Uploader, "delete_artifact", MagicMock()) @@ -404,8 +405,8 @@ def test_guided_prompts_ecr_companion_stack_present_execute_run( @patch("samcli.commands.delete.delete_context.get_uploaded_s3_object_name") @patch("samcli.commands.delete.delete_context.click.echo") @patch("samcli.commands.delete.delete_context.click.get_current_context") - @patch.object(CfnUtils, "has_stack", MagicMock(side_effect=(True, False))) - @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value=({"TemplateBody": "Hello World"}))) + @patch.object(CfnUtils, "can_delete_stack", MagicMock(side_effect=(True, False))) + @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value="Hello World")) @patch.object(CfnUtils, "delete_stack", MagicMock()) @patch.object(CfnUtils, "wait_for_delete", MagicMock()) @patch.object(S3Uploader, "delete_prefix_artifacts", MagicMock()) @@ -420,9 +421,9 @@ def test_no_prompts_input_is_ecr_companion_stack_present_execute_run( patched_click_echo, patched_get_cf_template_name, ): - CfnUtils.get_stack_template.return_value = { - "TemplateBody": {"Metadata": {"CompanionStackname": "Companion-Stack-Name"}} - } + CfnUtils.get_stack_template.return_value = json.dumps( + {"Metadata": {"CompanionStackname": "Companion-Stack-Name"}} + ) patched_get_cf_template_name.return_value = "hello.template" with DeleteContext( stack_name="Companion-Stack-Name", @@ -446,8 +447,8 @@ def test_no_prompts_input_is_ecr_companion_stack_present_execute_run( @patch("samcli.commands.delete.delete_context.get_uploaded_s3_object_name") @patch("samcli.commands.delete.delete_context.click.get_current_context") - @patch.object(CfnUtils, "has_stack", MagicMock(side_effect=(True, True))) - @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value=({"TemplateBody": "Hello World"}))) + @patch.object(CfnUtils, "can_delete_stack", MagicMock(side_effect=(True, True))) + @patch.object(CfnUtils, "get_stack_template", MagicMock(return_value="Hello World")) @patch.object(CfnUtils, "delete_stack", MagicMock()) @patch.object( CfnUtils, @@ -484,7 +485,7 @@ def test_retain_resources_delete_stack( delete_context.run() - self.assertEqual(CfnUtils.has_stack.call_count, 2) + self.assertEqual(CfnUtils.can_delete_stack.call_count, 2) self.assertEqual(CfnUtils.get_stack_template.call_count, 2) self.assertEqual(CfnUtils.delete_stack.call_count, 4) self.assertEqual(CfnUtils.wait_for_delete.call_count, 4) diff --git a/tests/unit/lib/delete/test_cfn_utils.py b/tests/unit/lib/delete/test_cfn_utils.py index 7cd2474aea..6521fa80da 100644 --- a/tests/unit/lib/delete/test_cfn_utils.py +++ b/tests/unit/lib/delete/test_cfn_utils.py @@ -1,8 +1,17 @@ -from unittest.mock import patch, MagicMock, ANY, call +from unittest.mock import patch, MagicMock from unittest import TestCase +from parameterized import parameterized -from samcli.commands.delete.exceptions import DeleteFailedError, FetchTemplateFailedError, CfDeleteFailedStatusError +from samcli.commands.delete.exceptions import ( + DeleteFailedError, + FetchChangeSetError, + FetchTemplateFailedError, + CfDeleteFailedStatusError, + NoChangeSetFoundError, + StackFetchError, + StackProtectionEnabledError, +) from botocore.exceptions import ClientError, BotoCoreError, WaiterError from samcli.lib.delete.cfn_utils import CfnUtils @@ -31,44 +40,38 @@ def test_cf_utils_init(self): def test_cf_utils_has_no_stack(self): self.cf_utils._client.describe_stacks = MagicMock(return_value={"Stacks": []}) - self.assertEqual(self.cf_utils.has_stack("test"), False) + self.assertEqual(self.cf_utils.can_delete_stack("test"), False) - def test_cf_utils_has_stack_exception_non_existent(self): + def test_cf_utils_can_delete_stack_exception_non_existent(self): self.cf_utils._client.describe_stacks = MagicMock( side_effect=ClientError( error_response={"Error": {"Message": "Stack with id test does not exist"}}, operation_name="stack_status", ) ) - self.assertEqual(self.cf_utils.has_stack("test"), False) + self.assertEqual(self.cf_utils.can_delete_stack("test"), False) - def test_cf_utils_has_stack_exception_client_error(self): + def test_cf_utils_can_delete_stack_exception_client_error(self): self.cf_utils._client.describe_stacks = MagicMock( side_effect=ClientError( error_response={"Error": {"Message": "Error: The security token included in the request is expired"}}, operation_name="stack_status", ) ) - with self.assertRaises(DeleteFailedError): - self.cf_utils.has_stack("test") + with self.assertRaises(StackFetchError): + self.cf_utils.can_delete_stack("test") - def test_cf_utils_has_stack_termination_protection_enabled(self): + def test_cf_utils_can_delete_stack_termination_protection_enabled(self): self.cf_utils._client.describe_stacks = MagicMock( return_value={"Stacks": [{"StackStatus": "CREATE_COMPLETE", "EnableTerminationProtection": True}]} ) - with self.assertRaises(DeleteFailedError): - self.cf_utils.has_stack("test") - - def test_cf_utils_has_stack_in_review(self): - self.cf_utils._client.describe_stacks = MagicMock( - return_value={"Stacks": [{"StackStatus": "REVIEW_IN_PROGRESS", "EnableTerminationProtection": False}]} - ) - self.assertEqual(self.cf_utils.has_stack("test"), False) + with self.assertRaises(StackProtectionEnabledError): + self.cf_utils.can_delete_stack("test") - def test_cf_utils_has_stack_exception_botocore(self): + def test_cf_utils_can_delete_stack_exception_botocore(self): self.cf_utils._client.describe_stacks = MagicMock(side_effect=BotoCoreError()) - with self.assertRaises(DeleteFailedError): - self.cf_utils.has_stack("test") + with self.assertRaises(StackFetchError): + self.cf_utils.can_delete_stack("test") def test_cf_utils_get_stack_template_exception_client_error(self): self.cf_utils._client.get_template = MagicMock( @@ -94,7 +97,7 @@ def test_cf_utils_get_stack_template_success(self): self.cf_utils._client.get_template = MagicMock(return_value=({"TemplateBody": "Hello World"})) response = self.cf_utils.get_stack_template("test", "Original") - self.assertEqual(response, {"TemplateBody": "Hello World"}) + self.assertEqual(response, "Hello World") def test_cf_utils_delete_stack_exception_botocore(self): self.cf_utils._client.delete_stack = MagicMock(side_effect=BotoCoreError()) @@ -187,3 +190,74 @@ def test_cf_utils_wait_for_delete_failed_status(self): ) with self.assertRaises(CfDeleteFailedStatusError): self.cf_utils.wait_for_delete("test") + + def test_cfn_utils_has_stack(self): + self.cf_utils._client.describe_stacks = MagicMock( + return_value={"Stacks": [{"EnableTerminationProtection": False}]} + ) + + result = self.cf_utils.can_delete_stack(MagicMock()) + + self.assertTrue(result) + + def test_cfn_utils_get_change_set_name(self): + change_set_name = "hello change set" + + self.cf_utils._client.list_change_sets = MagicMock( + return_value={"Summaries": [{"ChangeSetName": change_set_name}]} + ) + + result = self.cf_utils._get_change_set_name(MagicMock()) + + self.assertEqual(change_set_name, result) + + def test_cfn_utils_get_change_set_name_raises_no_change_sets(self): + self.cf_utils._client.list_change_sets = MagicMock() + + with self.assertRaises(NoChangeSetFoundError): + self.cf_utils._get_change_set_name(MagicMock()) + + @parameterized.expand( + [ + (ClientError(MagicMock(), MagicMock()),), + (BotoCoreError(),), + ] + ) + def test_cfn_utils_get_change_set_name_reraises_api_error(self, exception): + self.cf_utils._client.list_change_sets = MagicMock(side_effect=exception) + + with self.assertRaises(FetchChangeSetError): + self.cf_utils._get_change_set_name(MagicMock()) + + def test_get_template_use_change_set(self): + change_set_template = "from change set" + + self.cf_utils._client.get_template = MagicMock( + side_effect=[{"TemplateBody": ""}, {"TemplateBody": change_set_template}] + ) + self.cf_utils._get_change_set_name = MagicMock(return_value=MagicMock()) + + result = self.cf_utils.get_stack_template(MagicMock(), MagicMock()) + + self.assertEqual(change_set_template, result) + + def test_get_template_use_change_set_empty(self): + self.cf_utils._client.get_template = MagicMock(return_value={"TemplateBody": ""}) + self.cf_utils._get_change_set_name = MagicMock(return_value=MagicMock()) + + result = self.cf_utils.get_stack_template(MagicMock(), MagicMock()) + + self.assertEqual(result, "") + + @parameterized.expand( + [ + (FetchChangeSetError(MagicMock(), MagicMock()),), + (NoChangeSetFoundError(MagicMock()),), + ] + ) + def test_get_change_set_reraises_exceptions(self, caught_exception): + self.cf_utils._client.get_template = MagicMock(return_value={"TemplateBody": ""}) + self.cf_utils._get_change_set_name = MagicMock(side_effect=caught_exception) + + with self.assertRaises(FetchTemplateFailedError): + self.cf_utils.get_stack_template(MagicMock(), MagicMock())