Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support !Ref of template parameters #657

Merged
merged 10 commits into from
Sep 21, 2018
64 changes: 64 additions & 0 deletions samcli/cli/types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
"""
Implementation of custom click parameter types
"""

import re
import click


class CfnParameterOverridesType(click.ParamType):
"""
Custom Click options type to accept values for CloudFormation template parameters. You can pass values for
parameters as "ParameterKey=KeyPairName,ParameterValue=MyKey ParameterKey=InstanceType,ParameterValue=t1.micro"
"""

__EXAMPLE = "ParameterKey=KeyPairName,ParameterValue=MyKey ParameterKey=InstanceType,ParameterValue=t1.micro"

# Regex that parses CloudFormation parameter key-value pairs: https://regex101.com/r/xqfSjW/2
_pattern = r'(?:ParameterKey=([A-Za-z0-9\"]+),ParameterValue=(\"(?:\\.|[^\"\\]+)*\"|(?:\\.|[^ \"\\]+)+))'

name = ''

def convert(self, value, param, ctx):
result = {}
if not value:
return result

groups = re.findall(self._pattern, value)
if not groups:
return self.fail(
"{} is not in valid format. It must look something like '{}'".format(value, self.__EXAMPLE),
param,
ctx
)

# 'groups' variable is a list of tuples ex: [(key1, value1), (key2, value2)]
for key, param_value in groups:
result[self._unquote(key)] = self._unquote(param_value)

return result

@staticmethod
def _unquote(value):
r"""
jfuss marked this conversation as resolved.
Show resolved Hide resolved
Removes wrapping double quotes and any '\ ' characters. They are usually added to preserve spaces when passing
value thru shell.

Ex:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! I will add it..

val\ ue => value
"hel\ lo" => hello

Parameters
----------
value : str
Input to unquote

Returns
-------
Unquoted string
"""
if value and (value[0] == value[-1] == '"'):
jfuss marked this conversation as resolved.
Show resolved Hide resolved
# Remove quotes only if the string is wrapped in quotes
value = value.strip('"')

return value.replace("\\ ", " ").replace('\\"', '"')
21 changes: 19 additions & 2 deletions samcli/commands/local/cli_common/invoke_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def __init__(self,
debug_port=None,
debug_args=None,
debugger_path=None,
aws_region=None):
aws_region=None,
parameter_overrides=None):
"""
Initialize the context

Expand Down Expand Up @@ -85,6 +86,13 @@ def __init__(self,
Additional arguments passed to the debugger
debugger_path str
Path to the directory of the debugger to mount on Docker
aws_profile str
AWS Credential profile to use
aws_region str
AWS region to use
parameter_overrides dict
Values for the template parameters

"""
self._template_file = template_file
self._function_identifier = function_identifier
Expand All @@ -98,6 +106,7 @@ def __init__(self,
self._debug_port = debug_port
self._debug_args = debug_args
self._debugger_path = debugger_path
self._parameter_overrides = parameter_overrides or {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


self._template_dict = None
self._function_provider = None
Expand All @@ -114,7 +123,7 @@ def __enter__(self):

# Grab template from file and create a provider
self._template_dict = self._get_template_data(self._template_file)
self._function_provider = SamFunctionProvider(self._template_dict)
self._function_provider = SamFunctionProvider(self._template_dict, self.parameter_overrides)

self._env_vars_value = self._get_env_vars_value(self._env_vars_file)
self._log_file_handle = self._setup_log_file(self._log_file)
Expand Down Expand Up @@ -248,6 +257,14 @@ def get_cwd(self):

return cwd

@property
def parameter_overrides(self):
# Override certain CloudFormation pseudo-parameters based on values provided by customer
if self._aws_region:
self._parameter_overrides["AWS::Region"] = self._aws_region

return self._parameter_overrides

@staticmethod
def _get_template_data(template_file):
"""
Expand Down
7 changes: 7 additions & 0 deletions samcli/commands/local/cli_common/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import os
import click
from samcli.cli.types import CfnParameterOverridesType

_TEMPLATE_OPTION_DEFAULT_VALUE = "template.[yaml|yml]"

Expand Down Expand Up @@ -103,6 +104,12 @@ def invoke_common_options(f):
type=click.Path(exists=True),
help="JSON file containing values for Lambda function's environment variables."),

click.option("--parameter-overrides",
type=CfnParameterOverridesType(),
help="Optional. A string that contains CloudFormation parameter overrides encoded as key=value "
"pairs. Use the same format as the AWS CLI, e.g. 'ParameterKey=KeyPairName,"
"ParameterValue=MyKey ParameterKey=InstanceType,ParameterValue=t1.micro'"),

click.option('--debug-port', '-d',
help="When specified, Lambda function container will start in debug mode and will expose this "
"port on localhost.",
Expand Down
15 changes: 9 additions & 6 deletions samcli/commands/local/invoke/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,21 @@
@invoke_common_options
@cli_framework_options
@click.argument('function_identifier', required=False)
@pass_context
def cli(ctx, function_identifier, template, event, no_event, env_vars, debug_port, debug_args, debugger_path,
docker_volume_basedir, docker_network, log_file, skip_pull_image, profile, region):
@pass_context # pylint: disable=R0914
def cli(ctx, function_identifier, template, event, no_event, env_vars, debug_port,
debug_args, debugger_path, docker_volume_basedir, docker_network, log_file, skip_pull_image, profile, region,
parameter_overrides):

# All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing

do_cli(ctx, function_identifier, template, event, no_event, env_vars, debug_port, debug_args, debugger_path,
docker_volume_basedir, docker_network, log_file, skip_pull_image, profile, region) # pragma: no cover
docker_volume_basedir, docker_network, log_file, skip_pull_image, profile, region,
parameter_overrides) # pragma: no cover


def do_cli(ctx, function_identifier, template, event, no_event, env_vars, debug_port, # pylint: disable=R0914
debug_args, debugger_path, docker_volume_basedir, docker_network, log_file, skip_pull_image, profile,
region):
region, parameter_overrides):
"""
Implementation of the ``cli`` method, just separated out for unit testing purposes
"""
Expand Down Expand Up @@ -81,7 +83,8 @@ def do_cli(ctx, function_identifier, template, event, no_event, env_vars, debug_
debug_port=debug_port,
debug_args=debug_args,
debugger_path=debugger_path,
aws_region=region) as context:
aws_region=region,
parameter_overrides=parameter_overrides) as context:

# Invoke the function
context.local_lambda_runner.invoke(context.function_name,
Expand Down
4 changes: 3 additions & 1 deletion samcli/commands/local/lib/local_api_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ def __init__(self,
self.static_dir = static_dir

self.cwd = lambda_invoke_context.get_cwd()
self.api_provider = SamApiProvider(lambda_invoke_context.template, cwd=self.cwd)
self.api_provider = SamApiProvider(lambda_invoke_context.template,
parameter_overrides=lambda_invoke_context.parameter_overrides,
cwd=self.cwd)
self.lambda_runner = lambda_invoke_context.local_lambda_runner
self.stderr_stream = lambda_invoke_context.stderr

Expand Down
4 changes: 2 additions & 2 deletions samcli/commands/local/lib/sam_api_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class SamApiProvider(ApiProvider):
"OPTIONS",
"PATCH"]

def __init__(self, template_dict, cwd=None):
def __init__(self, template_dict, parameter_overrides=None, cwd=None):
"""
Initialize the class with SAM template data. The template_dict (SAM Templated) is assumed
to be valid, normalized and a dictionary. template_dict should be normalized by running any and all
Expand All @@ -52,7 +52,7 @@ def __init__(self, template_dict, cwd=None):
Optional working directory with respect to which we will resolve relative path to Swagger file
"""

self.template_dict = SamBaseProvider.get_template(template_dict)
self.template_dict = SamBaseProvider.get_template(template_dict, parameter_overrides)
self.resources = self.template_dict.get("Resources", {})

LOG.debug("%d resources found in the template", len(self.resources))
Expand Down
141 changes: 140 additions & 1 deletion samcli/commands/local/lib/sam_base_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,157 @@
Base class for SAM Template providers
"""

import logging

from samcli.lib.samlib.wrapper import SamTranslatorWrapper
from samtranslator.intrinsics.resolver import IntrinsicsResolver
from samtranslator.intrinsics.actions import RefAction


LOG = logging.getLogger(__name__)


class SamBaseProvider(object):
"""
Base class for SAM Template providers
"""

_DEFAULT_PSEUDO_PARAM_VALUES = {
"AWS::AccountId": "123456789012",
"AWS::Partition": "aws",

# There is not much value in inferring actual AWS region here. These values are just placeholders to help
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

# local testing and not representative of any environment. However, caller of this method can always override
# this value if they choose to.
"AWS::Region": "us-east-1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see an ask for making the Region reflect the --region a customer passes into the cli after we release this. You are claiming this doesn't have much value but we don't know how exactly a customer will be using this.

One example for have Region reflect the --region parameter would be to construct an ARN in the Environment Variables. Yes customers can override this but why should they have to if the Parameter Replacing does exactly what they need to (or assume so). Without the replacement, I would actually expect the Region to be reflected in AWS::Region and would become confused on why my expectation was wrong.

I think we alleviate some developer churn if we just make AWS::Partition, AWS::Region, and partition/region in the AWS::StackId reflect what it should be based on the --region that is passed into the commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Region is possible. Partition is harder because it requires us to maintain a list of region:partition mapping.

I don't want to pass region to the SAM Provider because it doesn't make sense. I will try to find another solution

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StackId is rarely used and partition is usually "aws". So I am going to skip those.


"AWS::StackName": "local",
"AWS::StackId": "arn:aws:cloudformation:us-east-1:123456789012:stack/"
"local/51af3dc0-da77-11e4-872e-1234567db123",
"AWS::URLSuffix": "localhost"
}

# Only Ref is supported when resolving template parameters
_SUPPORTED_INTRINSICS = [RefAction]

@staticmethod
def get_template(template_dict):
def get_template(template_dict, parameter_overrides=None):
"""
Given a SAM template dictionary, return a cleaned copy of the template where SAM plugins have been run
and parameter values have been substituted.

Parameters
----------
template_dict : dict
unprocessed SAM template dictionary

parameter_overrides: dict
Optional dictionary of values for template parameters

Returns
-------
dict
Processed SAM template
"""

template_dict = template_dict or {}
if template_dict:
template_dict = SamTranslatorWrapper(template_dict).run_plugins()

template_dict = SamBaseProvider._resolve_parameters(template_dict, parameter_overrides)
return template_dict

@staticmethod
def _resolve_parameters(template_dict, parameter_overrides):
"""
In the given template, apply parameter values to resolve intrinsic functions

Parameters
----------
template_dict : dict
SAM Template

parameter_overrides : dict
Values for template parameters provided by user

Returns
-------
dict
Resolved SAM template
"""

parameter_values = SamBaseProvider._get_parameter_values(template_dict, parameter_overrides)

supported_intrinsics = {action.intrinsic_name: action() for action in SamBaseProvider._SUPPORTED_INTRINSICS}

# Intrinsics resolver will mutate the original template
return IntrinsicsResolver(parameters=parameter_values, supported_intrinsics=supported_intrinsics)\
.resolve_parameter_refs(template_dict)

@staticmethod
def _get_parameter_values(template_dict, parameter_overrides):
"""
Construct a final list of values for CloudFormation template parameters based on user-supplied values,
default values provided in template, and sane defaults for pseudo-parameters.

Parameters
----------
template_dict : dict
SAM template dictionary

parameter_overrides : dict
User-supplied values for CloudFormation template parameters

Returns
-------
dict
Values for template parameters to substitute in template with
"""

default_values = SamBaseProvider._get_default_parameter_values(template_dict)

# NOTE: Ordering of following statements is important. It makes sure that any user-supplied values
# override the defaults
parameter_values = {}
parameter_values.update(SamBaseProvider._DEFAULT_PSEUDO_PARAM_VALUES)
parameter_values.update(default_values)
parameter_values.update(parameter_overrides or {})

return parameter_values

@staticmethod
def _get_default_parameter_values(sam_template):
"""
Method to read default values for template parameters and return it
Example:
If the template contains the following parameters defined
Parameters:
Param1:
Type: String
Default: default_value1
Param2:
Type: String
Default: default_value2

then, this method will grab default value for Param1 and return the following result:
{
Param1: "default_value1",
Param2: "default_value2"
}
:param dict sam_template: SAM template
:return dict: Default values for parameters
"""

default_values = {}

parameter_definition = sam_template.get("Parameters", None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not default this to{}?

if not parameter_definition or not isinstance(parameter_definition, dict):
LOG.debug("No Parameters detected in the template")
return default_values

for param_name, value in parameter_definition.items():
if isinstance(value, dict) and "Default" in value:
default_values[param_name] = value["Default"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a LOG.info or debug here? It will be helpful when customers submit Issues.

Also, you are doing a dict check here. This is checking for invalid Parameters definition. Can we add some sort of logging to tell the customer it either invalid or we are skipping for X reason. This will help customers understand what is going on and help them debug their issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add debug logs


LOG.debug("Collected default values for parameters: %s", default_values)
return default_values
6 changes: 4 additions & 2 deletions samcli/commands/local/lib/sam_function_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class SamFunctionProvider(FunctionProvider):
_LAMBDA_FUNCTION = "AWS::Lambda::Function"
_DEFAULT_CODEURI = "."

def __init__(self, template_dict):
def __init__(self, template_dict, parameter_overrides=None):
"""
Initialize the class with SAM template data. The SAM template passed to this provider is assumed
to be valid, normalized and a dictionary. It should be normalized by running all pre-processing
Expand All @@ -35,9 +35,11 @@ def __init__(self, template_dict):
You need to explicitly update the class with new template, if necessary.

:param dict template_dict: SAM Template as a dictionary
:param dict parameter_overrides: Optional dictionary of values for SAM template parameters that might want
to get substituted within the template
"""

self.template_dict = SamBaseProvider.get_template(template_dict)
self.template_dict = SamBaseProvider.get_template(template_dict, parameter_overrides)
self.resources = self.template_dict.get("Resources", {})

LOG.debug("%d resources found in the template", len(self.resources))
Expand Down
Loading