Skip to content

Commit

Permalink
fix: allow sam to deploy templates contain CFN Lambda Function resour…
Browse files Browse the repository at this point in the history
…ces with Image package type, and its code is local image uri. (aws#3529)

* (fix) allow sam to deploy templates contain CFN Lambda Function resources with Image package type, and its code is local image uri,

* make reformat

* (fix) fix sam build failure for templates contians function without Docker Metadata

* run black format

* resolve PR comments
  • Loading branch information
moelasmar committed Dec 22, 2021
1 parent 487c24b commit b570979
Show file tree
Hide file tree
Showing 12 changed files with 192 additions and 25 deletions.
15 changes: 14 additions & 1 deletion samcli/commands/build/build_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
import os
import pathlib
import shutil
from typing import Dict, Optional, List
from typing import Dict, Optional, List, cast

import click
from samcli.lib.utils.packagetype import IMAGE

from samcli.commands.build.exceptions import InvalidBuildDirException, MissingBuildMethodException
from samcli.lib.bootstrap.nested_stack.nested_stack_manager import NestedStackManager
Expand Down Expand Up @@ -465,6 +466,18 @@ def _is_function_buildable(function: Function):
if isinstance(function.codeuri, str) and function.codeuri.endswith(".zip"):
LOG.debug("Skip building zip function: %s", function.full_path)
return False

if function.packagetype == IMAGE:
metadata = function.metadata if function.metadata else {}
dockerfile = cast(str, metadata.get("Dockerfile", ""))
docker_context = cast(str, metadata.get("DockerContext", ""))
if not dockerfile or not docker_context:
LOG.debug(
"Skip Building %s function, as it does not contain either Dockerfile or DockerContext "
"metadata properties.",
function.full_path,
)
return False
return True

@staticmethod
Expand Down
6 changes: 4 additions & 2 deletions samcli/lib/build/app_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,7 @@ def update_template(
properties["ContentUri"] = store_path

if resource_type == AWS_LAMBDA_FUNCTION and properties.get("PackageType", ZIP) == IMAGE:
properties["Code"] = built_artifacts[full_path]

properties["Code"] = {"ImageUri": built_artifacts[full_path]}
if resource_type == AWS_SERVERLESS_FUNCTION and properties.get("PackageType", ZIP) == IMAGE:
properties["ImageUri"] = built_artifacts[full_path]

Expand Down Expand Up @@ -359,6 +358,9 @@ def _build_lambda_image(self, function_name: str, metadata: Dict, architecture:
docker_build_target = metadata.get("DockerBuildTarget", None)
docker_build_args = metadata.get("DockerBuildArgs", {})

if not dockerfile or not docker_context:
raise DockerBuildFailed("Docker file or Docker context metadata are missed.")

if not isinstance(docker_build_args, dict):
raise DockerBuildFailed("DockerBuildArgs needs to be a dictionary!")

Expand Down
2 changes: 1 addition & 1 deletion samcli/lib/package/packageable_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ class LambdaFunctionResource(ResourceWithS3UrlDict):
FORCE_ZIP = True


class LambdaFunctionImageResource(ResourceImageDict):
class LambdaFunctionImageResource(ResourceImage):
RESOURCE_TYPE = AWS_LAMBDA_FUNCTION
PROPERTY_NAME = RESOURCES_WITH_IMAGE_COMPONENT[RESOURCE_TYPE][0]
FORCE_ZIP = True
Expand Down
2 changes: 1 addition & 1 deletion samcli/lib/utils/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@

RESOURCES_WITH_IMAGE_COMPONENT = {
AWS_SERVERLESS_FUNCTION: ["ImageUri"],
AWS_LAMBDA_FUNCTION: ["Code"],
AWS_LAMBDA_FUNCTION: ["Code.ImageUri"],
AWS_ECR_REPOSITORY: ["RepositoryName"],
}

Expand Down
53 changes: 53 additions & 0 deletions tests/integration/buildcmd/test_build_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from typing import Set
from unittest import skipIf

import docker
import pytest
from parameterized import parameterized, parameterized_class

Expand Down Expand Up @@ -83,6 +84,58 @@ def test_with_default_requirements(self, runtime, use_container):
)


@skipIf(
# Hits public ECR pull limitation, move it to canary tests
((not RUN_BY_CANARY) or (IS_WINDOWS and RUNNING_ON_CI) and not CI_OVERRIDE),
"Skip build tests on windows when running in CI unless overridden",
)
@parameterized_class(
("template", "prop"),
[
("template_local_prebuilt_image.yaml", "ImageUri"),
("template_cfn_local_prebuilt_image.yaml", "Code.ImageUri"),
],
)
class TestSkipBuildingFunctionsWithLocalImageUri(BuildIntegBase):
EXPECTED_FILES_PROJECT_MANIFEST: Set[str] = set()

FUNCTION_LOGICAL_ID_IMAGE = "ImageFunction"

@parameterized.expand(["3.6", "3.7", "3.8", "3.9"])
@pytest.mark.flaky(reruns=3)
def test_with_default_requirements(self, runtime):
_tag = f"{random.randint(1,100)}"
image_uri = f"func:{_tag}"
docker_client = docker.from_env()
docker_client.images.build(
path=str(Path(self.test_data_path, "PythonImage")),
dockerfile="Dockerfile",
buildargs={"BASE_RUNTIME": runtime},
tag=image_uri,
)
overrides = {
"ImageUri": image_uri,
"Handler": "main.handler",
}
cmdlist = self.get_command_list(parameter_overrides=overrides)

LOG.info("Running Command: ")
LOG.info(cmdlist)
run_command(cmdlist, cwd=self.working_dir)

self._verify_image_build_artifact(
self.built_template,
self.FUNCTION_LOGICAL_ID_IMAGE,
self.prop,
{"Ref": "ImageUri"},
)

expected = {"pi": "3.14"}
self._verify_invoke_built_function(
self.built_template, self.FUNCTION_LOGICAL_ID_IMAGE, self._make_parameter_override_arg(overrides), expected
)


@skipIf(
((IS_WINDOWS and RUNNING_ON_CI) and not CI_OVERRIDE),
"Skip build tests on windows when running in CI unless overridden",
Expand Down
20 changes: 13 additions & 7 deletions tests/integration/deploy/test_deploy_command.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
from samcli.lib.bootstrap.companion_stack.data_types import CompanionStack
import shutil
import tempfile
import time
Expand Down Expand Up @@ -144,7 +145,7 @@ def test_no_package_and_deploy_with_s3_bucket_all_args(self, template_file):
deploy_process_execute = run_command(deploy_command_list)
self.assertEqual(deploy_process_execute.process.returncode, 0)

@parameterized.expand(["aws-serverless-function-image.yaml"])
@parameterized.expand(["aws-serverless-function-image.yaml", "aws-lambda-function-image.yaml"])
def test_no_package_and_deploy_with_s3_bucket_all_args_image_repository(self, template_file):
template_path = self.test_data_path.joinpath(template_file)

Expand All @@ -171,7 +172,9 @@ def test_no_package_and_deploy_with_s3_bucket_all_args_image_repository(self, te
deploy_process_execute = run_command(deploy_command_list)
self.assertEqual(deploy_process_execute.process.returncode, 0)

@parameterized.expand([("Hello", "aws-serverless-function-image.yaml")])
@parameterized.expand(
[("Hello", "aws-serverless-function-image.yaml"), ("MyLambdaFunction", "aws-lambda-function-image.yaml")]
)
def test_no_package_and_deploy_with_s3_bucket_all_args_image_repositories(self, resource_id, template_file):
template_path = self.test_data_path.joinpath(template_file)

Expand All @@ -198,7 +201,7 @@ def test_no_package_and_deploy_with_s3_bucket_all_args_image_repositories(self,
deploy_process_execute = run_command(deploy_command_list)
self.assertEqual(deploy_process_execute.process.returncode, 0)

@parameterized.expand(["aws-serverless-function-image.yaml"])
@parameterized.expand(["aws-serverless-function-image.yaml", "aws-lambda-function-image.yaml"])
def test_no_package_and_deploy_with_s3_bucket_all_args_resolve_image_repos(self, template_file):
template_path = self.test_data_path.joinpath(template_file)

Expand Down Expand Up @@ -593,7 +596,7 @@ def test_deploy_guided_zip(self, template_file):
# Remove samconfig.toml
os.remove(self.test_data_path.joinpath(DEFAULT_CONFIG_FILE_NAME))

@parameterized.expand(["aws-serverless-function-image.yaml"])
@parameterized.expand(["aws-serverless-function-image.yaml", "aws-lambda-function-image.yaml"])
def test_deploy_guided_image_auto(self, template_file):
template_path = self.test_data_path.joinpath(template_file)

Expand All @@ -613,8 +616,8 @@ def test_deploy_guided_image_auto(self, template_file):
# Remove samconfig.toml
os.remove(self.test_data_path.joinpath(DEFAULT_CONFIG_FILE_NAME))

@parameterized.expand(["aws-serverless-function-image.yaml"])
def test_deploy_guided_image_specify(self, template_file):
@parameterized.expand([("aws-serverless-function-image.yaml", True), ("aws-lambda-function-image.yaml", False)])
def test_deploy_guided_image_specify(self, template_file, does_ask_for_authorization):
template_path = self.test_data_path.joinpath(template_file)

stack_name = self._method_to_stack_name(self.id())
Expand All @@ -623,8 +626,11 @@ def test_deploy_guided_image_specify(self, template_file):
# Package and Deploy in one go without confirming change set.
deploy_command_list = self.get_deploy_command_list(template_file=template_path, guided=True)

autorization_question_answer = "\n" if does_ask_for_authorization else ""

deploy_process_execute = run_command_with_input(
deploy_command_list, f"{stack_name}\n\n\n\n\ny\n\n\n\nn\n{self.ecr_repo_name}\n\n\n\n".encode()
deploy_command_list,
f"{stack_name}\n\n\n\n\ny\n\n\n{autorization_question_answer}n\n{self.ecr_repo_name}\n\n\n\n".encode(),
)

# Deploy should succeed with a managed stack
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Parameters:
ImageUri:
Type: String
Handler:
Type: String

Resources:
ImageFunction:
Type: AWS::Lambda::Function
Properties:
PackageType: Image
ImageConfig:
Command:
- !Ref Handler
Code:
ImageUri: !Ref ImageUri
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
AWSTemplateFormatVersion : '2010-09-09'
Transform: AWS::Serverless-2016-10-31

Parameters:
ImageUri:
Type: String
Handler:
Type: String

Resources:

ImageFunction:
Type: AWS::Serverless::Function
Properties:
PackageType: Image
ImageUri: !Ref ImageUri
ImageConfig:
Command:
- !Ref Handler
Timeout: 600
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ Resources:
Type: AWS::Lambda::Function
Properties:
PackageType: Image
Code: "emulation-python3.8:latest"
Code:
ImageUri: "emulation-python3.8:latest"
Role:
Fn::GetAtt:
- "LambdaExecutionRole"
Expand Down
19 changes: 17 additions & 2 deletions tests/integration/testdata/package/aws-lambda-function-image.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,24 @@ Resources:
Type: AWS::Lambda::Function
Properties:
PackageType: Image
Code: emulation-python3.8:latest
Code:
ImageUri: emulation-python3.8:latest
Role:
Fn::GetAtt:
- "LambdaExecutionRole"
- "HelloWorldFunctionRole"
- "Arn"
Timeout: 25
HelloWorldFunctionRole:
Type: AWS::IAM::Role
Properties:
AssumeRolePolicyDocument:
Statement:
- Action:
- sts:AssumeRole
Effect: Allow
Principal:
Service:
- lambda.amazonaws.com
Version: '2012-10-17'
ManagedPolicyArns:
- arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole
20 changes: 15 additions & 5 deletions tests/unit/commands/buildcmd/test_build_context.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import os
from samcli.lib.build.app_builder import ApplicationBuilder, ApplicationBuildResult
from samcli.lib.build.app_builder import ApplicationBuildResult
from unittest import TestCase
from unittest.mock import patch, Mock, ANY, call

from parameterized import parameterized

from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR
from samcli.lib.utils.osutils import BUILD_DIR_PERMISSIONS
from samcli.lib.utils.packagetype import ZIP, IMAGE
from samcli.local.lambdafn.exceptions import ResourceNotFound
from samcli.commands.build.build_context import BuildContext
from samcli.commands.build.exceptions import InvalidBuildDirException, MissingBuildMethodException
Expand Down Expand Up @@ -92,7 +93,7 @@ def test_must_setup_context(
self.assertEqual(context.stacks, [stack])
self.assertEqual(context.manifest_path_override, os.path.abspath("manifest_path"))
self.assertEqual(context.mode, "buildmode")
resources_to_build = context.resources_to_build
resources_to_build = context.get_resources_to_build()
self.assertTrue(function1 in resources_to_build.functions)
self.assertTrue(layer1 in resources_to_build.layers)

Expand Down Expand Up @@ -356,9 +357,13 @@ def test_must_return_many_functions_to_build(
func2 = DummyFunction("func2")
func3_skipped = DummyFunction("func3", inlinecode="def handler(): pass", codeuri=None)
func4_skipped = DummyFunction("func4", codeuri="packaged_function.zip")
func5_skipped = DummyFunction("func5", codeuri=None, packagetype=IMAGE)
func6 = DummyFunction(
"func6", packagetype=IMAGE, metadata={"DockerContext": "/path", "Dockerfile": "DockerFile"}
)

func_provider_mock = Mock()
func_provider_mock.get_all.return_value = [func1, func2, func3_skipped, func4_skipped]
func_provider_mock.get_all.return_value = [func1, func2, func3_skipped, func4_skipped, func5_skipped, func6]
funcprovider = SamFunctionProviderMock.return_value = func_provider_mock

layer1 = DummyLayer("layer1", "buildMethod")
Expand Down Expand Up @@ -407,7 +412,7 @@ def test_must_return_many_functions_to_build(
self.assertEqual(context.mode, "buildmode")
self.assertFalse(context.is_building_specific_resource)
resources_to_build = context.resources_to_build
self.assertEqual(resources_to_build.functions, [func1, func2])
self.assertEqual(resources_to_build.functions, [func1, func2, func6])
self.assertEqual(resources_to_build.layers, [layer1])
get_buildable_stacks_mock.assert_called_once_with(
"template_file", parameter_overrides={"overrides": "value"}, global_parameter_overrides=None
Expand Down Expand Up @@ -927,9 +932,14 @@ def __init__(self, name, build_method, codeuri="layer_src"):


class DummyFunction:
def __init__(self, name, layers=[], inlinecode=None, codeuri="src"):
def __init__(
self, name, layers=[], inlinecode=None, codeuri="src", imageuri="image:latest", packagetype=ZIP, metadata=None
):
self.name = name
self.layers = layers
self.inlinecode = inlinecode
self.codeuri = codeuri
self.imageuri = imageuri
self.full_path = Mock()
self.packagetype = packagetype
self.metadata = metadata if metadata else {}

0 comments on commit b570979

Please sign in to comment.