From 5f1f2ce031830a90654cac7d1c463d0c6bd98d12 Mon Sep 17 00:00:00 2001 From: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Date: Wed, 15 Dec 2021 03:53:53 -0800 Subject: [PATCH 1/2] (fix) stop resolve relative paths for function imageUri --- samcli/commands/_utils/template.py | 8 +- .../integration/buildcmd/build_integ_base.py | 10 +++ tests/integration/buildcmd/test_build_cmd.py | 19 +++-- tests/unit/commands/_utils/test_template.py | 80 ++++++++++++++++++- 4 files changed, 107 insertions(+), 10 deletions(-) diff --git a/samcli/commands/_utils/template.py b/samcli/commands/_utils/template.py index 61b1c497e9..234e13f0bf 100644 --- a/samcli/commands/_utils/template.py +++ b/samcli/commands/_utils/template.py @@ -10,7 +10,7 @@ from botocore.utils import set_value_from_jmespath from samcli.commands.exceptions import UserException -from samcli.lib.utils.packagetype import ZIP +from samcli.lib.utils.packagetype import ZIP, IMAGE from samcli.yamlhelper import yaml_parse, yaml_dump from samcli.lib.utils.resources import ( METADATA_WITH_LOCAL_PATHS, @@ -155,6 +155,12 @@ def _update_relative_paths(template_dict, original_root, new_root): for path_prop_name in RESOURCES_WITH_LOCAL_PATHS[resource_type]: properties = resource.get("Properties", {}) + if ( + resource_type in [AWS_SERVERLESS_FUNCTION, AWS_LAMBDA_FUNCTION] + and properties.get("PackageType", ZIP) == IMAGE + ): + continue + path = jmespath.search(path_prop_name, properties) updated_path = _resolve_relative_to(path, original_root, new_root) diff --git a/tests/integration/buildcmd/build_integ_base.py b/tests/integration/buildcmd/build_integ_base.py index d89c23b29f..f0ab85714f 100644 --- a/tests/integration/buildcmd/build_integ_base.py +++ b/tests/integration/buildcmd/build_integ_base.py @@ -161,6 +161,16 @@ def verify_pulled_image(self, runtime, architecture=X86_64): def _make_parameter_override_arg(self, overrides): return " ".join(["ParameterKey={},ParameterValue={}".format(key, value) for key, value in overrides.items()]) + def _verify_image_build_artifact(self, template_path, image_function_logical_id, property, image_uri): + self.assertTrue(template_path.exists(), "Build directory should be created") + + build_dir = template_path.parent + build_dir_files = os.listdir(str(build_dir)) + self.assertNotIn(image_function_logical_id, build_dir_files) + + # Make sure the template has correct CodeUri for resource + self._verify_resource_property(str(template_path), image_function_logical_id, property, image_uri) + def _verify_resource_property(self, template_path, logical_id, property, expected_value): with open(template_path, "r") as fp: template_dict = yaml_parse(fp.read()) diff --git a/tests/integration/buildcmd/test_build_cmd.py b/tests/integration/buildcmd/test_build_cmd.py index d8c021f379..4b11c600c6 100644 --- a/tests/integration/buildcmd/test_build_cmd.py +++ b/tests/integration/buildcmd/test_build_cmd.py @@ -4,6 +4,7 @@ import shutil import sys from pathlib import Path +from typing import Set from unittest import skipIf import pytest @@ -49,24 +50,19 @@ class TestBuildCommand_PythonFunctions_Images(BuildIntegBase): template = "template_image.yaml" - EXPECTED_FILES_PROJECT_MANIFEST = { - "__init__.py", - "main.py", - "numpy", - # 'cryptography', - "requirements.txt", - } + EXPECTED_FILES_PROJECT_MANIFEST: Set[str] = set() FUNCTION_LOGICAL_ID_IMAGE = "ImageFunction" @parameterized.expand([("3.6", False), ("3.7", False), ("3.8", False), ("3.9", False)]) @pytest.mark.flaky(reruns=3) def test_with_default_requirements(self, runtime, use_container): + _tag = f"{random.randint(1,100)}" overrides = { "Runtime": runtime, "Handler": "main.handler", "DockerFile": "Dockerfile", - "Tag": f"{random.randint(1,100)}", + "Tag": _tag, } cmdlist = self.get_command_list(use_container=use_container, parameter_overrides=overrides) @@ -74,6 +70,13 @@ def test_with_default_requirements(self, runtime, use_container): LOG.info(cmdlist) run_command(cmdlist, cwd=self.working_dir) + self._verify_image_build_artifact( + self.built_template, + self.FUNCTION_LOGICAL_ID_IMAGE, + "ImageUri", + f"{self.FUNCTION_LOGICAL_ID_IMAGE.lower()}:{_tag}", + ) + expected = {"pi": "3.14"} self._verify_invoke_built_function( self.built_template, self.FUNCTION_LOGICAL_ID_IMAGE, self._make_parameter_override_arg(overrides), expected diff --git a/tests/unit/commands/_utils/test_template.py b/tests/unit/commands/_utils/test_template.py index c75db92c67..def0d1ec20 100644 --- a/tests/unit/commands/_utils/test_template.py +++ b/tests/unit/commands/_utils/test_template.py @@ -7,7 +7,7 @@ from botocore.utils import set_value_from_jmespath from parameterized import parameterized, param -from samcli.lib.utils.resources import AWS_SERVERLESS_FUNCTION, AWS_SERVERLESS_API +from samcli.lib.utils.resources import AWS_SERVERLESS_FUNCTION, AWS_SERVERLESS_API, RESOURCES_WITH_IMAGE_COMPONENT from samcli.commands._utils.template import ( get_template_data, METADATA_WITH_LOCAL_PATHS, @@ -140,6 +140,7 @@ def setUp(self): self.dest = os.path.abspath(os.path.join("src", "destination")) # /path/from/root/src/destination self.expected_result = os.path.join("..", "foo", "bar") + self.image_uri = "func12343:latest" @parameterized.expand([(resource_type, props) for resource_type, props in METADATA_WITH_LOCAL_PATHS.items()]) def test_must_update_relative_metadata_paths(self, resource_type, properties): @@ -199,6 +200,83 @@ def test_must_update_relative_resource_paths(self, resource_type, properties): self.maxDiff = None self.assertEqual(result, expected_template_dict) + @parameterized.expand([(resource_type, props) for resource_type, props in RESOURCES_WITH_IMAGE_COMPONENT.items()]) + def test_must_skip_image_components(self, resource_type, properties): + for propname in properties: + template_dict = { + "Resources": { + "ImageResource": {"Type": resource_type, "Properties": {}}, + } + } + + set_value_from_jmespath(template_dict, f"Resources.ImageResource.Properties.{propname}", self.image_uri) + + expected_template_dict = copy.deepcopy(template_dict) + + result = _update_relative_paths(template_dict, self.src, self.dest) + + self.maxDiff = None + self.assertEqual(result, expected_template_dict) + + @parameterized.expand( + [ + (image_resource_type, image_props, non_image_resource_type, non_image_props) + for image_resource_type, image_props in RESOURCES_WITH_IMAGE_COMPONENT.items() + for non_image_resource_type, non_image_props in RESOURCES_WITH_LOCAL_PATHS.items() + ] + ) + def test_must_skip_only_image_components_and_update_relative_resource_paths( + self, image_resource_type, image_properties, non_image_resource_type, non_image_properties + ): + for non_image_propname in non_image_properties: + for image_propname in image_properties: + template_dict = { + "Resources": { + "MyResourceWithRelativePath": {"Type": non_image_resource_type, "Properties": {}}, + "MyResourceWithS3Path": { + "Type": non_image_resource_type, + "Properties": {non_image_propname: self.s3path}, + }, + "MyResourceWithAbsolutePath": { + "Type": non_image_resource_type, + "Properties": {non_image_propname: self.abspath}, + }, + "MyResourceWithInvalidPath": { + "Type": non_image_resource_type, + "Properties": { + # Path is not a string + non_image_propname: {"foo": "bar"} + }, + }, + "MyResourceWithoutProperties": {"Type": non_image_resource_type}, + "UnsupportedResourceType": {"Type": "AWS::Ec2::Instance", "Properties": {"Code": "bar"}}, + "ResourceWithoutType": {"foo": "bar"}, + "ImageResource": {"Type": image_resource_type, "Properties": {}}, + }, + "Parameters": {"a": "b"}, + } + + set_value_from_jmespath( + template_dict, f"Resources.MyResourceWithRelativePath.Properties.{non_image_propname}", self.curpath + ) + + set_value_from_jmespath( + template_dict, f"Resources.ImageResource.Properties.{image_propname}", self.image_uri + ) + + expected_template_dict = copy.deepcopy(template_dict) + + set_value_from_jmespath( + expected_template_dict, + f"Resources.MyResourceWithRelativePath.Properties.{non_image_propname}", + self.expected_result, + ) + + result = _update_relative_paths(template_dict, self.src, self.dest) + + self.maxDiff = None + self.assertEqual(result, expected_template_dict) + def test_must_update_aws_include_also(self): template_dict = { "Resources": {"Fn::Transform": {"Name": "AWS::Include", "Parameters": {"Location": self.curpath}}}, From 15f54960b4364658dc8f87e3014221670e8f6bc3 Mon Sep 17 00:00:00 2001 From: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Date: Wed, 15 Dec 2021 05:56:51 -0800 Subject: [PATCH 2/2] fix unit testing --- tests/unit/commands/_utils/test_template.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/commands/_utils/test_template.py b/tests/unit/commands/_utils/test_template.py index def0d1ec20..7262ea261c 100644 --- a/tests/unit/commands/_utils/test_template.py +++ b/tests/unit/commands/_utils/test_template.py @@ -205,7 +205,7 @@ def test_must_skip_image_components(self, resource_type, properties): for propname in properties: template_dict = { "Resources": { - "ImageResource": {"Type": resource_type, "Properties": {}}, + "ImageResource": {"Type": resource_type, "Properties": {"PackageType": "Image"}}, } } @@ -251,7 +251,7 @@ def test_must_skip_only_image_components_and_update_relative_resource_paths( "MyResourceWithoutProperties": {"Type": non_image_resource_type}, "UnsupportedResourceType": {"Type": "AWS::Ec2::Instance", "Properties": {"Code": "bar"}}, "ResourceWithoutType": {"foo": "bar"}, - "ImageResource": {"Type": image_resource_type, "Properties": {}}, + "ImageResource": {"Type": image_resource_type, "Properties": {"PackageType": "Image"}}, }, "Parameters": {"a": "b"}, }