Skip to content

Tests for samconfig use with all commands #1575

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.

Sign up for GitHub

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

Merged
sriram-mv merged 9 commits into from
Nov 22, 2019
Merged

Tests for samconfig use with all commands #1575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
51b907c
test: Verify samconfig is accessible to all CLI commands
sanathkr Nov 22, 2019
88ad704
fix cases where comma vs space needs to be delimiter
sanathkr Nov 22, 2019
5f43d51
adding few more unit tests
sanathkr Nov 22, 2019
cdf59b6
adding unit tests for all commands
sanathkr Nov 22, 2019
9c2ddae
fix linter
sanathkr Nov 22, 2019
1632b82
Adding tests for overriding args thru config, CLI args, and envvars
sanathkr Nov 22, 2019
cf6fcd8
Merge branch 'release-v0.32.0' into samconfig-integtest
sanathkr Nov 22, 2019
8fcd4ed
Fixing a minor UX issue when sam template is invalid
sanathkr Nov 22, 2019
026861d
fixing mock imports
sanathkr Nov 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions samcli/cli/cli_config_file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,16 @@ def __call__(self, config_dir, config_env, cmd_names):
# NOTE(TheSriram): change from tomlkit table type to normal dictionary,
# so that click defaults work out of the box.
resolved_config = {k: v for k, v in samconfig.get_all(cmd_names, self.section, env=config_env).items()}
LOG.debug("Configuration values read from the file: %s", resolved_config)

except KeyError:
except KeyError as ex:
LOG.debug(
"Error reading configuration file at %s with config_env=%s, command=%s, section=%s",
"Error reading configuration file at %s with config_env=%s, command=%s, section=%s %s",
samconfig.path(),
config_env,
cmd_names,
self.section,
str(ex),
)
except Exception as ex:
LOG.debug("Error reading configuration file: %s %s", samconfig.path(), str(ex))
Expand Down Expand Up @@ -123,6 +125,16 @@ def configuration_option(*param_decls, **attrs):
"""
Adds configuration file support to a click application.

NOTE: This decorator should be added to the top of parameter chain, right below click.command, before
any options are declared.

Example:
>>> @click.command("hello")
@configuration_option(provider=TomlProvider(section="parameters"))
@click.option('--name', type=click.String)
def hello(name):
print("Hello " + name)

This will create an option of type `STRING` expecting the config_env in the
configuration file, by default this config_env is `default`. When specified,
the requisite portion of the configuration file is considered as the
Expand Down
27 changes: 17 additions & 10 deletions samcli/cli/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@
import click


def _value_regex(delim):
return f'(\\"(?:\\\\.|[^\\"\\\\]+)*\\"|(?:\\\\.|[^{delim}\\"\\\\]+)+)'


KEY_REGEX = '([A-Za-z0-9\\"]+)'
Copy link
Contributor

@sriram-mv sriram-mv Nov 22, 2019

Choose a reason for hiding this comment

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

Testing these regexes, will report back with results.

# Use this regex when you have space as delimiter Ex: "KeyName1=string KeyName2=string"
VALUE_REGEX_SPACE_DELIM = _value_regex(" ")
# Use this regex when you have comma as delimiter Ex: "KeyName1=string,KeyName2=string"
VALUE_REGEX_COMMA_DELIM = _value_regex(",")


class CfnParameterOverridesType(click.ParamType):
"""
Custom Click options type to accept values for CloudFormation template parameters. You can pass values for
Expand All @@ -25,11 +36,8 @@ class CfnParameterOverridesType(click.ParamType):
# If Both ParameterKey pattern and KeyPairName=MyKey should not be present
# while adding parameter overrides, if they are, it
# can result in unpredicatable behavior.
KEY_REGEX = '([A-Za-z0-9\\"]+)'
VALUE_REGEX = '(\\"(?:\\\\.|[^\\"\\\\]+)*\\"|(?:\\\\.|[^ \\"\\\\]+)+))'

_pattern_1 = r"(?:ParameterKey={key},ParameterValue={value}".format(key=KEY_REGEX, value=VALUE_REGEX)
_pattern_2 = r"(?:(?: ){key}={value}".format(key=KEY_REGEX, value=VALUE_REGEX)
_pattern_1 = r"(?:ParameterKey={key},ParameterValue={value})".format(key=KEY_REGEX, value=VALUE_REGEX_SPACE_DELIM)
Copy link
Contributor

@sriram-mv sriram-mv Nov 22, 2019

Choose a reason for hiding this comment

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

Ok, did some testing.
Things we may need to make a note of:
_pattern_1 works well when the entire string that is compared with the regex is under quotes.

Works:

"ParameterKey=Key,ParameterValue=Value" Groups: Key, Value
"ParameterKey=Key,ParameterValue=Value Groups: Key, Value
ParameterKey=Key,ParameterValue=Value" Groups: Key, Value
Doesnt work:

ParameterKey=Key,ParameterValue="Value" No Groups
ParameterKey="Key",ParameterValue="Value" No Groups
"ParameterKey="Key",ParameterValue="Value"" No Groups

_pattern_2 = r"(?:(?: ){key}={value})".format(key=KEY_REGEX, value=VALUE_REGEX_SPACE_DELIM)
Copy link
Contributor

@sriram-mv sriram-mv Nov 22, 2019

Choose a reason for hiding this comment

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

Works well, Gotchas with quotes.

Works
dd=gfbfg asdf=dadsf asdasd=qwe Groups (dd,gfbfg) (asdf,dadsf) (asdasd, qwe)
" "dd"='gfbfg'" Groups ("dd",'gfbfg')

Doesn't work

' "dd"="gfbfg" "asdf"="dadsf" "asdasd"="qwed"' No Groups

Copy link
Contributor Author

@sanathkr sanathkr Nov 22, 2019

Choose a reason for hiding this comment

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

I am not worried about the key not supporting quotes. that's out of scope. Also, value using single quotes is also out-of-scope.


ordered_pattern_match = [_pattern_1, _pattern_2]

Expand Down Expand Up @@ -114,7 +122,7 @@ class CfnMetadataType(click.ParamType):

_EXAMPLE = 'KeyName1=string,KeyName2=string or {"string":"string"}'

_pattern = r"([A-Za-z0-9\"]+)=([A-Za-z0-9\"]+)"
_pattern = r"(?:{key}={value})".format(key=KEY_REGEX, value=VALUE_REGEX_COMMA_DELIM)

# NOTE(TheSriram): name needs to be added to click.ParamType requires it.
name = ""
Expand Down Expand Up @@ -160,7 +168,7 @@ class CfnTags(click.ParamType):

_EXAMPLE = "KeyName1=string KeyName2=string"

_pattern = r"([A-Za-z0-9\"]+)=([A-Za-z0-9\"]+)"
_pattern = r"{key}={value}".format(key=KEY_REGEX, value=VALUE_REGEX_SPACE_DELIM)

# NOTE(TheSriram): name needs to be added to click.ParamType requires it.
name = ""
Expand All @@ -172,11 +180,10 @@ def convert(self, value, param, ctx):
if value == ("",):
return result

# if value comes in a via configuration file, we should still convert it.
# value = (value, ) if not isinstance(value, tuple) else value
# if value comes in a via configuration file, it will be a string. So we should still convert it.
value = (value,) if not isinstance(value, tuple) else value

for val in value:

groups = re.findall(self._pattern, val)

if not groups:
Expand Down
195 changes: 116 additions & 79 deletions samcli/commands/build/command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
import logging
import click

from samcli.commands._utils.options import template_option_without_build, docker_common_options, \
parameter_override_option
from samcli.commands._utils.options import (
template_option_without_build,
docker_common_options,
parameter_override_option,
)
from samcli.cli.main import pass_context, common_options as cli_framework_options, aws_creds_options
from samcli.lib.telemetry.metrics import track_command
from samcli.cli.cli_config_file import configuration_option, TomlProvider
Expand Down Expand Up @@ -54,73 +57,103 @@
"""


@configuration_option(provider=TomlProvider(section="parameters"))
@click.command("build", help=HELP_TEXT, short_help="Build your Lambda function code")
@click.option('--build-dir', '-b',
default=DEFAULT_BUILD_DIR,
type=click.Path(file_okay=False, dir_okay=True, writable=True), # Must be a directory
help="Path to a folder where the built artifacts will be stored. This directory will be first removed before starting a build.")
@click.option("--base-dir", "-s",
default=None,
type=click.Path(dir_okay=True, file_okay=False), # Must be a directory
help="Resolve relative paths to function's source code with respect to this folder. Use this if "
"SAM template and your source code are not in same enclosing folder. By default, relative paths "
"are resolved with respect to the SAM template's location")
@click.option("--use-container", "-u",
is_flag=True,
help="If your functions depend on packages that have natively compiled dependencies, use this flag "
"to build your function inside an AWS Lambda-like Docker container")
@click.option("--manifest", "-m",
default=None,
type=click.Path(),
help="Path to a custom dependency manifest (ex: package.json) to use instead of the default one")
@configuration_option(provider=TomlProvider(section="parameters"))
@click.option(
"--build-dir",
"-b",
default=DEFAULT_BUILD_DIR,
type=click.Path(file_okay=False, dir_okay=True, writable=True), # Must be a directory
help="Path to a folder where the built artifacts will be stored. This directory will be first removed before starting a build.",
)
@click.option(
"--base-dir",
"-s",
default=None,
type=click.Path(dir_okay=True, file_okay=False), # Must be a directory
help="Resolve relative paths to function's source code with respect to this folder. Use this if "
"SAM template and your source code are not in same enclosing folder. By default, relative paths "
"are resolved with respect to the SAM template's location",
)
@click.option(
"--use-container",
"-u",
is_flag=True,
help="If your functions depend on packages that have natively compiled dependencies, use this flag "
"to build your function inside an AWS Lambda-like Docker container",
)
@click.option(
"--manifest",
"-m",
default=None,
type=click.Path(),
help="Path to a custom dependency manifest (ex: package.json) to use instead of the default one",
)
@template_option_without_build
@parameter_override_option
@docker_common_options
@cli_framework_options
@aws_creds_options
@click.argument('function_identifier', required=False)
@click.argument("function_identifier", required=False)
@pass_context
@track_command
def cli(ctx,
def cli(
ctx,
function_identifier,
template_file,
base_dir,
build_dir,
use_container,
manifest,
docker_network,
skip_pull_image,
parameter_overrides,
):
# All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing

mode = _get_mode_value_from_envvar("SAM_BUILD_MODE", choices=["debug"])

do_cli(
function_identifier,
template_file,
base_dir,
build_dir,
True,
use_container,
manifest,
docker_network,
skip_pull_image,
parameter_overrides,
):
# All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing

mode = _get_mode_value_from_envvar("SAM_BUILD_MODE", choices=["debug"])

do_cli(function_identifier, template_file, base_dir, build_dir, True, use_container, manifest, docker_network,
skip_pull_image, parameter_overrides, mode) # pragma: no cover


def do_cli(function_identifier, # pylint: disable=too-many-locals, too-many-statements
template,
base_dir,
build_dir,
clean,
use_container,
manifest_path,
docker_network,
skip_pull_image,
parameter_overrides,
mode):
mode,
) # pragma: no cover


def do_cli( # pylint: disable=too-many-locals, too-many-statements
function_identifier,
template,
base_dir,
build_dir,
clean,
use_container,
manifest_path,
docker_network,
skip_pull_image,
parameter_overrides,
mode,
):
"""
Implementation of the ``cli`` method
"""

from samcli.commands.exceptions import UserException

from samcli.commands.build.build_context import BuildContext
from samcli.lib.build.app_builder import ApplicationBuilder, BuildError, UnsupportedBuilderLibraryVersionError, \
ContainerBuildNotSupported
from samcli.lib.build.app_builder import (
ApplicationBuilder,
BuildError,
UnsupportedBuilderLibraryVersionError,
ContainerBuildNotSupported,
)
from samcli.lib.build.workflow_config import UnsupportedRuntimeException
from samcli.local.lambdafn.exceptions import FunctionNotFound
from samcli.commands._utils.template import move_template
Expand All @@ -130,36 +163,36 @@ def do_cli(function_identifier, # pylint: disable=too-many-locals, too-many-sta
if use_container:
LOG.info("Starting Build inside a container")

with BuildContext(function_identifier,
template,
base_dir,
build_dir,
clean=clean,
manifest_path=manifest_path,
use_container=use_container,
parameter_overrides=parameter_overrides,
docker_network=docker_network,
skip_pull_image=skip_pull_image,
mode=mode) as ctx:
with BuildContext(
function_identifier,
template,
base_dir,
build_dir,
clean=clean,
manifest_path=manifest_path,
use_container=use_container,
parameter_overrides=parameter_overrides,
docker_network=docker_network,
skip_pull_image=skip_pull_image,
mode=mode,
) as ctx:
try:
builder = ApplicationBuilder(ctx.functions_to_build,
ctx.build_dir,
ctx.base_dir,
manifest_path_override=ctx.manifest_path_override,
container_manager=ctx.container_manager,
mode=ctx.mode)
builder = ApplicationBuilder(
ctx.functions_to_build,
ctx.build_dir,
ctx.base_dir,
manifest_path_override=ctx.manifest_path_override,
container_manager=ctx.container_manager,
mode=ctx.mode,
)
except FunctionNotFound as ex:
raise UserException(str(ex))

try:
artifacts = builder.build()
modified_template = builder.update_template(ctx.template_dict,
ctx.original_template_path,
artifacts)
modified_template = builder.update_template(ctx.template_dict, ctx.original_template_path, artifacts)

move_template(ctx.original_template_path,
ctx.output_template_path,
modified_template)
move_template(ctx.original_template_path, ctx.output_template_path, modified_template)

click.secho("\nBuild Succeeded", fg="green")

Expand All @@ -174,14 +207,20 @@ def do_cli(function_identifier, # pylint: disable=too-many-locals, too-many-sta
build_dir_in_success_message = ctx.build_dir
output_template_path_in_success_message = ctx.output_template_path

msg = gen_success_msg(build_dir_in_success_message,
output_template_path_in_success_message,
os.path.abspath(ctx.build_dir) == os.path.abspath(DEFAULT_BUILD_DIR))
msg = gen_success_msg(
build_dir_in_success_message,
output_template_path_in_success_message,
os.path.abspath(ctx.build_dir) == os.path.abspath(DEFAULT_BUILD_DIR),
)

click.secho(msg, fg="yellow")

except (UnsupportedRuntimeException, BuildError, UnsupportedBuilderLibraryVersionError,
ContainerBuildNotSupported) as ex:
except (
UnsupportedRuntimeException,
BuildError,
UnsupportedBuilderLibraryVersionError,
ContainerBuildNotSupported,
) as ex:
click.secho("\nBuild Failed", fg="red")
raise UserException(str(ex))

Expand All @@ -203,10 +242,9 @@ def gen_success_msg(artifacts_dir, output_template_path, is_default_build_dir):
=========================
[*] Invoke Function: {invokecmd}
[*] Deploy: {deploycmd}
""".format(invokecmd=invoke_cmd,
deploycmd=deploy_cmd,
artifacts_dir=artifacts_dir,
template=output_template_path)
""".format(
invokecmd=invoke_cmd, deploycmd=deploy_cmd, artifacts_dir=artifacts_dir, template=output_template_path
)

return msg

Expand All @@ -218,7 +256,6 @@ def _get_mode_value_from_envvar(name, choices):
return None

if mode not in choices:
raise click.UsageError("Invalid value for 'mode': invalid choice: {}. (choose from {})"
.format(mode, choices))
raise click.UsageError("Invalid value for 'mode': invalid choice: {}. (choose from {})".format(mode, choices))

return mode
Loading