From 50b1e48b4a75c7ebc8580ea8a385c5fbb01c4a80 Mon Sep 17 00:00:00 2001 From: Simon Heather <32168619+X-Guardian@users.noreply.github.com> Date: Tue, 30 May 2023 15:21:56 +0100 Subject: [PATCH] fix(workflow-hook): multiline output formatting (#3425) * Fix Workflow Hool Scripts Multiline Output * Update unit tests * revert bash cmd * Fix tests --- .../core/runtime/post_workflow_hook_runner.go | 5 +++-- .../runtime/post_workflow_hook_runner_test.go | 19 +++++++++---------- .../core/runtime/pre_workflow_hook_runner.go | 5 +++-- .../runtime/pre_workflow_hook_runner_test.go | 19 +++++++++---------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/server/core/runtime/post_workflow_hook_runner.go b/server/core/runtime/post_workflow_hook_runner.go index 0878d0175..2bcbfb7da 100644 --- a/server/core/runtime/post_workflow_hook_runner.go +++ b/server/core/runtime/post_workflow_hook_runner.go @@ -52,7 +52,8 @@ func (wh DefaultPostWorkflowHookRunner) Run(ctx models.WorkflowHookCommandContex cmd.Env = finalEnvVars out, err := cmd.CombinedOutput() - wh.OutputHandler.SendWorkflowHook(ctx, string(out), false) + outString := strings.ReplaceAll(string(out), "\n", "\r\n") + wh.OutputHandler.SendWorkflowHook(ctx, outString, false) wh.OutputHandler.SendWorkflowHook(ctx, "\n", true) if err != nil { @@ -75,5 +76,5 @@ func (wh DefaultPostWorkflowHookRunner) Run(ctx models.WorkflowHookCommandContex } ctx.Log.Info("successfully ran %q in %q", command, path) - return string(out), strings.Trim(string(customStatusOut), "\n"), nil + return outString, strings.Trim(string(customStatusOut), "\n"), nil } diff --git a/server/core/runtime/post_workflow_hook_runner_test.go b/server/core/runtime/post_workflow_hook_runner_test.go index e9ddfc546..f0a23edb6 100644 --- a/server/core/runtime/post_workflow_hook_runner_test.go +++ b/server/core/runtime/post_workflow_hook_runner_test.go @@ -31,7 +31,7 @@ func TestPostWorkflowHookRunner_Run(t *testing.T) { }, { Command: "echo hi", - ExpOut: "hi\n", + ExpOut: "hi\r\n", ExpErr: "", ExpDescription: "", }, @@ -43,37 +43,37 @@ func TestPostWorkflowHookRunner_Run(t *testing.T) { }, { Command: `printf 'your main.tf file does not provide default region.\ncheck'`, - ExpOut: "your main.tf file does not provide default region.\ncheck", + ExpOut: "your main.tf file does not provide default region.\r\ncheck", ExpErr: "", ExpDescription: "", }, { Command: "echo 'a", - ExpOut: "sh: 1: Syntax error: Unterminated quoted string\n", + ExpOut: "sh: 1: Syntax error: Unterminated quoted string\r\n", ExpErr: "exit status 2: running \"echo 'a\" in", ExpDescription: "", }, { Command: "echo hi >> file && cat file", - ExpOut: "hi\n", + ExpOut: "hi\r\n", ExpErr: "", ExpDescription: "", }, { Command: "lkjlkj", - ExpOut: "sh: 1: lkjlkj: not found\n", + ExpOut: "sh: 1: lkjlkj: not found\r\n", ExpErr: "exit status 127: running \"lkjlkj\" in", ExpDescription: "", }, { Command: "echo base_repo_name=$BASE_REPO_NAME base_repo_owner=$BASE_REPO_OWNER head_repo_name=$HEAD_REPO_NAME head_repo_owner=$HEAD_REPO_OWNER head_branch_name=$HEAD_BRANCH_NAME head_commit=$HEAD_COMMIT base_branch_name=$BASE_BRANCH_NAME pull_num=$PULL_NUM pull_url=$PULL_URL pull_author=$PULL_AUTHOR", - ExpOut: "base_repo_name=basename base_repo_owner=baseowner head_repo_name=headname head_repo_owner=headowner head_branch_name=add-feat head_commit=12345abcdef base_branch_name=main pull_num=2 pull_url=https://github.com/runatlantis/atlantis/pull/2 pull_author=acme\n", + ExpOut: "base_repo_name=basename base_repo_owner=baseowner head_repo_name=headname head_repo_owner=headowner head_branch_name=add-feat head_commit=12345abcdef base_branch_name=main pull_num=2 pull_url=https://github.com/runatlantis/atlantis/pull/2 pull_author=acme\r\n", ExpErr: "", ExpDescription: "", }, { Command: "echo user_name=$USER_NAME", - ExpOut: "user_name=acme-user\n", + ExpOut: "user_name=acme-user\r\n", ExpErr: "", ExpDescription: "", }, @@ -125,7 +125,7 @@ func TestPostWorkflowHookRunner_Run(t *testing.T) { }, Log: logger, } - out, desc, err := r.Run(ctx, c.Command, tmpDir) + _, desc, err := r.Run(ctx, c.Command, tmpDir) if c.ExpErr != "" { ErrContains(t, c.ExpErr, err) } else { @@ -134,9 +134,8 @@ func TestPostWorkflowHookRunner_Run(t *testing.T) { // Replace $DIR in the exp with the actual temp dir. We do this // here because when constructing the cases we don't yet know the // temp dir. - expOut := strings.Replace(c.ExpOut, "$DIR", tmpDir, -1) - Equals(t, expOut, out) Equals(t, c.ExpDescription, desc) + expOut := strings.Replace(c.ExpOut, "$DIR", tmpDir, -1) projectCmdOutputHandler.VerifyWasCalledOnce().SendWorkflowHook( runtimematchers.AnyModelsWorkflowHookCommandContext(), EqString(expOut), EqBool(false)) }) diff --git a/server/core/runtime/pre_workflow_hook_runner.go b/server/core/runtime/pre_workflow_hook_runner.go index 605db099a..c0da72129 100644 --- a/server/core/runtime/pre_workflow_hook_runner.go +++ b/server/core/runtime/pre_workflow_hook_runner.go @@ -52,7 +52,8 @@ func (wh DefaultPreWorkflowHookRunner) Run(ctx models.WorkflowHookCommandContext cmd.Env = finalEnvVars out, err := cmd.CombinedOutput() - wh.OutputHandler.SendWorkflowHook(ctx, string(out), false) + outString := strings.ReplaceAll(string(out), "\n", "\r\n") + wh.OutputHandler.SendWorkflowHook(ctx, outString, false) wh.OutputHandler.SendWorkflowHook(ctx, "\n", true) if err != nil { @@ -75,5 +76,5 @@ func (wh DefaultPreWorkflowHookRunner) Run(ctx models.WorkflowHookCommandContext } ctx.Log.Info("successfully ran %q in %q", command, path) - return string(out), strings.Trim(string(customStatusOut), "\n"), nil + return outString, strings.Trim(string(customStatusOut), "\n"), nil } diff --git a/server/core/runtime/pre_workflow_hook_runner_test.go b/server/core/runtime/pre_workflow_hook_runner_test.go index c1ad8c897..3da5dd71d 100644 --- a/server/core/runtime/pre_workflow_hook_runner_test.go +++ b/server/core/runtime/pre_workflow_hook_runner_test.go @@ -31,7 +31,7 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) { }, { Command: "echo hi", - ExpOut: "hi\n", + ExpOut: "hi\r\n", ExpErr: "", ExpDescription: "", }, @@ -43,37 +43,37 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) { }, { Command: `printf 'your main.tf file does not provide default region.\ncheck'`, - ExpOut: "your main.tf file does not provide default region.\ncheck", + ExpOut: "your main.tf file does not provide default region.\r\ncheck", ExpErr: "", ExpDescription: "", }, { Command: "echo 'a", - ExpOut: "sh: 1: Syntax error: Unterminated quoted string\n", + ExpOut: "sh: 1: Syntax error: Unterminated quoted string\r\n", ExpErr: "exit status 2: running \"echo 'a\" in", ExpDescription: "", }, { Command: "echo hi >> file && cat file", - ExpOut: "hi\n", + ExpOut: "hi\r\n", ExpErr: "", ExpDescription: "", }, { Command: "lkjlkj", - ExpOut: "sh: 1: lkjlkj: not found\n", + ExpOut: "sh: 1: lkjlkj: not found\r\n", ExpErr: "exit status 127: running \"lkjlkj\" in", ExpDescription: "", }, { Command: "echo base_repo_name=$BASE_REPO_NAME base_repo_owner=$BASE_REPO_OWNER head_repo_name=$HEAD_REPO_NAME head_repo_owner=$HEAD_REPO_OWNER head_branch_name=$HEAD_BRANCH_NAME head_commit=$HEAD_COMMIT base_branch_name=$BASE_BRANCH_NAME pull_num=$PULL_NUM pull_url=$PULL_URL pull_author=$PULL_AUTHOR", - ExpOut: "base_repo_name=basename base_repo_owner=baseowner head_repo_name=headname head_repo_owner=headowner head_branch_name=add-feat head_commit=12345abcdef base_branch_name=main pull_num=2 pull_url=https://github.com/runatlantis/atlantis/pull/2 pull_author=acme\n", + ExpOut: "base_repo_name=basename base_repo_owner=baseowner head_repo_name=headname head_repo_owner=headowner head_branch_name=add-feat head_commit=12345abcdef base_branch_name=main pull_num=2 pull_url=https://github.com/runatlantis/atlantis/pull/2 pull_author=acme\r\n", ExpErr: "", ExpDescription: "", }, { Command: "echo user_name=$USER_NAME", - ExpOut: "user_name=acme-user\n", + ExpOut: "user_name=acme-user\r\n", ExpErr: "", ExpDescription: "", }, @@ -125,7 +125,7 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) { }, Log: logger, } - out, desc, err := r.Run(ctx, c.Command, tmpDir) + _, desc, err := r.Run(ctx, c.Command, tmpDir) if c.ExpErr != "" { ErrContains(t, c.ExpErr, err) } else { @@ -134,9 +134,8 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) { // Replace $DIR in the exp with the actual temp dir. We do this // here because when constructing the cases we don't yet know the // temp dir. - expOut := strings.Replace(c.ExpOut, "$DIR", tmpDir, -1) - Equals(t, expOut, out) Equals(t, c.ExpDescription, desc) + expOut := strings.Replace(c.ExpOut, "$DIR", tmpDir, -1) projectCmdOutputHandler.VerifyWasCalledOnce().SendWorkflowHook( runtimematchers.AnyModelsWorkflowHookCommandContext(), EqString(expOut), EqBool(false)) })