Skip to content

Commit

Permalink
fix: use the correct content property for Lambda Layer Version (aws#3552
Browse files Browse the repository at this point in the history
)

* fix Lambda LayerVersion resource content property

* add integration test case
  • Loading branch information
moelasmar authored Dec 29, 2021
1 parent 9c55468 commit 279ba2e
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 30 deletions.
31 changes: 18 additions & 13 deletions samcli/lib/build/app_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,19 +306,9 @@ def update_template(
store_path = os.path.relpath(absolute_output_path, original_dir)

if has_build_artifact:
if resource_type == AWS_SERVERLESS_FUNCTION and properties.get("PackageType", ZIP) == ZIP:
properties["CodeUri"] = store_path

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

if resource_type in [AWS_SERVERLESS_LAYERVERSION, AWS_LAMBDA_LAYERVERSION]:
properties["ContentUri"] = store_path

if resource_type == AWS_LAMBDA_FUNCTION and properties.get("PackageType", ZIP) == IMAGE:
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]
ApplicationBuilder._update_built_resource(
built_artifacts[full_path], properties, resource_type, store_path
)

if is_stack:
if resource_type == AWS_SERVERLESS_APPLICATION:
Expand All @@ -329,6 +319,21 @@ def update_template(

return template_dict

@staticmethod
def _update_built_resource(path: str, resource_properties: Dict, resource_type: str, absolute_path: str) -> None:
if resource_type == AWS_SERVERLESS_FUNCTION and resource_properties.get("PackageType", ZIP) == ZIP:
resource_properties["CodeUri"] = absolute_path
if resource_type == AWS_LAMBDA_FUNCTION and resource_properties.get("PackageType", ZIP) == ZIP:
resource_properties["Code"] = absolute_path
if resource_type == AWS_LAMBDA_LAYERVERSION:
resource_properties["Content"] = absolute_path
if resource_type == AWS_SERVERLESS_LAYERVERSION:
resource_properties["ContentUri"] = absolute_path
if resource_type == AWS_LAMBDA_FUNCTION and resource_properties.get("PackageType", ZIP) == IMAGE:
resource_properties["Code"] = {"ImageUri": path}
if resource_type == AWS_SERVERLESS_FUNCTION and resource_properties.get("PackageType", ZIP) == IMAGE:
resource_properties["ImageUri"] = path

def _build_lambda_image(self, function_name: str, metadata: Dict, architecture: str) -> str:
"""
Build an Lambda image
Expand Down
13 changes: 10 additions & 3 deletions tests/integration/buildcmd/test_build_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -947,8 +947,15 @@ class TestBuildCommand_LayerBuilds(BuildIntegBase):
EXPECTED_FILES_PROJECT_MANIFEST = {"__init__.py", "main.py", "requirements.txt"}
EXPECTED_LAYERS_FILES_PROJECT_MANIFEST = {"__init__.py", "layer.py", "numpy", "requirements.txt"}

@parameterized.expand([("python3.7", False, "LayerOne"), ("python3.7", "use_container", "LayerOne")])
def test_build_single_layer(self, runtime, use_container, layer_identifier):
@parameterized.expand(
[
("python3.7", False, "LayerOne", "ContentUri"),
("python3.7", "use_container", "LayerOne", "ContentUri"),
("python3.7", False, "LambdaLayerOne", "Content"),
("python3.7", "use_container", "LambdaLayerOne", "Content"),
]
)
def test_build_single_layer(self, runtime, use_container, layer_identifier, content_property):
if use_container and (SKIP_DOCKER_TESTS or SKIP_DOCKER_BUILD):
self.skipTest(SKIP_DOCKER_MESSAGE)

Expand All @@ -966,7 +973,7 @@ def test_build_single_layer(self, runtime, use_container, layer_identifier):
self.default_build_dir,
layer_identifier,
self.EXPECTED_LAYERS_FILES_PROJECT_MANIFEST,
"ContentUri",
content_property,
"python",
)

Expand Down
10 changes: 10 additions & 0 deletions tests/integration/testdata/buildcmd/layers-functions-template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ Resources:
Metadata:
BuildMethod: !Ref LayerBuildMethod

LambdaLayerOne:
Type: AWS::Lambda::LayerVersion
Properties:
Description: Lambda Layer one
Content: !Ref LayerContentUri
CompatibleRuntimes:
- python3.7
Metadata:
BuildMethod: !Ref LayerBuildMethod

LayerTwo:
Type: AWS::Serverless::LayerVersion
Properties:
Expand Down
99 changes: 85 additions & 14 deletions tests/unit/lib/build_module/test_app_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from samcli.commands.local.cli_common.user_exceptions import InvalidFunctionPropertyType
from samcli.lib.utils.architecture import X86_64, ARM64
from samcli.lib.utils.packagetype import IMAGE, ZIP
from samcli.lib.utils.stream_writer import StreamWriter
from tests.unit.lib.build_module.test_build_graph import generate_function


Expand Down Expand Up @@ -70,7 +71,9 @@ def setUp(self):
resources_to_build_collector = ResourcesToBuildCollector()
resources_to_build_collector.add_functions([self.func1, self.func2, self.imageFunc1])
resources_to_build_collector.add_layers([self.layer1, self.layer2])
self.builder = ApplicationBuilder(resources_to_build_collector, "builddir", "basedir", "cachedir")
self.builder = ApplicationBuilder(
resources_to_build_collector, "builddir", "basedir", "cachedir", stream_writer=StreamWriter(sys.stderr)
)

@patch("samcli.lib.build.build_graph.BuildGraph._write")
def test_must_iterate_on_functions_and_layers(self, persist_mock):
Expand Down Expand Up @@ -258,7 +261,9 @@ def test_should_run_build_for_only_unique_builds(self, persist_mock, read_mock,
build_dir = "builddir"

# instantiate the builder and run build method
builder = ApplicationBuilder(resources_to_build_collector, "builddir", "basedir", "cachedir")
builder = ApplicationBuilder(
resources_to_build_collector, "builddir", "basedir", "cachedir", stream_writer=StreamWriter(sys.stderr)
)
builder._build_function = build_function_mock
build_function_mock.side_effect = [
function1_1.get_build_dir(build_dir),
Expand Down Expand Up @@ -319,7 +324,7 @@ def test_default_run_should_pick_default_strategy(self, mock_default_build_strat
build_graph_mock = Mock()
get_build_graph_mock = Mock(return_value=build_graph_mock)

builder = ApplicationBuilder(Mock(), "builddir", "basedir", "cachedir")
builder = ApplicationBuilder(Mock(), "builddir", "basedir", "cachedir", stream_writer=StreamWriter(sys.stderr))
builder._get_build_graph = get_build_graph_mock

result = builder.build().artifacts
Expand All @@ -338,7 +343,9 @@ def test_cached_run_should_pick_incremental_strategy(
build_graph_mock = Mock()
get_build_graph_mock = Mock(return_value=build_graph_mock)

builder = ApplicationBuilder(Mock(), "builddir", "basedir", "cachedir", cached=True)
builder = ApplicationBuilder(
Mock(), "builddir", "basedir", "cachedir", cached=True, stream_writer=StreamWriter(sys.stderr)
)
builder._get_build_graph = get_build_graph_mock

result = builder.build().artifacts
Expand All @@ -354,7 +361,9 @@ def test_parallel_run_should_pick_parallel_strategy(self, mock_parallel_build_st
build_graph_mock = Mock()
get_build_graph_mock = Mock(return_value=build_graph_mock)

builder = ApplicationBuilder(Mock(), "builddir", "basedir", "cachedir", parallel=True)
builder = ApplicationBuilder(
Mock(), "builddir", "basedir", "cachedir", parallel=True, stream_writer=StreamWriter(sys.stderr)
)
builder._get_build_graph = get_build_graph_mock

result = builder.build().artifacts
Expand All @@ -377,7 +386,15 @@ def test_parallel_and_cached_run_should_pick_parallel_with_incremental(
build_graph_mock = Mock()
get_build_graph_mock = Mock(return_value=build_graph_mock)

builder = ApplicationBuilder(Mock(), "builddir", "basedir", "cachedir", parallel=True, cached=True)
builder = ApplicationBuilder(
Mock(),
"builddir",
"basedir",
"cachedir",
parallel=True,
cached=True,
stream_writer=StreamWriter(sys.stderr),
)
builder._get_build_graph = get_build_graph_mock

result = builder.build().artifacts
Expand Down Expand Up @@ -422,7 +439,9 @@ def test_must_raise_for_functions_with_multi_architecture(self, persist_mock, re
build_dir = "builddir"

# instantiate the builder and run build method
builder = ApplicationBuilder(resources_to_build_collector, "builddir", "basedir", "cachedir")
builder = ApplicationBuilder(
resources_to_build_collector, "builddir", "basedir", "cachedir", stream_writer=StreamWriter(sys.stderr)
)
builder._build_function = build_function_mock
build_function_mock.side_effect = [function.get_build_dir(build_dir)]

Expand All @@ -447,7 +466,9 @@ def setUp(self):
self.container_manager = Mock()
resources_to_build_collector = ResourcesToBuildCollector()
resources_to_build_collector.add_layers([self.layer1, self.layer2])
self.builder = ApplicationBuilder(resources_to_build_collector, "builddir", "basedir", "cachedir")
self.builder = ApplicationBuilder(
resources_to_build_collector, "builddir", "basedir", "cachedir", stream_writer=StreamWriter(sys.stderr)
)

@patch("samcli.lib.build.app_builder.get_workflow_config")
@patch("samcli.lib.build.app_builder.osutils")
Expand Down Expand Up @@ -590,7 +611,9 @@ def make_root_template(self, resource_type, location_property_name):
}

def setUp(self):
self.builder = ApplicationBuilder(Mock(), "builddir", "basedir", "cachedir")
self.builder = ApplicationBuilder(
Mock(), "builddir", "basedir", "cachedir", stream_writer=StreamWriter(sys.stderr)
)

self.template_dict = {
"Resources": {
Expand All @@ -603,6 +626,16 @@ def setUp(self):
"Properties": {"PackageType": "Image"},
"Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "DockerContext", "DockerTag": "Tag"},
},
"MyServerlessLayer": {
"Type": "AWS::Serverless::LayerVersion",
"Properties": {"ContentUri": "oldvalue"},
"Metadata": {"BuildMethod": "python3.8"},
},
"MyLambdaLayer": {
"Type": "AWS::Lambda::LayerVersion",
"Properties": {"Content": "oldvalue"},
"Metadata": {"BuildMethod": "python3.8"},
},
}
}

Expand All @@ -612,6 +645,8 @@ def test_must_update_resources_with_build_artifacts(self):
built_artifacts = {
"MyFunction1": "/path/to/build/MyFunction1",
"MyFunction2": "/path/to/build/MyFunction2",
"MyServerlessLayer": "/path/to/build/ServerlessLayer",
"MyLambdaLayer": "/path/to/build/LambdaLayer",
"MyImageFunction1": "myimagefunction1:Tag",
"PreBuiltImageFunction1": "",
}
Expand All @@ -633,6 +668,16 @@ def test_must_update_resources_with_build_artifacts(self):
"Properties": {"Code": {"ImageUri": "myimagefunction1:Tag"}, "PackageType": IMAGE},
"Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "DockerContext", "DockerTag": "Tag"},
},
"MyServerlessLayer": {
"Type": "AWS::Serverless::LayerVersion",
"Properties": {"ContentUri": os.path.join("build", "ServerlessLayer")},
"Metadata": {"BuildMethod": "python3.8"},
},
"MyLambdaLayer": {
"Type": "AWS::Lambda::LayerVersion",
"Properties": {"Content": os.path.join("build", "LambdaLayer")},
"Metadata": {"BuildMethod": "python3.8"},
},
}
}

Expand All @@ -649,6 +694,8 @@ def test_must_update_resources_with_build_artifacts_and_template_paths_in_multi_
original_root_template_path = "/path/to/template.yaml"
built_artifacts = {
"MyFunction1": "/path/to/build/MyFunction1",
"ChildStackXXX/MyServerlessLayer": "/path/to/build/ChildStackXXX/ServerlessLayer",
"ChildStackXXX/MyLambdaLayer": "/path/to/build/ChildStackXXX/LambdaLayer",
"ChildStackXXX/MyFunction1": "/path/to/build/ChildStackXXX/MyFunction1",
"ChildStackXXX/MyFunction2": "/path/to/build/ChildStackXXX/MyFunction2",
"ChildStackXXX/MyImageFunction1": "myimagefunction1:Tag",
Expand All @@ -675,6 +722,16 @@ def test_must_update_resources_with_build_artifacts_and_template_paths_in_multi_
"Properties": {"Code": {"ImageUri": "myimagefunction1:Tag"}, "PackageType": IMAGE},
"Metadata": {"Dockerfile": "Dockerfile", "DockerContext": "DockerContext", "DockerTag": "Tag"},
},
"MyServerlessLayer": {
"Type": "AWS::Serverless::LayerVersion",
"Properties": {"ContentUri": os.path.join("build", "ChildStackXXX", "ServerlessLayer")},
"Metadata": {"BuildMethod": "python3.8"},
},
"MyLambdaLayer": {
"Type": "AWS::Lambda::LayerVersion",
"Properties": {"Content": os.path.join("build", "ChildStackXXX", "LambdaLayer")},
"Metadata": {"BuildMethod": "python3.8"},
},
}
}
expected_root = {
Expand Down Expand Up @@ -717,7 +774,9 @@ def test_must_skip_if_no_artifacts(self):

class TestApplicationBuilder_update_template_windows(TestCase):
def setUp(self):
self.builder = ApplicationBuilder(Mock(), "builddir", "basedir", "cachedir")
self.builder = ApplicationBuilder(
Mock(), "builddir", "basedir", "cachedir", stream_writer=StreamWriter(sys.stderr)
)

self.template_dict = {
"Resources": {
Expand Down Expand Up @@ -980,7 +1039,9 @@ def test_can_build_image_function_under_debug_with_target(self, mock_os):

class TestApplicationBuilder_build_function(TestCase):
def setUp(self):
self.builder = ApplicationBuilder(Mock(), "/build/dir", "/base/dir", "cachedir")
self.builder = ApplicationBuilder(
Mock(), "/build/dir", "/base/dir", "cachedir", stream_writer=StreamWriter(sys.stderr)
)

@patch("samcli.lib.build.app_builder.get_workflow_config")
@patch("samcli.lib.build.app_builder.osutils")
Expand Down Expand Up @@ -1214,7 +1275,9 @@ def test_must_build_in_container_with_custom_default_build_image(self, osutils_m

class TestApplicationBuilder_build_function_in_process(TestCase):
def setUp(self):
self.builder = ApplicationBuilder(Mock(), "/build/dir", "/base/dir", "/cache/dir", mode="mode")
self.builder = ApplicationBuilder(
Mock(), "/build/dir", "/base/dir", "/cache/dir", mode="mode", stream_writer=StreamWriter(sys.stderr)
)

@patch("samcli.lib.build.app_builder.LambdaBuilder")
def test_must_use_lambda_builder(self, lambda_builder_mock):
Expand Down Expand Up @@ -1284,7 +1347,13 @@ class TestApplicationBuilder_build_function_on_container(TestCase):
def setUp(self):
self.container_manager = Mock()
self.builder = ApplicationBuilder(
Mock(), "/build/dir", "/base/dir", "/cache/dir", container_manager=self.container_manager, mode="mode"
Mock(),
"/build/dir",
"/base/dir",
"/cache/dir",
container_manager=self.container_manager,
mode="mode",
stream_writer=StreamWriter(sys.stderr),
)
self.builder._parse_builder_response = Mock()

Expand Down Expand Up @@ -1392,7 +1461,9 @@ def test_must_raise_on_unsupported_container_build(self, supports_build_in_conta
class TestApplicationBuilder_parse_builder_response(TestCase):
def setUp(self):
self.image_name = "name"
self.builder = ApplicationBuilder(Mock(), "/build/dir", "/base/dir", "/cache/dir")
self.builder = ApplicationBuilder(
Mock(), "/build/dir", "/base/dir", "/cache/dir", stream_writer=StreamWriter(sys.stderr)
)

def test_must_parse_json(self):
data = {"valid": "json"}
Expand Down

0 comments on commit 279ba2e

Please sign in to comment.