From f4a2414f2b57f149bf447d1fb063b7d041bf4afa Mon Sep 17 00:00:00 2001 From: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Date: Fri, 11 Aug 2023 15:43:46 -0700 Subject: [PATCH] fix: read terraform hook parameters from samconfig file (#5751) * fix: read terraform hook parameters from samconfig file * run format * no need to run testing in containers --- .../_utils/custom_options/hook_name_option.py | 25 ++++---- samcli/commands/_utils/options.py | 2 +- ...uild_terraform_applications_other_cases.py | 58 +++++++++++++++++++ .../root_module/input_samconfig.yaml | 9 +++ .../test_hook_package_id_option.py | 56 +++++++++++++++++- 5 files changed, 137 insertions(+), 13 deletions(-) create mode 100644 tests/integration/testdata/buildcmd/terraform/application_outside_root_directory/root_module/input_samconfig.yaml diff --git a/samcli/commands/_utils/custom_options/hook_name_option.py b/samcli/commands/_utils/custom_options/hook_name_option.py index 6591a4d04f..af730ee2f5 100644 --- a/samcli/commands/_utils/custom_options/hook_name_option.py +++ b/samcli/commands/_utils/custom_options/hook_name_option.py @@ -61,7 +61,7 @@ def handle_parse_result(self, ctx, opts, args): return super().handle_parse_result(ctx, opts, args) try: - self._call_prepare_hook(iac_hook_wrapper, opts) + self._call_prepare_hook(iac_hook_wrapper, opts, ctx) except Exception as ex: # capture exceptions from prepare hook to emit in track_command c = Context.get_current_context() @@ -69,7 +69,7 @@ def handle_parse_result(self, ctx, opts, args): return super().handle_parse_result(ctx, opts, args) - def _call_prepare_hook(self, iac_hook_wrapper, opts): + def _call_prepare_hook(self, iac_hook_wrapper, opts, ctx): # call prepare hook built_template_path = DEFAULT_BUILT_TEMPLATE_PATH if not self._force_prepare and os.path.exists(built_template_path): @@ -82,14 +82,12 @@ def _call_prepare_hook(self, iac_hook_wrapper, opts): if not os.path.exists(output_dir_path): os.makedirs(output_dir_path, exist_ok=True) - debug = opts.get("debug", False) - aws_profile = opts.get("profile") - aws_region = opts.get("region") - skip_prepare_infra = opts.get("skip_prepare_infra", False) - plan_file = opts.get("terraform_plan_file") - project_root_dir = opts.get( - "terraform_project_root_path", - ) + debug = _read_parameter_value("debug", opts, ctx, False) + aws_profile = _read_parameter_value("profile", opts, ctx) + aws_region = _read_parameter_value("region", opts, ctx) + skip_prepare_infra = _read_parameter_value("skip_prepare_infra", opts, ctx, False) + plan_file = _read_parameter_value("terraform_plan_file", opts, ctx) + project_root_dir = _read_parameter_value("terraform_project_root_path", opts, ctx) metadata_file = iac_hook_wrapper.prepare( output_dir_path, @@ -210,3 +208,10 @@ def _get_iac_support_experimental_prompt_message(hook_name: str, command: str) - ) } return supported_iacs_messages.get(hook_name, "") + + +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)) diff --git a/samcli/commands/_utils/options.py b/samcli/commands/_utils/options.py index 7590492496..737c8ecdd2 100644 --- a/samcli/commands/_utils/options.py +++ b/samcli/commands/_utils/options.py @@ -731,7 +731,7 @@ def skip_prepare_infra_click_option(): Click option to skip the hook preparation stage """ return click.option( - "--skip-prepare-infra", + "--skip-prepare-infra/--prepare-infra", is_flag=True, required=False, callback=skip_prepare_infra_callback, 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 e9282013d8..a45a7a569e 100644 --- a/tests/integration/buildcmd/test_build_terraform_applications_other_cases.py +++ b/tests/integration/buildcmd/test_build_terraform_applications_other_cases.py @@ -439,6 +439,64 @@ def test_build_and_invoke_lambda_functions(self, function_identifier, expected_o "function_identifier": function_identifier, "project_root_dir": "./..", } + if self.build_in_container: + command_list_parameters["use_container"] = True + command_list_parameters["build_image"] = self.docker_tag + build_cmd_list = self.get_command_list(**command_list_parameters) + LOG.info("command list: %s", build_cmd_list) + stdout, stderr, return_code = self.run_command(build_cmd_list) + terraform_beta_feature_prompted_text = ( + f"Supporting Terraform applications is a beta feature.{os.linesep}" + f"Please confirm if you would like to proceed using AWS SAM CLI with terraform application.{os.linesep}" + "You can also enable this beta feature with 'sam build --beta-features'." + ) + experimental_warning = ( + f"{os.linesep}Experimental features are enabled for this session.{os.linesep}" + f"Visit the docs page to learn more about the AWS Beta terms " + f"https://aws.amazon.com/service-terms/.{os.linesep}" + ) + self.assertNotRegex(stdout.decode("utf-8"), terraform_beta_feature_prompted_text) + self.assertIn(Colored().yellow(experimental_warning), stderr.decode("utf-8")) + LOG.info("sam build stdout: %s", stdout.decode("utf-8")) + LOG.info("sam build stderr: %s", stderr.decode("utf-8")) + self.assertEqual(return_code, 0) + + self._verify_invoke_built_function( + function_logical_id=function_identifier, + 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 TestBuildTerraformApplicationsSourceCodeAndModulesAreNotInRootModuleDirectoryGetParametersFromSamConfig( + BuildTerraformApplicationIntegBase +): + terraform_application = Path("terraform/application_outside_root_directory") + + functions = [ + ("aws_lambda_function.function1", "hello world 1"), + ] + + def setUp(self): + super().setUp() + self.project_dir = self.working_dir + self.working_dir = f"{self.working_dir}/root_module" + + def tearDown(self): + if self.project_dir: + self.working_dir = self.project_dir + super().tearDown() + + @parameterized.expand(functions) + def test_build_and_invoke_lambda_functions(self, function_identifier, expected_output): + command_list_parameters = { + "config_file": "input_samconfig.yaml", + "function_identifier": function_identifier, + } build_cmd_list = self.get_command_list(**command_list_parameters) LOG.info("command list: %s", build_cmd_list) stdout, stderr, return_code = self.run_command(build_cmd_list) diff --git a/tests/integration/testdata/buildcmd/terraform/application_outside_root_directory/root_module/input_samconfig.yaml b/tests/integration/testdata/buildcmd/terraform/application_outside_root_directory/root_module/input_samconfig.yaml new file mode 100644 index 0000000000..12d6d000fc --- /dev/null +++ b/tests/integration/testdata/buildcmd/terraform/application_outside_root_directory/root_module/input_samconfig.yaml @@ -0,0 +1,9 @@ +version: 0.1 +default: + global: + parameters: + beta_features: true + hook_name: terraform + build: + parameters: + terraform_project_root_path: ./.. \ No newline at end of file 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 e12f351bba..ad6656fdd2 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 @@ -86,6 +86,7 @@ def test_valid_hook_package_with_only_hook_id_option( invalid_coexist_options=self.invalid_coexist_options, ) ctx = MagicMock() + ctx.default_map = {} opts = { "hook_name": self.terraform, } @@ -121,12 +122,15 @@ def test_valid_hook_package_with_other_options( invalid_coexist_options=self.invalid_coexist_options, ) ctx = MagicMock() + ctx.default_map = {} opts = { "hook_name": self.terraform, "debug": True, "profile": "test", "region": "us-east-1", "terraform_project_root_path": "/path/path", + "skip_prepare_infra": True, + "terraform_plan_file": "/path/plan/file", } args = [] hook_name_option.handle_parse_result(ctx, opts, args) @@ -136,8 +140,50 @@ def test_valid_hook_package_with_other_options( True, "test", "us-east-1", - False, - None, + True, + "/path/plan/file", + "/path/path", + ) + self.assertEqual(opts.get("template_file"), self.metadata_path) + + @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") + @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") + @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, prompt_experimental_mock, update_experimental_context_mock + ): + iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock + prompt_experimental_mock.return_value = True + + getcwd_mock.return_value = self.cwd_path + + hook_name_option = HookNameOption( + param_decls=(self.name, self.opt), + force_prepare=True, + invalid_coexist_options=self.invalid_coexist_options, + ) + ctx = MagicMock() + ctx.default_map = { + "hook_name": self.terraform, + "debug": True, + "profile": "test", + "region": "us-east-1", + "terraform_project_root_path": "/path/path", + "skip_prepare_infra": True, + "terraform_plan_file": "/path/plan/file", + } + opts = {} + args = [] + hook_name_option.handle_parse_result(ctx, opts, args) + self.iac_hook_wrapper_instance_mock.prepare.assert_called_once_with( + os.path.join(self.cwd_path, ".aws-sam-iacs", "iacs_metadata"), + self.cwd_path, + True, + "test", + "us-east-1", + True, + "/path/plan/file", "/path/path", ) self.assertEqual(opts.get("template_file"), self.metadata_path) @@ -272,6 +318,7 @@ def test_valid_hook_package_with_beta_feature_option( invalid_coexist_options=self.invalid_coexist_options, ) ctx = MagicMock() + ctx.default_map = {} opts = { "hook_name": self.terraform, "beta_features": True, @@ -420,6 +467,7 @@ def test_valid_hook_package_with_skipping_prepare_hook_and_built_path_does_not_e invalid_coexist_options=self.invalid_coexist_options, ) ctx = MagicMock() + ctx.default_map = {} opts = { "hook_name": self.terraform, } @@ -463,6 +511,7 @@ def test_valid_hook_package_with_use_container_and_build_image( invalid_coexist_options=self.invalid_coexist_options, ) ctx = MagicMock() + ctx.default_map = {} ctx.command.name = "build" opts = { "hook_name": self.terraform, @@ -586,6 +635,7 @@ def test_valid_parameter_hook_with_valid_absolute_project_root_directory( invalid_coexist_options=self.invalid_coexist_options, ) ctx = MagicMock() + ctx.default_map = {} ctx.command.name = "build" opts = { "hook_name": self.terraform, @@ -634,6 +684,7 @@ def test_valid_parameter_hook_with_valid_relative_project_root_directory( invalid_coexist_options=self.invalid_coexist_options, ) ctx = MagicMock() + ctx.default_map = {} ctx.command.name = "build" opts = { "hook_name": self.terraform, @@ -679,6 +730,7 @@ def test_valid_hook_package_with_use_container_false_and_no_build_image( invalid_coexist_options=self.invalid_coexist_options, ) ctx = MagicMock() + ctx.default_map = {} ctx.command.name = "build" opts = { "hook_name": self.terraform,