From 4145c48d8252b81466f0500775601946e81e0ad2 Mon Sep 17 00:00:00 2001 From: Alex Kennedy Date: Wed, 27 Nov 2024 14:29:24 -0800 Subject: [PATCH] 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 | 75 ++++++++++++++++--- kingpin/actors/aws/settings.py | 3 + .../actors/aws/test/test_cloudformation.py | 67 ++++++++++++++--- kingpin/version.py | 2 +- 4 files changed, 126 insertions(+), 21 deletions(-) diff --git a/kingpin/actors/aws/cloudformation.py b/kingpin/actors/aws/cloudformation.py index 987966f8..49086a75 100644 --- a/kingpin/actors/aws/cloudformation.py +++ b/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,7 +299,7 @@ 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") @@ -274,18 +307,18 @@ def _get_template_body(self, template: str, s3_region: Optional[str]): 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: diff --git a/kingpin/actors/aws/settings.py b/kingpin/actors/aws/settings.py index c2508cad..911a715c 100644 --- a/kingpin/actors/aws/settings.py +++ b/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") diff --git a/kingpin/actors/aws/test/test_cloudformation.py b/kingpin/actors/aws/test/test_cloudformation.py index d02640db..289145c6 100644 --- a/kingpin/actors/aws/test/test_cloudformation.py +++ b/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", diff --git a/kingpin/version.py b/kingpin/version.py index 97b01ee6..b16a4a99 100644 --- a/kingpin/version.py +++ b/kingpin/version.py @@ -13,4 +13,4 @@ # Copyright 2018 Nextdoor.com, Inc -__version__ = "3.0.4" +__version__ = "3.1.0"