Skip to content

Commit

Permalink
fix(workflow-hook): log output with or without errors (runatlantis#3422)
Browse files Browse the repository at this point in the history 
* fix: Errors in Pre or Post Workflow Hook Scripts do not Produce any Output

* Fix post workflow

* Update unit tests
  • Loading branch information
@X-Guardian @ijames-gc
X-Guardian authored and ijames-gc committed Feb 13, 2024
1 parent ceef5f5 commit 8704794
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 53 deletions.
10 changes: 5 additions & 5 deletions server/core/runtime/post_workflow_hook_runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ func (wh DefaultPostWorkflowHookRunner) Run(ctx models.WorkflowHookCommandContex
cmd.Env = finalEnvVars
out, err := cmd.CombinedOutput()

wh.OutputHandler.SendWorkflowHook(ctx, string(out), false)
wh.OutputHandler.SendWorkflowHook(ctx, "\n", true)

if err != nil {
err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, out)
ctx.Log.Debug("error: %s", err)
return "", "", err
return string(out), "", err
}

// Read the value from the "outputFilePath" file
Expand All @@ -67,13 +70,10 @@ func (wh DefaultPostWorkflowHookRunner) Run(ctx models.WorkflowHookCommandContex
if customStatusErr != nil {
err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, out)
ctx.Log.Debug("error: %s", err)
return "", "", err
return string(out), "", err
}
}

wh.OutputHandler.SendWorkflowHook(ctx, string(out), false)
wh.OutputHandler.SendWorkflowHook(ctx, "\n", true)

ctx.Log.Info("successfully ran %q in %q", command, path)
return string(out), strings.Trim(string(customStatusOut), "\n"), nil
}
68 changes: 46 additions & 22 deletions server/core/runtime/post_workflow_hook_runner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

. "github.com/petergtz/pegomock"
"github.com/runatlantis/atlantis/server/core/runtime"
runtimematchers "github.com/runatlantis/atlantis/server/core/runtime/mocks/matchers"
"github.com/runatlantis/atlantis/server/core/terraform/mocks"
matchers2 "github.com/runatlantis/atlantis/server/core/terraform/mocks/matchers"
"github.com/runatlantis/atlantis/server/events/mocks/matchers"
Expand All @@ -23,43 +24,63 @@ func TestPostWorkflowHookRunner_Run(t *testing.T) {
ExpDescription string
}{
{
Command: "",
ExpOut: "",
Command: "",
ExpOut: "",
ExpErr: "",
ExpDescription: "",
},
{
Command: "echo hi",
ExpOut: "hi\n",
Command: "echo hi",
ExpOut: "hi\n",
ExpErr: "",
ExpDescription: "",
},
{
Command: `printf \'your main.tf file does not provide default region.\\ncheck\'`,
ExpOut: `'your`,
Command: `printf \'your main.tf file does not provide default region.\\ncheck\'`,
ExpOut: `'your`,
ExpErr: "",
ExpDescription: "",
},
{
Command: `printf 'your main.tf file does not provide default region.\ncheck'`,
ExpOut: "your main.tf file does not provide default region.\ncheck",
Command: `printf 'your main.tf file does not provide default region.\ncheck'`,
ExpOut: "your main.tf file does not provide default region.\ncheck",
ExpErr: "",
ExpDescription: "",
},
{
Command: "echo 'a",
ExpErr: "exit status 2: running \"echo 'a\" in",
Command: "echo 'a",
ExpOut: "sh: 1: Syntax error: Unterminated quoted string\n",
ExpErr: "exit status 2: running \"echo 'a\" in",
ExpDescription: "",
},
{
Command: "echo hi >> file && cat file",
ExpOut: "hi\n",
Command: "echo hi >> file && cat file",
ExpOut: "hi\n",
ExpErr: "",
ExpDescription: "",
},
{
Command: "lkjlkj",
ExpErr: "exit status 127: running \"lkjlkj\" in",
Command: "lkjlkj",
ExpOut: "sh: 1: lkjlkj: not found\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",
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",
ExpErr: "",
ExpDescription: "",
},
{
Command: "echo user_name=$USER_NAME",
ExpOut: "user_name=acme-user\n",
Command: "echo user_name=$USER_NAME",
ExpOut: "user_name=acme-user\n",
ExpErr: "",
ExpDescription: "",
},
{
Command: "echo something > $OUTPUT_STATUS_FILE",
ExpOut: "",
ExpErr: "",
ExpDescription: "something",
},
}
Expand All @@ -77,8 +98,9 @@ func TestPostWorkflowHookRunner_Run(t *testing.T) {
logger := logging.NewNoopLogger(t)
tmpDir := t.TempDir()

r := runtime.DefaultPostWorkflowHookRunner{
OutputHandler: jobmocks.NewMockProjectCommandOutputHandler(),
projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler()
r := runtime.DefaultPreWorkflowHookRunner{
OutputHandler: projectCmdOutputHandler,
}
t.Run(c.Command, func(t *testing.T) {
ctx := models.WorkflowHookCommandContext{
Expand Down Expand Up @@ -106,15 +128,17 @@ func TestPostWorkflowHookRunner_Run(t *testing.T) {
out, desc, err := r.Run(ctx, c.Command, tmpDir)
if c.ExpErr != "" {
ErrContains(t, c.ExpErr, err)
return
} else {
Ok(t, err)
}
Ok(t, err)
// 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)
projectCmdOutputHandler.VerifyWasCalledOnce().SendWorkflowHook(
runtimematchers.AnyModelsWorkflowHookCommandContext(), EqString(expOut), EqBool(false))
})
}
}
10 changes: 5 additions & 5 deletions server/core/runtime/pre_workflow_hook_runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ func (wh DefaultPreWorkflowHookRunner) Run(ctx models.WorkflowHookCommandContext
cmd.Env = finalEnvVars
out, err := cmd.CombinedOutput()

wh.OutputHandler.SendWorkflowHook(ctx, string(out), false)
wh.OutputHandler.SendWorkflowHook(ctx, "\n", true)

if err != nil {
err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, out)
ctx.Log.Debug("error: %s", err)
return "", "", err
return string(out), "", err
}

// Read the value from the "outputFilePath" file
Expand All @@ -67,13 +70,10 @@ func (wh DefaultPreWorkflowHookRunner) Run(ctx models.WorkflowHookCommandContext
if customStatusErr != nil {
err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, out)
ctx.Log.Debug("error: %s", err)
return "", "", err
return string(out), "", err
}
}

wh.OutputHandler.SendWorkflowHook(ctx, string(out), false)
wh.OutputHandler.SendWorkflowHook(ctx, "\n", true)

ctx.Log.Info("successfully ran %q in %q", command, path)
return string(out), strings.Trim(string(customStatusOut), "\n"), nil
}
66 changes: 45 additions & 21 deletions server/core/runtime/pre_workflow_hook_runner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

. "github.com/petergtz/pegomock"
"github.com/runatlantis/atlantis/server/core/runtime"
runtimematchers "github.com/runatlantis/atlantis/server/core/runtime/mocks/matchers"
"github.com/runatlantis/atlantis/server/core/terraform/mocks"
matchers2 "github.com/runatlantis/atlantis/server/core/terraform/mocks/matchers"
"github.com/runatlantis/atlantis/server/events/mocks/matchers"
Expand All @@ -23,43 +24,63 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) {
ExpDescription string
}{
{
Command: "",
ExpOut: "",
Command: "",
ExpOut: "",
ExpErr: "",
ExpDescription: "",
},
{
Command: "echo hi",
ExpOut: "hi\n",
Command: "echo hi",
ExpOut: "hi\n",
ExpErr: "",
ExpDescription: "",
},
{
Command: `printf \'your main.tf file does not provide default region.\\ncheck\'`,
ExpOut: `'your`,
Command: `printf \'your main.tf file does not provide default region.\\ncheck\'`,
ExpOut: `'your`,
ExpErr: "",
ExpDescription: "",
},
{
Command: `printf 'your main.tf file does not provide default region.\ncheck'`,
ExpOut: "your main.tf file does not provide default region.\ncheck",
Command: `printf 'your main.tf file does not provide default region.\ncheck'`,
ExpOut: "your main.tf file does not provide default region.\ncheck",
ExpErr: "",
ExpDescription: "",
},
{
Command: "echo 'a",
ExpErr: "exit status 2: running \"echo 'a\" in",
Command: "echo 'a",
ExpOut: "sh: 1: Syntax error: Unterminated quoted string\n",
ExpErr: "exit status 2: running \"echo 'a\" in",
ExpDescription: "",
},
{
Command: "echo hi >> file && cat file",
ExpOut: "hi\n",
Command: "echo hi >> file && cat file",
ExpOut: "hi\n",
ExpErr: "",
ExpDescription: "",
},
{
Command: "lkjlkj",
ExpErr: "exit status 127: running \"lkjlkj\" in",
Command: "lkjlkj",
ExpOut: "sh: 1: lkjlkj: not found\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",
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",
ExpErr: "",
ExpDescription: "",
},
{
Command: "echo user_name=$USER_NAME",
ExpOut: "user_name=acme-user\n",
Command: "echo user_name=$USER_NAME",
ExpOut: "user_name=acme-user\n",
ExpErr: "",
ExpDescription: "",
},
{
Command: "echo something > $OUTPUT_STATUS_FILE",
ExpOut: "",
ExpErr: "",
ExpDescription: "something",
},
}
Expand All @@ -77,8 +98,9 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) {
logger := logging.NewNoopLogger(t)
tmpDir := t.TempDir()

projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler()
r := runtime.DefaultPreWorkflowHookRunner{
OutputHandler: jobmocks.NewMockProjectCommandOutputHandler(),
OutputHandler: projectCmdOutputHandler,
}
t.Run(c.Command, func(t *testing.T) {
ctx := models.WorkflowHookCommandContext{
Expand Down Expand Up @@ -106,15 +128,17 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) {
out, desc, err := r.Run(ctx, c.Command, tmpDir)
if c.ExpErr != "" {
ErrContains(t, c.ExpErr, err)
return
} else {
Ok(t, err)
}
Ok(t, err)
// 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)
projectCmdOutputHandler.VerifyWasCalledOnce().SendWorkflowHook(
runtimematchers.AnyModelsWorkflowHookCommandContext(), EqString(expOut), EqBool(false))
})
}
}

0 comments on commit 8704794

Please sign in to comment.