diff --git a/.github/workflows/comment-integration-tests.yaml b/.github/workflows/comment-integration-tests.yaml deleted file mode 100644 index a05829a8f..000000000 --- a/.github/workflows/comment-integration-tests.yaml +++ /dev/null @@ -1,18 +0,0 @@ -# The idea with this workflow is to allow users to trigger an integration test -# run from a PR however it doesn't work because github action does not allow -# access to the github token when triggered from a PR. The workflow fails with.. -# "Credentials could not be loaded, please check your action inputs: Could not load credentials from any providers" - -name: comment-integration-tests - -on: - pull_request_review: - types: [submitted] - -jobs: - integration-tests: - if: ${{ contains(github.event.review.body, '/integration-tests') }} - uses: "./.github/workflows/integration-tests.yaml" - with: - # role generated from https://github.com/Sceptre/sceptre-aws/blob/master/config/prod/gh-oidc-sceptre-tests.yaml - role-to-assume: "arn:aws:iam::743644221192:role/gh-oidc-sceptre-tests" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ff7965c05..139d05a37 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -17,7 +17,7 @@ repos: hooks: - id: yamllint - repo: https://github.com/awslabs/cfn-python-lint - rev: v1.2.5.a11 + rev: v1.8.2 hooks: - id: cfn-python-lint args: @@ -47,7 +47,7 @@ repos: language_version: python3.10 args: ['--check'] - repo: https://github.com/sirosen/check-jsonschema - rev: 0.28.5 + rev: 0.29.1 hooks: - id: check-github-workflows - id: check-github-actions diff --git a/integration-tests/features/create-change-set.feature b/integration-tests/features/create-change-set.feature index a53cc8563..91d829861 100644 --- a/integration-tests/features/create-change-set.feature +++ b/integration-tests/features/create-change-set.feature @@ -18,8 +18,7 @@ Feature: Create change set Given stack "1/A" does not exist and the template for stack "1/A" is "valid_template.json" When the user creates change set "A" for stack "1/A" - Then a "ClientError" is raised - and the user is told "stack does not exist" + Then stack "1/A" has change set "A" in "CREATE_COMPLETE" state Scenario: create new change set with updated template and ignore dependencies Given stack "1/A" exists in "CREATE_COMPLETE" state diff --git a/integration-tests/features/describe-change-set.feature b/integration-tests/features/describe-change-set.feature index d1fbfd6c7..5908d16a9 100644 --- a/integration-tests/features/describe-change-set.feature +++ b/integration-tests/features/describe-change-set.feature @@ -10,8 +10,7 @@ Feature: Describe change sets Given stack "1/A" exists in "CREATE_COMPLETE" state and stack "1/A" has no change sets When the user describes change set "A" for stack "1/A" - Then a "ClientError" is raised - and the user is told "change set does not exist" + Then the user is told "Failed describing Change Set" Scenario: describe a change set that exists with ignore dependencies Given stack "1/A" exists in "CREATE_COMPLETE" state diff --git a/integration-tests/features/describe-stack-group-resources.feature b/integration-tests/features/describe-stack-group-resources.feature deleted file mode 100644 index 1dd0f47f3..000000000 --- a/integration-tests/features/describe-stack-group-resources.feature +++ /dev/null @@ -1,23 +0,0 @@ -Feature: Describe stack_group resources - - Scenario: describe resources of a stack_group that does not exist - Given stack_group "2" does not exist - When the user describes resources in stack_group "2" - Then no resources are described - - Scenario: describe resources of a stack_group that already exists - Given all the stacks in stack_group "2" are in "CREATE_COMPLETE" - When the user describes resources in stack_group "2" - Then only all resources in stack_group "2" are described - - Scenario: describe a stack_group that partially exists - Given stack "2/A" exists in "CREATE_COMPLETE" state - and stack "2/B" does not exist - and stack "2/C" does not exist - When the user describes resources in stack_group "2" - Then only resources in stack "2/A" are described - - Scenario: describe resources of a stack_group that already exists with ignore dependencies - Given all the stacks in stack_group "2" are in "CREATE_COMPLETE" - When the user describes resources in stack_group "2" with ignore dependencies - Then only all resources in stack_group "2" are described diff --git a/integration-tests/features/describe-stack-group.feature b/integration-tests/features/describe-stack-group.feature deleted file mode 100644 index 8b1756de8..000000000 --- a/integration-tests/features/describe-stack-group.feature +++ /dev/null @@ -1,25 +0,0 @@ -Feature: Describe stack_group - - Scenario: describe a stack_group that does not exist - Given stack_group "2" does not exist - When the user describes stack_group "2" - Then no resources are described - - Scenario: describe a stack_group that already exists - Given all the stacks in stack_group "2" are in "CREATE_COMPLETE" - When the user describes stack_group "2" - Then all stacks in stack_group "2" are described as "CREATE_COMPLETE" - - Scenario: describe a stack_group that partially exists - Given stack "2/A" exists in "CREATE_COMPLETE" state - and stack "2/B" exists in "UPDATE_COMPLETE" state - and stack "2/C" does not exist - When the user describes stack_group "2" - Then stack "2/A" is described as "CREATE_COMPLETE" - and stack "2/B" is described as "UPDATE_COMPLETE" - and stack "2/C" is described as "PENDING" - - Scenario: describe a stack_group that already exists with ignore dependencies - Given all the stacks in stack_group "2" are in "CREATE_COMPLETE" - When the user describes stack_group "2" with ignore dependencies - Then all stacks in stack_group "2" are described as "CREATE_COMPLETE" diff --git a/integration-tests/features/describe-stack-resources.feature b/integration-tests/features/describe-stack-resources.feature deleted file mode 100644 index f53c77b93..000000000 --- a/integration-tests/features/describe-stack-resources.feature +++ /dev/null @@ -1,11 +0,0 @@ -Feature: Describe-stack-resources - - Scenario: describe the resources of a stack that exists - Given stack "1/A" exists in "CREATE_COMPLETE" state - When the user describes the resources of stack "1/A" - Then the resources of stack "1/A" are described - - Scenario: describe the resources of a stack that exists with ignore dependencies - Given stack "1/A" exists in "CREATE_COMPLETE" state - When the user describes the resources of stack "1/A" with ignore dependencies - Then the resources of stack "1/A" are described diff --git a/integration-tests/features/execute-change-set.feature b/integration-tests/features/execute-change-set.feature index 3ae3f650c..c9046919d 100644 --- a/integration-tests/features/execute-change-set.feature +++ b/integration-tests/features/execute-change-set.feature @@ -11,8 +11,7 @@ Feature: Execute change set Given stack "1/A" exists in "CREATE_COMPLETE" state And stack "1/A" does not have change set "A" When the user executes change set "A" for stack "1/A" - Then a "ClientError" is raised - And the user is told "change set does not exist" + Then the user is told "change set does not exist" Scenario: execute a change set that exists with ignore dependencies Given stack "1/A" exists in "CREATE_COMPLETE" state diff --git a/integration-tests/steps/helpers.py b/integration-tests/steps/helpers.py index ddf920690..7743ea3e0 100644 --- a/integration-tests/steps/helpers.py +++ b/integration-tests/steps/helpers.py @@ -17,14 +17,15 @@ def step_impl(context, message): msg = context.error.response["Error"]["Message"] assert msg.endswith("does not exist") elif message == "change set does not exist": - msg = context.error.response["Error"]["Message"] - assert msg.endswith("does not exist") + assert context.log_capture.find_event("does not exist") elif message == "the template is valid": for stack, status in context.response.items(): assert status["ResponseMetadata"]["HTTPStatusCode"] == 200 elif message == "the template is malformed": msg = context.error.response["Error"]["Message"] assert msg.startswith("Template format error") + elif message == "Failed describing Change Set": + assert context.log_capture.find_event(message) else: raise Exception("Step has incorrect message") diff --git a/integration-tests/steps/stack_groups.py b/integration-tests/steps/stack_groups.py index b6ef69b4d..07a3282a3 100644 --- a/integration-tests/steps/stack_groups.py +++ b/integration-tests/steps/stack_groups.py @@ -97,68 +97,6 @@ def step_impl(context, stack_group_name): sceptre_plan.delete() -@when('the user describes stack_group "{stack_group_name}"') -def step_impl(context, stack_group_name): - sceptre_context = SceptreContext( - command_path=stack_group_name, project_path=context.sceptre_dir - ) - - sceptre_plan = SceptrePlan(sceptre_context) - responses = sceptre_plan.describe() - - stack_names = get_full_stack_names(context, stack_group_name) - cfn_stacks = {} - - for response in responses.values(): - if response is None: - continue - for stack in response["Stacks"]: - cfn_stacks[stack["StackName"]] = stack["StackStatus"] - - context.response = [ - {short_name: cfn_stacks[full_name]} - for short_name, full_name in stack_names.items() - if cfn_stacks.get(full_name) - ] - - -@when('the user describes stack_group "{stack_group_name}" with ignore dependencies') -def step_impl(context, stack_group_name): - sceptre_context = SceptreContext( - command_path=stack_group_name, - project_path=context.sceptre_dir, - ignore_dependencies=True, - ) - - sceptre_plan = SceptrePlan(sceptre_context) - responses = sceptre_plan.describe() - - stack_names = get_full_stack_names(context, stack_group_name) - cfn_stacks = {} - - for response in responses.values(): - if response is None: - continue - for stack in response["Stacks"]: - cfn_stacks[stack["StackName"]] = stack["StackStatus"] - - context.response = [ - {short_name: cfn_stacks[full_name]} - for short_name, full_name in stack_names.items() - if cfn_stacks.get(full_name) - ] - - -@when('the user describes resources in stack_group "{stack_group_name}"') -def step_impl(context, stack_group_name): - sceptre_context = SceptreContext( - command_path=stack_group_name, project_path=context.sceptre_dir - ) - - sceptre_plan = SceptrePlan(sceptre_context) - context.response = sceptre_plan.describe_resources().values() - - @when( 'the user describes resources in stack_group "{stack_group_name}" with ignore dependencies' ) @@ -196,21 +134,6 @@ def step_impl(context, stack_group_name): check_stack_status(context, full_stack_names, None) -@then('all stacks in stack_group "{stack_group_name}" are described as "{status}"') -def step_impl(context, stack_group_name, status): - stacks_names = get_stack_names(context, stack_group_name) - expected_response = [{stack_name: status} for stack_name in stacks_names] - for response in context.response: - assert response in expected_response - - -@then("no resources are described") -def step_impl(context): - for stack_resources in context.response: - stack_name = next(iter(stack_resources)) - assert stack_resources == {stack_name: []} - - @then('stack "{stack_name}" is described as "{status}"') def step_impl(context, stack_name, status): response = next( @@ -221,51 +144,6 @@ def step_impl(context, stack_name, status): assert response[stack_name] == status -@then('only all resources in stack_group "{stack_group_name}" are described') -def step_impl(context, stack_group_name): - stacks_names = get_full_stack_names(context, stack_group_name) - expected_resources = {} - sceptre_response = [] - for stack_resources in context.response: - for resource in stack_resources.values(): - sceptre_response.append(resource[0]["PhysicalResourceId"]) - - for short_name, full_name in stacks_names.items(): - time.sleep(1) - response = retry_boto_call( - context.client.describe_stack_resources, StackName=full_name - ) - expected_resources[short_name] = response["StackResources"] - - for short_name, resources in expected_resources.items(): - for resource in resources: - sceptre_response.remove(resource["PhysicalResourceId"]) - - assert sceptre_response == [] - - -@then('only resources in stack "{stack_name}" are described') -def step_impl(context, stack_name): - expected_resources = {} - sceptre_response = [] - for stack_resources in context.response: - for resource in stack_resources.values(): - if resource: - sceptre_response.append(resource[0].get("PhysicalResourceId")) - - response = retry_boto_call( - context.client.describe_stack_resources, - StackName=get_cloudformation_stack_name(context, stack_name), - ) - expected_resources[stack_name] = response["StackResources"] - - for short_name, resources in expected_resources.items(): - for resource in resources: - sceptre_response.remove(resource["PhysicalResourceId"]) - - assert sceptre_response == [] - - @then('that stack "{first_stack}" was created before "{second_stack}"') def step_impl(context, first_stack, second_stack): stacks = [ diff --git a/integration-tests/steps/stacks.py b/integration-tests/steps/stacks.py index c286920d0..f072d1a49 100644 --- a/integration-tests/steps/stacks.py +++ b/integration-tests/steps/stacks.py @@ -310,30 +310,6 @@ def step_impl(context, stack_name): launch_stack(context, stack_name, False, True) -@when('the user describes the resources of stack "{stack_name}"') -def step_impl(context, stack_name): - sceptre_context = SceptreContext( - command_path=stack_name + ".yaml", project_path=context.sceptre_dir - ) - - sceptre_plan = SceptrePlan(sceptre_context) - context.output = list(sceptre_plan.describe_resources().values()) - - -@when( - 'the user describes the resources of stack "{stack_name}" with ignore dependencies' -) -def step_impl(context, stack_name): - sceptre_context = SceptreContext( - command_path=stack_name + ".yaml", - project_path=context.sceptre_dir, - ignore_dependencies=True, - ) - - sceptre_plan = SceptrePlan(sceptre_context) - context.output = list(sceptre_plan.describe_resources().values()) - - @when('the user diffs stack "{stack_name}" with "{diff_type}"') def step_impl(context, stack_name, diff_type): sceptre_context = SceptreContext( @@ -381,21 +357,6 @@ def step_impl(context, stack_name): assert status is None -@then('the resources of stack "{stack_name}" are described') -def step_impl(context, stack_name): - full_name = get_cloudformation_stack_name(context, stack_name) - response = retry_boto_call( - context.client.describe_stack_resources, StackName=full_name - ) - properties = {"LogicalResourceId", "PhysicalResourceId"} - formatted_response = [ - {k: v for k, v in item.items() if k in properties} - for item in response["StackResources"] - ] - - assert [{stack_name: formatted_response}] == context.output - - @then( 'stack "{stack_name}" does not exist and stack "{dependant_stack_name}" exists in "{desired_state}"' ) diff --git a/pyproject.toml b/pyproject.toml index 91d78e158..2c686a808 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,6 +22,7 @@ classifiers = [ [tool.poetry.plugins."sceptre.hooks"] "asg_scheduled_actions" = "sceptre.hooks.asg_scaling_processes:ASGScalingProcesses" +"asg_scaling_processes" = "sceptre.hooks.asg_scaling_processes:ASGScalingProcesses" "cmd" = "sceptre.hooks.cmd:Cmd" [tool.poetry.plugins."sceptre.resolvers"] diff --git a/sceptre/cli/helpers.py b/sceptre/cli/helpers.py index c5eeb6dee..4fd98c0d5 100644 --- a/sceptre/cli/helpers.py +++ b/sceptre/cli/helpers.py @@ -337,6 +337,9 @@ def simplify_change_set_description(response): :returns: A more concise description of the change set. :rtype: dict """ + if not response: + return {"ChangeSetName": "ChangeSetNotFound"} + desired_response_items = [ "ChangeSetName", "CreationTime", diff --git a/sceptre/diffing/stack_differ.py b/sceptre/diffing/stack_differ.py index e49ef665d..a575c6afc 100644 --- a/sceptre/diffing/stack_differ.py +++ b/sceptre/diffing/stack_differ.py @@ -23,6 +23,8 @@ from sceptre.plan.actions import StackActions from sceptre.stack import Stack +from botocore.exceptions import ClientError + DiffType = TypeVar("DiffType") logger = logging.getLogger(__name__) @@ -192,10 +194,12 @@ def _extract_parameters_from_generated_stack(self, stack: Stack) -> dict: def _create_deployed_stack_config( self, stack_actions: StackActions ) -> Optional[StackConfiguration]: - description = stack_actions.describe() - if description is None: + try: + description = stack_actions.describe() + except ClientError as err: # This means the stack has not been deployed yet - return None + if err.response["Error"]["Message"].endswith("does not exist"): + return None stacks = description["Stacks"] for stack in stacks: diff --git a/sceptre/plan/actions.py b/sceptre/plan/actions.py index 478c16df1..520775575 100644 --- a/sceptre/plan/actions.py +++ b/sceptre/plan/actions.py @@ -212,7 +212,11 @@ def launch(self) -> StackStatus: if existing_status == "PENDING": status = self.create() - elif existing_status in ["CREATE_FAILED", "ROLLBACK_COMPLETE"]: + elif existing_status in [ + "CREATE_FAILED", + "ROLLBACK_COMPLETE", + "REVIEW_IN_PROGRESS", + ]: self.delete() status = self.create() elif existing_status.endswith("COMPLETE"): @@ -303,16 +307,11 @@ def describe(self): :returns: A Stack description. :rtype: dict """ - try: - return self.connection_manager.call( - service="cloudformation", - command="describe_stacks", - kwargs={"StackName": self.stack.external_name}, - ) - except botocore.exceptions.ClientError as e: - if e.response["Error"]["Message"].endswith("does not exist"): - return - raise + return self.connection_manager.call( + service="cloudformation", + command="describe_stacks", + kwargs={"StackName": self.stack.external_name}, + ) def describe_events(self): """ @@ -364,15 +363,11 @@ def describe_outputs(self): """ Returns the Stack's outputs. - :returns: The Stack's outputs. + :returns: The stack's outputs. :rtype: list """ self.logger.debug("%s - Describing stack outputs", self.stack.name) - - try: - response = self._describe() - except botocore.exceptions.ClientError: - return [] + response = self.describe() return {self.stack.name: response["Stacks"][0].get("Outputs", [])} @@ -440,6 +435,21 @@ def create_change_set(self, change_set_name): :param change_set_name: The name of the Change Set. :type change_set_name: str """ + try: + existing_status = self._get_status() + except StackDoesNotExistError: + existing_status = "PENDING" + + self.logger.info( + "%s - Stack is in the %s state", self.stack.name, existing_status + ) + + change_set_type = ( + "CREATE" + if existing_status in ["PENDING", "REVIEW_IN_PROGRESS"] + else "UPDATE" + ) + create_change_set_kwargs = { "StackName": self.stack.external_name, "Parameters": self._format_parameters(self.stack.parameters), @@ -449,13 +459,27 @@ def create_change_set(self, change_set_name): "CAPABILITY_AUTO_EXPAND", ], "ChangeSetName": change_set_name, + "ChangeSetType": change_set_type, "NotificationARNs": self.stack.notifications, "Tags": [ {"Key": str(k), "Value": str(v)} for k, v in self.stack.tags.items() ], } + create_change_set_kwargs.update(self.stack.template.get_boto_call_parameter()) create_change_set_kwargs.update(self._get_role_arn()) + + try: + self._create_change_set(change_set_name, create_change_set_kwargs) + except Exception as err: + self.logger.info( + "%s - Failed creating Change Set '%s'\n%s", + self.stack.name, + change_set_name, + err, + ) + + def _create_change_set(self, change_set_name, create_change_set_kwargs): self.logger.debug( "%s - Creating Change Set '%s'", self.stack.name, change_set_name ) @@ -479,6 +503,24 @@ def delete_change_set(self, change_set_name): :param change_set_name: The name of the Change Set. :type change_set_name: str """ + # If the call successfully completes, AWS CloudFormation + # successfully deleted the Change Set. + try: + self._delete_change_set(change_set_name) + self.logger.info( + "%s - Successfully deleted Change Set '%s'", + self.stack.name, + change_set_name, + ) + except Exception as err: + self.logger.info( + "%s - Failed deleting Change Set '%s'\n%s", + self.stack.name, + change_set_name, + err, + ) + + def _delete_change_set(self, change_set_name): self.logger.debug( "%s - Deleting Change Set '%s'", self.stack.name, change_set_name ) @@ -490,13 +532,6 @@ def delete_change_set(self, change_set_name): "StackName": self.stack.external_name, }, ) - # If the call successfully completes, AWS CloudFormation - # successfully deleted the Change Set. - self.logger.info( - "%s - Successfully deleted Change Set '%s'", - self.stack.name, - change_set_name, - ) def describe_change_set(self, change_set_name): """ @@ -507,6 +542,21 @@ def describe_change_set(self, change_set_name): :returns: The description of the Change Set. :rtype: dict """ + return_val = {} + + try: + return_val = self._describe_change_set(change_set_name) + except Exception as err: + self.logger.info( + "%s - Failed describing Change Set '%s'\n%s", + self.stack.name, + change_set_name, + err, + ) + + return return_val + + def _describe_change_set(self, change_set_name): self.logger.debug( "%s - Describing Change Set '%s'", self.stack.name, change_set_name ) @@ -532,7 +582,10 @@ def execute_change_set(self, change_set_name): change_set = self.describe_change_set(change_set_name) status = change_set.get("Status") reason = change_set.get("StatusReason") - if status == "FAILED" and self.change_set_creation_failed_due_to_no_changes( + + return_val = 0 + + if status == "FAILED" and self._change_set_creation_failed_due_to_no_changes( reason ): self.logger.info( @@ -540,8 +593,21 @@ def execute_change_set(self, change_set_name): change_set.get("StackName") ) ) - return 0 + return return_val + + try: + return_val = self._execute_change_set(change_set_name) + except Exception as err: + self.logger.info( + "%s - Failed describing Change Set '%s'\n%s", + self.stack.name, + change_set_name, + err, + ) + + return return_val + def _execute_change_set(self, change_set_name): self.logger.debug( "%s - Executing Change Set '%s'", self.stack.name, change_set_name ) @@ -556,8 +622,9 @@ def execute_change_set(self, change_set_name): status = self._wait_for_completion(boto_response=response) return status - def change_set_creation_failed_due_to_no_changes(self, reason: str) -> bool: - """Indicates the change set failed when it was created because there were actually + def _change_set_creation_failed_due_to_no_changes(self, reason: str) -> bool: + """ + Indicates the change set failed when it was created because there were actually no changes introduced from the change set. :param reason: The reason reported by CloudFormation for the Change Set failure @@ -784,16 +851,9 @@ def timed_out(elapsed): return status - def _describe(self): - return self.connection_manager.call( - service="cloudformation", - command="describe_stacks", - kwargs={"StackName": self.stack.external_name}, - ) - def _get_status(self): try: - status = self._describe()["Stacks"][0]["StackStatus"] + status = self.describe()["Stacks"][0]["StackStatus"] except botocore.exceptions.ClientError as exp: if exp.response["Error"]["Message"].endswith("does not exist"): raise StackDoesNotExistError(exp.response["Error"]["Message"]) @@ -1106,9 +1166,6 @@ def _log_drift_status(self, response: dict) -> None: self.logger.debug(f"{self.stack.name} - {key} - {response[key]}") def _detect_stack_drift(self) -> dict: - """ - Run detect_stack_drift. - """ self.logger.info(f"{self.stack.name} - Detecting Stack Drift") return self.connection_manager.call( @@ -1118,9 +1175,6 @@ def _detect_stack_drift(self) -> dict: ) def _describe_stack_drift_detection_status(self, detection_id: str) -> dict: - """ - Run describe_stack_drift_detection_status. - """ self.logger.info(f"{self.stack.name} - Describing Stack Drift Detection Status") return self.connection_manager.call( diff --git a/sceptre/plan/plan.py b/sceptre/plan/plan.py index 27e551a64..69f5e0f9b 100644 --- a/sceptre/plan/plan.py +++ b/sceptre/plan/plan.py @@ -8,6 +8,7 @@ """ import functools import itertools +import pathlib from os import path, walk from typing import Dict, List, Set, Callable, Iterable, Optional @@ -55,6 +56,26 @@ def _execute(self, *args): executor = SceptrePlanExecutor(self.command, self.launch_order) return executor.execute(*args) + def _raise_no_launch_order_error(self): + MAX_VALID_STACK_PATH_COUNT = 10 + + error_text = f"No stacks detected from the given path '{sceptreise_path(self.context.command_path)}'." + command_path_parent = pathlib.PurePath(self.context.command_path).parent + + all_paths = self._valid_stack_paths() + filtered_valid_stack_paths = [ + p for p in all_paths if pathlib.PurePath(p).parent == command_path_parent + ] or all_paths + if ( + filtered_valid_stack_paths + and len(filtered_valid_stack_paths) < MAX_VALID_STACK_PATH_COUNT + ): + error_text += ( + f" Valid stack paths under '{sceptreise_path(str(command_path_parent))}' " + + f"are: {filtered_valid_stack_paths}" + ) + raise ConfigFileNotFoundError(error_text) + def _generate_launch_order(self, reverse=False) -> List[Set[Stack]]: if self.context.ignore_dependencies: return [self.command_stacks] @@ -73,12 +94,7 @@ def _generate_launch_order(self, reverse=False) -> List[Set[Stack]]: graph.remove_stack(stack) if not launch_order: - raise ConfigFileNotFoundError( - "No stacks detected from the given path '{}'. Valid stack paths are: {}".format( - sceptreise_path(self.context.command_path), - self._valid_stack_paths(), - ) - ) + self._raise_no_launch_order_error() return launch_order diff --git a/sceptre/stack.py b/sceptre/stack.py index fa5639a8b..7990e8126 100644 --- a/sceptre/stack.py +++ b/sceptre/stack.py @@ -263,7 +263,7 @@ def __init__( ) self.s3_details = s3_details - self.parameters = self._ensure_parameters(parameters or {}) + self.parameters = self._cast_parameters(parameters or {}) self.sceptre_user_data = sceptre_user_data or {} self.notifications = notifications or [] @@ -276,10 +276,21 @@ def _ensure_boolean(self, config_name: str, value: Any) -> bool: ) return value - def _ensure_parameters( + def _cast_parameters( self, parameters: Dict[str, Any] ) -> Dict[str, Union[str, List[Union[str, Resolver]], Resolver]]: - """Ensure CloudFormation parameters are of valid types""" + """Cast CloudFormation parameters to valid types""" + + def cast_value(value: Any) -> Union[str, List[Union[str, Resolver]], Resolver]: + if isinstance(value, bool): + return "true" if value else "false" + elif isinstance(value, (int, float)): + return str(value) + elif isinstance(value, list): + return [cast_value(item) for item in value] + elif isinstance(value, Resolver): + return value + return value def is_valid(value: Any) -> bool: return ( @@ -294,11 +305,19 @@ def is_valid(value: Any) -> bool: or isinstance(value, Resolver) ) - if not all(is_valid(value) for value in parameters.values()): + if not isinstance(parameters, dict): raise InvalidConfigFileError( - f"{self.name}: Values for parameters must be strings, lists or resolvers, got {parameters}" + f"{self.name}: parameters must be a dictionary of key-value pairs, got {parameters}" ) - return parameters + + casted_parameters = {k: cast_value(v) for k, v in parameters.items()} + + if not all(is_valid(value) for value in casted_parameters.values()): + raise InvalidConfigFileError( + f"{self.name}: Values for parameters must be strings, lists or resolvers, got {casted_parameters}" + ) + + return casted_parameters def __repr__(self): return ( diff --git a/tests/test_actions.py b/tests/test_actions.py index c3c5a007d..113ccf6c8 100644 --- a/tests/test_actions.py +++ b/tests/test_actions.py @@ -397,6 +397,19 @@ def test_launch_with_stack_that_failed_to_create( mock_create.assert_called_once_with() assert response == sentinel.launch_response + @patch("sceptre.plan.actions.StackActions.create") + @patch("sceptre.plan.actions.StackActions.delete") + @patch("sceptre.plan.actions.StackActions._get_status") + def test_launch_with_stack_in_review_in_progress( + self, mock_get_status, mock_delete, mock_create + ): + mock_get_status.return_value = "REVIEW_IN_PROGRESS" + mock_create.return_value = sentinel.launch_response + response = self.actions.launch() + mock_delete.assert_called_once_with() + mock_create.assert_called_once_with() + assert response == sentinel.launch_response + @patch("sceptre.plan.actions.StackActions.update") @patch("sceptre.plan.actions.StackActions._get_status") def test_launch_with_complete_stack_with_updates_to_perform( @@ -556,14 +569,15 @@ def test_describe_resources_sends_correct_request(self): ] } - @patch("sceptre.plan.actions.StackActions._describe") + @patch("sceptre.plan.actions.StackActions.describe") def test_describe_outputs_sends_correct_request(self, mock_describe): mock_describe.return_value = {"Stacks": [{"Outputs": sentinel.outputs}]} response = self.actions.describe_outputs() + mock_describe.assert_called_once_with() assert response == {self.stack.name: sentinel.outputs} - @patch("sceptre.plan.actions.StackActions._describe") + @patch("sceptre.plan.actions.StackActions.describe") def test_describe_outputs_handles_stack_with_no_outputs(self, mock_describe): mock_describe.return_value = {"Stacks": [{}]} response = self.actions.describe_outputs() @@ -631,6 +645,7 @@ def test_create_change_set_sends_correct_request(self): "CAPABILITY_AUTO_EXPAND", ], "ChangeSetName": sentinel.change_set_name, + "ChangeSetType": "UPDATE", "RoleARN": sentinel.cloudformation_service_role, "NotificationARNs": [sentinel.notification], "Tags": [{"Key": "tag1", "Value": "val1"}], @@ -658,12 +673,38 @@ def test_create_change_set_sends_correct_request_no_notifications(self): "CAPABILITY_AUTO_EXPAND", ], "ChangeSetName": sentinel.change_set_name, + "ChangeSetType": "UPDATE", "RoleARN": sentinel.cloudformation_service_role, "NotificationARNs": [], "Tags": [{"Key": "tag1", "Value": "val1"}], }, ) + @patch("sceptre.plan.actions.StackActions._get_status") + def test_create_change_set_with_non_existent_stack(self, mock_get_status): + mock_get_status.side_effect = StackDoesNotExistError() + self.template._body = sentinel.template + self.actions.create_change_set(sentinel.change_set_name) + self.actions.connection_manager.call.assert_called_with( + service="cloudformation", + command="create_change_set", + kwargs={ + "StackName": sentinel.external_name, + "TemplateBody": sentinel.template, + "Parameters": [{"ParameterKey": "key1", "ParameterValue": "val1"}], + "Capabilities": [ + "CAPABILITY_IAM", + "CAPABILITY_NAMED_IAM", + "CAPABILITY_AUTO_EXPAND", + ], + "ChangeSetName": sentinel.change_set_name, + "ChangeSetType": "CREATE", + "RoleARN": sentinel.cloudformation_service_role, + "NotificationARNs": [sentinel.notification], + "Tags": [{"Key": "tag1", "Value": "val1"}], + }, + ) + def test_delete_change_set_sends_correct_request(self): self.actions.delete_change_set(sentinel.change_set_name) self.actions.connection_manager.call.assert_called_with( @@ -892,13 +933,13 @@ def test_format_parameters_with_none_list_and_string_values(self): {"ParameterKey": "key2", "ParameterValue": "value4"}, ] - @patch("sceptre.plan.actions.StackActions._describe") + @patch("sceptre.plan.actions.StackActions.describe") def test_get_status_with_created_stack(self, mock_describe): mock_describe.return_value = {"Stacks": [{"StackStatus": "CREATE_COMPLETE"}]} status = self.actions.get_status() assert status == "CREATE_COMPLETE" - @patch("sceptre.plan.actions.StackActions._describe") + @patch("sceptre.plan.actions.StackActions.describe") def test_get_status_with_non_existent_stack(self, mock_describe): mock_describe.side_effect = ClientError( { @@ -911,7 +952,7 @@ def test_get_status_with_non_existent_stack(self, mock_describe): ) assert self.actions.get_status() == "PENDING" - @patch("sceptre.plan.actions.StackActions._describe") + @patch("sceptre.plan.actions.StackActions.describe") def test_get_status_with_unknown_clinet_error(self, mock_describe): mock_describe.side_effect = ClientError( {"Error": {"Code": "DoesNotExistException", "Message": "Boom!"}}, diff --git a/tests/test_diffing/test_stack_differ.py b/tests/test_diffing/test_stack_differ.py index 83bf7b441..b05a2f045 100644 --- a/tests/test_diffing/test_stack_differ.py +++ b/tests/test_diffing/test_stack_differ.py @@ -20,6 +20,8 @@ from sceptre.plan.actions import StackActions from sceptre.stack import Stack +from botocore.exceptions import ClientError + class ImplementedStackDiffer(StackDiffer): def __init__(self, command_capturer: Mock): @@ -224,14 +226,22 @@ def test_diff__deployed_stack_exists__returns_is_deployed_as_true(self): assert diff.is_deployed is True def test_diff__deployed_stack_does_not_exist__returns_is_deployed_as_false(self): - self.actions.describe.return_value = self.actions.describe.side_effect = None + self.actions.describe.side_effect = ClientError( + {"Error": {"Code": "Whatevs", "Message": "Stack does not exist"}}, + "DescribeStacks", + ) + self.actions.describe.return_value = None diff = self.differ.diff(self.actions) assert diff.is_deployed is False def test_diff__deployed_stack_does_not_exist__compares_none_to_generated_config( self, ): - self.actions.describe.return_value = self.actions.describe.side_effect = None + self.actions.describe.side_effect = ClientError( + {"Error": {"Code": "Whatevs", "Message": "Stack does not exist"}}, + "DescribeStacks", + ) + self.actions.describe.return_value = None self.differ.diff(self.actions) self.command_capturer.compare_stack_configurations.assert_called_with( diff --git a/tests/test_stack.py b/tests/test_stack.py index 6f67e08cd..e67edc49b 100644 --- a/tests/test_stack.py +++ b/tests/test_stack.py @@ -190,12 +190,7 @@ def test_init__non_boolean_obsolete_value__raises_invalid_config_file_error(self @pytest.mark.parametrize( "parameters", - [ - {"someNum": 1}, - {"someBool": True}, - {"aBadList": [1, 2, 3]}, - {"aDict": {"foo": "bar"}}, - ], + [{"Dict": {"foo": "bar"}}, {"List": ["of", "stuff", {"including": "aDict"}]}], ) def test_init__invalid_parameters_raise_invalid_config_file_error(self, parameters): with pytest.raises(InvalidConfigFileError): @@ -207,13 +202,30 @@ def test_init__invalid_parameters_raise_invalid_config_file_error(self, paramete parameters=parameters, ) + @pytest.mark.parametrize( + "parameters", + [["this", "is", "a", "list"], "a_string"], + ) + def test_init__invalid_parameters__parameters_a_list(self, parameters): + with pytest.raises(InvalidConfigFileError): + Stack( + name="stack_name", + project_code="project_code", + template_handler_config={"type": "file"}, + region="region", + parameters=parameters, + ) + @pytest.mark.parametrize( "parameters", [ - {"someNum": "1"}, - {"someBool": "true"}, - {"aList": ["aString", FakeResolver()]}, - {"aResolver": FakeResolver()}, + {"IntAsString": "1"}, + {"Int": 1}, + {"BoolAsString": "true"}, + {"Bool": True}, + {"List": ["aStringAndA", FakeResolver()]}, + {"ListOfInts": [1, 2, 3]}, + {"Resolver": FakeResolver()}, ], ) def test_init__valid_parameters_do_not_raise_invalid_config_file_error(