Cloudformation package command must resolve relative file paths when processing AWS::Include transform

awscli/customizations/cloudformation/artifact_exporter.py

@@ -495,12 +495,23 @@ class ServerlessApplicationResource(CloudFormationStackResource):
 ]
 
 
-def include_transform_export_handler(template_dict, uploader):
+def include_transform_export_handler(template_dict, uploader, parent_dir):
     if template_dict.get("Name", None) != "AWS::Include":
         return template_dict
-    include_location = template_dict.get("Parameters", {}).get("Location", {})
-    if (is_local_file(include_location)):
-        template_dict["Parameters"]["Location"] = uploader.upload_with_dedup(include_location)
+
+    include_location = template_dict.get("Parameters", {}).get("Location", None)
+    if not include_location or is_s3_url(include_location):
+        return template_dict
+
+    abs_include_location = os.path.join(parent_dir, include_location)
+    if is_local_file(abs_include_location):
+        template_dict["Parameters"]["Location"] = uploader.upload_with_dedup(abs_include_location)
+    else:
+        raise exceptions.InvalidLocalPathError(
+            resource_id="AWS::Include",
+            property_name="Location",
+            local_path=abs_include_location)
+
     return template_dict
 
 
@@ -547,7 +558,7 @@ def export_global_artifacts(self, template_dict):
         """
         for key, val in template_dict.items():
             if key in GLOBAL_EXPORT_DICT:
-                template_dict[key] = GLOBAL_EXPORT_DICT[key](val, self.uploader)
+                template_dict[key] = GLOBAL_EXPORT_DICT[key](val, self.uploader, self.template_dir)
             elif isinstance(val, dict):
                 self.export_global_artifacts(val)
             elif isinstance(val, list):

tests/unit/customizations/cloudformation/test_artifact_exporter.py

@@ -1027,32 +1027,74 @@ def test_template_global_export(self, yaml_parse_mock):
                 self.assertTrue({"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}} in call_args)
                 self.assertTrue({"Name": "AWS::OtherTransform"} in call_args)
                 self.assertTrue({"Name": "AWS::Include", "Parameters": {"Location": "bar.yaml"}} in call_args)
+                self.assertTrue(template_dir in first_call_args)
+                self.assertTrue(template_dir in second_call_args)
+                self.assertTrue(template_dir in third_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):
+    def test_include_transform_export_handler_with_relative_file_path(self, is_local_file_mock):
         #exports transform
+        parent_dir = os.path.abspath("someroot")
         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")
+        abs_file_path = os.path.join(parent_dir, "foo.yaml")
+
+        handler_output = include_transform_export_handler({"Name": "AWS::Include", "Parameters": {"Location": "foo.yaml"}}, self.s3_uploader_mock, parent_dir)
+        self.s3_uploader_mock.upload_with_dedup.assert_called_once_with(abs_file_path)
+        is_local_file_mock.assert_called_with(abs_file_path)
+        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_with_absolute_file_path(self, is_local_file_mock):
+        #exports transform
+        parent_dir = os.path.abspath("someroot")
+        self.s3_uploader_mock.upload_with_dedup.return_value = "s3://foo"
+        is_local_file_mock.return_value = True
+        abs_file_path = os.path.abspath(os.path.join("my", "file.yaml"))
+
+        handler_output = include_transform_export_handler({"Name": "AWS::Include", "Parameters": {"Location": abs_file_path}}, self.s3_uploader_mock, parent_dir)
+        self.s3_uploader_mock.upload_with_dedup.assert_called_once_with(abs_file_path)
+        is_local_file_mock.assert_called_with(abs_file_path)
         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_with_s3_uri(self, is_local_file_mock):
+
+        handler_output = include_transform_export_handler({"Name": "AWS::Include", "Parameters": {"Location": "s3://bucket/foo.yaml"}}, self.s3_uploader_mock, "parent_dir")
+        # Input is returned unmodified
+        self.assertEquals(handler_output, {"Name": "AWS::Include", "Parameters": {"Location": "s3://bucket/foo.yaml"}})
+
+        is_local_file_mock.assert_not_called()
+        self.s3_uploader_mock.assert_not_called()
+
+    @patch("awscli.customizations.cloudformation.artifact_exporter.is_local_file")
+    def test_include_transform_export_handler_with_no_path(self, is_local_file_mock):
+
+        handler_output = include_transform_export_handler({"Name": "AWS::Include", "Parameters": {"Location": ""}}, self.s3_uploader_mock, "parent_dir")
+        # Input is returned unmodified
+        self.assertEquals(handler_output, {"Name": "AWS::Include", "Parameters": {"Location": ""}})
+
+        is_local_file_mock.assert_not_called()
+        self.s3_uploader_mock.assert_not_called()
+
     @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
+        #returns unchanged template dict if transform not a local file, and not a S3 URI
         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"}})
+
+        with self.assertRaises(exceptions.InvalidLocalPathError):
+            include_transform_export_handler({"Name": "AWS::Include", "Parameters": {"Location": "http://foo.yaml"}}, self.s3_uploader_mock, "parent_dir")
+            is_local_file_mock.assert_called_with("http://foo.yaml")
+            self.s3_uploader_mock.assert_not_called()
 
     @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)
+        handler_output = include_transform_export_handler({"Name": "AWS::OtherTransform", "Parameters": {"Location": "foo.yaml"}}, self.s3_uploader_mock, "parent_dir")
         self.s3_uploader_mock.upload_with_dedup.assert_not_called()
         self.assertEquals(handler_output, {"Name": "AWS::OtherTransform", "Parameters": {"Location": "foo.yaml"}})