diff --git a/samcli/commands/_utils/custom_options/hook_name_option.py b/samcli/commands/_utils/custom_options/hook_name_option.py index a226fd68e9..b3f43dd550 100644 --- a/samcli/commands/_utils/custom_options/hook_name_option.py +++ b/samcli/commands/_utils/custom_options/hook_name_option.py @@ -4,6 +4,7 @@ import logging import os +from typing import Any, Mapping import click @@ -11,9 +12,12 @@ from samcli.commands._utils.constants import DEFAULT_BUILT_TEMPLATE_PATH from samcli.lib.hook.exceptions import InvalidHookWrapperException from samcli.lib.hook.hook_wrapper import IacHookWrapper, get_available_hook_packages_ids +from samcli.lib.telemetry.event import EventName, EventTracker LOG = logging.getLogger(__name__) +PLAN_FILE_OPTION = "terraform_plan_file" + class HookNameOption(click.Option): """ @@ -56,6 +60,8 @@ def handle_parse_result(self, ctx, opts, args): # capture exceptions from prepare hook to emit in track_command c = Context.get_current_context() c.exception = ex + finally: + record_hook_telemetry(opts, ctx) return super().handle_parse_result(ctx, opts, args) @@ -132,3 +138,19 @@ def _read_parameter_value(param_name, opts, ctx, default_value=None): Read SAM CLI parameter value either from the parameters list or from the samconfig values """ return opts.get(param_name, ctx.default_map.get(param_name, default_value)) + + +def record_hook_telemetry(opts: Mapping[str, Any], ctx: click.Context): + """ + Emit metrics related to hooks based on the options passed into the command + + Parameters + ---------- + opts: Mapping[str, Any] + Mapping between a command line option and its value + ctx: Context + Command context properties + """ + plan_file_param = _read_parameter_value(PLAN_FILE_OPTION, opts, ctx) + if plan_file_param: + EventTracker.track_event(EventName.HOOK_CONFIGURATIONS_USED.value, "TerraformPlanFile") diff --git a/samcli/commands/remote/invoke/cli.py b/samcli/commands/remote/invoke/cli.py index 0318566b4a..030a8dbcac 100644 --- a/samcli/commands/remote/invoke/cli.py +++ b/samcli/commands/remote/invoke/cli.py @@ -117,6 +117,12 @@ def do_cli( """ Implementation of the ``cli`` method """ + from botocore.exceptions import ( + NoCredentialsError, + NoRegionError, + ProfileNotFound, + ) + from samcli.commands.exceptions import UserException from samcli.commands.remote.remote_invoke_context import RemoteInvokeContext from samcli.lib.remote_invoke.exceptions import ( @@ -127,9 +133,9 @@ def do_cli( from samcli.lib.remote_invoke.remote_invoke_executors import RemoteInvokeExecutionInfo from samcli.lib.utils.boto_utils import get_boto_client_provider_with_config, get_boto_resource_provider_with_config - boto_client_provider = get_boto_client_provider_with_config(region_name=region) - boto_resource_provider = get_boto_resource_provider_with_config(region_name=region) try: + boto_client_provider = get_boto_client_provider_with_config(region_name=region, profile=profile) + boto_resource_provider = get_boto_resource_provider_with_config(region_name=region, profile=profile) with RemoteInvokeContext( boto_client_provider=boto_client_provider, boto_resource_provider=boto_resource_provider, @@ -142,5 +148,12 @@ def do_cli( ) remote_invoke_context.run(remote_invoke_input=remote_invoke_input) - except (ErrorBotoApiCallException, InvalideBotoResponseException, InvalidResourceBotoParameterException) as ex: + except ( + ErrorBotoApiCallException, + InvalideBotoResponseException, + InvalidResourceBotoParameterException, + ProfileNotFound, + NoCredentialsError, + NoRegionError, + ) as ex: raise UserException(str(ex), wrapped_from=ex.__class__.__name__) from ex diff --git a/samcli/hook_packages/terraform/copy_terraform_built_artifacts.py b/samcli/hook_packages/terraform/copy_terraform_built_artifacts.py index 53a7d4d4ec..d4be990d61 100644 --- a/samcli/hook_packages/terraform/copy_terraform_built_artifacts.py +++ b/samcli/hook_packages/terraform/copy_terraform_built_artifacts.py @@ -56,6 +56,16 @@ LOG = logging.getLogger(__name__) TF_BACKEND_OVERRIDE_FILENAME = "z_samcli_backend_override" +TF_BLOCKED_ARGUMENTS = [ + "-target", + "-destroy", +] +TF_ENVIRONMENT_VARIABLE_DELIM = "=" +TF_ENVIRONMENT_VARIABLES = [ + "TF_CLI_ARGS", + "TF_CLI_ARGS_plan", + "TF_CLI_ARGS_apply", +] class ResolverException(Exception): @@ -276,6 +286,31 @@ def create_backend_override(): cli_exit() +def validate_environment_variables(): + """ + Validate that the Terraform environment variables do not contain blocked arguments. + """ + for env_var in TF_ENVIRONMENT_VARIABLES: + env_value = os.environ.get(env_var, "") + + trimmed_arguments = [] + # get all trimmed arguments in a list and split on delim + # eg. + # "-foo=bar -hello" => ["-foo", "-hello"] + for argument in env_value.split(" "): + cleaned_argument = argument.strip() + cleaned_argument = cleaned_argument.split(TF_ENVIRONMENT_VARIABLE_DELIM)[0] + + trimmed_arguments.append(cleaned_argument) + + if any([argument in TF_BLOCKED_ARGUMENTS for argument in trimmed_arguments]): + LOG.error( + "Environment variable '%s' contains " + "a blocked argument, please validate it does not contain: %s" % (env_var, TF_BLOCKED_ARGUMENTS) + ) + cli_exit() + + if __name__ == "__main__": # Gather inputs and clean them argparser = argparse.ArgumentParser( @@ -314,6 +349,9 @@ def create_backend_override(): target = arguments.target json_str = arguments.json + # validate environment variables do not contain blocked arguments + validate_environment_variables() + if target and json_str: LOG.error("Provide either --target or --json. Do not provide both.") cli_exit() diff --git a/samcli/hook_packages/terraform/hooks/prepare/hook.py b/samcli/hook_packages/terraform/hooks/prepare/hook.py index b9adf26c56..0d8c8ce322 100644 --- a/samcli/hook_packages/terraform/hooks/prepare/hook.py +++ b/samcli/hook_packages/terraform/hooks/prepare/hook.py @@ -12,7 +12,11 @@ from samcli.hook_packages.terraform.hooks.prepare.constants import CFN_CODE_PROPERTIES from samcli.hook_packages.terraform.hooks.prepare.translate import translate_to_cfn -from samcli.lib.hook.exceptions import PrepareHookException, TerraformCloudException +from samcli.lib.hook.exceptions import ( + PrepareHookException, + TerraformCloudException, + UnallowedEnvironmentVariableArgumentException, +) from samcli.lib.utils import osutils from samcli.lib.utils.subprocess_utils import LoadingPatternError, invoke_subprocess_with_loading_pattern @@ -32,6 +36,17 @@ "a plan file using the --terraform-plan-file flag." ) +TF_BLOCKED_ARGUMENTS = [ + "-target", + "-destroy", +] +TF_ENVIRONMENT_VARIABLE_DELIM = "=" +TF_ENVIRONMENT_VARIABLES = [ + "TF_CLI_ARGS", + "TF_CLI_ARGS_plan", + "TF_CLI_ARGS_apply", +] + def prepare(params: dict) -> dict: """ @@ -55,6 +70,8 @@ def prepare(params: dict) -> dict: if not output_dir_path: raise PrepareHookException("OutputDirPath was not supplied") + _validate_environment_variables() + LOG.debug("Normalize the terraform application root module directory path %s", terraform_application_dir) if not os.path.isabs(terraform_application_dir): terraform_application_dir = os.path.normpath(os.path.join(os.getcwd(), terraform_application_dir)) @@ -215,3 +232,36 @@ def _generate_plan_file(skip_prepare_infra: bool, terraform_application_dir: str raise PrepareHookException(f"Error occurred when invoking a process:\n{e}") from e return dict(json.loads(result.stdout)) + + +def _validate_environment_variables() -> None: + """ + Validate that the Terraform environment variables do not contain blocked arguments. + + Raises + ------ + UnallowedEnvironmentVariableArgumentException + Raised when a Terraform related environment variable contains a blocked value + """ + for env_var in TF_ENVIRONMENT_VARIABLES: + env_value = os.environ.get(env_var, "") + + trimmed_arguments = [] + # get all trimmed arguments in a list and split on delim + # eg. + # "-foo=bar -hello" => ["-foo", "-hello"] + for argument in env_value.split(" "): + cleaned_argument = argument.strip() + cleaned_argument = cleaned_argument.split(TF_ENVIRONMENT_VARIABLE_DELIM)[0] + + trimmed_arguments.append(cleaned_argument) + + if any([argument in TF_BLOCKED_ARGUMENTS for argument in trimmed_arguments]): + message = ( + "Environment variable '%s' contains a blocked argument, please validate it does not contain: %s" + % ( + env_var, + TF_BLOCKED_ARGUMENTS, + ) + ) + raise UnallowedEnvironmentVariableArgumentException(message) diff --git a/samcli/lib/hook/exceptions.py b/samcli/lib/hook/exceptions.py index 72cbc3e69d..a4b162826b 100644 --- a/samcli/lib/hook/exceptions.py +++ b/samcli/lib/hook/exceptions.py @@ -24,3 +24,7 @@ class PrepareHookException(UserException): class TerraformCloudException(UserException): pass + + +class UnallowedEnvironmentVariableArgumentException(UserException): + pass diff --git a/samcli/lib/telemetry/event.py b/samcli/lib/telemetry/event.py index 2c35748951..96e5fe45cb 100644 --- a/samcli/lib/telemetry/event.py +++ b/samcli/lib/telemetry/event.py @@ -28,6 +28,7 @@ class EventName(Enum): SYNC_FLOW_END = "SyncFlowEnd" BUILD_WORKFLOW_USED = "BuildWorkflowUsed" CONFIG_FILE_EXTENSION = "SamConfigFileExtension" + HOOK_CONFIGURATIONS_USED = "HookConfigurationsUsed" class UsedFeature(Enum): @@ -61,6 +62,10 @@ class EventType: ] _WORKFLOWS = [f"{config.language}-{config.dependency_manager}" for config in ALL_CONFIGS] + _HOOK_CONFIGURATIONS = [ + "TerraformPlanFile", + ] + _event_values = { # Contains allowable values for Events EventName.USED_FEATURE: [event.value for event in UsedFeature], EventName.BUILD_FUNCTION_RUNTIME: INIT_RUNTIMES, @@ -72,6 +77,7 @@ class EventType: EventName.SYNC_FLOW_END: _SYNC_FLOWS, EventName.BUILD_WORKFLOW_USED: _WORKFLOWS, EventName.CONFIG_FILE_EXTENSION: list(FILE_MANAGER_MAPPER.keys()), + EventName.HOOK_CONFIGURATIONS_USED: _HOOK_CONFIGURATIONS, } @staticmethod @@ -148,6 +154,7 @@ class EventTracker: _events: List[Event] = [] _event_lock = threading.Lock() _session_id: Optional[str] = None + _command_name: Optional[str] = None MAX_EVENTS: int = 50 # Maximum number of events to store before sending @@ -205,8 +212,9 @@ def track_event( Event(event_name, event_value, thread_id=thread_id, exception_name=exception_name) ) - # Get the session ID (needed for multithreading sending) - EventTracker._set_session_id() + # Get properties from the click context (needed for multithreading sending) + EventTracker._set_context_property("_session_id", "session_id") + EventTracker._set_context_property("_command_name", "command_path") if len(EventTracker._events) >= EventTracker.MAX_EVENTS: should_send = True @@ -235,17 +243,27 @@ def send_events() -> threading.Thread: return send_thread @staticmethod - def _set_session_id() -> None: + def _set_context_property(event_prop: str, context_prop: str) -> None: """ - Get the session ID from click and save it locally. + Set a click context property in the event so that it is emitted when the metric is sent. + This is required since the event is sent in a separate thread and no longer has access + to the click context that the command was initially called with. As a workaround, we set + the property here first so that it's available when calling the metrics endpoint. + + Parameters + ---------- + event_prop: str + Property name to be stored in the event and consumed when emitting the metric + context_prop: str + Property name for the target property from the context object """ - if not EventTracker._session_id: + if not getattr(EventTracker, event_prop): try: ctx = Context.get_current_context() if ctx: - EventTracker._session_id = ctx.session_id + setattr(EventTracker, event_prop, getattr(ctx, context_prop)) except RuntimeError: - LOG.debug("EventTracker: Unable to obtain session ID") + LOG.debug("EventTracker: Unable to obtain %s", context_prop) @staticmethod def _send_events_in_thread(): @@ -264,6 +282,7 @@ def _send_events_in_thread(): telemetry = Telemetry() metric = Metric("events") metric.add_data("sessionId", EventTracker._session_id) + metric.add_data("commandName", EventTracker._command_name) metric.add_data("metricSpecificAttributes", msa) telemetry.emit(metric) diff --git a/tests/integration/buildcmd/test_build_terraform_applications_other_cases.py b/tests/integration/buildcmd/test_build_terraform_applications_other_cases.py index b3225b81e8..8c3d87ddae 100644 --- a/tests/integration/buildcmd/test_build_terraform_applications_other_cases.py +++ b/tests/integration/buildcmd/test_build_terraform_applications_other_cases.py @@ -466,3 +466,37 @@ def test_build_and_invoke_lambda_functions(self, function_identifier, expected_o overrides=None, expected_result={"statusCode": 200, "body": expected_output}, ) + + +@skipIf( + (not RUN_BY_CANARY and not CI_OVERRIDE), + "Skip Terraform test cases unless running in CI", +) +class TestBuildTerraformApplicationsWithBlockedEnvironVariables(BuildTerraformApplicationIntegBase): + terraform_application = Path("terraform/simple_application") + + @parameterized.expand( + [ + ("TF_CLI_ARGS", "-destroy"), + ("TF_CLI_ARGS", "-target=some.module"), + ("TF_CLI_ARGS_plan", "-destroy"), + ("TF_CLI_ARGS_plan", "-target=some.module"), + ("TF_CLI_ARGS_apply", "-destroy"), + ("TF_CLI_ARGS_apply", "-target=some.module"), + ] + ) + def test_blocked_env_variables(self, env_name, env_value): + cmdlist = self.get_command_list(hook_name="terraform", beta_features=True) + + env_variables = os.environ.copy() + env_variables[env_name] = env_value + + _, stderr, return_code = self.run_command(cmdlist, env=env_variables) + + process_stderr = stderr.strip() + self.assertRegex( + process_stderr.decode("utf-8"), + "Error: Environment variable '%s' contains a blocked argument, please validate it does not contain: ['-destroy', '-target']" + % env_name, + ) + self.assertNotEqual(return_code, 0) diff --git a/tests/unit/commands/_utils/custom_options/test_hook_package_id_option.py b/tests/unit/commands/_utils/custom_options/test_hook_package_id_option.py index 5af9ab1c0f..d4eed80902 100644 --- a/tests/unit/commands/_utils/custom_options/test_hook_package_id_option.py +++ b/tests/unit/commands/_utils/custom_options/test_hook_package_id_option.py @@ -1,10 +1,10 @@ from unittest import TestCase -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, patch, Mock import os import click -from samcli.commands._utils.custom_options.hook_name_option import HookNameOption +from samcli.commands._utils.custom_options.hook_name_option import HookNameOption, record_hook_telemetry from samcli.lib.hook.exceptions import InvalidHookWrapperException @@ -68,9 +68,12 @@ def test_invalid_coexist_options(self, iac_hook_wrapper_mock): f"Parameters hook-name, and {','.join(self.invalid_coexist_options)} cannot be used together", ) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") - def test_valid_hook_package_with_only_hook_id_option(self, iac_hook_wrapper_mock, getcwd_mock): + def test_valid_hook_package_with_only_hook_id_option( + self, iac_hook_wrapper_mock, getcwd_mock, record_hook_telemetry_mock + ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock getcwd_mock.return_value = self.cwd_path @@ -99,9 +102,12 @@ def test_valid_hook_package_with_only_hook_id_option(self, iac_hook_wrapper_mock ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") - def test_valid_hook_package_with_other_options(self, iac_hook_wrapper_mock, getcwd_mock): + def test_valid_hook_package_with_other_options( + self, iac_hook_wrapper_mock, getcwd_mock, record_hook_telemetry_mock + ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock getcwd_mock.return_value = self.cwd_path @@ -135,10 +141,14 @@ def test_valid_hook_package_with_other_options(self, iac_hook_wrapper_mock, getc "/path/path", ) self.assertEqual(opts.get("template_file"), self.metadata_path) + record_hook_telemetry_mock.assert_called_once() + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") - def test_valid_hook_package_with_other_options_from_sam_config(self, iac_hook_wrapper_mock, getcwd_mock): + def test_valid_hook_package_with_other_options_from_sam_config( + self, iac_hook_wrapper_mock, getcwd_mock, record_hook_telemetry_mock + ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock getcwd_mock.return_value = self.cwd_path @@ -173,6 +183,7 @@ def test_valid_hook_package_with_other_options_from_sam_config(self, iac_hook_wr ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.os.path.exists") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") @@ -181,6 +192,7 @@ def test_valid_hook_package_with_skipping_prepare_hook_and_built_path_exists( iac_hook_wrapper_mock, path_exists_mock, getcwd_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock @@ -202,6 +214,7 @@ def test_valid_hook_package_with_skipping_prepare_hook_and_built_path_exists( self.iac_hook_wrapper_instance_mock.prepare.assert_not_called() self.assertEqual(opts.get("template_file"), None) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.os.path.exists") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") @@ -210,6 +223,7 @@ def test_valid_hook_package_with_skipping_prepare_hook_and_built_path_does_not_e iac_hook_wrapper_mock, path_exists_mock, getcwd_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock @@ -241,6 +255,7 @@ def test_valid_hook_package_with_skipping_prepare_hook_and_built_path_does_not_e ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.os.path.exists") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") @@ -249,6 +264,7 @@ def test_valid_hook_package_with_use_container_and_build_image( iac_hook_wrapper_mock, path_exists_mock, getcwd_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock @@ -283,6 +299,7 @@ def test_valid_hook_package_with_use_container_and_build_image( ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.os.path.exists") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") @@ -291,6 +308,7 @@ def test_invalid_hook_package_with_use_container_and_no_build_image( iac_hook_wrapper_mock, path_exists_mock, getcwd_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock @@ -316,12 +334,14 @@ def test_invalid_hook_package_with_use_container_and_no_build_image( ): hook_name_option.handle_parse_result(ctx, opts, args) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") def test_invalid_parameter_hook_with_invalid_project_root_directory( self, iac_hook_wrapper_mock, getcwd_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock @@ -347,6 +367,7 @@ def test_invalid_parameter_hook_with_invalid_project_root_directory( ): hook_name_option.handle_parse_result(ctx, opts, args) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.os.path.exists") @patch("samcli.commands._utils.custom_options.hook_name_option.os.path.isabs") @@ -357,6 +378,7 @@ def test_valid_parameter_hook_with_valid_absolute_project_root_directory( path_isabs_mock, path_exists_mock, getcwd_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock @@ -391,6 +413,7 @@ def test_valid_parameter_hook_with_valid_absolute_project_root_directory( ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.os.path.exists") @patch("samcli.commands._utils.custom_options.hook_name_option.os.path.isabs") @@ -401,6 +424,7 @@ def test_valid_parameter_hook_with_valid_relative_project_root_directory( path_isabs_mock, path_exists_mock, getcwd_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock @@ -435,6 +459,7 @@ def test_valid_parameter_hook_with_valid_relative_project_root_directory( ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.os.path.exists") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") @@ -443,6 +468,7 @@ def test_valid_hook_package_with_use_container_false_and_no_build_image( iac_hook_wrapper_mock, path_exists_mock, getcwd_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock @@ -475,3 +501,9 @@ def test_valid_hook_package_with_use_container_false_and_no_build_image( None, ) self.assertEqual(opts.get("template_file"), self.metadata_path) + + @patch("samcli.commands._utils.custom_options.hook_name_option.EventTracker") + def test_record_hook_telemetry(self, event_tracker_mock): + opts = {"terraform_plan_file": "my_plan.json"} + record_hook_telemetry(opts, Mock()) + event_tracker_mock.track_event.assert_called_once_with("HookConfigurationsUsed", "TerraformPlanFile") diff --git a/tests/unit/commands/remote/invoke/test_cli.py b/tests/unit/commands/remote/invoke/test_cli.py index fe9179a891..2d30ccabc5 100644 --- a/tests/unit/commands/remote/invoke/test_cli.py +++ b/tests/unit/commands/remote/invoke/test_cli.py @@ -3,6 +3,11 @@ from parameterized import parameterized +from botocore.exceptions import ( + ProfileNotFound, + NoCredentialsError, + NoRegionError, +) from samcli.commands.remote.invoke.cli import do_cli from samcli.lib.remote_invoke.remote_invoke_executors import RemoteInvokeOutputFormat from samcli.lib.remote_invoke.exceptions import ( @@ -85,8 +90,8 @@ def test_remote_invoke_command( config_env=self.config_env, ) - patched_get_boto_client_provider_with_config.assert_called_with(region_name=self.region) - patched_get_boto_resource_provider_with_config.assert_called_with(region_name=self.region) + patched_get_boto_client_provider_with_config.assert_called_with(region_name=self.region, profile=self.profile) + patched_get_boto_resource_provider_with_config.assert_called_with(region_name=self.region, profile=self.profile) mock_remote_invoke_context.assert_called_with( boto_client_provider=given_client_provider, @@ -106,6 +111,9 @@ def test_remote_invoke_command( (InvalideBotoResponseException,), (ErrorBotoApiCallException,), (InvalidResourceBotoParameterException,), + (ProfileNotFound,), + (NoCredentialsError,), + (NoRegionError,), ] ) @patch("samcli.commands.remote.remote_invoke_context.RemoteInvokeContext") diff --git a/tests/unit/hook_packages/terraform/hooks/prepare/test_hook.py b/tests/unit/hook_packages/terraform/hooks/prepare/test_hook.py index 6fa264e788..99413b33b2 100644 --- a/tests/unit/hook_packages/terraform/hooks/prepare/test_hook.py +++ b/tests/unit/hook_packages/terraform/hooks/prepare/test_hook.py @@ -6,12 +6,18 @@ from tests.unit.hook_packages.terraform.hooks.prepare.prepare_base import PrepareHookUnitBase from samcli.hook_packages.terraform.hooks.prepare.hook import ( + TF_ENVIRONMENT_VARIABLES, + _validate_environment_variables, prepare, _update_resources_paths, TF_CLOUD_EXCEPTION_MESSAGE, TF_CLOUD_HELP_MESSAGE, ) -from samcli.lib.hook.exceptions import PrepareHookException, TerraformCloudException +from samcli.lib.hook.exceptions import ( + PrepareHookException, + TerraformCloudException, + UnallowedEnvironmentVariableArgumentException, +) from samcli.lib.utils.subprocess_utils import LoadingPatternError @@ -492,3 +498,38 @@ def test_prints_tf_cloud_help_message(self, mock_subprocess_loader): prepare(self.prepare_params) self.assertEqual(ex.exception.message, TF_CLOUD_HELP_MESSAGE) + + @parameterized.expand( + [ + ("-destroy",), + ("-target=my.module.resource",), + ("-destroy -target=my.module.resource",), + ("-target=my.module.resource -destroy",), + ] + ) + @patch("samcli.hook_packages.terraform.hooks.prepare.hook.os") + def test_environment_variable_check_fails(self, argument, get_mock): + get_mock.environ.get.return_value = argument + + with self.assertRaises(UnallowedEnvironmentVariableArgumentException): + _validate_environment_variables() + + @parameterized.expand( + [ + ("-not-actually-argument",), + ("something",), + ("",), + ] + ) + @patch("samcli.hook_packages.terraform.hooks.prepare.hook.os") + def test_environment_variable_check_passes(self, argument, get_mock): + get_mock.environ.get.return_value = argument + + _validate_environment_variables() + + @patch("samcli.hook_packages.terraform.hooks.prepare.hook._validate_environment_variables") + def test_prepare_method_fails_environment_variables(self, validate_mock): + validate_mock.side_effect = [UnallowedEnvironmentVariableArgumentException("message")] + + with self.assertRaises(UnallowedEnvironmentVariableArgumentException): + prepare(self.prepare_params) diff --git a/tests/unit/lib/telemetry/test_event.py b/tests/unit/lib/telemetry/test_event.py index b8dc01fd34..33202303cf 100644 --- a/tests/unit/lib/telemetry/test_event.py +++ b/tests/unit/lib/telemetry/test_event.py @@ -7,6 +7,9 @@ from typing import List, Tuple from unittest import TestCase from unittest.mock import ANY, Mock, patch + +from parameterized import parameterized + from samcli.cli.context import Context from uuid import UUID, uuid4 @@ -163,6 +166,7 @@ def test_events_get_sent(self, telemetry_mock): "ci": ANY, "pyversion": ANY, "samcliVersion": ANY, + "commandName": ANY, "metricSpecificAttributes": { "events": [ { @@ -212,10 +216,44 @@ def test_session_id_set(self, context_mock): context_mock.return_value = mock EventTracker._session_id = None - EventTracker._set_session_id() + EventTracker._set_context_property("_session_id", "session_id") self.assertEqual(EventTracker._session_id, "123") + @patch("samcli.cli.context.Context.get_current_context") + def test_command_name_set(self, context_mock): + mock = Mock() + mock.command_path = "sam cool command" + context_mock.return_value = mock + + EventTracker._command_name = None + EventTracker._set_context_property("_command_name", "command_path") + + self.assertEqual(EventTracker._command_name, "sam cool command") + + @patch("samcli.cli.context.Context.get_current_context") + def test_set_context_properties(self, context_mock): + event_prop = "event_prop" + context_prop = "context_prop" + context_value = "context_value" + + mock = Mock() + setattr(mock, context_prop, context_value) + context_mock.return_value = mock + + setattr(EventTracker, event_prop, None) + EventTracker._set_context_property(event_prop, context_prop) + + self.assertEqual(getattr(EventTracker, event_prop), context_value) + + @patch("samcli.lib.telemetry.event.LOG") + @patch("samcli.cli.context.Context.get_current_context") + def test_throws_handles_context_not_read(self, context_mock, log_mock): + context_mock.side_effect = RuntimeError("Failed") + EventTracker._command_name = None + EventTracker._set_context_property("_command_name", "command_path") + log_mock.debug.assert_called_once_with("EventTracker: Unable to obtain %s", "command_path") + class TestTrackLongEvent(TestCase): @patch("samcli.lib.telemetry.event.EventTracker.send_events")