diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py index ef38c4fea9..31caa4eeea 100644 --- a/samcli/commands/build/build_context.py +++ b/samcli/commands/build/build_context.py @@ -9,8 +9,11 @@ from typing import Dict, Optional, List, cast import click + +from samcli.lib.providers.sam_api_provider import SamApiProvider from samcli.lib.utils.packagetype import IMAGE +from samcli.commands._utils.template import get_template_data from samcli.commands.build.exceptions import InvalidBuildDirException, MissingBuildMethodException from samcli.lib.bootstrap.nested_stack.nested_stack_manager import NestedStackManager from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR @@ -154,6 +157,12 @@ def get_resources_to_build(self): def run(self): """Runs the building process by creating an ApplicationBuilder.""" + template_dict = get_template_data(self._template_file) + template_transform = template_dict.get("Transform", "") + is_sam_template = isinstance(template_transform, str) and template_transform.startswith("AWS::Serverless") + if is_sam_template: + SamApiProvider.check_implicit_api_resource_ids(self.stacks) + try: builder = ApplicationBuilder( self.get_resources_to_build(), diff --git a/samcli/lib/providers/sam_api_provider.py b/samcli/lib/providers/sam_api_provider.py index 1fd48dfde5..5493baedb2 100644 --- a/samcli/lib/providers/sam_api_provider.py +++ b/samcli/lib/providers/sam_api_provider.py @@ -7,6 +7,7 @@ from samcli.lib.providers.cfn_base_api_provider import CfnBaseApiProvider from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException from samcli.lib.providers.provider import Stack +from samcli.lib.utils.colors import Colored from samcli.local.apigw.local_apigw_service import Route from samcli.lib.utils.resources import AWS_SERVERLESS_FUNCTION, AWS_SERVERLESS_API, AWS_SERVERLESS_HTTPAPI @@ -327,3 +328,20 @@ def _get_route_stack_depth(route: Route) -> int: if not route.stack_path: return 0 return route.stack_path.count("/") + 1 + + @staticmethod + def check_implicit_api_resource_ids(stacks: List[Stack]) -> None: + for stack in stacks: + for logical_id in stack.resources: + if logical_id in ( + SamApiProvider.IMPLICIT_API_RESOURCE_ID, + SamApiProvider.IMPLICIT_HTTP_API_RESOURCE_ID, + ): + LOG.warning( + Colored().yellow( + 'Your template contains a resource with logical ID "%s", ' + "which is a reserved logical ID in AWS SAM. " + "It could result in unexpected behaviors and is not recommended." + ), + logical_id, + ) diff --git a/tests/unit/commands/buildcmd/test_build_context.py b/tests/unit/commands/buildcmd/test_build_context.py index ec2c7f6e8e..747b9606d3 100644 --- a/tests/unit/commands/buildcmd/test_build_context.py +++ b/tests/unit/commands/buildcmd/test_build_context.py @@ -27,6 +27,7 @@ class DeepWrap(Exception): class TestBuildContext__enter__(TestCase): + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @@ -39,6 +40,7 @@ def test_must_setup_context( SamLayerProviderMock, SamFunctionProviderMock, get_buildable_stacks_mock, + get_template_data_mock, ): template_dict = "template dict" stack = Mock() @@ -108,6 +110,7 @@ def test_must_setup_context( ContainerManagerMock.assert_called_once_with(docker_network_id="network", skip_pull_image=True) func_provider_mock.get.assert_called_once_with("function_identifier") + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @@ -120,6 +123,7 @@ def test_must_fail_with_illegal_identifier( SamLayerProviderMock, SamFunctionProviderMock, get_buildable_stacks_mock, + get_template_data_mock, ): template_dict = "template dict" stack = Mock() @@ -163,6 +167,7 @@ def test_must_fail_with_illegal_identifier( with self.assertRaises(ResourceNotFound): context.resources_to_build + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @@ -175,6 +180,7 @@ def test_must_return_only_layer_when_layer_is_build( SamLayerProviderMock, SamFunctionProviderMock, get_buildable_stacks_mock, + get_template_data_mock, ): template_dict = "template dict" stack = Mock() @@ -216,6 +222,7 @@ def test_must_return_only_layer_when_layer_is_build( context.__enter__() self.assertTrue(layer1 in context.resources_to_build.layers) + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @@ -228,6 +235,7 @@ def test_must_return_buildable_dependent_layer_when_function_is_build( SamLayerProviderMock, SamFunctionProviderMock, get_buildable_stacks_mock, + get_template_data_mock, ): template_dict = "template dict" stack = Mock() @@ -275,6 +283,7 @@ def test_must_return_buildable_dependent_layer_when_function_is_build( self.assertTrue(layer2 not in context.resources_to_build.layers) self.assertTrue(context.is_building_specific_resource) + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @@ -287,6 +296,7 @@ def test_must_fail_when_layer_is_build_without_buildmethod( SamLayerProviderMock, SamFunctionProviderMock, get_buildable_stacks_mock, + get_template_data_mock, ): template_dict = "template dict" stack = Mock() @@ -329,6 +339,7 @@ def test_must_fail_when_layer_is_build_without_buildmethod( with self.assertRaises(MissingBuildMethodException): context.resources_to_build + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @@ -341,6 +352,7 @@ def test_must_return_many_functions_to_build( SamLayerProviderMock, SamFunctionProviderMock, get_buildable_stacks_mock, + get_template_data_mock, ): """ In this unit test, we also verify @@ -435,6 +447,7 @@ def test_must_return_many_functions_to_build( @parameterized.expand([(["remote_stack_1", "stack.remote_stack_2"], "print_warning"), ([], False)]) @patch("samcli.commands.build.build_context.LOG") + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @@ -449,6 +462,7 @@ def test_must_print_remote_url_warning( SamLayerProviderMock, SamFunctionProviderMock, get_buildable_stacks_mock, + get_template_data_mock, log_mock, ): get_buildable_stacks_mock.return_value = ([], remote_stack_full_paths) @@ -649,6 +663,7 @@ def test_cached_dir_and_deps_dir_creation( class TestBuildContext_run(TestCase): @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") + @patch("samcli.commands.build.build_context.SamApiProvider") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @patch("samcli.commands.build.build_context.pathlib") @@ -657,10 +672,12 @@ class TestBuildContext_run(TestCase): @patch("samcli.commands.build.build_context.ApplicationBuilder") @patch("samcli.commands.build.build_context.BuildContext.get_resources_to_build") @patch("samcli.commands.build.build_context.move_template") + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.os") def test_run_build_context( self, os_mock, + get_template_data_mock, move_template_mock, resources_mock, ApplicationBuilderMock, @@ -669,6 +686,7 @@ def test_run_build_context( pathlib_mock, SamLayerProviderMock, SamFunctionProviderMock, + SamApiProviderMock, get_buildable_stacks_mock, ): @@ -787,6 +805,7 @@ def test_run_build_context( ] ) @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") + @patch("samcli.commands.build.build_context.SamApiProvider") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @patch("samcli.commands.build.build_context.pathlib") @@ -795,12 +814,14 @@ def test_run_build_context( @patch("samcli.commands.build.build_context.ApplicationBuilder") @patch("samcli.commands.build.build_context.BuildContext.get_resources_to_build") @patch("samcli.commands.build.build_context.move_template") + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.os") def test_must_catch_known_exceptions( self, exception, wrapped_exception, os_mock, + get_template_data_mock, move_template_mock, resources_mock, ApplicationBuilderMock, @@ -809,6 +830,7 @@ def test_must_catch_known_exceptions( pathlib_mock, SamLayerProviderMock, SamFunctionProviderMock, + SamApiProviderMock, get_buildable_stacks_mock, ): @@ -862,6 +884,7 @@ def test_must_catch_known_exceptions( self.assertEqual(wrapped_exception, ctx.exception.wrapped_from) @patch("samcli.commands.build.build_context.SamLocalStackProvider.get_stacks") + @patch("samcli.commands.build.build_context.SamApiProvider") @patch("samcli.commands.build.build_context.SamFunctionProvider") @patch("samcli.commands.build.build_context.SamLayerProvider") @patch("samcli.commands.build.build_context.pathlib") @@ -870,10 +893,12 @@ def test_must_catch_known_exceptions( @patch("samcli.commands.build.build_context.ApplicationBuilder") @patch("samcli.commands.build.build_context.BuildContext.get_resources_to_build") @patch("samcli.commands.build.build_context.move_template") + @patch("samcli.commands.build.build_context.get_template_data") @patch("samcli.commands.build.build_context.os") def test_must_catch_function_not_found_exception( self, os_mock, + get_template_data_mock, move_template_mock, resources_mock, ApplicationBuilderMock, @@ -882,6 +907,7 @@ def test_must_catch_function_not_found_exception( pathlib_mock, SamLayerProviderMock, SamFunctionProviderMock, + SamApiProviderMock, get_buildable_stacks_mock, ): stack = Mock() diff --git a/tests/unit/commands/local/lib/test_api_provider.py b/tests/unit/commands/local/lib/test_api_provider.py index 7d10663430..f671f77bb7 100644 --- a/tests/unit/commands/local/lib/test_api_provider.py +++ b/tests/unit/commands/local/lib/test_api_provider.py @@ -239,3 +239,24 @@ def test_apis_in_child_stack_overridden_by_apis_in_parents_within_implicit_or_ex (logicalId, [route2]), ] self.assertEqual(SamApiProvider.merge_routes(collector), [route1]) + + +class TestApiProvider_check_implicit_api_resource_ids(TestCase): + @patch("samcli.lib.providers.sam_api_provider.LOG.warning") + def test_check_implicit_api_resource_ids_false(self, warning_mock): + SamApiProvider.check_implicit_api_resource_ids([Mock(resources={"Api1": {"Properties": Mock()}})]) + warning_mock.assert_not_called() + + @patch("samcli.lib.providers.sam_api_provider.LOG.warning") + def test_check_implicit_api_resource_ids_rest_api(self, warning_mock): + SamApiProvider.check_implicit_api_resource_ids( + [Mock(resources={"Api1": {"Properties": Mock()}, "ServerlessRestApi": {"Properties": Mock()}})] + ) + warning_mock.assert_called_once() + + @patch("samcli.lib.providers.sam_api_provider.LOG.warning") + def test_check_implicit_api_resource_ids_http_api(self, warning_mock): + SamApiProvider.check_implicit_api_resource_ids( + [Mock(resources={"Api1": {"Properties": Mock()}, "ServerlessHttpApi": {"Properties": Mock()}})] + ) + warning_mock.assert_called_once()