From 9af1a96174c9ab47ead772f731faf27b043e2f82 Mon Sep 17 00:00:00 2001 From: Kai Xia Date: Sat, 7 Aug 2021 17:02:22 +1000 Subject: [PATCH] make FakeChangeSet a subclass of BaseModel, not FakeStack. This fixes #4141, and pave ways for future changes around changeset. We had subclassed FakeChangeSet from FakeStack, not from BaseModel. This made us easier to send the response for describe_change_set calls, but when we are handling the details of change set, the old approach won't work at all. For example, when we were creating a changeset, we were actually creating a stack without registering it (self.stacks), and future update onto this stack is not really possible. Signed-off-by: Kai Xia --- moto/cloudformation/models.py | 157 ++++++++++-------- moto/cloudformation/responses.py | 4 +- .../test_cloudformation_stack_crud_boto3.py | 23 ++- 3 files changed, 110 insertions(+), 74 deletions(-) diff --git a/moto/cloudformation/models.py b/moto/cloudformation/models.py index 1d5b300c6..61650513e 100644 --- a/moto/cloudformation/models.py +++ b/moto/cloudformation/models.py @@ -214,7 +214,6 @@ class FakeStack(BaseModel): tags=None, role_arn=None, cross_stack_resources=None, - create_change_set=False, ): self.stack_id = stack_id self.name = name @@ -231,24 +230,10 @@ class FakeStack(BaseModel): self.role_arn = role_arn self.tags = tags if tags else {} self.events = [] - if create_change_set: - self._add_stack_event( - "REVIEW_IN_PROGRESS", resource_status_reason="User Initiated" - ) - else: - self._add_stack_event( - "CREATE_IN_PROGRESS", resource_status_reason="User Initiated" - ) self.cross_stack_resources = cross_stack_resources or {} self.resource_map = self._create_resource_map() self.output_map = self._create_output_map() - if create_change_set: - self.status = "CREATE_COMPLETE" - self.execution_status = "AVAILABLE" - else: - self.create_resources() - self._add_stack_event("CREATE_COMPLETE") self.creation_time = datetime.utcnow() def _create_resource_map(self): @@ -372,47 +357,54 @@ class FakeChange(BaseModel): self.resource_type = resource_type -class FakeChangeSet(FakeStack): +class FakeChangeSet(BaseModel): def __init__( self, - stack_id, - stack_name, - stack_template, + change_set_type, change_set_id, change_set_name, + stack, template, parameters, - region_name, + description, notification_arns=None, tags=None, role_arn=None, - cross_stack_resources=None, ): - super(FakeChangeSet, self).__init__( - stack_id, - stack_name, - stack_template, - parameters, - region_name, - notification_arns=notification_arns, - tags=tags, - role_arn=role_arn, - cross_stack_resources=cross_stack_resources, - create_change_set=True, - ) - self.stack_name = stack_name + self.change_set_type = change_set_type self.change_set_id = change_set_id self.change_set_name = change_set_name - self.changes = self.diff(template=template, parameters=parameters) - if self.description is None: - self.description = self.template_dict.get("Description") - self.creation_time = datetime.utcnow() - def diff(self, template, parameters=None): + self.stack = stack + self.stack_id = self.stack.stack_id + self.stack_name = self.stack.name + self.notification_arns = notification_arns + self.description = description + self.tags = tags + self.role_arn = role_arn self.template = template + self.parameters = parameters self._parse_template() + + self.creation_time = datetime.utcnow() + self.changes = self.diff() + + def _parse_template(self): + yaml.add_multi_constructor("", yaml_tag_constructor) + try: + self.template_dict = yaml.load(self.template, Loader=yaml.Loader) + except (yaml.parser.ParserError, yaml.scanner.ScannerError): + self.template_dict = json.loads(self.template) + + @property + def creation_time_iso_8601(self): + return iso_8601_datetime_without_milliseconds(self.creation_time) + + def diff(self): changes = [] - resources_by_action = self.resource_map.diff(self.template_dict, parameters) + resources_by_action = self.stack.resource_map.diff( + self.template_dict, self.parameters + ) for action, resources in resources_by_action.items(): for resource_name, resource in resources.items(): changes.append( @@ -424,6 +416,9 @@ class FakeChangeSet(FakeStack): ) return changes + def apply(self): + self.stack.resource_map.update(self.template_dict, self.parameters) + class FakeEvent(BaseModel): def __init__( @@ -590,7 +585,6 @@ class CloudFormationBackend(BaseBackend): notification_arns=None, tags=None, role_arn=None, - create_change_set=False, ): stack_id = generate_stack_id(name) new_stack = FakeStack( @@ -603,12 +597,16 @@ class CloudFormationBackend(BaseBackend): tags=tags, role_arn=role_arn, cross_stack_resources=self.exports, - create_change_set=create_change_set, ) self.stacks[stack_id] = new_stack self._validate_export_uniqueness(new_stack) for export in new_stack.exports: self.exports[export.name] = export + new_stack._add_stack_event( + "CREATE_IN_PROGRESS", resource_status_reason="User Initiated" + ) + new_stack.create_resources() + new_stack._add_stack_event("CREATE_COMPLETE") return new_stack def create_change_set( @@ -617,46 +615,55 @@ class CloudFormationBackend(BaseBackend): change_set_name, template, parameters, + description, region_name, change_set_type, notification_arns=None, tags=None, role_arn=None, ): - stack_id = None - stack_template = None if change_set_type == "UPDATE": - stacks = self.stacks.values() - stack = None - for s in stacks: - if s.name == stack_name: - stack = s - stack_id = stack.stack_id - stack_template = stack.template - if stack is None: + for stack in self.stacks.values(): + if stack.name == stack_name: + break + else: raise ValidationError(stack_name) else: stack_id = generate_stack_id(stack_name, region_name) - stack_template = {} + stack = FakeStack( + stack_id=stack_id, + name=stack_name, + template={}, + parameters=parameters, + region_name=region_name, + notification_arns=notification_arns, + tags=tags, + role_arn=role_arn, + ) + self.stacks[stack_id] = stack + stack.status = "REVIEW_IN_PROGRESS" + stack._add_stack_event( + "REVIEW_IN_PROGRESS", resource_status_reason="User Initiated" + ) change_set_id = generate_changeset_id(change_set_name, region_name) + new_change_set = FakeChangeSet( - stack_id=stack_id, - stack_name=stack_name, - stack_template=stack_template, + change_set_type=change_set_type, change_set_id=change_set_id, change_set_name=change_set_name, + stack=stack, template=template, parameters=parameters, - region_name=region_name, + description=description, notification_arns=notification_arns, tags=tags, role_arn=role_arn, - cross_stack_resources=self.exports, ) + new_change_set.status = "CREATE_COMPLETE" + new_change_set.execution_status = "AVAILABLE" self.change_sets[change_set_id] = new_change_set - self.stacks[stack_id] = new_change_set - return change_set_id, stack_id + return change_set_id, stack.stack_id def delete_change_set(self, change_set_name, stack_name=None): if change_set_name in self.change_sets: @@ -683,25 +690,35 @@ class CloudFormationBackend(BaseBackend): return change_set def execute_change_set(self, change_set_name, stack_name=None): - stack = None if change_set_name in self.change_sets: # This means arn was passed in - stack = self.change_sets[change_set_name] + change_set = self.change_sets[change_set_name] else: for cs in self.change_sets: if self.change_sets[cs].change_set_name == change_set_name: - stack = self.change_sets[cs] - if stack is None: + change_set = self.change_sets[cs] + + if change_set is None: raise ValidationError(stack_name) - if stack.events[-1].resource_status == "REVIEW_IN_PROGRESS": - stack._add_stack_event( + + if change_set.change_set_type == "CREATE": + change_set.stack._add_stack_event( "CREATE_IN_PROGRESS", resource_status_reason="User Initiated" ) - stack._add_stack_event("CREATE_COMPLETE") + change_set.apply() + change_set.stack._add_stack_event("CREATE_COMPLETE") else: - stack._add_stack_event("UPDATE_IN_PROGRESS") - stack._add_stack_event("UPDATE_COMPLETE") - stack.create_resources() + change_set.stack._add_stack_event("UPDATE_IN_PROGRESS") + change_set.apply() + change_set.stack._add_stack_event("UPDATE_COMPLETE") + + # set the execution status of the changeset + change_set.execution_status = "EXECUTE_COMPLETE" + + # set the status of the stack + self.stacks[ + change_set.stack_id + ].status = f"{change_set.change_set_type}_COMPLETE" return True def describe_stacks(self, name_or_stack_id): diff --git a/moto/cloudformation/responses.py b/moto/cloudformation/responses.py index 7213b99e1..d5a57a1f3 100644 --- a/moto/cloudformation/responses.py +++ b/moto/cloudformation/responses.py @@ -125,6 +125,7 @@ class CloudFormationResponse(BaseResponse): change_set_name = self._get_param("ChangeSetName") stack_body = self._get_param("TemplateBody") template_url = self._get_param("TemplateURL") + description = self._get_param("Description") role_arn = self._get_param("RoleARN") update_or_create = self._get_param("ChangeSetType", "CREATE") parameters_list = self._get_list_prefix("Parameters.member") @@ -144,6 +145,7 @@ class CloudFormationResponse(BaseResponse): change_set_name=change_set_name, template=stack_body, parameters=parameters, + description=description, region_name=self.region, notification_arns=stack_notification_arns, tags=tags, @@ -638,7 +640,7 @@ DESCRIBE_CHANGE_SET_RESPONSE_TEMPLATE = """ {{ change_set.stack_name }} {{ change_set.description }} - {% for param_name, param_value in change_set.stack_parameters.items() %} + {% for param_name, param_value in change_set.parameters.items() %} {{ param_name }} {{ param_value }} diff --git a/tests/test_cloudformation/test_cloudformation_stack_crud_boto3.py b/tests/test_cloudformation/test_cloudformation_stack_crud_boto3.py index 36e716d79..def515be2 100644 --- a/tests/test_cloudformation/test_cloudformation_stack_crud_boto3.py +++ b/tests/test_cloudformation/test_cloudformation_stack_crud_boto3.py @@ -1013,6 +1013,7 @@ def test_create_change_set_from_s3_url(): @mock_cloudformation +@mock_ec2 def test_describe_change_set(): cf_conn = boto3.client("cloudformation", region_name="us-east-1") cf_conn.create_change_set( @@ -1048,10 +1049,19 @@ def test_describe_change_set(): # Execute change set cf_conn.execute_change_set(ChangeSetName="NewChangeSet") - # Verify that the changes have been applied - stack = cf_conn.describe_change_set(ChangeSetName="NewChangeSet") - stack["Changes"].should.have.length_of(1) + # Verify that the changes have been applied + ec2 = boto3.client("ec2", region_name="us-east-1") + ec2.describe_instances()["Reservations"].should.have.length_of(1) + + change_set = cf_conn.describe_change_set(ChangeSetName="NewChangeSet") + change_set["Changes"].should.have.length_of(1) + change_set["ExecutionStatus"].should.equal("EXECUTE_COMPLETE") + + stack = cf_conn.describe_stacks(StackName="NewStack")["Stacks"][0] + stack["StackStatus"].should.equal("CREATE_COMPLETE") + + # create another change set to update the stack cf_conn.create_change_set( StackName="NewStack", TemplateBody=dummy_update_template_json, @@ -1063,6 +1073,13 @@ def test_describe_change_set(): stack["StackName"].should.equal("NewStack") stack["Changes"].should.have.length_of(2) + # Execute change set + cf_conn.execute_change_set(ChangeSetName="NewChangeSet2") + + # Verify that the changes have been applied + stack = cf_conn.describe_stacks(StackName="NewStack")["Stacks"][0] + stack["StackStatus"].should.equal("UPDATE_COMPLETE") + @mock_cloudformation @mock_ec2