diff --git a/samcli/commands/_utils/command_exception_handler.py b/samcli/commands/_utils/command_exception_handler.py new file mode 100644 index 0000000000..c4da732b34 --- /dev/null +++ b/samcli/commands/_utils/command_exception_handler.py @@ -0,0 +1,70 @@ +""" +Contains method decorator which can be used to convert common exceptions into click exceptions +which will end exeecution gracefully +""" +from functools import wraps +from typing import Callable, Dict, Any, Optional + +from botocore.exceptions import NoRegionError, ClientError + +from samcli.commands._utils.options import parameterized_option +from samcli.commands.exceptions import CredentialsError, RegionError +from samcli.lib.utils.boto_utils import get_client_error_code + + +@parameterized_option +def command_exception_handler(f, additional_mapping: Optional[Dict[Any, Callable[[Any], None]]] = None): + """ + This function returns a wrapped function definition, which handles configured exceptions gracefully + """ + + def decorator_command_exception_handler(func): + @wraps(func) + def wrapper_command_exception_handler(*args, **kwargs): + try: + return func(*args, **kwargs) + except Exception as ex: + exception_type = type(ex) + + # check if there is a custom handling defined + exception_handler = (additional_mapping or {}).get(exception_type) + if exception_handler: + exception_handler(ex) + + # if no custom handling defined search for default handlers + exception_handler = COMMON_EXCEPTION_HANDLER_MAPPING.get(exception_type) + if exception_handler: + exception_handler(ex) + + # if no handler defined, raise the exception + raise ex + + return wrapper_command_exception_handler + + return decorator_command_exception_handler(f) + + +def _handle_no_region_error(ex: NoRegionError) -> None: + raise RegionError( + "No region information found. Please provide --region parameter or configure default region settings. " + "\nFor more information please visit https://docs.aws.amazon.com/sdk-for-java/v1/developer-guide/" + "setup-credentials.html#setup-credentials-setting-region" + ) + + +def _handle_client_errors(ex: ClientError) -> None: + error_code = get_client_error_code(ex) + + if error_code in ("ExpiredToken", "ExpiredTokenException"): + raise CredentialsError( + "Your credential configuration is invalid or has expired token value. \nFor more information please " + "visit: https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html" + ) + + raise ex + + +COMMON_EXCEPTION_HANDLER_MAPPING: Dict[Any, Callable] = { + NoRegionError: _handle_no_region_error, + ClientError: _handle_client_errors, +} diff --git a/samcli/commands/_utils/experimental.py b/samcli/commands/_utils/experimental.py index 1f238f160e..51dcfb44d0 100644 --- a/samcli/commands/_utils/experimental.py +++ b/samcli/commands/_utils/experimental.py @@ -240,7 +240,7 @@ def prompt_experimental( if is_experimental_enabled(config_entry): update_experimental_context() return True - confirmed = click.confirm(prompt, default=False) + confirmed = click.confirm(Colored().yellow(prompt), default=False) if confirmed: set_experimental(config_entry=config_entry, enabled=True) update_experimental_context() diff --git a/samcli/commands/deploy/deploy_context.py b/samcli/commands/deploy/deploy_context.py index 25d6068944..b96374288b 100644 --- a/samcli/commands/deploy/deploy_context.py +++ b/samcli/commands/deploy/deploy_context.py @@ -285,7 +285,7 @@ def deploy( s3_uploader=s3_uploader, tags=tags, ) - LOG.info(result) + LOG.debug(result) except deploy_exceptions.DeployFailedError as ex: LOG.error(str(ex)) diff --git a/samcli/commands/exceptions.py b/samcli/commands/exceptions.py index 10a8dbf337..07229d5063 100644 --- a/samcli/commands/exceptions.py +++ b/samcli/commands/exceptions.py @@ -90,3 +90,9 @@ class InvalidImageException(UserException): """ Value provided to --build-image or --invoke-image is invalid URI """ + + +class InvalidStackNameException(UserException): + """ + Value provided to --stack-name is invalid + """ diff --git a/samcli/commands/logs/command.py b/samcli/commands/logs/command.py index ca3d01f9b9..71a228df2f 100644 --- a/samcli/commands/logs/command.py +++ b/samcli/commands/logs/command.py @@ -8,15 +8,20 @@ from samcli.cli.cli_config_file import configuration_option, TomlProvider from samcli.cli.main import pass_context, common_options as cli_framework_options, aws_creds_options, print_cmdline_args -from samcli.commands._utils.options import common_observability_options -from samcli.lib.telemetry.metric import track_command -from samcli.lib.utils.version_checker import check_newer_version from samcli.commands._utils.experimental import ( ExperimentalFlag, force_experimental_option, experimental, prompt_experimental, ) +from samcli.commands._utils.options import common_observability_options +from samcli.commands.logs.validation_and_exception_handlers import ( + SAM_LOGS_ADDITIONAL_EXCEPTION_HANDLERS, + stack_name_cw_log_group_validation, +) +from samcli.lib.telemetry.metric import track_command +from samcli.commands._utils.command_exception_handler import command_exception_handler +from samcli.lib.utils.version_checker import check_newer_version LOG = logging.getLogger(__name__) @@ -97,6 +102,8 @@ @force_experimental_option("include_traces", config_entry=ExperimentalFlag.Accelerate) # pylint: disable=E1120 @force_experimental_option("cw_log_group", config_entry=ExperimentalFlag.Accelerate) # pylint: disable=E1120 @force_experimental_option("output", config_entry=ExperimentalFlag.Accelerate) # pylint: disable=E1120 +@command_exception_handler(SAM_LOGS_ADDITIONAL_EXCEPTION_HANDLERS) +@stack_name_cw_log_group_validation def cli( ctx, name, @@ -169,7 +176,9 @@ def do_cli( boto_client_provider = get_boto_client_provider_with_config(region=region, profile=profile) boto_resource_provider = get_boto_resource_provider_with_config(region=region, profile=profile) - resource_logical_id_resolver = ResourcePhysicalIdResolver(boto_resource_provider, stack_name, names) + resource_logical_id_resolver = ResourcePhysicalIdResolver( + boto_resource_provider, boto_client_provider, stack_name, names + ) # only fetch all resources when no CloudWatch log group defined fetch_all_when_no_resource_name_given = not cw_log_groups diff --git a/samcli/commands/logs/logs_context.py b/samcli/commands/logs/logs_context.py index 7dea7b51d1..46bffab3f2 100644 --- a/samcli/commands/logs/logs_context.py +++ b/samcli/commands/logs/logs_context.py @@ -73,11 +73,13 @@ class ResourcePhysicalIdResolver: def __init__( self, boto_resource_provider: BotoProviderType, + boto_client_provider: BotoProviderType, stack_name: str, resource_names: Optional[List[str]] = None, supported_resource_types: Optional[Set[str]] = None, ): self._boto_resource_provider = boto_resource_provider + self._boto_client_provider = boto_client_provider self._stack_name = stack_name if resource_names is None: resource_names = [] @@ -126,7 +128,10 @@ def _fetch_resources_from_stack( """ LOG.debug("Getting logical id of the all resources for stack '%s'", self._stack_name) stack_resources = get_resource_summaries( - self._boto_resource_provider, self._stack_name, ResourcePhysicalIdResolver.DEFAULT_SUPPORTED_RESOURCES + self._boto_resource_provider, + self._boto_client_provider, + self._stack_name, + ResourcePhysicalIdResolver.DEFAULT_SUPPORTED_RESOURCES, ) if selected_resource_names: @@ -161,4 +166,10 @@ def _get_selected_resources( selected_resource = resource_summaries.get(selected_resource_name) if selected_resource: resources.append(selected_resource) + else: + LOG.warning( + "Resource name (%s) does not exist. Available resource names: %s", + selected_resource_name, + ", ".join(resource_summaries.keys()), + ) return resources diff --git a/samcli/commands/logs/puller_factory.py b/samcli/commands/logs/puller_factory.py index 3cd5246d1e..09e745aff8 100644 --- a/samcli/commands/logs/puller_factory.py +++ b/samcli/commands/logs/puller_factory.py @@ -5,6 +5,8 @@ import logging from typing import List, Optional +from botocore.exceptions import ClientError + from samcli.commands.exceptions import UserException from samcli.commands.logs.console_consumers import CWConsoleEventConsumer from samcli.commands.traces.traces_puller_factory import generate_trace_puller @@ -25,7 +27,7 @@ ObservabilityCombinedPuller, ) from samcli.lib.observability.util import OutputOption -from samcli.lib.utils.boto_utils import BotoProviderType +from samcli.lib.utils.boto_utils import BotoProviderType, get_client_error_code from samcli.lib.utils.cloudformation import CloudFormationResourceSummary from samcli.lib.utils.colors import Colored @@ -101,9 +103,11 @@ def generate_puller( # populate puller instances for the additional CloudWatch log groups for cw_log_group in additional_cw_log_groups: consumer = generate_consumer(filter_pattern, output) + logs_client = boto_client_provider("logs") + _validate_cw_log_group_name(cw_log_group, logs_client) pullers.append( CWLogPuller( - boto_client_provider("logs"), + logs_client, consumer, cw_log_group, ) @@ -122,6 +126,14 @@ def generate_puller( return ObservabilityCombinedPuller(pullers) +def _validate_cw_log_group_name(cw_log_group, logs_client): + try: + _ = logs_client.describe_log_streams(logGroupName=cw_log_group, limit=1) + except ClientError as ex: + if get_client_error_code(ex) == "ResourceNotFoundException": + LOG.warning("CloudWatch log group name (%s) does not exist.", cw_log_group) + + def generate_consumer( filter_pattern: Optional[str] = None, output: OutputOption = OutputOption.text, resource_name: Optional[str] = None ): diff --git a/samcli/commands/logs/validation_and_exception_handlers.py b/samcli/commands/logs/validation_and_exception_handlers.py new file mode 100644 index 0000000000..edf3224802 --- /dev/null +++ b/samcli/commands/logs/validation_and_exception_handlers.py @@ -0,0 +1,68 @@ +""" +Contains helper functions for validation and exception handling of "sam logs" command +""" +from functools import wraps +from typing import Dict, Any, Callable + +import click +from botocore.exceptions import ClientError +from click import Context, BadOptionUsage + +from samcli.commands.exceptions import InvalidStackNameException +from samcli.lib.utils.boto_utils import get_client_error_code + + +def stack_name_cw_log_group_validation(func): + """ + Wrapper Validation function that will run last after the all cli parmaters have been loaded + to check for conditions surrounding `--stack-name` and `--cw-log-group`. The + reason they are done last instead of in callback functions, is because the options depend + on each other, and this breaks cyclic dependencies. + + :param func: Click command function + :return: Click command function after validation + """ + + @wraps(func) + def wrapped(*args, **kwargs): + ctx = click.get_current_context() + stack_name = ctx.params.get("stack_name") + cw_log_groups = ctx.params.get("cw_log_group") + names = ctx.params.get("name") + + # if --name is provided --stack-name should be provided as well + if names and not stack_name: + raise BadOptionUsage( + option_name="--stack-name", + ctx=ctx, + message="Missing option. Please provide '--stack-name' when using '--name' option", + ) + + # either --stack-name or --cw-log-group flags should be provided + if not stack_name and not cw_log_groups: + raise BadOptionUsage( + option_name="--stack-name", + ctx=ctx, + message="Missing option. Please provide '--stack-name' or '--cw-log-group'", + ) + + return func(*args, **kwargs) + + return wrapped + + +def _handle_client_error(ex: ClientError) -> None: + """ + Handles client error which was caused by ListStackResources event + """ + operation_name = ex.operation_name + client_error_code = get_client_error_code(ex) + if client_error_code == "ValidationError" and operation_name == "ListStackResources": + click_context: Context = click.get_current_context() + stack_name_value = click_context.params.get("stack_name") + raise InvalidStackNameException( + f"Invalid --stack-name parameter. Stack with id '{stack_name_value}' does not exist" + ) + + +SAM_LOGS_ADDITIONAL_EXCEPTION_HANDLERS: Dict[Any, Callable] = {ClientError: _handle_client_error} diff --git a/samcli/commands/sync/command.py b/samcli/commands/sync/command.py index 415c41758a..5d0e9cc3be 100644 --- a/samcli/commands/sync/command.py +++ b/samcli/commands/sync/command.py @@ -242,18 +242,6 @@ def do_cli( from samcli.commands.package.package_context import PackageContext from samcli.commands.deploy.deploy_context import DeployContext - s3_bucket = manage_stack(profile=profile, region=region) - click.echo(f"\n\t\tManaged S3 bucket: {s3_bucket}") - - click.echo(f"\n\t\tDefault capabilities applied: {DEFAULT_CAPABILITIES}") - click.echo("To override with customized capabilities, use --capabilities flag or set it in samconfig.toml") - - build_dir = DEFAULT_BUILD_DIR_WITH_AUTO_DEPENDENCY_LAYER if dependency_layer else DEFAULT_BUILD_DIR - LOG.debug("Using build directory as %s", build_dir) - - build_dir = DEFAULT_BUILD_DIR_WITH_AUTO_DEPENDENCY_LAYER if dependency_layer else DEFAULT_BUILD_DIR - LOG.debug("Using build directory as %s", build_dir) - confirmation_text = SYNC_CONFIRMATION_TEXT if not is_experimental_enabled(ExperimentalFlag.Accelerate): @@ -265,6 +253,11 @@ def do_cli( set_experimental(ExperimentalFlag.Accelerate) update_experimental_context() + s3_bucket = manage_stack(profile=profile, region=region) + + build_dir = DEFAULT_BUILD_DIR_WITH_AUTO_DEPENDENCY_LAYER if dependency_layer else DEFAULT_BUILD_DIR + LOG.debug("Using build directory as %s", build_dir) + with BuildContext( resource_identifier=None, template_file=template_file, diff --git a/samcli/commands/traces/command.py b/samcli/commands/traces/command.py index f014d2320b..f5542582e6 100644 --- a/samcli/commands/traces/command.py +++ b/samcli/commands/traces/command.py @@ -7,6 +7,7 @@ from samcli.cli.cli_config_file import configuration_option, TomlProvider from samcli.cli.main import pass_context, common_options as cli_framework_options, aws_creds_options, print_cmdline_args +from samcli.commands._utils.command_exception_handler import command_exception_handler from samcli.commands._utils.options import common_observability_options from samcli.lib.observability.util import OutputOption from samcli.lib.telemetry.metric import track_command @@ -36,6 +37,7 @@ @track_command @check_newer_version @print_cmdline_args +@command_exception_handler def cli( ctx, trace_id, diff --git a/samcli/lib/build/build_graph.py b/samcli/lib/build/build_graph.py index 1a3b8e28b2..ffd51f4959 100644 --- a/samcli/lib/build/build_graph.py +++ b/samcli/lib/build/build_graph.py @@ -6,6 +6,7 @@ import logging import os import threading +from abc import abstractmethod from pathlib import Path from typing import Sequence, Tuple, List, Any, Optional, Dict, cast, NamedTuple from copy import deepcopy @@ -501,6 +502,10 @@ def dependencies_dir(self) -> str: def env_vars(self) -> Dict: return deepcopy(self._env_vars) + @abstractmethod + def get_resource_full_paths(self) -> str: + """Returns string representation of resources' full path information for this build definition""" + class LayerBuildDefinition(AbstractBuildDefinition): """ @@ -527,6 +532,12 @@ def __init__( # this and move "layer" out of LayerBuildDefinition to take advantage of type check. self.layer: LayerVersion = None # type: ignore + def get_resource_full_paths(self) -> str: + if not self.layer: + LOG.debug("LayerBuildDefinition with uuid (%s) doesn't have a layer assigned to it", self.uuid) + return "" + return self.layer.full_path + def __str__(self) -> str: return ( f"LayerBuildDefinition({self.full_path}, {self.codeuri}, {self.source_hash}, {self.uuid}, " @@ -616,6 +627,10 @@ def get_build_dir(self, artifact_root_dir: str) -> str: self._validate_functions() return self.functions[0].get_build_dir(artifact_root_dir) + def get_resource_full_paths(self) -> str: + """Returns list of functions' full path information as a list of str""" + return ", ".join([function.full_path for function in self.functions]) + def _validate_functions(self) -> None: if not self.functions: raise InvalidBuildGraphException("Build definition doesn't have any function definition to build") diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py index 2af7e16f57..c41ffd964a 100644 --- a/samcli/lib/build/build_strategy.py +++ b/samcli/lib/build/build_strategy.py @@ -139,7 +139,7 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini build_definition.runtime, build_definition.metadata, build_definition.architecture, - [function.full_path for function in build_definition.functions], + build_definition.get_resource_full_paths(), ) # build into one of the functions from this build definition @@ -259,8 +259,8 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini if not cache_function_dir.exists() or build_definition.source_hash != source_hash: LOG.info( - "Cache is invalid, running build and copying resources to function build definition of %s", - build_definition.uuid, + "Cache is invalid, running build and copying resources for following functions (%s)", + build_definition.get_resource_full_paths(), ) build_result = self._delegate_build_strategy.build_single_function_definition(build_definition) function_build_results.update(build_result) @@ -275,8 +275,8 @@ def build_single_function_definition(self, build_definition: FunctionBuildDefini break else: LOG.info( - "Valid cache found, copying previously built resources from function build definition of %s", - build_definition.uuid, + "Valid cache found, copying previously built resources for following functions (%s)", + build_definition.get_resource_full_paths(), ) for function in build_definition.functions: # artifacts directory will be created by the builder @@ -298,8 +298,8 @@ def build_single_layer_definition(self, layer_definition: LayerBuildDefinition) if not cache_function_dir.exists() or layer_definition.source_hash != source_hash: LOG.info( - "Cache is invalid, running build and copying resources to layer build definition of %s", - layer_definition.uuid, + "Cache is invalid, running build and copying resources for following layers (%s)", + layer_definition.get_resource_full_paths(), ) build_result = self._delegate_build_strategy.build_single_layer_definition(layer_definition) layer_build_result.update(build_result) @@ -314,8 +314,8 @@ def build_single_layer_definition(self, layer_definition: LayerBuildDefinition) break else: LOG.info( - "Valid cache found, copying previously built resources from layer build definition of %s", - layer_definition.uuid, + "Valid cache found, copying previously built resources for following layers (%s)", + layer_definition.get_resource_full_paths(), ) # artifacts directory will be created by the builder artifacts_dir = str(pathlib.Path(self._build_dir, layer_definition.layer.full_path)) @@ -446,14 +446,17 @@ def _check_whether_manifest_is_changed( if is_manifest_changed or is_dependencies_dir_missing: build_definition.manifest_hash = manifest_hash LOG.info( - "Manifest file is changed (new hash: %s) or dependency folder (%s) is missing for %s, " + "Manifest file is changed (new hash: %s) or dependency folder (%s) is missing for (%s), " "downloading dependencies and copying/building source", manifest_hash, build_definition.dependencies_dir, - build_definition.uuid, + build_definition.get_resource_full_paths(), ) else: - LOG.info("Manifest is not changed for %s, running incremental build", build_definition.uuid) + LOG.info( + "Manifest is not changed for (%s), running incremental build", + build_definition.get_resource_full_paths(), + ) build_definition.download_dependencies = is_manifest_changed or is_dependencies_dir_missing @@ -513,32 +516,32 @@ def build(self) -> Dict[str, str]: def build_single_function_definition(self, build_definition: FunctionBuildDefinition) -> Dict[str, str]: if self._is_incremental_build_supported(build_definition.runtime): LOG.debug( - "Running incremental build for runtime %s for build definition %s", + "Running incremental build for runtime %s for following resources (%s)", build_definition.runtime, - build_definition.uuid, + build_definition.get_resource_full_paths(), ) return self._incremental_build_strategy.build_single_function_definition(build_definition) LOG.debug( - "Running incremental build for runtime %s for build definition %s", + "Running incremental build for runtime %s for following resources (%s)", build_definition.runtime, - build_definition.uuid, + build_definition.get_resource_full_paths(), ) return self._cached_build_strategy.build_single_function_definition(build_definition) def build_single_layer_definition(self, layer_definition: LayerBuildDefinition) -> Dict[str, str]: if self._is_incremental_build_supported(layer_definition.build_method): LOG.debug( - "Running incremental build for runtime %s for build definition %s", + "Running incremental build for runtime %s for following resources (%s)", layer_definition.build_method, - layer_definition.uuid, + layer_definition.get_resource_full_paths(), ) return self._incremental_build_strategy.build_single_layer_definition(layer_definition) LOG.debug( - "Running cached build for runtime %s for build definition %s", + "Running cached build for runtime %s for following resources (%s)", layer_definition.build_method, - layer_definition.uuid, + layer_definition.get_resource_full_paths, ) return self._cached_build_strategy.build_single_layer_definition(layer_definition) diff --git a/samcli/lib/observability/cw_logs/cw_log_group_provider.py b/samcli/lib/observability/cw_logs/cw_log_group_provider.py index 537c06d50f..d42d4d17fc 100644 --- a/samcli/lib/observability/cw_logs/cw_log_group_provider.py +++ b/samcli/lib/observability/cw_logs/cw_log_group_provider.py @@ -155,6 +155,6 @@ def for_step_functions( step_function_name, log_group_arn, ) - LOG.warning("Logging is not configured for StepFunctions (%s)") + LOG.warning("Logging is not configured for StepFunctions (%s)", step_function_name) return None diff --git a/samcli/lib/sync/sync_flow_factory.py b/samcli/lib/sync/sync_flow_factory.py index fe1c252003..cb613f9d99 100644 --- a/samcli/lib/sync/sync_flow_factory.py +++ b/samcli/lib/sync/sync_flow_factory.py @@ -16,7 +16,7 @@ from samcli.lib.sync.flows.rest_api_sync_flow import RestApiSyncFlow from samcli.lib.sync.flows.http_api_sync_flow import HttpApiSyncFlow from samcli.lib.sync.flows.stepfunctions_sync_flow import StepFunctionsSyncFlow -from samcli.lib.utils.boto_utils import get_boto_resource_provider_with_config +from samcli.lib.utils.boto_utils import get_boto_resource_provider_with_config, get_boto_client_provider_with_config from samcli.lib.utils.cloudformation import get_resource_summaries from samcli.lib.utils.resources import ( AWS_SERVERLESS_FUNCTION, @@ -74,12 +74,17 @@ def __init__( def load_physical_id_mapping(self) -> None: """Load physical IDs of the stack resources from remote""" LOG.debug("Loading physical ID mapping") - provider = get_boto_resource_provider_with_config( + resource_provider = get_boto_resource_provider_with_config( + region=self._deploy_context.region, profile=self._deploy_context.profile + ) + client_provider = get_boto_client_provider_with_config( region=self._deploy_context.region, profile=self._deploy_context.profile ) resource_mapping = get_resource_summaries( - boto_resource_provider=provider, stack_name=self._deploy_context.stack_name + boto_resource_provider=resource_provider, + boto_client_provider=client_provider, + stack_name=self._deploy_context.stack_name, ) # get the resource_id -> physical_id mapping diff --git a/samcli/lib/utils/boto_utils.py b/samcli/lib/utils/boto_utils.py index 887d8df323..eab922f7dd 100644 --- a/samcli/lib/utils/boto_utils.py +++ b/samcli/lib/utils/boto_utils.py @@ -4,9 +4,9 @@ from typing import Any, Optional from boto3 import Session -from typing_extensions import Protocol - from botocore.config import Config +from botocore.exceptions import ClientError +from typing_extensions import Protocol from samcli import __version__ from samcli.cli.global_config import GlobalConfig @@ -38,7 +38,7 @@ def get_boto_config_with_user_agent(**kwargs) -> Config: # Type definition of following boto providers, which is equal to Callable[[str], Any] class BotoProviderType(Protocol): def __call__(self, service_name: str) -> Any: - ... + ... # pragma: no cover def get_boto_client_provider_from_session_with_config(session: Session, **kwargs) -> BotoProviderType: @@ -135,3 +135,8 @@ def get_boto_resource_provider_with_config( return get_boto_resource_provider_from_session_with_config( Session(region_name=region, profile_name=profile), **kwargs ) + + +def get_client_error_code(client_error: ClientError) -> Optional[str]: + """Extracts error code from boto ClientError""" + return client_error.response.get("Error", {}).get("Code") diff --git a/samcli/lib/utils/cloudformation.py b/samcli/lib/utils/cloudformation.py index 25a85458f6..c75f3d8220 100644 --- a/samcli/lib/utils/cloudformation.py +++ b/samcli/lib/utils/cloudformation.py @@ -3,17 +3,38 @@ """ import logging import posixpath -from typing import Dict, Set, Optional +from typing import Dict, Set, Optional, Iterable, Any from attr import dataclass from botocore.exceptions import ClientError -from samcli.lib.utils.boto_utils import BotoProviderType +from samcli.lib.utils.boto_utils import BotoProviderType, get_client_error_code from samcli.lib.utils.resources import AWS_CLOUDFORMATION_STACK LOG = logging.getLogger(__name__) +# list of possible values for active stacks +# CFN console has a way to display active stacks but it is not possible in API calls +STACK_ACTIVE_STATUS = [ + "CREATE_IN_PROGRESS", + "CREATE_COMPLETE", + "ROLLBACK_IN_PROGRESS", + "ROLLBACK_FAILED", + "ROLLBACK_COMPLETE", + "DELETE_IN_PROGRESS", + "DELETE_FAILED", + "UPDATE_IN_PROGRESS", + "UPDATE_COMPLETE_CLEANUP_IN_PROGRESS", + "UPDATE_COMPLETE", + "UPDATE_ROLLBACK_IN_PROGRESS", + "UPDATE_ROLLBACK_FAILED", + "UPDATE_ROLLBACK_COMPLETE_CLEANUP_IN_PROGRESS", + "UPDATE_ROLLBACK_COMPLETE", + "REVIEW_IN_PROGRESS", +] + + @dataclass class CloudFormationResourceSummary: """ @@ -27,6 +48,7 @@ class CloudFormationResourceSummary: def get_resource_summaries( boto_resource_provider: BotoProviderType, + boto_client_provider: BotoProviderType, stack_name: str, resource_types: Optional[Set[str]] = None, nested_stack_prefix: Optional[str] = None, @@ -38,6 +60,8 @@ def get_resource_summaries( ---------- boto_resource_provider : BotoProviderType A callable which will return boto3 resource + boto_client_provider : BotoProviderType + A callable which will return boto3 client stack_name : str Name of the stack which is deployed to CFN resource_types : Optional[Set[str]] @@ -52,7 +76,18 @@ def get_resource_summaries( """ LOG.debug("Fetching stack (%s) resources", stack_name) - cfn_resource_summaries = boto_resource_provider("cloudformation").Stack(stack_name).resource_summaries.all() + try: + cfn_resource_summaries = list( + boto_resource_provider("cloudformation").Stack(stack_name).resource_summaries.all() + ) + except ClientError as ex: + if get_client_error_code(ex) == "ValidationError" and LOG.isEnabledFor(logging.DEBUG): + LOG.debug( + "Invalid stack name (%s). Available stack names: %s", + stack_name, + ", ".join(list_active_stack_names(boto_client_provider)), + ) + raise ex resource_summaries: Dict[str, CloudFormationResourceSummary] = {} for cfn_resource_summary in cfn_resource_summaries: @@ -68,6 +103,7 @@ def get_resource_summaries( resource_summaries.update( get_resource_summaries( boto_resource_provider, + boto_client_provider, resource_summary.physical_resource_id, resource_types, new_nested_stack_prefix, @@ -90,7 +126,9 @@ def get_resource_summaries( return resource_summaries -def get_resource_summary(boto_resource_provider: BotoProviderType, stack_name: str, resource_logical_id: str): +def get_resource_summary( + boto_resource_provider: BotoProviderType, stack_name: str, resource_logical_id: str +) -> Optional[CloudFormationResourceSummary]: """ Returns resource summary of given single resource with its logical id @@ -120,3 +158,35 @@ def get_resource_summary(boto_resource_provider: BotoProviderType, stack_name: s "Failed to pull resource (%s) information from stack (%s)", resource_logical_id, stack_name, exc_info=e ) return None + + +def list_active_stack_names(boto_client_provider: BotoProviderType, show_nested_stacks: bool = False) -> Iterable[str]: + """ + Returns list of active cloudformation stack names + + Parameters + ---------- + boto_client_provider : BotoProviderType + A callable which will return boto3 client + show_nested_stacks : bool + True; will display nested stack names as well. False; will hide nested stack names from the list. + + Returns + ------- + Iterable[str] List of stack names that is currently active + """ + cfn_client = boto_client_provider("cloudformation") + first_call = True + next_token: Optional[str] = None + + while first_call or next_token: + first_call = False + kwargs: Dict[str, Any] = {"StackStatusFilter": STACK_ACTIVE_STATUS} + if next_token: + kwargs["NextToken"] = next_token + list_stacks_result = cfn_client.list_stacks(**kwargs) + for stack_summary in list_stacks_result.get("StackSummaries", []): + if not show_nested_stacks and stack_summary.get("RootId"): + continue + yield stack_summary.get("StackName") + next_token = list_stacks_result.get("NextToken") diff --git a/tests/integration/buildcmd/build_integ_base.py b/tests/integration/buildcmd/build_integ_base.py index b15de58a9c..ea1db08554 100644 --- a/tests/integration/buildcmd/build_integ_base.py +++ b/tests/integration/buildcmd/build_integ_base.py @@ -816,7 +816,7 @@ def _verify_process_code_and_output(self, command_result, function_full_paths): for function_full_path in function_full_paths: self.assertRegex( command_result.stderr.decode("utf-8"), - f"Building codeuri: .* runtime: .* metadata: .* functions: \\[.*'{function_full_path}'.*\\]", + f"Building codeuri: .* runtime: .* metadata: .* functions: .*{function_full_path}.*", ) def _verify_invoke_built_functions(self, template_path, overrides, function_and_expected): diff --git a/tests/integration/buildcmd/test_build_cmd.py b/tests/integration/buildcmd/test_build_cmd.py index d21cb9c600..0d29fde5e6 100644 --- a/tests/integration/buildcmd/test_build_cmd.py +++ b/tests/integration/buildcmd/test_build_cmd.py @@ -1654,7 +1654,7 @@ def test_no_cached_override_build(self): cmdlist.extend(["--config-file", config_file]) command_result = run_command(cmdlist, cwd=self.working_dir) self.assertTrue( - "Valid cache found, copying previously built resources from function build definition of" + "Valid cache found, copying previously built resources for following functions" in str(command_result.stderr), "Should have built using cache", ) @@ -1996,7 +1996,7 @@ def test_nested_build(self, use_container, cached, parallel): command_result = run_command(cmdlist, cwd=self.working_dir) # make sure functions are deduplicated properly, in stderr they will show up in the same line. - self.assertRegex(command_result.stderr.decode("utf-8"), r"Building .+'Function2',.+LocalNestedStack/Function2") + self.assertRegex(command_result.stderr.decode("utf-8"), r"Building .+Function2,.+LocalNestedStack/Function2") function_full_paths = ["Function", "Function2", "LocalNestedStack/Function1", "LocalNestedStack/Function2"] stack_paths = ["", "LocalNestedStack"] diff --git a/tests/integration/telemetry/test_experimental_metric.py b/tests/integration/telemetry/test_experimental_metric.py index 4d9a4f2fee..36111ddf3a 100644 --- a/tests/integration/telemetry/test_experimental_metric.py +++ b/tests/integration/telemetry/test_experimental_metric.py @@ -81,7 +81,7 @@ def test_must_send_experimental_metrics_if_experimental_option(self): process = self.run_cmd(cmd_list=[self.cmd, "logs", "--include-traces"], optout_envvar_value="1") process.communicate() - self.assertEqual(process.returncode, 1, "Command should fail") + self.assertEqual(process.returncode, 2, "Command should fail") all_requests = server.get_all_requests() self.assertEqual(1, len(all_requests), "Command run metric must be sent") request = all_requests[0] @@ -188,7 +188,7 @@ def test_must_send_not_experimental_metrics_if_not_experimental(self): process = self.run_cmd(cmd_list=[self.cmd, "logs", "--name", "abc"], optout_envvar_value="1") process.communicate() - self.assertEqual(process.returncode, 1, "Command should fail") + self.assertEqual(process.returncode, 2, "Command should fail") all_requests = server.get_all_requests() self.assertEqual(1, len(all_requests), "Command run metric must be sent") request = all_requests[0] diff --git a/tests/unit/commands/_utils/test_command_exception_handler.py b/tests/unit/commands/_utils/test_command_exception_handler.py new file mode 100644 index 0000000000..f206de5e98 --- /dev/null +++ b/tests/unit/commands/_utils/test_command_exception_handler.py @@ -0,0 +1,80 @@ +from typing import Callable +from unittest import TestCase + +from botocore.exceptions import NoRegionError, ClientError +from parameterized import parameterized + +from samcli.commands._utils.command_exception_handler import command_exception_handler +from samcli.commands.exceptions import RegionError, CredentialsError, UserException + + +@command_exception_handler +def echo_command(proxy_function: Callable): + return proxy_function() + + +class UnhandledException(Exception): + pass + + +class TestCommandExceptionHandler(TestCase): + def test_no_exception(self): + self.assertEqual(echo_command(lambda: 5), 5) + + def test_no_region_error(self): + def _proxy_function_that_raises_region_error(): + raise NoRegionError() + + with self.assertRaises(RegionError): + echo_command(_proxy_function_that_raises_region_error) + + @parameterized.expand([("ExpiredToken",), ("ExpiredTokenException",)]) + def test_expired_token_error(self, error_code): + def _proxy_function_that_raises_expired_token(): + raise ClientError({"Error": {"Code": error_code}}, "mock") + + with self.assertRaises(CredentialsError): + echo_command(_proxy_function_that_raises_expired_token) + + def test_unhandled_client_error(self): + client_error = ClientError({"Error": {"Code": "UnhandledCode"}}, "mock") + + def _proxy_function_that_raises_unhandled_client_error(): + raise client_error + + with self.assertRaises(ClientError) as ex: + echo_command(_proxy_function_that_raises_unhandled_client_error) + self.assertEqual(client_error, ex) + + def test_unhandled_exception(self): + def _proxy_function_that_raises_unhandled_exception(): + raise UnhandledException() + + with self.assertRaises(UnhandledException): + echo_command(_proxy_function_that_raises_unhandled_exception) + + +class CustomException(Exception): + pass + + +class CustomUserException(UserException): + pass + + +def _custom_handler(ex: CustomException): + raise CustomUserException("Error") + + +@command_exception_handler({CustomException: _custom_handler}) +def command_with_custom_exception_handler(proxy_function: Callable): + proxy_function() + + +class TestCommandExceptionHandlerWithCustomHandler(TestCase): + def test_custom_exception(self): + def _proxy_custom_exception(): + raise CustomException() + + with self.assertRaises(CustomUserException): + command_with_custom_exception_handler(_proxy_custom_exception) diff --git a/tests/unit/commands/_utils/test_experimental.py b/tests/unit/commands/_utils/test_experimental.py index 4e20daaaf3..67709912cf 100644 --- a/tests/unit/commands/_utils/test_experimental.py +++ b/tests/unit/commands/_utils/test_experimental.py @@ -15,6 +15,7 @@ ExperimentalEntry, ExperimentalFlag, ) +from samcli.lib.utils.colors import Colored class TestExperimental(TestCase): @@ -118,5 +119,5 @@ def test_prompt_experimental(self, update_experimental_context, enabled_mock, co prompt_experimental(config_entry, prompt) set_experimental_mock.assert_called_once_with(config_entry=config_entry, enabled=True) enabled_mock.assert_called_once_with(config_entry) - confirm_mock.assert_called_once_with(prompt, default=False) + confirm_mock.assert_called_once_with(Colored().yellow(prompt), default=False) update_experimental_context.assert_called_once() diff --git a/tests/unit/commands/logs/test_command.py b/tests/unit/commands/logs/test_command.py index ac65365674..d2e1d6c0d8 100644 --- a/tests/unit/commands/logs/test_command.py +++ b/tests/unit/commands/logs/test_command.py @@ -1,10 +1,13 @@ import itertools from unittest import TestCase -from unittest.mock import Mock, patch, call, ANY +from unittest.mock import Mock, patch, call +import pytest +from botocore.exceptions import ClientError +from click.testing import CliRunner from parameterized import parameterized -from samcli.commands.logs.command import do_cli +from samcli.commands.logs.command import do_cli, cli from samcli.lib.observability.util import OutputOption @@ -88,7 +91,7 @@ def test_logs_command( patched_boto_resource_provider.assert_called_with(region=self.region, profile=self.profile) patched_resource_physical_id_resolver.assert_called_with( - mocked_resource_provider, self.stack_name, self.function_name + mocked_resource_provider, mocked_client_provider, self.stack_name, self.function_name ) fetch_param = not bool(len(cw_log_group)) @@ -109,3 +112,64 @@ def test_logs_command( mocked_puller.assert_has_calls( [call.load_time_period(mocked_start_time, mocked_end_time, self.filter_pattern)] ) + + def test_without_stack_name_or_cw_log_group( + self, patched_is_experimental_enabled, patched_update_experimental_context + ): + cli_runner = CliRunner() + result = cli_runner.invoke(cli, []) + self.assertIn("Please provide '--stack-name' or '--cw-log-group'", result.output) + + @patch("samcli.commands.logs.logs_context.ResourcePhysicalIdResolver.get_resource_information") + @patch("samcli.commands.logs.puller_factory.generate_puller") + def test_with_stack_name_but_without_cw_log_group_should_succeed( + self, + patched_generate_puller, + patched_get_resource_information, + patched_is_experimental_enabled, + patched_update_experimental_context, + ): + cli_runner = CliRunner() + cli_runner.invoke(cli, ["--stack-name", "abcdef"]) + patched_get_resource_information.assert_called_with(True) + patched_generate_puller.assert_called_once() + + @patch("samcli.commands.logs.logs_context.ResourcePhysicalIdResolver.get_resource_information") + @patch("samcli.commands.logs.puller_factory.generate_puller") + def test_with_cw_log_group_but_without_stack_name_should_succeed( + self, + patched_generate_puller, + patched_get_resource_information, + patched_is_experimental_enabled, + patched_update_experimental_context, + ): + cli_runner = CliRunner() + cli_runner.invoke(cli, ["--cw-log-group", "abcdef"]) + patched_get_resource_information.assert_called_with(False) + patched_generate_puller.assert_called_once() + + def test_with_name_but_without_stack_name_should_fail( + self, patched_is_experimental_enabled, patched_update_experimental_context + ): + cli_runner = CliRunner() + result = cli_runner.invoke(cli, ["--name", "abcdef"]) + self.assertIn("Missing option. Please provide '--stack-name' when using '--name' option", result.output) + + @pytest.fixture(autouse=True) + def inject_fixtures(self, caplog): + self._caplog = caplog + + @patch("samcli.commands.logs.logs_context.ResourcePhysicalIdResolver.get_resource_information") + def test_invalid_stack_name_should_fail( + self, patched_get_resource_information, patched_is_experimental_enabled, patched_update_experimental_context + ): + patched_get_resource_information.side_effect = ClientError( + {"Error": {"Code": "ValidationError"}}, "ListStackResources" + ) + self._caplog.set_level(100000) + cli_runner = CliRunner() + invalid_stack_name = "my-invalid-stack-name" + result = cli_runner.invoke(cli, ["--stack-name", invalid_stack_name, "--region", "us-west-2"]) + self.assertIn( + f"Invalid --stack-name parameter. Stack with id '{invalid_stack_name}' does not exist", result.output + ) diff --git a/tests/unit/commands/logs/test_logs_context.py b/tests/unit/commands/logs/test_logs_context.py index be5e6304d8..61e8f5052f 100644 --- a/tests/unit/commands/logs/test_logs_context.py +++ b/tests/unit/commands/logs/test_logs_context.py @@ -54,7 +54,7 @@ def test_parse_time_empty_time(self): class TestResourcePhysicalIdResolver(TestCase): def test_get_resource_information_with_resources(self): - resource_physical_id_resolver = ResourcePhysicalIdResolver(Mock(), "stack_name", ["resource_name"]) + resource_physical_id_resolver = ResourcePhysicalIdResolver(Mock(), Mock(), "stack_name", ["resource_name"]) with mock.patch( "samcli.commands.logs.logs_context.ResourcePhysicalIdResolver._fetch_resources_from_stack" ) as mocked_fetch: diff --git a/tests/unit/lib/build_module/test_build_strategy.py b/tests/unit/lib/build_module/test_build_strategy.py index 48023dcdd6..a2e6ae6dd7 100644 --- a/tests/unit/lib/build_module/test_build_strategy.py +++ b/tests/unit/lib/build_module/test_build_strategy.py @@ -32,15 +32,15 @@ def setUp(self): self.function1_1 = Mock() self.function1_1.inlinecode = None self.function1_1.get_build_dir = Mock() - self.function1_1.full_path = Mock() + self.function1_1.full_path = "function1_1" self.function1_2 = Mock() self.function1_2.inlinecode = None self.function1_2.get_build_dir = Mock() - self.function1_2.full_path = Mock() + self.function1_2.full_path = "function1_2" self.function2 = Mock() self.function2.inlinecode = None self.function2.get_build_dir = Mock() - self.function2.full_path = Mock() + self.function2.full_path = "function2" self.function_build_definition1 = FunctionBuildDefinition("runtime", "codeuri", ZIP, X86_64, {}, "handler") self.function_build_definition2 = FunctionBuildDefinition("runtime2", "codeuri", ZIP, X86_64, {}, "handler") @@ -563,7 +563,7 @@ def test_will_call_incremental_build_strategy( ): patched_experimental.return_value = experimental_enabled build_definition = FunctionBuildDefinition(runtime, "codeuri", "packate_type", X86_64, {}, "handler") - self.build_graph.put_function_build_definition(build_definition, Mock()) + self.build_graph.put_function_build_definition(build_definition, Mock(full_path="function_full_path")) with patch.object( self.build_strategy, "_incremental_build_strategy" ) as patched_incremental_build_strategy, patch.object( @@ -587,7 +587,7 @@ def test_will_call_incremental_build_strategy( ) def test_will_call_cached_build_strategy(self, mocked_read, mocked_write, runtime): build_definition = FunctionBuildDefinition(runtime, "codeuri", "packate_type", X86_64, {}, "handler") - self.build_graph.put_function_build_definition(build_definition, Mock()) + self.build_graph.put_function_build_definition(build_definition, Mock(full_path="function_full_path")) with patch.object( self.build_strategy, "_incremental_build_strategy" ) as patched_incremental_build_strategy, patch.object( diff --git a/tests/unit/lib/package/test_uploaders.py b/tests/unit/lib/package/test_uploaders.py new file mode 100644 index 0000000000..754a85d89c --- /dev/null +++ b/tests/unit/lib/package/test_uploaders.py @@ -0,0 +1,26 @@ +from unittest import TestCase +from unittest.mock import Mock + +from parameterized import parameterized + +from samcli.lib.package.uploaders import Destination, Uploaders + + +class TestUploaders(TestCase): + @parameterized.expand([(Destination.S3,), (Destination.ECR,), (None,)]) + def test_uploader_get(self, destination): + ecr_uploader = Mock() + s3_uploader = Mock() + + uploaders = Uploaders(s3_uploader, ecr_uploader) + + if not destination: + with self.assertRaises(ValueError): + uploaders.get(destination) + elif destination == Destination.S3: + self.assertEqual(uploaders.get(destination), s3_uploader) + elif destination == Destination.ECR: + self.assertEqual(uploaders.get(destination), ecr_uploader) + + self.assertEqual(s3_uploader, uploaders.s3) + self.assertEqual(ecr_uploader, uploaders.ecr) diff --git a/tests/unit/lib/sync/test_sync_flow_factory.py b/tests/unit/lib/sync/test_sync_flow_factory.py index ea6625b66a..c7d86b3a03 100644 --- a/tests/unit/lib/sync/test_sync_flow_factory.py +++ b/tests/unit/lib/sync/test_sync_flow_factory.py @@ -31,7 +31,10 @@ def create_factory(self, auto_dependency_layer: bool = False): @patch("samcli.lib.sync.sync_flow_factory.get_resource_summaries") @patch("samcli.lib.sync.sync_flow_factory.get_boto_resource_provider_with_config") - def test_load_physical_id_mapping(self, get_boto_resource_provider_mock, get_resource_summaries_mock): + @patch("samcli.lib.sync.sync_flow_factory.get_boto_client_provider_with_config") + def test_load_physical_id_mapping( + self, get_boto_client_provider_mock, get_boto_resource_provider_mock, get_resource_summaries_mock + ): resource_summary_1 = CloudFormationResourceSummary( resource_type="", logical_resource_id="", physical_resource_id="PhysicalResource1" ) diff --git a/tests/unit/lib/utils/test_boto_utils.py b/tests/unit/lib/utils/test_boto_utils.py index 9cde1d3fb0..626d9d1ab2 100644 --- a/tests/unit/lib/utils/test_boto_utils.py +++ b/tests/unit/lib/utils/test_boto_utils.py @@ -9,6 +9,7 @@ get_boto_resource_provider_with_config, get_boto_resource_provider_from_session_with_config, get_boto_client_provider_from_session_with_config, + get_client_error_code, ) TEST_VERSION = "1.0.0" @@ -121,3 +122,7 @@ def test_get_boto_resource_provider_from_session_with_config(self, patched_get_c self.assertEqual(resource, given_resource) patched_get_config.assert_called_with(param=given_config_param) given_session.resource.assert_called_with(given_resource_name, config=given_config) + + @parameterized.expand([({}, None), ({"Error": {}}, None), ({"Error": {"Code": "ErrorCode"}}, "ErrorCode")]) + def test_get_client_error_code(self, response, expected): + self.assertEqual(expected, get_client_error_code(Mock(response=response))) diff --git a/tests/unit/lib/utils/test_cloudformation.py b/tests/unit/lib/utils/test_cloudformation.py index 7bddd47710..ea52adf6b4 100644 --- a/tests/unit/lib/utils/test_cloudformation.py +++ b/tests/unit/lib/utils/test_cloudformation.py @@ -7,6 +7,7 @@ CloudFormationResourceSummary, get_resource_summaries, get_resource_summary, + list_active_stack_names, ) from samcli.lib.utils.resources import AWS_CLOUDFORMATION_STACK @@ -31,6 +32,7 @@ def test_cfn_resource_summary(self): class TestCloudformationUtils(TestCase): def test_get_resource_summaries(self): resource_provider_mock = Mock() + client_provider_mock = Mock() given_stack_name = "stack_name" given_resource_types = {"ResourceType0"} @@ -68,7 +70,9 @@ def test_get_resource_summaries(self): given_nested_stack_resource_array, ] - resource_summaries = get_resource_summaries(resource_provider_mock, given_stack_name, given_resource_types) + resource_summaries = get_resource_summaries( + resource_provider_mock, client_provider_mock, given_stack_name, given_resource_types + ) self.assertEqual(len(resource_summaries), 4) self.assertEqual( @@ -127,3 +131,47 @@ def test_get_resource_summary_fail(self): resource_summary = get_resource_summary(resource_provider_mock, given_stack_name, given_resource_logical_id) self.assertIsNone(resource_summary) + + @patch("samcli.lib.utils.cloudformation.LOG") + @patch("samcli.lib.utils.cloudformation.list_active_stack_names") + def test_get_resource_summaries_invalid_stack(self, patched_list_active_stack_names, patched_log): + resource_provider_mock = Mock() + client_provider_mock = Mock() + patched_log.isEnabledFor.return_value = True + patched_list_active_stack_names.return_value = [] + + resource_provider_mock.side_effect = ClientError({"Error": {"Code": "ValidationError"}}, "operation") + + with self.assertRaises(ClientError): + get_resource_summaries(resource_provider_mock, client_provider_mock, "invalid-stack") + patched_log.debug.assert_called_with( + "Invalid stack name (%s). Available stack names: %s", "invalid-stack", ", ".join([]) + ) + + def test_list_active_stack_names(self): + cfn_client_mock = Mock() + cfn_client_mock.list_stacks.side_effect = [ + { + "StackSummaries": [{"StackName": "A"}, {"StackName": "B"}, {"StackName": "C", "RootId": "A"}], + "NextToken": "X", + }, + {"StackSummaries": [{"StackName": "D"}, {"StackName": "E"}, {"StackName": "F", "RootId": "A"}]}, + ] + client_provider_mock = Mock() + client_provider_mock.return_value = cfn_client_mock + + self.assertEqual(["A", "B", "D", "E"], list(list_active_stack_names(client_provider_mock))) + + def test_list_active_stack_names_with_nested_stacks(self): + cfn_client_mock = Mock() + cfn_client_mock.list_stacks.side_effect = [ + { + "StackSummaries": [{"StackName": "A"}, {"StackName": "B"}, {"StackName": "C", "RootId": "A"}], + "NextToken": "X", + }, + {"StackSummaries": [{"StackName": "D"}, {"StackName": "E"}, {"StackName": "F", "RootId": "A"}]}, + ] + client_provider_mock = Mock() + client_provider_mock.return_value = cfn_client_mock + + self.assertEqual(["A", "B", "C", "D", "E", "F"], list(list_active_stack_names(client_provider_mock, True)))