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 <kaix+github@fastmail.com>

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,
+        change_set_type,
-        stack_name,
-        stack_template,
         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__(
+        self.change_set_type = change_set_type
-            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_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()
+            for stack in self.stacks.values():
-            stack = None
+                if stack.name == stack_name:
-            for s in stacks:
+                    break
-                if s.name == stack_name:
+            else:
-                    stack = s
-                    stack_id = stack.stack_id
-                    stack_template = stack.template
-            if stack is None:
                 raise ValidationError(stack_name)
         else:
             stack_id = generate_stack_id(stack_name, region_name)
-            stack_template = {}
+            stack = FakeStack(
-
-        change_set_id = generate_changeset_id(change_set_name, region_name)
-        new_change_set = FakeChangeSet(
                 stack_id=stack_id,
-            stack_name=stack_name,
+                name=stack_name,
-            stack_template=stack_template,
+                template={},
-            change_set_id=change_set_id,
-            change_set_name=change_set_name,
-            template=template,
                 parameters=parameters,
                 region_name=region_name,
                 notification_arns=notification_arns,
                 tags=tags,
                 role_arn=role_arn,
-            cross_stack_resources=self.exports,
             )
+            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(
+            change_set_type=change_set_type,
+            change_set_id=change_set_id,
+            change_set_name=change_set_name,
+            stack=stack,
+            template=template,
+            parameters=parameters,
+            description=description,
+            notification_arns=notification_arns,
+            tags=tags,
+            role_arn=role_arn,
+        )
+        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.stack_id
-        return change_set_id, 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]
+                    change_set = self.change_sets[cs]
-        if stack is None:
+
+        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")
+            change_set.stack._add_stack_event("UPDATE_IN_PROGRESS")
-            stack._add_stack_event("UPDATE_COMPLETE")
+            change_set.apply()
-        stack.create_resources()
+            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):

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 = """<DescribeChangeSetResponse>
     <StackName>{{ change_set.stack_name }}</StackName>
     <Description>{{ change_set.description }}</Description>
     <Parameters>
-      {% for param_name, param_value in change_set.stack_parameters.items() %}
+      {% for param_name, param_value in change_set.parameters.items() %}
        <member>
           <ParameterKey>{{ param_name }}</ParameterKey>
           <ParameterValue>{{ param_value }}</ParameterValue>

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