Support !Ref of template parameters by sanathkr · Pull Request #657 · aws/aws-sam-cli

sanathkr · 2018-09-07T15:29:19Z

Issue #, if available:
#573, #572, #528, #490

Description of changes:
Still very much a work in progress. This PR supports resolving !Ref of template parameters anywhere within a SAM template. It does not yet support other intrinsic functions like !Sub or !FindInMap.

Customers can provide values for template parameters using --parameter-overrides CLI option
Automatically picks up default values of parameters from template
Assigns sane defaults for CFN pseudo-parameters like AWS::Region. This can also be overridden using --parameter-overrides CLI option

Remaining:

Docstrings
Unit tests
Integration test for start-api and start-lambda
Linter fixes

Thanks to @jfuss for providing the initial version of implementation. This PR is a cleaned up version of Jacob's implementation.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

- Provide values for template parameters - Automatically picks up default values - Assigns sane defaults for CFN pseudo-parameters like AWS::Region

jfuss · 2018-09-14T12:50:15Z

            Additional arguments passed to the debugger
        debugger_path str
            Path to the directory of the debugger to mount on Docker
+        aws_profile


Can you add the type to aws_profile and aws_region.

jfuss · 2018-09-14T12:50:39Z

+            AWS Credential profile to use
+        aws_region
+            AWS region to use
+        parameter_overrides : dict


Nit: remove the : to remain consistent with the other docstrings.

jfuss · 2018-09-14T12:52:12Z


+        click.option("--parameter-overrides",
+                     type=CfnParameterOverridesType(),
+                     help="Optional. A string that contains CloudFormation parameter overrides encoded as key-value "


Should this be key=value instead of key-value? You have an example but keeping this consistent will help communicate what we are expecting.

jfuss · 2018-09-14T13:17:50Z

        error_output = process_stderr.decode('utf-8')
        self.assertIn("no_event and event cannot be used together. Please provide only one.", error_output)
+
+    def test_invoke_with_timeout_set_by_parameters(self):


This is almost identical to the 'test_invoke_with_timeout_set' test. Can we parameterize them into one test? This will be help as we build out the tests more to cover different cases.

good idea. will do

jfuss · 2018-09-14T13:21:09Z

+        )
+
+    ])
+    def test_successful_parsing(self, input, expected):


jfuss · 2018-09-14T13:21:49Z

+    ])
+    def test_successful_parsing(self, input, expected):
+        result = self.param_type.convert(input, None, None)
+        print(input)


remove prints from tests.

jfuss · 2018-09-19T16:13:50Z

+        "AWS::AccountId": "123456789012",
+        "AWS::Partition": "aws",
+
+        # There is not much value in inferring actual AWS region here. These values are just placeholders to help


Can you update this comment, to reflect the AWS::Region override change.

jfuss · 2018-09-19T16:20:18Z

+
+        default_values = {}
+
+        parameter_definition = sam_template.get("Parameters", None)


Why not default this to{}?

sriram-mv · 2018-09-21T17:45:02Z

+        Removes wrapping double quotes and any '\ ' characters. They are usually added to preserve spaces when passing
+        value thru shell.
+
+        Ex:


just a nit:

Making it look like in the docstring, looks nice in the editor, because it interprets it as a python prompt. https://numpydoc.readthedocs.io/en/latest/format.html#docstring-standard

Examples ---------- >>> _unquote('val\ ue') value

Good idea! I will add it..

sriram-mv · 2018-09-21T17:51:16Z

        self._debug_port = debug_port
        self._debug_args = debug_args
        self._debugger_path = debugger_path
+        self._parameter_overrides = parameter_overrides or {}


We set this to be None by default everywhere else, should we do that here too?

Yes it is set as None everywhere else, but as a good practice I set to empty dictionary if customers don't specify any value.

alytle · 2018-09-21T19:46:56Z

Yay for this one. Just started using SAM, and this is a blocker. Thanks for adding it!

@thesriram

Approved by @thesriram

webwizrd · 2019-11-04T13:12:47Z

I have the following issue with !Ref:
My template.yaml contains 2 Lambda functions that each !Ref a globally defined parameter in their individual environments section:

Parameters:
  GlobalClientId:
    Type: String
    Description: Issued client ID

Resources:
AuthorizeFunction:
    Type: AWS::Serverless::Function
    Properties:
    (...)
   Environment:
        Variables:
          CLIENT_ID: !Ref GlobalClientId
CallbackFunction:
    Type: AWS::Serverless::Function
    Properties:
    (...)
   Environment:
        Variables:
          CLIENT_ID: !Ref GlobalClientId

When executing via sam local and --env-vars .env with the .env file being:

{
    "AuthorizeFunction": 
        {
        "CLIENT_ID":"ABCD" 
        },
   "CallbackFunction": 
        {
        "CLIENT_ID":"ABCD" 
        }
}

only the first Function receives the value in the .env file (process.env.CLIENT_ID=="ABCD").
The second Function receives the referenced parameter name as a value !? (process.env.CLIENT_ID=="GlobalClientId").

Am I using this wrong?

sanathkr added 2 commits September 7, 2018 08:19

WIP: Support !Ref of template parameters

98eba88

- Provide values for template parameters - Automatically picks up default values - Assigns sane defaults for CFN pseudo-parameters like AWS::Region

Add parameter_overrides to start-lambda

c70c9f6

sanathkr mentioned this pull request Sep 7, 2018

Local SSM Parameter Store #616

Closed

sanathkr added 4 commits September 7, 2018 13:55

Unit test for some parts of parameter substitution

28a050a

Unit test for methods doing ref subsitution

f1acff4

linter fixes

fe310be

Merge branch 'develop' into parameter-substitution

be35ca4

jfuss previously requested changes Sep 14, 2018

View reviewed changes

addressing PR comments

474ace1

jfuss reviewed Sep 19, 2018

View reviewed changes

Comment explaining why pseudo params don't have real values

642eb78

sanathkr changed the title ~~WIP: Support !Ref of template parameters~~ Sep 21, 2018

sriram-mv reviewed Sep 21, 2018

View reviewed changes

sanathkr added 2 commits September 21, 2018 11:02

Updated examples

c5fa2fd

Merge branch 'develop' into parameter-substitution

7eccccc

sriram-mv approved these changes Sep 21, 2018

View reviewed changes

sanathkr merged commit a48238c into aws:develop Sep 21, 2018

jfuss mentioned this pull request Oct 3, 2018

Environment variables ignored/don't get set with referenced Parameters default values. #450

Closed

Tanbouz mentioned this pull request Oct 3, 2018

Mutliple API stages but in a single document - best practice aws/serverless-application-model#198

Closed

jfuss mentioned this pull request Oct 18, 2018

Support for --parameter-values in 0.3.0 #456

Closed

sriram-mv mentioned this pull request Oct 19, 2018

Release: 0.6.1 Release #720

Merged

jfuss mentioned this pull request Oct 24, 2018

Global variables are undefined when running local invoke #380

Closed

mnapoli mentioned this pull request Feb 8, 2019

Missing !Sub and !If instrinsic functions #528

Closed

Navigation Menu

Uh oh!

Support !Ref of template parameters - #657

Support !Ref of template parameters#657
sanathkr merged 10 commits into
aws:developfrom
sanathkr:parameter-substitution

sanathkr commented Sep 7, 2018 •

edited

Loading

Uh oh!

Uh oh!

jfuss Sep 14, 2018

sanathkr Sep 19, 2018

jfuss Sep 14, 2018

jfuss Sep 14, 2018

jfuss Sep 14, 2018

sanathkr Sep 19, 2018

Uh oh!

jfuss Sep 14, 2018

Uh oh!

jfuss Sep 14, 2018

jfuss Sep 19, 2018

jfuss Sep 19, 2018

sriram-mv Sep 21, 2018

sanathkr Sep 21, 2018

sriram-mv Sep 21, 2018

sanathkr Sep 21, 2018

alytle commented Sep 21, 2018

webwizrd commented Nov 4, 2019

Labels

5 participants


		default_values = {}

		parameter_definition = sam_template.get("Parameters", None)

Uh oh!

Conversation

sanathkr commented Sep 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alytle commented Sep 21, 2018

webwizrd commented Nov 4, 2019

Labels

5 participants

sanathkr commented Sep 7, 2018 •

edited

Loading