From 344560043407f18098bfaec3e8bab50b3b115f76 Mon Sep 17 00:00:00 2001 From: Elliot Korte Date: Mon, 16 Jul 2018 12:28:44 -0500 Subject: [PATCH 1/7] add support for AWS::Include transform exports in cloudformation package template by adding a global export function for artifacts that are not specific to a resource type --- .../cloudformation/artifact_exporter.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/awscli/customizations/cloudformation/artifact_exporter.py b/awscli/customizations/cloudformation/artifact_exporter.py index 60db11a05160..b0a0e3d56322 100644 --- a/awscli/customizations/cloudformation/artifact_exporter.py +++ b/awscli/customizations/cloudformation/artifact_exporter.py @@ -405,6 +405,19 @@ def do_export(self, resource_id, resource_dict, parent_dir): } +def includes_transform_export_handler(template_dict, s3_uploader): + if template_dict.get("Name", None) != "AWS::Include": + return template_dict + includes_location = template_dict.get("Parameters", {}).get("Location", {}) + if (is_local_file(includes_location)): + template_dict["Parameters"]["Location"] = s3_uploader.upload_with_dedup(includes_location) + return template_dict + +GLOBAL_EXPORT_DICT = { + "Fn::Transform": includes_transform_export_handler +} + + class Template(object): """ Class to export a CloudFormation template @@ -432,6 +445,20 @@ def __init__(self, template_path, parent_dir, uploader, self.resources_to_export = resources_to_export self.uploader = uploader + def export_global_artifacts(self, template_dict): + """ + Template params such as AWS::Include transforms are not specific to + any resource type but contain artifacts that should be exported, + here we iterate through the template dict and export params with a + handler defined in GLOBAL_EXPORT_DICT + """ + for key, val in template_dict.iteritems(): + if key in GLOBAL_EXPORT_DICT: + val = GLOBAL_EXPORT_DICT[key](val, self.uploader) + elif type(val) is dict: + self.export_global_artifacts(val) + + def export(self): """ Exports the local artifacts referenced by the given template to an @@ -443,6 +470,8 @@ def export(self): if "Resources" not in self.template_dict: return self.template_dict + self.export_global_artifacts(self.template_dict) + for resource_id, resource in self.template_dict["Resources"].items(): resource_type = resource.get("Type", None) From c4a425969e56e03e168b0330735ec757c7a21066 Mon Sep 17 00:00:00 2001 From: Elliot Korte Date: Mon, 16 Jul 2018 17:52:26 -0500 Subject: [PATCH 2/7] added tests for include upload, added assignments to template change steps, rename includes to include --- .../cloudformation/artifact_exporter.py | 15 ++-- .../cloudformation/test_artifact_exporter.py | 86 ++++++++++++++++++- 2 files changed, 92 insertions(+), 9 deletions(-) diff --git a/awscli/customizations/cloudformation/artifact_exporter.py b/awscli/customizations/cloudformation/artifact_exporter.py index b0a0e3d56322..4bf3bdfb6c5f 100644 --- a/awscli/customizations/cloudformation/artifact_exporter.py +++ b/awscli/customizations/cloudformation/artifact_exporter.py @@ -405,16 +405,16 @@ def do_export(self, resource_id, resource_dict, parent_dir): } -def includes_transform_export_handler(template_dict, s3_uploader): +def include_transform_export_handler(template_dict, uploader): if template_dict.get("Name", None) != "AWS::Include": return template_dict - includes_location = template_dict.get("Parameters", {}).get("Location", {}) - if (is_local_file(includes_location)): - template_dict["Parameters"]["Location"] = s3_uploader.upload_with_dedup(includes_location) + include_location = template_dict.get("Parameters", {}).get("Location", {}) + if (is_local_file(include_location)): + template_dict["Parameters"]["Location"] = uploader.upload_with_dedup(include_location) return template_dict GLOBAL_EXPORT_DICT = { - "Fn::Transform": includes_transform_export_handler + "Fn::Transform": include_transform_export_handler } @@ -454,9 +454,10 @@ def export_global_artifacts(self, template_dict): """ for key, val in template_dict.iteritems(): if key in GLOBAL_EXPORT_DICT: - val = GLOBAL_EXPORT_DICT[key](val, self.uploader) + template_dict[key] = GLOBAL_EXPORT_DICT[key](val, self.uploader) elif type(val) is dict: self.export_global_artifacts(val) + return self.template_dict def export(self): @@ -470,7 +471,7 @@ def export(self): if "Resources" not in self.template_dict: return self.template_dict - self.export_global_artifacts(self.template_dict) + self.template_dict = self.export_global_artifacts(self.template_dict) for resource_id, resource in self.template_dict["Resources"].items(): diff --git a/tests/unit/customizations/cloudformation/test_artifact_exporter.py b/tests/unit/customizations/cloudformation/test_artifact_exporter.py index cd8c032d3b7a..13d811135fc0 100644 --- a/tests/unit/customizations/cloudformation/test_artifact_exporter.py +++ b/tests/unit/customizations/cloudformation/test_artifact_exporter.py @@ -7,7 +7,7 @@ import zipfile from nose.tools import assert_true, assert_false, assert_equal -from contextlib import contextmanager,closing +from contextlib import contextmanager, closing, nested from mock import patch, Mock, MagicMock from botocore.stub import Stubber from awscli.testutils import unittest, FileCreator @@ -19,7 +19,7 @@ ServerlessFunctionResource, GraphQLSchemaResource, \ LambdaFunctionResource, ApiGatewayRestApiResource, \ ElasticBeanstalkApplicationVersion, CloudFormationStackResource, \ - copy_to_temp_dir + copy_to_temp_dir, include_transform_export_handler, GLOBAL_EXPORT_DICT def test_is_s3_url(): @@ -770,6 +770,88 @@ def test_template_export(self, yaml_parse_mock): resource_type2_instance.export.assert_called_once_with( "Resource2", mock.ANY, template_dir) + @patch("awscli.customizations.cloudformation.artifact_exporter.yaml_parse") + def test_template_global_export(self, yaml_parse_mock): + parent_dir = os.path.sep + template_dir = os.path.join(parent_dir, 'foo', 'bar') + template_path = os.path.join(template_dir, 'path') + template_str = self.example_yaml_template() + + resource_type1_class = Mock() + resource_type1_instance = Mock() + resource_type1_class.return_value = resource_type1_instance + resource_type2_class = Mock() + resource_type2_instance = Mock() + resource_type2_class.return_value = resource_type2_instance + + resources_to_export = { + "resource_type1": resource_type1_class, + "resource_type2": resource_type2_class + } + properties1 = {"foo": "bar", "Fn::Transform": {"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}}} + properties2 = {"foo": "bar", "Fn::Transform": {"Name": "AWS::OtherTransform"}} + template_dict = { + "Resources": { + "Resource1": { + "Type": "resource_type1", + "Properties": properties1 + }, + "Resource2": { + "Type": "resource_type2", + "Properties": properties2 + } + } + } + open_mock = mock.mock_open() + include_transform_export_handler_mock = Mock() + include_transform_export_handler_mock.return_value = {"Name": "AWS::Include", "Parameters": {"Location": "s3://foo"}} + yaml_parse_mock.return_value = template_dict + + with nested( + patch("awscli.customizations.cloudformation.artifact_exporter.open", + open_mock(read_data=template_str)), + patch.dict(GLOBAL_EXPORT_DICT, {"Fn::Transform": include_transform_export_handler_mock})) as (open_mock): + + dict_patch = patch.dict(GLOBAL_EXPORT_DICT) + template_exporter = Template( + template_path, parent_dir, self.s3_uploader_mock, + resources_to_export) + + exported_template = template_exporter.export_global_artifacts(template_exporter.template_dict) + + first_call_args, kwargs = include_transform_export_handler_mock.call_args_list[0] + self.assertEquals(first_call_args[0], {"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}}) + second_call_args, kwargs = include_transform_export_handler_mock.call_args_list[1] + self.assertEquals(second_call_args[0], {"Name": "AWS::OtherTransform"}) + self.assertEquals(include_transform_export_handler_mock.call_count, 2) + #new s3 url is added to include location + self.assertEquals(exported_template["Resources"]["Resource1"]["Properties"]["Fn::Transform"], {"Name": "AWS::Include", "Parameters": {"Location": "s3://foo"}}) + + @patch("awscli.customizations.cloudformation.artifact_exporter.is_local_file") + def test_include_transform_export_handler(self, is_local_file_mock): + #exports transform + self.s3_uploader_mock.upload_with_dedup.return_value = "s3://foo" + is_local_file_mock.return_value = True + handler_output = include_transform_export_handler({"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}}, self.s3_uploader_mock) + self.s3_uploader_mock.upload_with_dedup.assert_called_once_with("foo.yaml") + self.assertEquals(handler_output, {'Name': 'AWS::Include', 'Parameters': {'Location': 's3://foo'}}) + + @patch("awscli.customizations.cloudformation.artifact_exporter.is_local_file") + def test_include_transform_export_handler_non_local_file(self, is_local_file_mock): + #returns unchanged template dict if transform not a local file + is_local_file_mock.return_value = False + handler_output = include_transform_export_handler({"Name": "AWS::Include", "Parameters": {"Location": "http://foo.yaml"}}, self.s3_uploader_mock) + self.s3_uploader_mock.upload_with_dedup.assert_not_called() + self.assertEquals(handler_output, {"Name": "AWS::Include", "Parameters": {"Location": "http://foo.yaml"}}) + + @patch("awscli.customizations.cloudformation.artifact_exporter.is_local_file") + def test_include_transform_export_handler_non_include_transform(self, is_local_file_mock): + #ignores transform that is not aws::include + handler_output = include_transform_export_handler({"Name": "AWS::OtherTransform", "Parameters": {"Location": "foo.yaml"}}, self.s3_uploader_mock) + self.s3_uploader_mock.upload_with_dedup.assert_not_called() + self.assertEquals(handler_output, {"Name": "AWS::OtherTransform", "Parameters": {"Location": "foo.yaml"}}) + + def test_template_export_path_be_folder(self): template_path = "/path/foo" From fe803492e74c6c133be0b2cb698bb9f8db079cd1 Mon Sep 17 00:00:00 2001 From: Elliot Korte Date: Tue, 17 Jul 2018 10:18:53 -0500 Subject: [PATCH 3/7] remove use of nested which broke build for python3 --- .../cloudformation/test_artifact_exporter.py | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/tests/unit/customizations/cloudformation/test_artifact_exporter.py b/tests/unit/customizations/cloudformation/test_artifact_exporter.py index 13d811135fc0..3a17feab2e00 100644 --- a/tests/unit/customizations/cloudformation/test_artifact_exporter.py +++ b/tests/unit/customizations/cloudformation/test_artifact_exporter.py @@ -7,7 +7,7 @@ import zipfile from nose.tools import assert_true, assert_false, assert_equal -from contextlib import contextmanager, closing, nested +from contextlib import contextmanager, closing from mock import patch, Mock, MagicMock from botocore.stub import Stubber from awscli.testutils import unittest, FileCreator @@ -807,25 +807,23 @@ def test_template_global_export(self, yaml_parse_mock): include_transform_export_handler_mock.return_value = {"Name": "AWS::Include", "Parameters": {"Location": "s3://foo"}} yaml_parse_mock.return_value = template_dict - with nested( - patch("awscli.customizations.cloudformation.artifact_exporter.open", - open_mock(read_data=template_str)), - patch.dict(GLOBAL_EXPORT_DICT, {"Fn::Transform": include_transform_export_handler_mock})) as (open_mock): - - dict_patch = patch.dict(GLOBAL_EXPORT_DICT) - template_exporter = Template( - template_path, parent_dir, self.s3_uploader_mock, - resources_to_export) - - exported_template = template_exporter.export_global_artifacts(template_exporter.template_dict) - - first_call_args, kwargs = include_transform_export_handler_mock.call_args_list[0] - self.assertEquals(first_call_args[0], {"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}}) - second_call_args, kwargs = include_transform_export_handler_mock.call_args_list[1] - self.assertEquals(second_call_args[0], {"Name": "AWS::OtherTransform"}) - self.assertEquals(include_transform_export_handler_mock.call_count, 2) - #new s3 url is added to include location - self.assertEquals(exported_template["Resources"]["Resource1"]["Properties"]["Fn::Transform"], {"Name": "AWS::Include", "Parameters": {"Location": "s3://foo"}}) + with patch( + "awscli.customizations.cloudformation.artifact_exporter.open", + open_mock(read_data=template_str)) as open_mock: + with patch.dict(GLOBAL_EXPORT_DICT, {"Fn::Transform": include_transform_export_handler_mock}): + template_exporter = Template( + template_path, parent_dir, self.s3_uploader_mock, + resources_to_export) + + exported_template = template_exporter.export_global_artifacts(template_exporter.template_dict) + + first_call_args, kwargs = include_transform_export_handler_mock.call_args_list[0] + self.assertEquals(first_call_args[0], {"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}}) + second_call_args, kwargs = include_transform_export_handler_mock.call_args_list[1] + self.assertEquals(second_call_args[0], {"Name": "AWS::OtherTransform"}) + self.assertEquals(include_transform_export_handler_mock.call_count, 2) + #new s3 url is added to include location + self.assertEquals(exported_template["Resources"]["Resource1"]["Properties"]["Fn::Transform"], {"Name": "AWS::Include", "Parameters": {"Location": "s3://foo"}}) @patch("awscli.customizations.cloudformation.artifact_exporter.is_local_file") def test_include_transform_export_handler(self, is_local_file_mock): From ebdec6dbdc587538dc46014a0c55f6e005205d1d Mon Sep 17 00:00:00 2001 From: Elliot Korte Date: Tue, 17 Jul 2018 10:37:18 -0500 Subject: [PATCH 4/7] change dictionary iteration to be python 3 compatible --- awscli/customizations/cloudformation/artifact_exporter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awscli/customizations/cloudformation/artifact_exporter.py b/awscli/customizations/cloudformation/artifact_exporter.py index 4bf3bdfb6c5f..b89e6be370c6 100644 --- a/awscli/customizations/cloudformation/artifact_exporter.py +++ b/awscli/customizations/cloudformation/artifact_exporter.py @@ -452,7 +452,7 @@ def export_global_artifacts(self, template_dict): here we iterate through the template dict and export params with a handler defined in GLOBAL_EXPORT_DICT """ - for key, val in template_dict.iteritems(): + for key, val in iter(template_dict.items()): if key in GLOBAL_EXPORT_DICT: template_dict[key] = GLOBAL_EXPORT_DICT[key](val, self.uploader) elif type(val) is dict: From 760b11c52c223668dd88c12f25774c2638d1c010 Mon Sep 17 00:00:00 2001 From: Elliot Korte Date: Tue, 17 Jul 2018 12:00:25 -0500 Subject: [PATCH 5/7] make call args test order agnostic for differences in dictionary item iteration --- .../customizations/cloudformation/test_artifact_exporter.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/customizations/cloudformation/test_artifact_exporter.py b/tests/unit/customizations/cloudformation/test_artifact_exporter.py index 3a17feab2e00..aa1650b7fee4 100644 --- a/tests/unit/customizations/cloudformation/test_artifact_exporter.py +++ b/tests/unit/customizations/cloudformation/test_artifact_exporter.py @@ -818,9 +818,10 @@ def test_template_global_export(self, yaml_parse_mock): exported_template = template_exporter.export_global_artifacts(template_exporter.template_dict) first_call_args, kwargs = include_transform_export_handler_mock.call_args_list[0] - self.assertEquals(first_call_args[0], {"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}}) second_call_args, kwargs = include_transform_export_handler_mock.call_args_list[1] - self.assertEquals(second_call_args[0], {"Name": "AWS::OtherTransform"}) + call_args = [first_call_args[0], second_call_args[0]] + self.assertTrue({"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}} in call_args) + self.assertTrue({"Name": "AWS::OtherTransform"} in call_args) self.assertEquals(include_transform_export_handler_mock.call_count, 2) #new s3 url is added to include location self.assertEquals(exported_template["Resources"]["Resource1"]["Properties"]["Fn::Transform"], {"Name": "AWS::Include", "Parameters": {"Location": "s3://foo"}}) From 17fced5fad7d7561bef80b87d2db29e8b29a9e0a Mon Sep 17 00:00:00 2001 From: Elliot Korte Date: Tue, 16 Oct 2018 13:21:45 -0500 Subject: [PATCH 6/7] add include transform to documentation in aws cloudformation package help page --- awscli/examples/cloudformation/_package_description.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/awscli/examples/cloudformation/_package_description.rst b/awscli/examples/cloudformation/_package_description.rst index 4acc4fc98530..12474d389b7a 100644 --- a/awscli/examples/cloudformation/_package_description.rst +++ b/awscli/examples/cloudformation/_package_description.rst @@ -8,7 +8,7 @@ Use this command to quickly upload local artifacts that might be required by your template. After you package your template's artifacts, run the deploy command to ``deploy`` the returned template. -This command can upload local artifacts specified by following properties of a resource: +This command can upload local artifacts referenced in the following places: - ``BodyS3Location`` property for the ``AWS::ApiGateway::RestApi`` resource @@ -16,6 +16,7 @@ This command can upload local artifacts specified by following properties of a r - ``CodeUri`` property for the ``AWS::Serverless::Function`` resource - ``DefinitionS3Location`` property for the ``AWS::AppSync::GraphQLSchema`` resource - ``DefinitionUri`` property for the ``AWS::Serverless::Api`` resource + - ``Location`` parameter for the ``AWS::Include`` transform - ``SourceBundle`` property for the ``AWS::ElasticBeanstalk::ApplicationVersion`` resource - ``TemplateURL`` property for the ``AWS::CloudFormation::Stack`` resource From 1eaa24f2b2d3bd2eda6ab8bdbbefe9d85d40c3ce Mon Sep 17 00:00:00 2001 From: Elliot Korte Date: Wed, 17 Oct 2018 12:04:00 -0500 Subject: [PATCH 7/7] add support for include transforms in lists, minor changes per PR review --- .../cloudformation/artifact_exporter.py | 10 +++++++--- .../cloudformation/test_artifact_exporter.py | 13 +++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/awscli/customizations/cloudformation/artifact_exporter.py b/awscli/customizations/cloudformation/artifact_exporter.py index b89e6be370c6..6cfdaa5bf217 100644 --- a/awscli/customizations/cloudformation/artifact_exporter.py +++ b/awscli/customizations/cloudformation/artifact_exporter.py @@ -452,12 +452,16 @@ def export_global_artifacts(self, template_dict): here we iterate through the template dict and export params with a handler defined in GLOBAL_EXPORT_DICT """ - for key, val in iter(template_dict.items()): + for key, val in template_dict.items(): if key in GLOBAL_EXPORT_DICT: template_dict[key] = GLOBAL_EXPORT_DICT[key](val, self.uploader) - elif type(val) is dict: + elif isinstance(val, dict): self.export_global_artifacts(val) - return self.template_dict + elif isinstance(val, list): + for item in val: + if isinstance(item, dict): + self.export_global_artifacts(item) + return template_dict def export(self): diff --git a/tests/unit/customizations/cloudformation/test_artifact_exporter.py b/tests/unit/customizations/cloudformation/test_artifact_exporter.py index aa1650b7fee4..f12a47070fdc 100644 --- a/tests/unit/customizations/cloudformation/test_artifact_exporter.py +++ b/tests/unit/customizations/cloudformation/test_artifact_exporter.py @@ -790,6 +790,7 @@ def test_template_global_export(self, yaml_parse_mock): } properties1 = {"foo": "bar", "Fn::Transform": {"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}}} properties2 = {"foo": "bar", "Fn::Transform": {"Name": "AWS::OtherTransform"}} + properties_in_list = {"Fn::Transform": {"Name": "AWS::Include", "Parameters": {"Location": "bar.yaml"}}} template_dict = { "Resources": { "Resource1": { @@ -798,9 +799,10 @@ def test_template_global_export(self, yaml_parse_mock): }, "Resource2": { "Type": "resource_type2", - "Properties": properties2 + "Properties": properties2, } - } + }, + "List": ["foo", properties_in_list] } open_mock = mock.mock_open() include_transform_export_handler_mock = Mock() @@ -819,12 +821,15 @@ def test_template_global_export(self, yaml_parse_mock): first_call_args, kwargs = include_transform_export_handler_mock.call_args_list[0] second_call_args, kwargs = include_transform_export_handler_mock.call_args_list[1] - call_args = [first_call_args[0], second_call_args[0]] + third_call_args, kwargs = include_transform_export_handler_mock.call_args_list[2] + call_args = [first_call_args[0], second_call_args[0], third_call_args[0]] self.assertTrue({"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}} in call_args) self.assertTrue({"Name": "AWS::OtherTransform"} in call_args) - self.assertEquals(include_transform_export_handler_mock.call_count, 2) + self.assertTrue({"Name": "AWS::Include", "Parameters": {"Location": "bar.yaml"}} in call_args) + self.assertEquals(include_transform_export_handler_mock.call_count, 3) #new s3 url is added to include location self.assertEquals(exported_template["Resources"]["Resource1"]["Properties"]["Fn::Transform"], {"Name": "AWS::Include", "Parameters": {"Location": "s3://foo"}}) + self.assertEquals(exported_template["List"][1]["Fn::Transform"], {"Name": "AWS::Include", "Parameters": {"Location": "s3://foo"}}) @patch("awscli.customizations.cloudformation.artifact_exporter.is_local_file") def test_include_transform_export_handler(self, is_local_file_mock):