fix: allow parameters to be passed through to SamTranslatorWrapper (#…

…1961)

* fix: allow parameters to be passed through to `SamTranslatorWrapper`

Why is this change necessary?

* Some plugins require that intrinsics are resolved and parameter
information be present.

How does it address the issue?

* Passing parameters that have parameters and pseudo parameters resolved
fixes the issue.
* Also tested with local invoke and deploy

What side effects does this change have?

* None at this point.

* fix: integration tests

Why is this change necessary?

* Integration testing for intrinsics.

How does it address the issue?

* Test use cases where the function invoke and deploys work and shouldnt
work.

* fix: test assertions - line numbers

Why is this change necessary?

* Container Implementation may have changed, which changes the line
numbers in the assertion.

How does it address the issue?

* Updated to correct line number in the assertion.

What side effects does this change have?

* None

* fix: address comments

samcli/commands/deploy/deploy_context.py

@@ -24,6 +24,7 @@
 from samcli.commands._utils.template import get_template_data
 from samcli.commands.deploy import exceptions as deploy_exceptions
 from samcli.commands.deploy.auth_utils import auth_per_resource
+from samcli.commands.deploy.utils import sanitize_parameter_overrides
 from samcli.lib.deploy.deployer import Deployer
 from samcli.lib.package.s3_uploader import S3Uploader
 from samcli.lib.utils.botoconfig import get_boto_config_with_user_agent
@@ -149,7 +150,9 @@ def deploy(
         confirm_changeset=False,
     ):
 
-        auth_required_per_resource = auth_per_resource(parameters, get_template_data(self.template_file))
+        auth_required_per_resource = auth_per_resource(
+            sanitize_parameter_overrides(self.parameter_overrides), get_template_data(self.template_file)
+        )
 
         for resource, authorization_required in auth_required_per_resource:
             if not authorization_required:

samcli/commands/deploy/guided_context.py

@@ -14,6 +14,7 @@
 from samcli.commands.deploy.exceptions import GuidedDeployFailedError
 from samcli.commands.deploy.guided_config import GuidedConfig
 from samcli.commands.deploy.auth_utils import auth_per_resource
+from samcli.commands.deploy.utils import sanitize_parameter_overrides
 from samcli.lib.bootstrap.bootstrap import manage_stack
 from samcli.lib.utils.colors import Colored
 
@@ -99,7 +100,7 @@ def guided_prompts(self, parameter_override_keys):
                 type=FuncParamType(func=_space_separated_list_func_type),
             )
 
-        self.prompt_authorization(input_parameter_overrides)
+        self.prompt_authorization(sanitize_parameter_overrides(input_parameter_overrides))
 
         save_to_config = confirm(f"\t{self.start_bold}Save arguments to samconfig.toml{self.end_bold}", default=True)
 

samcli/lib/providers/sam_base_provider.py

@@ -37,14 +37,14 @@ def get_template(template_dict, parameter_overrides=None):
             Processed SAM template
         """
         template_dict = template_dict or {}
+        parameters_values = SamBaseProvider._get_parameter_values(template_dict, parameter_overrides)
         if template_dict:
-            template_dict = SamTranslatorWrapper(template_dict).run_plugins()
+            template_dict = SamTranslatorWrapper(template_dict, parameter_values=parameters_values).run_plugins()
         ResourceMetadataNormalizer.normalize(template_dict)
-        logical_id_translator = SamBaseProvider._get_parameter_values(template_dict, parameter_overrides)
 
         resolver = IntrinsicResolver(
             template=template_dict,
-            symbol_resolver=IntrinsicsSymbolTable(logical_id_translator=logical_id_translator, template=template_dict),
+            symbol_resolver=IntrinsicsSymbolTable(logical_id_translator=parameters_values, template=template_dict),
         )
         template_dict = resolver.resolve_template(ignore_errors=True)
         return template_dict

samcli/lib/samlib/wrapper.py

@@ -37,17 +37,20 @@ class SamTranslatorWrapper:
     _thisdir = os.path.dirname(os.path.abspath(__file__))
     _DEFAULT_MANAGED_POLICIES_FILE = os.path.join(_thisdir, "default_managed_policies.json")
 
-    def __init__(self, sam_template, offline_fallback=True):
+    def __init__(self, sam_template, parameter_values=None, offline_fallback=True):
         """
 
         Parameters
         ----------
         sam_template dict:
             SAM Template dictionary
+        parameter_values dict:
+            SAM Template parameters (must contain psuedo and default parameters)
         offline_fallback bool:
             Set it to True to make the translator work entirely offline, if internet is not available
         """
         self.local_uri_plugin = SupportLocalUriPlugin()
+        self.parameter_values = parameter_values
         self.extra_plugins = [
             # Extra plugin specific to the SAM CLI that will support local paths for CodeUri & DefinitionUri
             self.local_uri_plugin
@@ -65,7 +68,9 @@ def run_plugins(self, convert_local_uris=True):
             additional_plugins.append(self.local_uri_plugin)
 
         parser = _SamParserReimplemented()
-        all_plugins = prepare_plugins(additional_plugins)
+        all_plugins = prepare_plugins(
+            additional_plugins, parameters=self.parameter_values if self.parameter_values else {}
+        )
 
         try:
             parser.parse(template_copy, all_plugins)  # parse() will run all configured plugins

tests/integration/local/invoke/test_integrations_cli.py

@@ -15,8 +15,8 @@
 from .invoke_integ_base import InvokeIntegBase
 from tests.testing_utils import IS_WINDOWS, RUNNING_ON_CI, RUNNING_TEST_FOR_MASTER_ON_CI, RUN_BY_CANARY
 
-# Layers tests require credentials and Travis will only add credentials to the env if the PR is from the same repo.
-# This is to restrict layers tests to run outside of Travis, when the branch is not master and tests are not run by Canary.
+# Layers tests require credentials and Appveyor will only add credentials to the env if the PR is from the same repo.
+# This is to restrict layers tests to run outside of Appveyor, when the branch is not master and tests are not run by Canary.
 SKIP_LAYERS_TESTS = RUNNING_ON_CI and RUNNING_TEST_FOR_MASTER_ON_CI and not RUN_BY_CANARY
 
 from pathlib import Path
@@ -401,9 +401,9 @@ def test_skip_pull_image_in_env_var(self):
         self.assertIn("Requested to skip pulling images", process_stderr.decode("utf-8"))
 
     @skipIf(SKIP_LAYERS_TESTS, "Skip layers tests in Appveyor only")
-    @skipIf(IS_WINDOWS, "This test failes on windows due to unix permissions not set properly on unzipped binary")
+    @skipIf(IS_WINDOWS, "This test fails on windows due to unix permissions not set properly on unzipped binary")
     @pytest.mark.flaky(reruns=3)
-    def test_invoke_returns_execpted_results_from_git_function(self):
+    def test_invoke_returns_expected_results_from_git_function(self):
         command_list = self.get_command_list(
             "GitLayerFunction", template_path=self.template_path, event_path=self.event_path
         )
@@ -418,6 +418,48 @@ def test_invoke_returns_execpted_results_from_git_function(self):
         process_stdout = stdout.strip()
         self.assertEqual(process_stdout.decode("utf-8"), '"git init passed"')
 
+    @skipIf(SKIP_LAYERS_TESTS, "Skip layers tests in Appveyor only")
+    @skipIf(IS_WINDOWS, "This test fails on windows due to unix permissions not set properly on unzipped binary")
+    @pytest.mark.flaky(reruns=3)
+    def test_invoke_returns_expected_results_from_git_function_with_parameters(self):
+        command_list = self.get_command_list(
+            "GitLayerFunctionParameters",
+            template_path=self.template_path,
+            event_path=self.event_path,
+            parameter_overrides={"LayerVersion": "5"},
+        )
+
+        process = Popen(command_list, stdout=PIPE)
+        try:
+            stdout, _ = process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
+
+        process_stdout = stdout.strip()
+        self.assertEqual(process_stdout.decode("utf-8"), '"git init passed"')
+
+
+class TestSamInstrinsicsAndPlugins(InvokeIntegBase):
+    template = "template-pseudo-params.yaml"
+
+    @pytest.mark.flaky(reruns=3)
+    def test_resolve_instrincs_which_runs_plugins(self):
+        command_list = self.get_command_list(
+            "HelloWorldServerlessFunction", template_path=self.template_path, event_path=self.event_path
+        )
+
+        process = Popen(command_list, stdout=PIPE)
+        try:
+            stdout, _ = process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
+
+        process_stdout = stdout.strip()
+        # Returned result is dependent on region, but should not be None.
+        self.assertIsNotNone(process_stdout.decode("utf-8"), "Invalid ApplicationId")
+
 
 class TestUsingConfigFiles(InvokeIntegBase):
     template = Path("template.yml")
@@ -603,7 +645,7 @@ def _create_cred_file(self, profile):
         return custom_cred
 
 
-@skipIf(SKIP_LAYERS_TESTS, "Skip layers tests in Travis only")
+@skipIf(SKIP_LAYERS_TESTS, "Skip layers tests in Appveyor only")
 class TestLayerVersion(InvokeIntegBase):
     template = Path("layers", "layer-template.yml")
     region = "us-west-2"
@@ -815,7 +857,7 @@ def test_caching_two_layers_with_layer_cache_env_set(self):
         self.assertEqual(2, len(os.listdir(str(self.layer_cache))))
 
 
-@skipIf(SKIP_LAYERS_TESTS, "Skip layers tests in Travis only")
+@skipIf(SKIP_LAYERS_TESTS, "Skip layers tests in Appveyor only")
 class TestLayerVersionThatDoNotCreateCache(InvokeIntegBase):
     template = Path("layers", "layer-template.yml")
     region = "us-west-2"
@@ -883,3 +925,33 @@ def test_account_does_not_exist_for_layer(self):
         )
 
         self.assertIn(expected_error_output, error_output)
+
+
+@skipIf(SKIP_LAYERS_TESTS, "Skip layers tests in Appveyor only")
+class TestBadLayerVersion(InvokeIntegBase):
+    template = Path("layers", "layer-bad-template.yaml")
+    region = "us-west-2"
+    layer_utils = LayerUtils(region=region)
+
+    def test_unresolved_layer_due_to_bad_instrinsic(self):
+        command_list = self.get_command_list(
+            "LayerBadInstrinsic",
+            template_path=self.template_path,
+            no_event=True,
+            region=self.region,
+            parameter_overrides={"LayerVersion": "1"},
+        )
+
+        process = Popen(command_list, stderr=PIPE)
+        try:
+            _, stderr = process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
+
+        process_stderr = stderr.strip()
+        error_output = process_stderr.decode("utf-8")
+
+        expected_error_output = "Error: arn:aws:lambda:us-west-2:111111111101:layer:layerDoesNotExist:${LayerVersion} is an Invalid Layer Arn."
+
+        self.assertIn(expected_error_output, error_output)

tests/integration/local/start_lambda/test_start_lambda.py

@@ -192,7 +192,7 @@ def test_lambda_function_raised_error(self):
                 "errorMessage": "Lambda is raising an exception",
                 "errorType": "Exception",
                 "stackTrace": [
-                    '  File "/var/task/main.py", line 49, in raise_exception\n    raise Exception("Lambda is raising an exception")\n'
+                    '  File "/var/task/main.py", line 51, in raise_exception\n    raise Exception("Lambda is raising an exception")\n'
                 ],
             },
         )

tests/integration/testdata/invoke/layers/layer-bad-template.yaml

@@ -0,0 +1,20 @@
+AWSTemplateFormatVersion : '2010-09-09'
+Transform: AWS::Serverless-2016-10-31
+Description: Invalid template with bad layer intrinsic.
+
+Parameters:
+
+  LayerVersion:
+    Default: 1
+    Type: String
+
+Resources:
+  LayerBadInstrinsic:
+    Type: AWS::Serverless::Function
+    Properties:
+      Handler: layer-main.handler
+      Runtime: python3.6
+      CodeUri: .
+      Timeout: 20
+      Layers:
+        - "arn:aws:lambda:us-west-2:111111111101:layer:layerDoesNotExist:${LayerVersion}"

tests/integration/testdata/invoke/main.py

@@ -15,6 +15,8 @@ def handler(event, context):
 
     return "Hello world"
 
+def intrinsics_handler(event, context):
+    return os.environ.get("ApplicationId")
 
 def sleep_handler(event, context):
     time.sleep(10)

tests/integration/testdata/invoke/template-pseudo-params.yaml

@@ -0,0 +1,67 @@
+AWSTemplateFormatVersion: '2010-09-09'
+Transform: AWS::Serverless-2016-10-31
+Description: Run intrinsics and fill in pseudo parameters first, ApplicationIds used are aws sample app arns.
+
+Mappings:
+  MappingExample:
+    eu-west-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    eu-west-2:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    eu-west-3:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    eu-central-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    eu-north-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    us-east-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    us-east-2:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    us-west-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    us-west-2:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    ap-east-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    ap-south-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    ap-northeast-2:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    ap-southeast-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    ap-southeast-2:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    ap-northeast-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    ca-central-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    me-south-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+    sa-east-1:
+      ApplicationId: arn:aws:serverlessrepo:us-east-1:077246666028:applications/hello-world
+
+Resources:
+
+  HelloworldApplication:
+    Type: AWS::Serverless::Application
+    Properties:
+      Location:
+        ApplicationId: !FindInMap [MappingExample, !Ref AWS::Region, ApplicationId]
+        SemanticVersion: 1.0.4
+      Parameters:
+        IdentityNameParameter: TestName
+
+  HelloWorldServerlessFunction:
+    Type: AWS::Serverless::Function
+    Properties:
+      Handler: main.intrinsics_handler
+      Runtime: python3.7
+      CodeUri: .
+      Timeout: 600
+      Environment:
+        Variables:
+          ApplicationId: !FindInMap [MappingExample, !Ref AWS::Region, ApplicationId]
+
+
+

tests/integration/testdata/invoke/template.yml

@@ -22,6 +22,10 @@ Parameters:
     Type: String
     Default: "MyReallyCoolFunction"
 
+  LayerVersion:
+    Type: String
+    Default: "2"
+
 Resources:
   HelloWorldServerlessFunction:
     Type: AWS::Serverless::Function
@@ -188,3 +192,12 @@ Resources:
       Runtime: python3.8
       Layers:
         - arn:aws:lambda:us-east-1:553035198032:layer:git-lambda2:5
+
+  GitLayerFunctionParameters:
+    Type: AWS::Serverless::Function
+    Properties:
+      CodeUri: .
+      Handler: main.execute_git
+      Runtime: python3.8
+      Layers:
+        - !Sub "arn:aws:lambda:us-east-1:553035198032:layer:git-lambda2:${LayerVersion}"

tests/unit/commands/local/lib/test_sam_base_provider.py

@@ -2,6 +2,7 @@
 from unittest.mock import Mock, patch
 from samcli.lib.providers.sam_base_provider import SamBaseProvider
 from samcli.lib.intrinsic_resolver.intrinsic_property_resolver import IntrinsicResolver
+from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable
 
 
 class TestSamBaseProvider_get_template(TestCase):
@@ -19,6 +20,7 @@ def test_must_run_translator_plugins(
         overrides = {"some": "value"}
 
         SamBaseProvider.get_template(template, overrides)
-
-        SamTranslatorWrapperMock.assert_called_once_with(template)
+        called_parameter_values = IntrinsicsSymbolTable.DEFAULT_PSEUDO_PARAM_VALUES.copy()
+        called_parameter_values.update(overrides)
+        SamTranslatorWrapperMock.assert_called_once_with(template, parameter_values=called_parameter_values)
         translator_instance.run_plugins.assert_called_once()