From 46c374f79c4a21869cd02423f91bd0eda1dbee79 Mon Sep 17 00:00:00 2001 From: Haresh Nasit <84355507+hnnasit@users.noreply.github.com> Date: Thu, 20 Jul 2023 14:50:13 -0700 Subject: [PATCH] fix: Use posix path for dockerfile path passed to docker build API (#5528) * Use posix path for dockerfile * Updated dockerfile path to posix before Docker API call and Added integ tests --- samcli/lib/build/app_builder.py | 2 +- .../samlib/resource_metadata_normalizer.py | 2 +- tests/integration/buildcmd/test_build_cmd.py | 75 +++++ .../__init__.py | 0 .../myimage/Dockerfile | 7 + .../myimage/main.py | 2 + ...d_template_image_function_shared_code.json | 306 ++++++++++++++++++ .../test_resource_metadata_normalizer.py | 6 +- 8 files changed, 395 insertions(+), 5 deletions(-) create mode 100644 tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/__init__.py create mode 100644 tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/myimage/Dockerfile create mode 100644 tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/myimage/main.py create mode 100644 tests/integration/testdata/buildcmd/cdk_v2_synthesized_template_image_function_shared_code.json diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py index 24df7dbad9..7078bc7cec 100644 --- a/samcli/lib/build/app_builder.py +++ b/samcli/lib/build/app_builder.py @@ -404,7 +404,7 @@ def _build_lambda_image(self, function_name: str, metadata: Dict, architecture: build_args = { "path": str(docker_context_dir), - "dockerfile": dockerfile, + "dockerfile": str(pathlib.Path(dockerfile).as_posix()), "tag": docker_tag, "buildargs": docker_build_args, "platform": get_docker_platform(architecture), diff --git a/samcli/lib/samlib/resource_metadata_normalizer.py b/samcli/lib/samlib/resource_metadata_normalizer.py index ea7ec931be..d12a930be9 100644 --- a/samcli/lib/samlib/resource_metadata_normalizer.py +++ b/samcli/lib/samlib/resource_metadata_normalizer.py @@ -186,7 +186,7 @@ def _extract_image_asset_metadata(metadata): asset_path = Path(metadata.get(ASSET_PATH_METADATA_KEY, "")) dockerfile_path = Path(metadata.get(ASSET_DOCKERFILE_PATH_KEY), "") return { - SAM_METADATA_DOCKERFILE_KEY: str(dockerfile_path), + SAM_METADATA_DOCKERFILE_KEY: str(dockerfile_path.as_posix()), SAM_METADATA_DOCKER_CONTEXT_KEY: str(asset_path), SAM_METADATA_DOCKER_BUILD_ARGS_KEY: metadata.get(ASSET_DOCKERFILE_BUILD_ARGS_KEY, {}), } diff --git a/tests/integration/buildcmd/test_build_cmd.py b/tests/integration/buildcmd/test_build_cmd.py index 6e7787381e..7a1b0f1c75 100644 --- a/tests/integration/buildcmd/test_build_cmd.py +++ b/tests/integration/buildcmd/test_build_cmd.py @@ -274,6 +274,40 @@ def test_intermediate_container_deleted(self, dockerfile, expected): _num_of_containers_before_build, _num_of_containers_after_build, "Intermediate containers are not removed" ) + @parameterized.expand( + [ + ("feature_phi\\Dockerfile", {"phi": "1.62"}), + ("feature_pi\\Dockerfile", {"pi": "3.14"}), + ] + ) + @pytest.mark.flaky(reruns=3) + @skipIf(not IS_WINDOWS, "Skipping passing Windows path for dockerfile path on non Windows platform") + def test_windows_dockerfile_present_sub_dir(self, dockerfile, expected): + _tag = f"{random.randint(1, 100)}" + overrides = { + "Runtime": "3.9", + "Handler": "main.handler", + "DockerFile": dockerfile, + "Tag": _tag, + } + cmdlist = self.get_command_list(use_container=False, parameter_overrides=overrides) + + LOG.info("Running Command: ") + LOG.info(cmdlist) + command_result = run_command(cmdlist, cwd=self.working_dir) + self.assertEqual(command_result.process.returncode, 0) + + self._verify_image_build_artifact( + self.built_template, + self.FUNCTION_LOGICAL_ID_IMAGE, + "ImageUri", + f"{self.FUNCTION_LOGICAL_ID_IMAGE.lower()}:{_tag}", + ) + + self._verify_invoke_built_function( + self.built_template, self.FUNCTION_LOGICAL_ID_IMAGE, self._make_parameter_override_arg(overrides), expected + ) + @skipIf( # Hits public ECR pull limitation, move it to canary tests @@ -546,6 +580,47 @@ def test_cdk_app_with_default_requirements(self): ) +@skipIf( + # Hits public ECR pull limitation, move it to canary tests + SKIP_DOCKER_TESTS, + "Skip build tests that requires Docker in CI environment", +) +@parameterized_class( + ( + "template", + "FUNCTION_LOGICAL_ID", + "overrides", + "use_container", + "prop", + ), + [ + ( + "cdk_v2_synthesized_template_image_function_shared_code.json", + "TestLambdaFunctionC089708A", + False, + False, + "Code.ImageUri", + ), + ], +) +class TestBuildCommandCDKPythonImageFunctionSharedCode(BuildIntegPythonBase): + @pytest.mark.flaky(reruns=3) + def test_cdk_app_with_default_requirements(self): + expected = "Hello World" + cmdlist = self.get_command_list(use_container=self.use_container) + command_result = run_command(cmdlist, cwd=self.working_dir) + self.assertEqual(command_result.process.returncode, 0) + + self._verify_image_build_artifact( + self.built_template, + self.FUNCTION_LOGICAL_ID, + self.prop, + f"{self.FUNCTION_LOGICAL_ID.lower()}:latest", + ) + + self._verify_invoke_built_function(self.built_template, self.FUNCTION_LOGICAL_ID, {}, expected) + + class TestBuildCommand_PythonFunctions_With_Specified_Architecture(BuildIntegPythonBase): template = "template_with_architecture.yaml" diff --git a/tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/__init__.py b/tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/myimage/Dockerfile b/tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/myimage/Dockerfile new file mode 100644 index 0000000000..93d8fd9d02 --- /dev/null +++ b/tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/myimage/Dockerfile @@ -0,0 +1,7 @@ +ARG BASE_RUNTIME + +FROM public.ecr.aws/lambda/python:$BASE_RUNTIME + +COPY myimage/main.py $FUNCTION_DIR + +CMD [ "main.handler" ] \ No newline at end of file diff --git a/tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/myimage/main.py b/tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/myimage/main.py new file mode 100644 index 0000000000..a4e0923dda --- /dev/null +++ b/tests/integration/testdata/buildcmd/asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428/myimage/main.py @@ -0,0 +1,2 @@ +def handler(event, context): + return "Hello World" \ No newline at end of file diff --git a/tests/integration/testdata/buildcmd/cdk_v2_synthesized_template_image_function_shared_code.json b/tests/integration/testdata/buildcmd/cdk_v2_synthesized_template_image_function_shared_code.json new file mode 100644 index 0000000000..37ed78e7a9 --- /dev/null +++ b/tests/integration/testdata/buildcmd/cdk_v2_synthesized_template_image_function_shared_code.json @@ -0,0 +1,306 @@ +{ + "Resources": { + "TestLambdaFunctionServiceRole0C9E0634": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "lambda.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + }, + "ManagedPolicyArns": [ + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ] + ] + } + ] + }, + "Metadata": { + "aws:cdk:path": "TestcdkStack/TestLambdaFunction/ServiceRole/Resource" + } + }, + "TestLambdaFunctionC089708A": { + "Type": "AWS::Lambda::Function", + "Properties": { + "Code": { + "ImageUri": { + "Fn::Sub": "${AWS::AccountId}.dkr.ecr.${AWS::Region}.${AWS::URLSuffix}/cdk-hnb659fds-container-assets-${AWS::AccountId}-${AWS::Region}:31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428" + } + }, + "Role": { + "Fn::GetAtt": [ + "TestLambdaFunctionServiceRole0C9E0634", + "Arn" + ] + }, + "FunctionName": "TestLambdaFunction", + "PackageType": "Image" + }, + "DependsOn": [ + "TestLambdaFunctionServiceRole0C9E0634" + ], + "Metadata": { + "aws:cdk:path": "TestcdkStack/TestLambdaFunction/Resource", + "aws:asset:path": "asset.31c00c94a012bbfa9021c9ef3d6589c251d6e9743f9cf6d38694d29e7372c428", + "aws:asset:dockerfile-path": "myimage/Dockerfile", + "aws:asset:property": "Code.ImageUri", + "aws:asset:docker-build-args": { + "BASE_RUNTIME": "3.9" + } + } + }, + "CDKMetadata": { + "Type": "AWS::CDK::Metadata", + "Properties": { + "Analytics": "v2:deflate64:H4sIAAAAAAAA/01PQQ6CMBB8i/eyigfiVTEmXusDSCkrWaCt6RaNafi7BULiaSYzszu7RygKyHfqw5lu+mygGuIjKN2LJFVxUKZuFMSr0z36u1Et3karAzkryqfd+CRIGYjSDTjLM04Cta8UMwaGv/HzrMweSHw5puD896I45SWyG71GsUTSES3Zdlm3GYmXzja0VlrXIHS8f+fpgxMcdh0TZX60gQyCXPEHgvB57+AAAAA=" + }, + "Metadata": { + "aws:cdk:path": "TestcdkStack/CDKMetadata/Default" + }, + "Condition": "CDKMetadataAvailable" + } + }, + "Conditions": { + "CDKMetadataAvailable": { + "Fn::Or": [ + { + "Fn::Or": [ + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "af-south-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "ap-east-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "ap-northeast-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "ap-northeast-2" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "ap-south-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "ap-southeast-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "ap-southeast-2" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "ca-central-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "cn-north-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "cn-northwest-1" + ] + } + ] + }, + { + "Fn::Or": [ + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "eu-central-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "eu-north-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "eu-south-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "eu-west-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "eu-west-2" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "eu-west-3" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "me-south-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "sa-east-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "us-east-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "us-east-2" + ] + } + ] + }, + { + "Fn::Or": [ + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "us-west-1" + ] + }, + { + "Fn::Equals": [ + { + "Ref": "AWS::Region" + }, + "us-west-2" + ] + } + ] + } + ] + } + }, + "Parameters": { + "BootstrapVersion": { + "Type": "AWS::SSM::Parameter::Value", + "Default": "/cdk-bootstrap/hnb659fds/version", + "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]" + } + }, + "Rules": { + "CheckBootstrapVersion": { + "Assertions": [ + { + "Assert": { + "Fn::Not": [ + { + "Fn::Contains": [ + [ + "1", + "2", + "3", + "4", + "5" + ], + { + "Ref": "BootstrapVersion" + } + ] + } + ] + }, + "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI." + } + ] + } + } + } \ No newline at end of file diff --git a/tests/unit/lib/samlib/test_resource_metadata_normalizer.py b/tests/unit/lib/samlib/test_resource_metadata_normalizer.py index e027a527c4..7c60be1124 100644 --- a/tests/unit/lib/samlib/test_resource_metadata_normalizer.py +++ b/tests/unit/lib/samlib/test_resource_metadata_normalizer.py @@ -108,7 +108,7 @@ def test_replace_all_resources_that_contain_image_metadata(self): self.assertEqual( expected_docker_context_path, template_data["Resources"]["Function1"]["Metadata"]["DockerContext"] ) - expected_dockerfile_path = str(pathlib.Path("path", "to", "Dockerfile")) + expected_dockerfile_path = str(pathlib.Path("path", "to", "Dockerfile").as_posix()) self.assertEqual(expected_dockerfile_path, template_data["Resources"]["Function1"]["Metadata"]["Dockerfile"]) self.assertEqual(docker_build_args, template_data["Resources"]["Function1"]["Metadata"]["DockerBuildArgs"]) self.assertEqual("Function1", template_data["Resources"]["Function1"]["Metadata"]["SamResourceId"]) @@ -145,7 +145,7 @@ def test_replace_all_resources_that_contain_image_metadata_dockerfile_extensions expected_docker_context_path, template_data["Resources"]["Function1"]["Metadata"]["DockerContext"] ) self.assertEqual( - str(pathlib.Path("path/to/Dockerfile.production")), + str(pathlib.Path("path/to/Dockerfile.production").as_posix()), template_data["Resources"]["Function1"]["Metadata"]["Dockerfile"], ) self.assertEqual(docker_build_args, template_data["Resources"]["Function1"]["Metadata"]["DockerBuildArgs"]) @@ -182,7 +182,7 @@ def test_replace_all_resources_that_contain_image_metadata_windows_paths(self): self.assertEqual( expected_docker_context_path, template_data["Resources"]["Function1"]["Metadata"]["DockerContext"] ) - expected_dockerfile_path = str(pathlib.Path("rel/path/to/Dockerfile")) + expected_dockerfile_path = str(pathlib.Path("rel/path/to/Dockerfile").as_posix()) self.assertEqual(expected_dockerfile_path, template_data["Resources"]["Function1"]["Metadata"]["Dockerfile"]) self.assertEqual(docker_build_args, template_data["Resources"]["Function1"]["Metadata"]["DockerBuildArgs"]) self.assertEqual("Function1", template_data["Resources"]["Function1"]["Metadata"]["SamResourceId"])