feat(cfn): Append hash to outputs (#534)

* feat(cfn): Append hash to outputs

* pyblack

* fix hash date

* fix hash

* split strip_hash function

* format

* reenabl test

* move

* add new tests

* use env var as bool

* bump version

kingpin/actors/aws/cloudformation.py

@@ -17,6 +17,7 @@
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 """
 
+from hashlib import md5
 import json
 from json import JSONEncoder
 
@@ -35,6 +36,7 @@
 from kingpin import utils
 from kingpin.actors import exceptions
 from kingpin.actors.aws import base
+from kingpin.actors.aws.settings import KINGPIN_CFN_HASH_OUTPUT_KEY
 from kingpin.actors.utils import dry
 from kingpin.constants import REQUIRED, STATE
 from kingpin.constants import SchemaCompareBase, StringCompareBase
@@ -230,6 +232,34 @@ def _discover_default_params(self, template_body):
         }
         return default_params
 
+    def _strip_hash_dict(self, template: dict) -> dict:
+        """Strips the hash from the template.
+
+        Note: This will also strip the "Outputs" section if no other output exists. This might cause
+        issues when diffiing a template that contains an outputs section with no outputs.
+        """
+
+        # Bail if the user has disabled this feature.
+        if not KINGPIN_CFN_HASH_OUTPUT_KEY:
+            return template
+
+        # Bail if the "Outputs" section is missing or a type we do not expect.
+        if not isinstance(template.get("Outputs", None), dict):
+            return template
+
+        # Remove the hash from the Outputs section.
+        if KINGPIN_CFN_HASH_OUTPUT_KEY in template["Outputs"].keys():
+            del template["Outputs"][KINGPIN_CFN_HASH_OUTPUT_KEY]
+
+            # If there are no other outputs, remove the Outputs section entirely.
+            if len(template["Outputs"].keys()) == 0:
+                del template["Outputs"]
+
+        return template
+
+    def _strip_hash_str(self, template: str) -> str:
+        return json.dumps(self._strip_hash_dict(json.loads(template)))
+
     def _get_template_body(self, template: str, s3_region: Optional[str]):
         """Reads in a local template file and returns the contents.
 
@@ -249,6 +279,9 @@ def _get_template_body(self, template: str, s3_region: Optional[str]):
         if template is None:
             return None, None
 
+        ret_template: str = ""
+        ret_url: Optional[str] = None
+
         if template.startswith("s3://"):
             match = S3_REGEX.match(template)
             if match:
@@ -266,26 +299,26 @@ def _get_template_body(self, template: str, s3_region: Optional[str]):
                     s3_region = "us-east-1"
             # AWS has a multitude of different s3 url formats, but not all are
             # supported. Use this one.
-            url = f"https://{bucket}.s3.{s3_region}.amazonaws.com/{key}"
+            ret_url = f"https://{bucket}.s3.{s3_region}.amazonaws.com/{key}"
 
             s3 = self.get_s3_client(s3_region)
             log.debug("Downloading template stored in s3")
             try:
                 resp = s3.get_object(Bucket=bucket, Key=key)
             except ClientError as e:
                 raise InvalidTemplate(e)
-            remote_template = resp["Body"].read()
-            return remote_template, url
+            ret_template = resp["Body"].read()
         else:
             # The template is provided inline.
             try:
-                return (
-                    json.dumps(self._parse_policy_json(template), cls=DateEncoder),
-                    None,
+                ret_template = json.dumps(
+                    self._parse_policy_json(template), cls=DateEncoder
                 )
             except exceptions.UnrecoverableActorFailure as e:
                 raise InvalidTemplate(e)
 
+        return self._strip_hash_str(ret_template), ret_url
+
     def get_s3_client(self, region):
         """Get a boto3 S3 client for a given region.
 
@@ -382,7 +415,7 @@ def _get_stack(self, stack):
 
     @gen.coroutine
     def _get_stack_template(self, stack):
-        """Returns the live policy attached to a CF Stack.
+        """Returns the live template used by the CFN Stack.
 
         args:
             stack: Stack name or stack ID
@@ -394,7 +427,8 @@ def _get_stack_template(self, stack):
         except ClientError as e:
             raise CloudFormationError(e)
 
-        raise gen.Return(ret["TemplateBody"])
+        template_body: dict = ret["TemplateBody"]
+        raise gen.Return(self._strip_hash_dict(template_body))
 
     @gen.coroutine
     def _wait_until_state(self, stack_name, desired_states, sleep=15):
@@ -1080,6 +1114,29 @@ def _diff_params_safely(self, remote, local):
 
         return False
 
+    def _template_body_with_hash(self) -> str:
+        """Add a hash to the template to force a change in the stack."""
+
+        # Bail if the user has disabled this feature.
+        if not KINGPIN_CFN_HASH_OUTPUT_KEY:
+            return self._template_body
+
+        template_obj = json.loads(self._template_body)
+
+        if not isinstance(template_obj.get("Outputs", None), dict):
+            # overwrite the outputs with an empty dict
+            template_obj["Outputs"] = {}
+
+        template_obj["Outputs"][KINGPIN_CFN_HASH_OUTPUT_KEY] = {
+            "Value": md5(
+                # datetime.datetime.now(datetime.timezone.utc).isoformat().encode()  # requires freezegun during testing
+                # or
+                json.dumps(template_obj).encode()
+            ).hexdigest()
+        }
+
+        return json.dumps(template_obj)
+
     @gen.coroutine
     def _create_change_set(self, stack, uuid=uuid.uuid4().hex):
         """Generates a Change Set.
@@ -1108,7 +1165,7 @@ def _create_change_set(self, stack, uuid=uuid.uuid4().hex):
         if self._template_url:
             change_opts["TemplateURL"] = self._template_url
         else:
-            change_opts["TemplateBody"] = self._template_body
+            change_opts["TemplateBody"] = self._template_body_with_hash()
 
         self.log.info("Generating a stack Change Set...")
         try:

kingpin/actors/aws/settings.py

@@ -41,3 +41,6 @@
 # Docs: https://boto3.amazonaws.com/v1/documentation/api/latest/guide/retries.html
 AWS_MAX_ATTEMPTS = int(os.getenv("AWS_MAX_ATTEMPTS", 10))
 AWS_RETRY_MODE = os.getenv("AWS_RETRY_MODE", "standard")
+
+# Set to "" (an empty string) to disable.
+KINGPIN_CFN_HASH_OUTPUT_KEY = os.getenv("KINGPIN_CFN_HASH_OUTPUT_KEY", "KingpinCfnHash")

kingpin/actors/aws/test/test_cloudformation.py

@@ -92,7 +92,7 @@ def test_get_template_body_s3(self):
             "LocationConstraint": None
         }
 
-        expected_template = "i am a cfn template"
+        expected_template = '{"fake": "template"}'
         with mock.patch.object(self.actor, "get_s3_client") as mock_get:
             mock_s3 = mock.MagicMock()
             mock_body = mock.MagicMock()
@@ -248,7 +248,9 @@ def test_get_stack_template(self):
             "TemplateBody": {"Fake": "Stack"},
         }
         self.actor.cf3_conn.get_template.return_value = fake_stack_template
+
         ret = yield self.actor._get_stack_template("test")
+
         self.actor.cf3_conn.get_template.assert_has_calls(
             [mock.call(StackName="test", TemplateStage="Original")]
         )
@@ -516,14 +518,22 @@ def test_create_stack_file_with_termination_protection_true(self):
     @testing.gen_test
     def test_create_stack_url(self):
         with mock.patch.object(boto3, "client"):
-            actor = cloudformation.Create(
-                "Unit Test Action",
-                {
-                    "name": "unit-test-cf",
-                    "region": "us-west-2",
-                    "template": "s3://bucket/key",
-                },
-            )
+            with mock.patch.object(
+                cloudformation.CloudFormationBaseActor,
+                "_get_template_body",
+                return_value=(
+                    '{"fake": "template"}',
+                    "https://bucket.s3.us-west-2.amazonaws.com/key",
+                ),
+            ):
+                actor = cloudformation.Create(
+                    "Unit Test Action",
+                    {
+                        "name": "unit-test-cf",
+                        "region": "us-west-2",
+                        "template": "s3://bucket/key",
+                    },
+                )
         actor._wait_until_state = mock.MagicMock(name="_wait_until_state")
         actor._wait_until_state.side_effect = [tornado_value(None)]
         actor.cf3_conn.create_stack = mock.MagicMock(name="create_stack_mock")
@@ -698,6 +708,7 @@ def setUp(self):
         settings.AWS_ACCESS_KEY_ID = "unit-test"
         settings.AWS_SECRET_ACCESS_KEY = "unit-test"
         settings.AWS_SESSION_TOKEN = "unit-test"
+        settings.KINGPIN_CFN_HASH_OUTPUT_KEY = "KingpinCfnHash"
         importlib.reload(cloudformation)
         # Need to recreate the api call queues between tests
         # because nose creates a new ioloop per test run.
@@ -1067,6 +1078,40 @@ def test_ensure_template_exc(self):
         with self.assertRaises(cloudformation.StackFailed):
             yield self.actor._ensure_template(fake_stack)
 
+    def test_hash_feature_disabled(self):
+        settings.KINGPIN_CFN_HASH_OUTPUT_KEY = ""
+        importlib.reload(cloudformation)
+        self.actor = cloudformation.Stack(
+            options={
+                "name": "unit-test-cf",
+                "state": "present",
+                "region": "us-west-2",
+                "template": "examples/test/aws.cloudformation/cf.unittest.json",
+                "parameters": {"key1": "value1"},
+            }
+        )
+        self.actor._template_body = json.dumps({"blank": "json"})
+
+        ret1 = self.actor._template_body_with_hash()
+        ret2 = self.actor._strip_hash_str(self.actor._template_body)
+
+        self.assertEqual(ret1, json.dumps({"blank": "json"}))
+        self.assertEqual(ret2, json.dumps({"blank": "json"}))
+
+    def test_strip_hash_str(self):
+        self.actor._template_body = json.dumps(
+            {
+                "blank": "json",
+                "Outputs": {
+                    "KingpinCfnHash": {"Value": "251693d288f81514f8f49b594fc83e47"}
+                },
+            }
+        )
+
+        ret = self.actor._strip_hash_str(self.actor._template_body)
+
+        self.assertEqual(ret, json.dumps({"blank": "json"}))
+
     @testing.gen_test
     def test_create_change_set_body(self):
         self.actor.cf3_conn.create_change_set.return_value = {"Id": "abcd"}
@@ -1077,7 +1122,7 @@ def test_create_change_set_body(self):
             [
                 mock.call(
                     StackName="arn:aws:cloudformation:us-east-1:xxxx:stack/fake/x",
-                    TemplateBody='{"blank": "json"}',
+                    TemplateBody='{"blank": "json", "Outputs": {"KingpinCfnHash": {"Value": "251693d288f81514f8f49b594fc83e47"}}}',
                     Capabilities=[],
                     ChangeSetName="kingpin-uuid",
                     Parameters=[{"ParameterValue": "value1", "ParameterKey": "key1"}],
@@ -1097,7 +1142,7 @@ def test_create_change_set_body_with_role(self):
             [
                 mock.call(
                     StackName="arn:aws:cloudformation:us-east-1:xxxx:stack/fake/x",
-                    TemplateBody='{"blank": "json"}',
+                    TemplateBody='{"blank": "json", "Outputs": {"KingpinCfnHash": {"Value": "251693d288f81514f8f49b594fc83e47"}}}',
                     RoleARN="test_role_arn",
                     Capabilities=[],
                     ChangeSetName="kingpin-uuid",

kingpin/version.py

@@ -13,4 +13,4 @@
 # Copyright 2018 Nextdoor.com, Inc
 
 
-__version__ = "3.0.4"
+__version__ = "3.1.0"