Tests for samconfig use with all commands

samcli/cli/cli_config_file.py

@@ -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))
@@ -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

samcli/cli/types.py

@@ -9,6 +9,17 @@
 import click
 
 
+def _value_regex(delim):
+    return f'(\\"(?:\\\\.|[^\\"\\\\]+)*\\"|(?:\\\\.|[^{delim}\\"\\\\]+)+)'
+
+
+KEY_REGEX = '([A-Za-z0-9\\"]+)'
+# 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
@@ -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)
+    _pattern_2 = r"(?:(?: ){key}={value})".format(key=KEY_REGEX, value=VALUE_REGEX_SPACE_DELIM)
 
     ordered_pattern_match = [_pattern_1, _pattern_2]
 
@@ -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 = ""
@@ -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 = ""
@@ -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:

samcli/commands/build/command.py

@@ -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
@@ -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
@@ -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")
 
@@ -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))
 
@@ -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
 
@@ -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