fix the hanging problem in windows if the terraform plan command failed

samcli/hook_packages/terraform/hooks/prepare/hook.py

@@ -190,7 +190,8 @@ def _generate_plan_file(skip_prepare_infra: bool, terraform_application_dir: str
             command_args={
                 "args": ["terraform", "init", "-input=false"],
                 "cwd": terraform_application_dir,
-            }
+            },
+            is_running_terraform_command=True,
         )
 
         # get json output of terraform plan
@@ -202,7 +203,8 @@ def _generate_plan_file(skip_prepare_infra: bool, terraform_application_dir: str
                 command_args={
                     "args": ["terraform", "plan", "-out", temp_file.name, "-input=false"],
                     "cwd": terraform_application_dir,
-                }
+                },
+                is_running_terraform_command=True,
             )
 
             result = run(

samcli/lib/utils/subprocess_utils.py

@@ -3,15 +3,20 @@
 """
 import logging
 import os
+import platform
 import sys
 from concurrent.futures.thread import ThreadPoolExecutor
-from subprocess import PIPE, Popen
+from subprocess import PIPE, STDOUT, Popen
 from time import sleep
 from typing import Any, AnyStr, Callable, Dict, Optional, Union
 
 from samcli.commands.exceptions import UserException
 from samcli.lib.utils.stream_writer import StreamWriter
 
+# These are the bytes that used as a prefix for a some string to color them in red to show errors.
+TERRAFORM_ERROR_PREFIX = [27, 91, 51, 49]
+
+IS_WINDOWS = platform.system().lower() == "windows"
 LOG = logging.getLogger(__name__)
 
 
@@ -43,6 +48,7 @@ def invoke_subprocess_with_loading_pattern(
     command_args: Dict[str, Any],
     loading_pattern: Callable[[StreamWriter], None] = default_loading_pattern,
     stream_writer: Optional[StreamWriter] = None,
+    is_running_terraform_command: Optional[bool] = None,
 ) -> Optional[Union[str, bytes]]:
     """
     Wrapper for Popen to asynchronously invoke a subprocess while
@@ -57,6 +63,9 @@ def invoke_subprocess_with_loading_pattern(
         A function generating a pattern to the given stream
     stream_writer: Optional[StreamWriter]
         The stream to which to write the pattern
+    is_running_terraform_command: Optional[bool]
+        Flag to refer if the subprocess is for Terraform commands. This flag is used to help reading the subprocess
+        errors in case of windows.
 
     Returns
     -------
@@ -73,7 +82,7 @@ def invoke_subprocess_with_loading_pattern(
         command_args["stdout"] = PIPE
 
     if not command_args.get("stderr"):
-        command_args["stderr"] = PIPE
+        command_args["stderr"] = STDOUT if IS_WINDOWS else PIPE
 
     try:
         keep_printing = LOG.getEffectiveLevel() >= logging.INFO
@@ -82,7 +91,7 @@ def _print_loading_pattern():
             while keep_printing:
                 loading_pattern(stream_writer)
 
-        # Popen is async as opposed to run so we can print while we wait
+        # Popen is async as opposed to run, so we can print while we wait
         with Popen(**command_args) as process:
             with ThreadPoolExecutor() as executor:
                 executor.submit(_print_loading_pattern)
@@ -92,10 +101,19 @@ def _print_loading_pattern():
                     # we read from subprocess stdout to avoid the deadlock process.wait function
                     # for more detail check this python bug https://bugs.python.org/issue1256
                     for line in process.stdout:
-                        decoded_line = _check_and_process_bytes(line)
+                        is_error = (
+                            is_running_terraform_command
+                            and IS_WINDOWS
+                            and len(line) >= len(TERRAFORM_ERROR_PREFIX)
+                            and line[0:4] == bytes(TERRAFORM_ERROR_PREFIX)
+                        )
+                        decoded_line = _check_and_process_bytes(line, preserve_whitespace=is_error)
                         if LOG.getEffectiveLevel() < logging.INFO:
                             LOG.debug(decoded_line)
-                        process_output += decoded_line
+                        if not is_error:
+                            process_output += decoded_line
+                        else:
+                            process_stderr += decoded_line
 
                 if process.stderr:
                     for line in process.stderr:

tests/integration/buildcmd/test_build_terraform_applications.py

@@ -55,10 +55,10 @@ def setUp(self):
         shutil.rmtree(Path(self.working_dir))
         shutil.copytree(Path(self.terraform_application_path), Path(self.working_dir))
 
-    def run_command(self, command_list, env=None, input=None):
+    def run_command(self, command_list, env=None, input=None, timeout=None):
         process = Popen(command_list, stdout=PIPE, stderr=PIPE, stdin=PIPE, env=env, cwd=self.working_dir)
         try:
-            (stdout, stderr) = process.communicate(input=input)
+            (stdout, stderr) = process.communicate(input=input, timeout=timeout)
             return stdout, stderr, process.returncode
         except TimeoutExpired:
             process.kill()

tests/integration/buildcmd/test_build_terraform_applications_other_cases.py

@@ -362,7 +362,7 @@ def test_invoke_function(self):
         (True,),
     ],
 )
-class TestBuildTerraformApplicationsSourceCodeAndModulesAreNotInRootModuleDirectory(BuildTerraformApplicationIntegBase):
+class TestBuildTerraformNestedDirectories(BuildTerraformApplicationIntegBase):
     terraform_application = (
         Path("terraform/application_outside_root_directory")
         if not IS_WINDOWS
@@ -429,10 +429,12 @@ def test_build_and_invoke_lambda_functions(self, function_identifier, expected_o
     (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")
+class TestBuildTerraformApplicationsNestedDirectoriesGetParametersFromSamConfig(BuildTerraformApplicationIntegBase):
+    terraform_application = (
+        Path("terraform/application_outside_root_directory")
+        if not IS_WINDOWS
+        else Path("terraform/application_outside_root_directory_windows")
+    )
 
     functions = [
         ("aws_lambda_function.function1", "hello world 1"),
@@ -509,12 +511,25 @@ def test_blocked_env_variables(self, env_name, env_value):
 class TestTerraformHandlesExceptionFromBinary(BuildTerraformApplicationIntegBase):
     terraform_application = Path("terraform/broken_tf")
 
-    def test_subprocess_handler(self):
-        err_message = "Terraform encountered problems during initialisation"
+    @parameterized.expand([True, False])
+    def test_subprocess_handler(self, debug_flag):
+        err_message = (
+            "Failed to execute the subprocess. The process ['terraform', 'init', '-input=false'] returned a non-zero "
+            "exit code 1."
+        )
+        terraform_error_message = "Error: Unclosed configuration block"
         stack_trace_error = "unexpected error was encountered while executing 'sam build'"
-        cmdlist = self.get_command_list(hook_name="terraform", beta_features=True)
-        _, stderr, return_code = self.run_command(cmdlist)
+        cmdlist = self.get_command_list(
+            hook_name="terraform",
+            debug=debug_flag,
+        )
+        # add time out, so if the process hangs, the testing will not hang, but the sam command will be timed out.
+        # terraform plan command should fail within seconds, as there is an error in syntax, but we will wait for 5 mins
+        # in case if terraform init takes time.
+        _, stderr, return_code = self.run_command(cmdlist, timeout=300)
         err_string = stderr.decode("utf-8").strip()
+        LOG.info("sam build stderr: %s", err_string)
         self.assertEqual(return_code, 1)
         self.assertIn(err_message, err_string)
+        self.assertIn(terraform_error_message, err_string)
         self.assertNotIn(stack_trace_error, err_string)

tests/integration/local/start_api/test_start_api_with_terraform_application.py

@@ -138,6 +138,7 @@ def test_successful_request(self):
 class TestStartApiTerraformApplicationCustomPlanFile(TerraformStartApiIntegrationBase):
     terraform_application = "terraform-v1-api-simple"
     terraform_plan_file = "custom-plan.json"
+    testing_urls = ["hello"]
 
     def setUp(self):
         self.url = "http://127.0.0.1:{}".format(self.port)

tests/integration/testdata/buildcmd/terraform/application_outside_root_directory_windows/root_module/input_samconfig.yaml

@@ -0,0 +1,8 @@
+version: 0.1
+default:
+  global:
+    parameters:
+      hook_name: terraform
+  build:
+    parameters:
+      terraform_project_root_path: ./..

tests/unit/hook_packages/terraform/hooks/prepare/test_hook.py

@@ -85,13 +85,15 @@ def test_prepare(
                     command_args={
                         "args": ["terraform", "init", "-input=false"],
                         "cwd": "iac/project/path",
-                    }
+                    },
+                    is_running_terraform_command=True,
                 ),
                 call(
                     command_args={
                         "args": ["terraform", "plan", "-out", tf_plan_filename, "-input=false"],
                         "cwd": "iac/project/path",
-                    }
+                    },
+                    is_running_terraform_command=True,
                 ),
             ]
         )
@@ -176,13 +178,15 @@ def test_prepare_with_project_root_dir_param(
                     command_args={
                         "args": ["terraform", "init", "-input=false"],
                         "cwd": "iac/project/path",
-                    }
+                    },
+                    is_running_terraform_command=True,
                 ),
                 call(
                     command_args={
                         "args": ["terraform", "plan", "-out", tf_plan_filename, "-input=false"],
                         "cwd": "iac/project/path",
-                    }
+                    },
+                    is_running_terraform_command=True,
                 ),
             ]
         )
@@ -272,13 +276,15 @@ def test_prepare_with_relative_paths(
                     command_args={
                         "args": ["terraform", "init", "-input=false"],
                         "cwd": "/current/dir/iac/project/path",
-                    }
+                    },
+                    is_running_terraform_command=True,
                 ),
                 call(
                     command_args={
                         "args": ["terraform", "plan", "-out", tf_plan_filename, "-input=false"],
                         "cwd": "/current/dir/iac/project/path",
-                    }
+                    },
+                    is_running_terraform_command=True,
                 ),
             ]
         )

tests/unit/lib/utils/test_subprocess_utils.py

@@ -1,4 +1,4 @@
-from subprocess import DEVNULL, PIPE
+from subprocess import DEVNULL, PIPE, STDOUT
 
 import logging
 from io import StringIO
@@ -18,6 +18,7 @@
     LoadingPatternError,
     _check_and_process_bytes,
 )
+from tests.testing_utils import IS_WINDOWS
 
 
 class TestSubprocessUtils(TestCase):
@@ -35,8 +36,10 @@ def test_loader_stream_logs_when_debug_level(self, patched_Popen, patched_log, p
         mock_pattern = Mock()
         mock_stream_writer = Mock()
         patched_check_and_process_byes.side_effect = [f"Line 1{os.linesep}", "Line 2"]
-        actual_output = invoke_subprocess_with_loading_pattern({"args": ["ls"]}, mock_pattern, mock_stream_writer)
-        patched_check_and_process_byes.assert_has_calls([call(f"Line 1{os.linesep}"), call("Line 2")])
+        actual_output = invoke_subprocess_with_loading_pattern({"args": ["ls"]}, mock_pattern, mock_stream_writer, True)
+        patched_check_and_process_byes.assert_has_calls(
+            [call(f"Line 1{os.linesep}", preserve_whitespace=False), call("Line 2", preserve_whitespace=False)]
+        )
         mock_pattern.assert_not_called()
         self.assertEqual(actual_output, expected_output)
 
@@ -54,7 +57,7 @@ def test_loader_stream_uses_passed_in_stdout(self, patched_Popen, patched_log, p
         patched_Popen.return_value.__enter__.return_value = mock_process
         patched_log.getEffectiveLevel.return_value = logging.INFO
         invoke_subprocess_with_loading_pattern({"args": ["ls"], "stdout": DEVNULL}, mock_pattern, mock_stream_writer)
-        patched_Popen.assert_called_once_with(args=["ls"], stdout=DEVNULL, stderr=PIPE)
+        patched_Popen.assert_called_once_with(args=["ls"], stdout=DEVNULL, stderr=PIPE if not IS_WINDOWS else STDOUT)
 
     @patch("samcli.lib.utils.subprocess_utils.Popen")
     def test_loader_raises_exception_non_zero_exit_code(self, patched_Popen):