From f7cd127cb1eda2870725cf94d90961fbefcdb00f Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Mon, 16 May 2022 23:22:53 -0700 Subject: [PATCH 01/22] Start threading job output to RunStepRunner --- .../events/events_controller_e2e_test.go | 5 +- server/core/runtime/env_step_runner_test.go | 7 ++- .../core/runtime/multienv_step_runner_test.go | 7 ++- server/core/runtime/run_step_runner.go | 46 +++++++++++++++++-- server/core/runtime/run_step_runner_test.go | 13 ++++-- server/events/project_command_runner_test.go | 7 ++- server/server.go | 7 +-- 7 files changed, 72 insertions(+), 20 deletions(-) diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 4d2204e256..8d41d3669a 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -973,8 +973,9 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl TerraformExecutor: terraformClient, }, RunStepRunner: &runtime.RunStepRunner{ - TerraformExecutor: terraformClient, - DefaultTFVersion: defaultTFVersion, + TerraformExecutor: terraformClient, + DefaultTFVersion: defaultTFVersion, + ProjectCmdOutputHandler: projectCmdOutputHandler, }, WorkingDir: workingDir, Webhooks: &mockWebhookSender{}, diff --git a/server/core/runtime/env_step_runner_test.go b/server/core/runtime/env_step_runner_test.go index 0084ca97b8..1bf1d872ba 100644 --- a/server/core/runtime/env_step_runner_test.go +++ b/server/core/runtime/env_step_runner_test.go @@ -8,6 +8,7 @@ import ( "github.com/runatlantis/atlantis/server/core/terraform/mocks" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" + jobmocks "github.com/runatlantis/atlantis/server/jobs/mocks" "github.com/runatlantis/atlantis/server/logging" . "github.com/petergtz/pegomock" @@ -40,9 +41,11 @@ func TestEnvStepRunner_Run(t *testing.T) { tfClient := mocks.NewMockClient() tfVersion, err := version.NewVersion("0.12.0") Ok(t, err) + projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() runStepRunner := runtime.RunStepRunner{ - TerraformExecutor: tfClient, - DefaultTFVersion: tfVersion, + TerraformExecutor: tfClient, + DefaultTFVersion: tfVersion, + ProjectCmdOutputHandler: projectCmdOutputHandler, } envRunner := runtime.EnvStepRunner{ RunStepRunner: &runStepRunner, diff --git a/server/core/runtime/multienv_step_runner_test.go b/server/core/runtime/multienv_step_runner_test.go index 7628ea95ea..72ce92352c 100644 --- a/server/core/runtime/multienv_step_runner_test.go +++ b/server/core/runtime/multienv_step_runner_test.go @@ -9,6 +9,7 @@ import ( "github.com/runatlantis/atlantis/server/core/terraform/mocks" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" + jobmocks "github.com/runatlantis/atlantis/server/jobs/mocks" "github.com/runatlantis/atlantis/server/logging" . "github.com/runatlantis/atlantis/testing" ) @@ -31,9 +32,11 @@ func TestMultiEnvStepRunner_Run(t *testing.T) { tfClient := mocks.NewMockClient() tfVersion, err := version.NewVersion("0.12.0") Ok(t, err) + projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() runStepRunner := runtime.RunStepRunner{ - TerraformExecutor: tfClient, - DefaultTFVersion: tfVersion, + TerraformExecutor: tfClient, + DefaultTFVersion: tfVersion, + ProjectCmdOutputHandler: projectCmdOutputHandler, } multiEnvStepRunner := runtime.MultiEnvStepRunner{ RunStepRunner: &runStepRunner, diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 6a7673bf3e..ab465f13f2 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -1,14 +1,19 @@ package runtime import ( + "bufio" + "bytes" "fmt" + "io" "os" "os/exec" "path/filepath" "strings" + "sync" "github.com/hashicorp/go-version" "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/jobs" ) // RunStepRunner runs custom commands. @@ -16,7 +21,8 @@ type RunStepRunner struct { TerraformExecutor TerraformExec DefaultTFVersion *version.Version // TerraformBinDir is the directory where Atlantis downloads Terraform binaries. - TerraformBinDir string + TerraformBinDir string + ProjectCmdOutputHandler jobs.ProjectCommandOutputHandler } func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path string, envs map[string]string) (string, error) { @@ -66,13 +72,45 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str finalEnvVars = append(finalEnvVars, fmt.Sprintf("%s=%s", key, val)) } cmd.Env = finalEnvVars - out, err := cmd.CombinedOutput() + stdout, err := cmd.StdoutPipe() + if err != nil { + err = fmt.Errorf("%s: unable to create stdout buffer", err) + ctx.Log.Debug("error: %s", err) + return "", err + } + stderr, err := cmd.StderrPipe() + if err != nil { + err = fmt.Errorf("%s: unable to create stderr buffer", err) + ctx.Log.Debug("error: %s", err) + return "", err + } + cmd.Start() + + var output bytes.Buffer + var mutex sync.Mutex + + go r.streamOutput(ctx, stdout, &output, &mutex) + go r.streamOutput(ctx, stderr, &output, &mutex) + + err = cmd.Wait() if err != nil { - err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, out) + err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, output.String()) ctx.Log.Debug("error: %s", err) return "", err } ctx.Log.Info("successfully ran %q in %q", command, path) - return string(out), nil + return string(output.String()), nil +} + +func (r RunStepRunner) streamOutput(ctx command.ProjectContext, reader io.Reader, buffer *bytes.Buffer, mutex *sync.Mutex) { + scanner := bufio.NewScanner(reader) + for scanner.Scan() { + line := scanner.Text() + r.ProjectCmdOutputHandler.Send(ctx, line, false) + mutex.Lock() + buffer.WriteString(line) + buffer.WriteString("\n") + mutex.Unlock() + } } diff --git a/server/core/runtime/run_step_runner_test.go b/server/core/runtime/run_step_runner_test.go index 8536a331ea..89d3f2db27 100644 --- a/server/core/runtime/run_step_runner_test.go +++ b/server/core/runtime/run_step_runner_test.go @@ -14,6 +14,7 @@ import ( "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/mocks/matchers" "github.com/runatlantis/atlantis/server/events/models" + jobmocks "github.com/runatlantis/atlantis/server/jobs/mocks" "github.com/runatlantis/atlantis/server/logging" . "github.com/runatlantis/atlantis/testing" ) @@ -38,11 +39,11 @@ func TestRunStepRunner_Run(t *testing.T) { }, { Command: `printf \'your main.tf file does not provide default region.\\ncheck\'`, - ExpOut: `'your`, + ExpOut: "'your\n", }, { 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.\ncheck\n", }, { Command: "echo 'a", @@ -104,11 +105,13 @@ func TestRunStepRunner_Run(t *testing.T) { ThenReturn(nil) logger := logging.NewNoopLogger(t) + projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() r := runtime.RunStepRunner{ - TerraformExecutor: terraform, - DefaultTFVersion: defaultVersion, - TerraformBinDir: "/bin/dir", + TerraformExecutor: terraform, + DefaultTFVersion: defaultVersion, + TerraformBinDir: "/bin/dir", + ProjectCmdOutputHandler: projectCmdOutputHandler, } t.Run(c.Command, func(t *testing.T) { tmpDir, cleanup := TempDir(t) diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go index d057581847..b0bb776668 100644 --- a/server/events/project_command_runner_test.go +++ b/server/events/project_command_runner_test.go @@ -30,6 +30,7 @@ import ( eventmocks "github.com/runatlantis/atlantis/server/events/mocks" "github.com/runatlantis/atlantis/server/events/mocks/matchers" "github.com/runatlantis/atlantis/server/events/models" + jobmocks "github.com/runatlantis/atlantis/server/jobs/mocks" "github.com/runatlantis/atlantis/server/logging" . "github.com/runatlantis/atlantis/testing" ) @@ -537,9 +538,11 @@ func TestDefaultProjectCommandRunner_RunEnvSteps(t *testing.T) { tfClient := tmocks.NewMockClient() tfVersion, err := version.NewVersion("0.12.0") Ok(t, err) + projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() run := runtime.RunStepRunner{ - TerraformExecutor: tfClient, - DefaultTFVersion: tfVersion, + TerraformExecutor: tfClient, + DefaultTFVersion: tfVersion, + ProjectCmdOutputHandler: projectCmdOutputHandler, } env := runtime.EnvStepRunner{ RunStepRunner: &run, diff --git a/server/server.go b/server/server.go index a6546a70ca..c06fe54c13 100644 --- a/server/server.go +++ b/server/server.go @@ -466,9 +466,10 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { defaultTfVersion := terraformClient.DefaultVersion() pendingPlanFinder := &events.DefaultPendingPlanFinder{} runStepRunner := &runtime.RunStepRunner{ - TerraformExecutor: terraformClient, - DefaultTFVersion: defaultTfVersion, - TerraformBinDir: terraformClient.TerraformBinDir(), + TerraformExecutor: terraformClient, + DefaultTFVersion: defaultTfVersion, + TerraformBinDir: terraformClient.TerraformBinDir(), + ProjectCmdOutputHandler: projectCmdOutputHandler, } drainer := &events.Drainer{} statusController := &controllers.StatusController{ From 0200c62ec261f2a51d334a3d2c1172f60085e5d4 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Mon, 16 May 2022 23:42:54 -0700 Subject: [PATCH 02/22] Strip ANSI --- server/core/runtime/run_step_runner.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index ab465f13f2..ab37739032 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/go-version" "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/events/terraform/ansi" "github.com/runatlantis/atlantis/server/jobs" ) @@ -100,7 +101,7 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str return "", err } ctx.Log.Info("successfully ran %q in %q", command, path) - return string(output.String()), nil + return ansi.Strip(output.String()), nil } func (r RunStepRunner) streamOutput(ctx command.ProjectContext, reader io.Reader, buffer *bytes.Buffer, mutex *sync.Mutex) { From 22388bc5cdab17782449ac8732a5fb41d32204ae Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 00:07:11 -0700 Subject: [PATCH 03/22] Fix lint --- server/core/runtime/run_step_runner.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index ab37739032..36b56d6b88 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -85,13 +85,17 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str ctx.Log.Debug("error: %s", err) return "", err } - cmd.Start() + if err := cmd.Start(); err != nil { + err = fmt.Errorf("%s: unable to start command %q", err, command) + ctx.Log.Debug("error: %s", err) + return "", err + } - var output bytes.Buffer - var mutex sync.Mutex + output := &bytes.Buffer{} + mutex := &sync.Mutex{} - go r.streamOutput(ctx, stdout, &output, &mutex) - go r.streamOutput(ctx, stderr, &output, &mutex) + go r.streamOutput(ctx, stdout, output, mutex) + go r.streamOutput(ctx, stderr, output, mutex) err = cmd.Wait() @@ -104,7 +108,7 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str return ansi.Strip(output.String()), nil } -func (r RunStepRunner) streamOutput(ctx command.ProjectContext, reader io.Reader, buffer *bytes.Buffer, mutex *sync.Mutex) { +func (r RunStepRunner) streamOutput(ctx command.ProjectContext, reader io.Reader, buffer io.StringWriter, mutex *sync.Mutex) { scanner := bufio.NewScanner(reader) for scanner.Scan() { line := scanner.Text() From efb38a8c664f9716248d74ab2540fa4c0a651cb1 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 00:13:21 -0700 Subject: [PATCH 04/22] Use waitgroup to avoid test flakiness --- server/core/runtime/run_step_runner.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 36b56d6b88..43cda7a787 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -91,11 +91,21 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str return "", err } - output := &bytes.Buffer{} - mutex := &sync.Mutex{} + output := new(bytes.Buffer) + mutex := new(sync.Mutex) - go r.streamOutput(ctx, stdout, output, mutex) - go r.streamOutput(ctx, stderr, output, mutex) + // Use a waitgroup to block until our stdout/err copying is complete. + wg := new(sync.WaitGroup) + wg.Add(2) + + go func() { + r.streamOutput(ctx, stdout, output, mutex) + wg.Done() + }() + go func() { + r.streamOutput(ctx, stderr, output, mutex) + wg.Done() + }() err = cmd.Wait() @@ -105,6 +115,7 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str return "", err } ctx.Log.Info("successfully ran %q in %q", command, path) + wg.Wait() return ansi.Strip(output.String()), nil } From 831a8f6fe12d6759eb16305dc855de1ed20bbee6 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 00:13:49 -0700 Subject: [PATCH 05/22] Move waitgroup higher --- server/core/runtime/run_step_runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 43cda7a787..5c927d2b67 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -108,6 +108,7 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str }() err = cmd.Wait() + wg.Wait() if err != nil { err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, output.String()) @@ -115,7 +116,6 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str return "", err } ctx.Log.Info("successfully ran %q in %q", command, path) - wg.Wait() return ansi.Strip(output.String()), nil } From 589b43c3ea7b0f4d0e94049b08a31989fd500bc2 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 08:34:45 -0700 Subject: [PATCH 06/22] Add ANSI test and use strings.Builder --- server/core/runtime/run_step_runner.go | 3 +-- server/core/runtime/run_step_runner_test.go | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 5c927d2b67..9a59863e5f 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -2,7 +2,6 @@ package runtime import ( "bufio" - "bytes" "fmt" "io" "os" @@ -91,7 +90,7 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str return "", err } - output := new(bytes.Buffer) + output := new(strings.Builder) mutex := new(sync.Mutex) // Use a waitgroup to block until our stdout/err copying is complete. diff --git a/server/core/runtime/run_step_runner_test.go b/server/core/runtime/run_step_runner_test.go index 89d3f2db27..8ce664beeb 100644 --- a/server/core/runtime/run_step_runner_test.go +++ b/server/core/runtime/run_step_runner_test.go @@ -45,6 +45,10 @@ func TestRunStepRunner_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\n", }, + { + Command: `echo "\e[0;32mgreen"`, + ExpOut: "green\n", + }, { Command: "echo 'a", ExpErr: "exit status 2: running \"echo 'a\" in", From 51cfc3923917190de2774f775ad44bf218bcb094 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 08:49:27 -0700 Subject: [PATCH 07/22] Fix lint --- server/core/runtime/run_step_runner.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 9a59863e5f..e2cfd16602 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -118,14 +118,14 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str return ansi.Strip(output.String()), nil } -func (r RunStepRunner) streamOutput(ctx command.ProjectContext, reader io.Reader, buffer io.StringWriter, mutex *sync.Mutex) { +func (r RunStepRunner) streamOutput(ctx command.ProjectContext, reader io.Reader, buffer io.StringWriter, mutex sync.Locker) { scanner := bufio.NewScanner(reader) for scanner.Scan() { line := scanner.Text() r.ProjectCmdOutputHandler.Send(ctx, line, false) mutex.Lock() - buffer.WriteString(line) - buffer.WriteString("\n") + _, _ = buffer.WriteString(line) + _, _ = buffer.WriteString("\n") mutex.Unlock() } } From bdae433866e1f7263d7210d5862369d134e7a39f Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 09:29:49 -0700 Subject: [PATCH 08/22] Use errors.Wrap per style guide --- server/core/runtime/run_step_runner.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index e2cfd16602..2a01d7352f 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -11,6 +11,7 @@ import ( "sync" "github.com/hashicorp/go-version" + "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/terraform/ansi" "github.com/runatlantis/atlantis/server/jobs" @@ -74,18 +75,18 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str cmd.Env = finalEnvVars stdout, err := cmd.StdoutPipe() if err != nil { - err = fmt.Errorf("%s: unable to create stdout buffer", err) + err = errors.Wrap(err, "opening stdout stream") ctx.Log.Debug("error: %s", err) return "", err } stderr, err := cmd.StderrPipe() if err != nil { - err = fmt.Errorf("%s: unable to create stderr buffer", err) + err = errors.Wrap(err, "opening stderr stream") ctx.Log.Debug("error: %s", err) return "", err } if err := cmd.Start(); err != nil { - err = fmt.Errorf("%s: unable to start command %q", err, command) + err = errors.Wrapf(err, "starting command %q", command) ctx.Log.Debug("error: %s", err) return "", err } From f1fda9d7734c2e063e667ee3ba34e8eac86dbdac Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 11:31:50 -0700 Subject: [PATCH 09/22] Create ShellCommandRunner to encapsulate streaming --- server/core/runtime/run_step_runner.go | 63 +------- server/core/runtime/shell_command_runner.go | 156 ++++++++++++++++++++ 2 files changed, 160 insertions(+), 59 deletions(-) create mode 100644 server/core/runtime/shell_command_runner.go diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 2a01d7352f..87402f464d 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -1,19 +1,13 @@ package runtime import ( - "bufio" "fmt" - "io" "os" - "os/exec" "path/filepath" "strings" - "sync" "github.com/hashicorp/go-version" - "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/events/command" - "github.com/runatlantis/atlantis/server/events/terraform/ansi" "github.com/runatlantis/atlantis/server/jobs" ) @@ -39,9 +33,6 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str return "", err } - cmd := exec.Command("sh", "-c", command) // #nosec - cmd.Dir = path - baseEnvVars := os.Environ() customEnvVars := map[string]string{ "ATLANTIS_TERRAFORM_VERSION": tfVersion.String(), @@ -72,61 +63,15 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str for key, val := range envs { finalEnvVars = append(finalEnvVars, fmt.Sprintf("%s=%s", key, val)) } - cmd.Env = finalEnvVars - stdout, err := cmd.StdoutPipe() - if err != nil { - err = errors.Wrap(err, "opening stdout stream") - ctx.Log.Debug("error: %s", err) - return "", err - } - stderr, err := cmd.StderrPipe() - if err != nil { - err = errors.Wrap(err, "opening stderr stream") - ctx.Log.Debug("error: %s", err) - return "", err - } - if err := cmd.Start(); err != nil { - err = errors.Wrapf(err, "starting command %q", command) - ctx.Log.Debug("error: %s", err) - return "", err - } - - output := new(strings.Builder) - mutex := new(sync.Mutex) - - // Use a waitgroup to block until our stdout/err copying is complete. - wg := new(sync.WaitGroup) - wg.Add(2) - go func() { - r.streamOutput(ctx, stdout, output, mutex) - wg.Done() - }() - go func() { - r.streamOutput(ctx, stderr, output, mutex) - wg.Done() - }() - - err = cmd.Wait() - wg.Wait() + runner := NewShellCommandRunner(command, finalEnvVars, path, r.ProjectCmdOutputHandler) + output, err := runner.Run(ctx) if err != nil { - err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, output.String()) + err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, output) ctx.Log.Debug("error: %s", err) return "", err } ctx.Log.Info("successfully ran %q in %q", command, path) - return ansi.Strip(output.String()), nil -} - -func (r RunStepRunner) streamOutput(ctx command.ProjectContext, reader io.Reader, buffer io.StringWriter, mutex sync.Locker) { - scanner := bufio.NewScanner(reader) - for scanner.Scan() { - line := scanner.Text() - r.ProjectCmdOutputHandler.Send(ctx, line, false) - mutex.Lock() - _, _ = buffer.WriteString(line) - _, _ = buffer.WriteString("\n") - mutex.Unlock() - } + return output, nil } diff --git a/server/core/runtime/shell_command_runner.go b/server/core/runtime/shell_command_runner.go new file mode 100644 index 0000000000..9ccc93415f --- /dev/null +++ b/server/core/runtime/shell_command_runner.go @@ -0,0 +1,156 @@ +package runtime + +import ( + "bufio" + "io" + "os/exec" + "strings" + "sync" + + "github.com/pkg/errors" + "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/events/terraform/ansi" + "github.com/runatlantis/atlantis/server/jobs" +) + +// Setting the buffer size to 10mb +const BufioScannerBufferSize = 10 * 1024 * 1024 + +// Line represents a line that was output from a shell command. +type Line struct { + // Line is the contents of the line (without the newline). + Line string + // Err is set if there was an error. + Err error +} + +// ShellCommandRunner runs a command via `exec.Command` and streams output to the +// `ProjectCommandOutputHandler`. +type ShellCommandRunner struct { + command string + environ []string + workingDir string + outputHandler jobs.ProjectCommandOutputHandler + cmd *exec.Cmd +} + +func NewShellCommandRunner(command string, environ []string, workingDir string, outputHandler jobs.ProjectCommandOutputHandler) *ShellCommandRunner { + cmd := exec.Command("sh", "-c", command) // #nosec + cmd.Env = environ + cmd.Dir = workingDir + + return &ShellCommandRunner{ + command: command, + workingDir: workingDir, + outputHandler: outputHandler, + cmd: cmd, + } +} + +func (s *ShellCommandRunner) Run(ctx command.ProjectContext) (string, error) { + _, outCh := s.runCommandAsync(ctx) + + outbuf := new(strings.Builder) + var err error + for line := range outCh { + if line.Err != nil { + err = line.Err + break + } + outbuf.WriteString(line.Line) + outbuf.WriteString("\n") + } + + // sanitize output by stripping out any ansi characters. + output := ansi.Strip(outbuf.String()) + return output, err +} + +// RunCommandAsync runs terraform with args. It immediately returns an +// input and output channel. Callers can use the output channel to +// get the realtime output from the command. +// Callers can use the input channel to pass stdin input to the command. +// If any error is passed on the out channel, there will be no +// further output (so callers are free to exit). +func (s *ShellCommandRunner) runCommandAsync(ctx command.ProjectContext) (chan<- string, <-chan Line) { + outCh := make(chan Line) + inCh := make(chan string) + + // We start a goroutine to do our work asynchronously and then immediately + // return our channels. + go func() { + // Ensure we close our channels when we exit. + defer func() { + close(outCh) + close(inCh) + }() + + stdout, _ := s.cmd.StdoutPipe() + stderr, _ := s.cmd.StderrPipe() + stdin, _ := s.cmd.StdinPipe() + + ctx.Log.Debug("starting %q in %q", s.cmd, s.workingDir) + err := s.cmd.Start() + if err != nil { + err = errors.Wrapf(err, "running %q in %q", s.cmd, s.workingDir) + ctx.Log.Err(err.Error()) + outCh <- Line{Err: err} + return + } + + // If we get anything on inCh, write it to stdin. + // This function will exit when inCh is closed which we do in our defer. + go func() { + for line := range inCh { + ctx.Log.Debug("writing %q to remote command's stdin", line) + _, err := io.WriteString(stdin, line) + if err != nil { + ctx.Log.Err(errors.Wrapf(err, "writing %q to process", line).Error()) + } + } + }() + + wg := new(sync.WaitGroup) + wg.Add(2) + // Asynchronously copy from stdout/err to outCh. + go func() { + scanner := bufio.NewScanner(stdout) + buf := []byte{} + scanner.Buffer(buf, BufioScannerBufferSize) + + for scanner.Scan() { + message := scanner.Text() + outCh <- Line{Line: message} + s.outputHandler.Send(ctx, message, false) + } + wg.Done() + }() + go func() { + scanner := bufio.NewScanner(stderr) + for scanner.Scan() { + message := scanner.Text() + outCh <- Line{Line: message} + s.outputHandler.Send(ctx, message, false) + } + wg.Done() + }() + + // Wait for our copying to complete. This *must* be done before + // calling cmd.Wait(). (see https://github.com/golang/go/issues/19685) + wg.Wait() + + // Wait for the command to complete. + err = s.cmd.Wait() + + // We're done now. Send an error if there was one. + if err != nil { + err = errors.Wrapf(err, "running %q in %q", s.cmd, s.workingDir) + ctx.Log.Err(err.Error()) + outCh <- Line{Err: err} + } else { + ctx.Log.Info("successfully ran %q in %q", s.cmd, s.workingDir) + } + }() + + return inCh, outCh +} From a6567b5f2865329663acb8a7a015a6a4c657152a Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 12:02:40 -0700 Subject: [PATCH 10/22] WIP: shell command runner --- server/core/runtime/apply_step_runner.go | 72 +++++---- .../{ => models}/shell_command_runner.go | 6 +- server/core/runtime/plan_step_runner.go | 37 ++--- server/core/runtime/run_step_runner.go | 3 +- server/core/runtime/runtime.go | 4 +- .../terraform/mocks/mock_terraform_client.go | 30 ++-- server/core/terraform/terraform_client.go | 153 +++++------------- 7 files changed, 109 insertions(+), 196 deletions(-) rename server/core/runtime/{ => models}/shell_command_runner.go (97%) diff --git a/server/core/runtime/apply_step_runner.go b/server/core/runtime/apply_step_runner.go index 67e1f58444..9f01919d3e 100644 --- a/server/core/runtime/apply_step_runner.go +++ b/server/core/runtime/apply_step_runner.go @@ -133,48 +133,50 @@ func (a *ApplyStepRunner) runRemoteApply( // Start the async command execution. ctx.Log.Debug("starting async tf remote operation") - inCh, outCh := a.AsyncTFExec.RunCommandAsync(ctx, filepath.Clean(path), applyArgs, envs, tfVersion, ctx.Workspace) + inCh, outCh, err := a.AsyncTFExec.RunCommandAsync(ctx, filepath.Clean(path), applyArgs, envs, tfVersion, ctx.Workspace) var lines []string nextLineIsRunURL := false var runURL string var planChangedErr error - for line := range outCh { - if line.Err != nil { - err = line.Err - break - } - lines = append(lines, line.Line) - - // Here we're checking for the run url and updating the status - // if found. - if line.Line == lineBeforeRunURL { - nextLineIsRunURL = true - } else if nextLineIsRunURL { - runURL = strings.TrimSpace(line.Line) - ctx.Log.Debug("remote run url found, updating commit status") - updateStatusF(models.PendingCommitStatus, runURL) - nextLineIsRunURL = false - } - - // If the plan is complete and it's waiting for us to verify the apply, - // check if the plan is the same and if so, input "yes". - if a.atConfirmApplyPrompt(lines) { - ctx.Log.Debug("remote apply is waiting for confirmation") - - // Check if the plan is as expected. - planChangedErr = a.remotePlanChanged(string(planfileBytes), strings.Join(lines, "\n"), tfVersion) - if planChangedErr != nil { - ctx.Log.Err("plan generated during apply does not match expected plan, aborting") - inCh <- "no\n" - // Need to continue so we read all the lines, otherwise channel - // sender (in TerraformClient) will block indefinitely waiting - // for us to read. - continue + if err != nil { + for line := range outCh { + if line.Err != nil { + err = line.Err + break + } + lines = append(lines, line.Line) + + // Here we're checking for the run url and updating the status + // if found. + if line.Line == lineBeforeRunURL { + nextLineIsRunURL = true + } else if nextLineIsRunURL { + runURL = strings.TrimSpace(line.Line) + ctx.Log.Debug("remote run url found, updating commit status") + updateStatusF(models.PendingCommitStatus, runURL) + nextLineIsRunURL = false } - ctx.Log.Debug("plan generated during apply matches expected plan, continuing") - inCh <- "yes\n" + // If the plan is complete and it's waiting for us to verify the apply, + // check if the plan is the same and if so, input "yes". + if a.atConfirmApplyPrompt(lines) { + ctx.Log.Debug("remote apply is waiting for confirmation") + + // Check if the plan is as expected. + planChangedErr = a.remotePlanChanged(string(planfileBytes), strings.Join(lines, "\n"), tfVersion) + if planChangedErr != nil { + ctx.Log.Err("plan generated during apply does not match expected plan, aborting") + inCh <- "no\n" + // Need to continue so we read all the lines, otherwise channel + // sender (in TerraformClient) will block indefinitely waiting + // for us to read. + continue + } + + ctx.Log.Debug("plan generated during apply matches expected plan, continuing") + inCh <- "yes\n" + } } } diff --git a/server/core/runtime/shell_command_runner.go b/server/core/runtime/models/shell_command_runner.go similarity index 97% rename from server/core/runtime/shell_command_runner.go rename to server/core/runtime/models/shell_command_runner.go index 9ccc93415f..f83e544337 100644 --- a/server/core/runtime/shell_command_runner.go +++ b/server/core/runtime/models/shell_command_runner.go @@ -1,4 +1,4 @@ -package runtime +package models import ( "bufio" @@ -48,7 +48,7 @@ func NewShellCommandRunner(command string, environ []string, workingDir string, } func (s *ShellCommandRunner) Run(ctx command.ProjectContext) (string, error) { - _, outCh := s.runCommandAsync(ctx) + _, outCh := s.RunCommandAsync(ctx) outbuf := new(strings.Builder) var err error @@ -72,7 +72,7 @@ func (s *ShellCommandRunner) Run(ctx command.ProjectContext) (string, error) { // Callers can use the input channel to pass stdin input to the command. // If any error is passed on the out channel, there will be no // further output (so callers are free to exit). -func (s *ShellCommandRunner) runCommandAsync(ctx command.ProjectContext) (chan<- string, <-chan Line) { +func (s *ShellCommandRunner) RunCommandAsync(ctx command.ProjectContext) (chan<- string, <-chan Line) { outCh := make(chan Line) inCh := make(chan string) diff --git a/server/core/runtime/plan_step_runner.go b/server/core/runtime/plan_step_runner.go index a328c09daa..1f435124ae 100644 --- a/server/core/runtime/plan_step_runner.go +++ b/server/core/runtime/plan_step_runner.go @@ -253,28 +253,29 @@ func (p *PlanStepRunner) runRemotePlan( // Start the async command execution. ctx.Log.Debug("starting async tf remote operation") - _, outCh := p.AsyncTFExec.RunCommandAsync(ctx, filepath.Clean(path), cmdArgs, envs, tfVersion, ctx.Workspace) + _, outCh, err := p.AsyncTFExec.RunCommandAsync(ctx, filepath.Clean(path), cmdArgs, envs, tfVersion, ctx.Workspace) var lines []string nextLineIsRunURL := false var runURL string - var err error - for line := range outCh { - if line.Err != nil { - err = line.Err - break - } - lines = append(lines, line.Line) - - // Here we're checking for the run url and updating the status - // if found. - if line.Line == lineBeforeRunURL { - nextLineIsRunURL = true - } else if nextLineIsRunURL { - runURL = strings.TrimSpace(line.Line) - ctx.Log.Debug("remote run url found, updating commit status") - updateStatusF(models.PendingCommitStatus, runURL) - nextLineIsRunURL = false + if err != nil { + for line := range outCh { + if line.Err != nil { + err = line.Err + break + } + lines = append(lines, line.Line) + + // Here we're checking for the run url and updating the status + // if found. + if line.Line == lineBeforeRunURL { + nextLineIsRunURL = true + } else if nextLineIsRunURL { + runURL = strings.TrimSpace(line.Line) + ctx.Log.Debug("remote run url found, updating commit status") + updateStatusF(models.PendingCommitStatus, runURL) + nextLineIsRunURL = false + } } } diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 87402f464d..e8e8f6c98f 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/hashicorp/go-version" + "github.com/runatlantis/atlantis/server/core/runtime/models" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/jobs" ) @@ -64,7 +65,7 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str finalEnvVars = append(finalEnvVars, fmt.Sprintf("%s=%s", key, val)) } - runner := NewShellCommandRunner(command, finalEnvVars, path, r.ProjectCmdOutputHandler) + runner := models.NewShellCommandRunner(command, finalEnvVars, path, r.ProjectCmdOutputHandler) output, err := runner.Run(ctx) if err != nil { diff --git a/server/core/runtime/runtime.go b/server/core/runtime/runtime.go index 7bd575308d..0f15d761f2 100644 --- a/server/core/runtime/runtime.go +++ b/server/core/runtime/runtime.go @@ -10,7 +10,7 @@ import ( version "github.com/hashicorp/go-version" "github.com/pkg/errors" - "github.com/runatlantis/atlantis/server/core/terraform" + runtimemodels "github.com/runatlantis/atlantis/server/core/runtime/models" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/logging" @@ -41,7 +41,7 @@ type AsyncTFExec interface { // Callers can use the input channel to pass stdin input to the command. // If any error is passed on the out channel, there will be no // further output (so callers are free to exit). - RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan terraform.Line) + RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line, error) } // StatusUpdater brings the interface from CommitStatusUpdater into this package diff --git a/server/core/terraform/mocks/mock_terraform_client.go b/server/core/terraform/mocks/mock_terraform_client.go index 76ccc3da48..746f39a0c7 100644 --- a/server/core/terraform/mocks/mock_terraform_client.go +++ b/server/core/terraform/mocks/mock_terraform_client.go @@ -4,14 +4,12 @@ package mocks import ( - "reflect" - "time" - go_version "github.com/hashicorp/go-version" pegomock "github.com/petergtz/pegomock" - "github.com/runatlantis/atlantis/server/core/terraform" - "github.com/runatlantis/atlantis/server/events/command" + command "github.com/runatlantis/atlantis/server/events/command" logging "github.com/runatlantis/atlantis/server/logging" + "reflect" + "time" ) type MockClient struct { @@ -48,16 +46,6 @@ func (mock *MockClient) RunCommandWithVersion(ctx command.ProjectContext, path s return ret0, ret1 } -func (mock *MockClient) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *go_version.Version, workspace string) (chan<- string, <-chan terraform.Line) { - if mock == nil { - panic("mock must not be nil. Use myMock := NewMockClient().") - } - outCh := make(chan terraform.Line) - inCh := make(chan string) - - return inCh, outCh -} - func (mock *MockClient) EnsureVersion(log logging.SimpleLogging, v *go_version.Version) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") @@ -121,17 +109,17 @@ type MockClient_RunCommandWithVersion_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockClient_RunCommandWithVersion_OngoingVerification) GetCapturedArguments() (logging.SimpleLogging, string, []string, map[string]string, *go_version.Version, string) { - log, path, args, envs, v, workspace := c.GetAllCapturedArguments() - return log[len(log)-1], path[len(path)-1], args[len(args)-1], envs[len(envs)-1], v[len(v)-1], workspace[len(workspace)-1] +func (c *MockClient_RunCommandWithVersion_OngoingVerification) GetCapturedArguments() (command.ProjectContext, string, []string, map[string]string, *go_version.Version, string) { + ctx, path, args, envs, v, workspace := c.GetAllCapturedArguments() + return ctx[len(ctx)-1], path[len(path)-1], args[len(args)-1], envs[len(envs)-1], v[len(v)-1], workspace[len(workspace)-1] } -func (c *MockClient_RunCommandWithVersion_OngoingVerification) GetAllCapturedArguments() (_param0 []logging.SimpleLogging, _param1 []string, _param2 [][]string, _param3 []map[string]string, _param4 []*go_version.Version, _param5 []string) { +func (c *MockClient_RunCommandWithVersion_OngoingVerification) GetAllCapturedArguments() (_param0 []command.ProjectContext, _param1 []string, _param2 [][]string, _param3 []map[string]string, _param4 []*go_version.Version, _param5 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]logging.SimpleLogging, len(c.methodInvocations)) + _param0 = make([]command.ProjectContext, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(logging.SimpleLogging) + _param0[u] = param.(command.ProjectContext) } _param1 = make([]string, len(c.methodInvocations)) for u, param := range params[1] { diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index f6e5c92a42..6516491ffe 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -15,9 +15,7 @@ package terraform import ( - "bufio" "fmt" - "io" "os" "os/exec" "path/filepath" @@ -31,6 +29,7 @@ import ( "github.com/mitchellh/go-homedir" "github.com/pkg/errors" + "github.com/runatlantis/atlantis/server/core/runtime/models" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/terraform/ansi" "github.com/runatlantis/atlantis/server/jobs" @@ -280,16 +279,17 @@ func (c *DefaultClient) EnsureVersion(log logging.SimpleLogging, v *version.Vers // See Client.RunCommandWithVersion. func (c *DefaultClient) RunCommandWithVersion(ctx command.ProjectContext, path string, args []string, customEnvVars map[string]string, v *version.Version, workspace string) (string, error) { if isAsyncEligibleCommand(args[0]) { - _, outCh := c.RunCommandAsync(ctx, path, args, customEnvVars, v, workspace) + _, outCh, err := c.RunCommandAsync(ctx, path, args, customEnvVars, v, workspace) var lines []string - var err error - for line := range outCh { - if line.Err != nil { - err = line.Err - break + if err != nil { + for line := range outCh { + if line.Err != nil { + err = line.Err + break + } + lines = append(lines, line.Line) } - lines = append(lines, line.Line) } output := strings.Join(lines, "\n") @@ -297,7 +297,7 @@ func (c *DefaultClient) RunCommandWithVersion(ctx command.ProjectContext, path s output = ansi.Strip(output) return fmt.Sprintf("%s\n", output), err } - tfCmd, cmd, err := c.prepCmd(ctx.Log, v, workspace, path, args) + tfCmd, cmd, err := c.prepExecCmd(ctx.Log, v, workspace, path, args) if err != nil { return "", err } @@ -317,10 +317,23 @@ func (c *DefaultClient) RunCommandWithVersion(ctx command.ProjectContext, path s return ansi.Strip(string(out)), nil } -// prepCmd builds a ready to execute command based on the version of terraform +// prepExecCmd builds a ready to execute command based on the version of terraform // v, and args. It returns a printable representation of the command that will // be run and the actual command. -func (c *DefaultClient) prepCmd(log logging.SimpleLogging, v *version.Version, workspace string, path string, args []string) (string, *exec.Cmd, error) { +func (c *DefaultClient) prepExecCmd(log logging.SimpleLogging, v *version.Version, workspace string, path string, args []string) (string, *exec.Cmd, error) { + tfCmd, envVars, err := c.prepCmd(log, v, workspace, path, args) + if err != nil { + return "", nil, err + } + cmd := exec.Command("sh", "-c", tfCmd) + cmd.Dir = path + cmd.Env = envVars + return tfCmd, cmd, nil +} + +// prepCmd prepares a shell command (to be interpreted with `sh -c `) and set of environment +// variables for running terraform. +func (c *DefaultClient) prepCmd(log logging.SimpleLogging, v *version.Version, workspace string, path string, args []string) (string, []string, error) { if v == nil { v = c.defaultVersion } @@ -356,18 +369,7 @@ func (c *DefaultClient) prepCmd(log logging.SimpleLogging, v *version.Version, w // AWS_ACCESS_KEY. envVars = append(envVars, os.Environ()...) tfCmd := fmt.Sprintf("%s %s", binPath, strings.Join(args, " ")) - cmd := exec.Command("sh", "-c", tfCmd) - cmd.Dir = path - cmd.Env = envVars - return tfCmd, cmd, nil -} - -// Line represents a line that was output from a terraform command. -type Line struct { - // Line is the contents of the line (without the newline). - Line string - // Err is set if there was an error. - Err error + return tfCmd, envVars, nil } // RunCommandAsync runs terraform with args. It immediately returns an @@ -376,100 +378,19 @@ type Line struct { // Callers can use the input channel to pass stdin input to the command. // If any error is passed on the out channel, there will be no // further output (so callers are free to exit). -func (c *DefaultClient) RunCommandAsync(ctx command.ProjectContext, path string, args []string, customEnvVars map[string]string, v *version.Version, workspace string) (chan<- string, <-chan Line) { - outCh := make(chan Line) - inCh := make(chan string) - - // We start a goroutine to do our work asynchronously and then immediately - // return our channels. - go func() { - - // Ensure we close our channels when we exit. - defer func() { - close(outCh) - close(inCh) - }() - - tfCmd, cmd, err := c.prepCmd(ctx.Log, v, workspace, path, args) - if err != nil { - ctx.Log.Err(err.Error()) - outCh <- Line{Err: err} - return - } - stdout, _ := cmd.StdoutPipe() - stderr, _ := cmd.StderrPipe() - stdin, _ := cmd.StdinPipe() - envVars := cmd.Env - for key, val := range customEnvVars { - envVars = append(envVars, fmt.Sprintf("%s=%s", key, val)) - } - cmd.Env = envVars - - ctx.Log.Debug("starting %q in %q", tfCmd, path) - err = cmd.Start() - if err != nil { - err = errors.Wrapf(err, "running %q in %q", tfCmd, path) - ctx.Log.Err(err.Error()) - outCh <- Line{Err: err} - return - } - - // If we get anything on inCh, write it to stdin. - // This function will exit when inCh is closed which we do in our defer. - go func() { - for line := range inCh { - ctx.Log.Debug("writing %q to remote command's stdin", line) - _, err := io.WriteString(stdin, line) - if err != nil { - ctx.Log.Err(errors.Wrapf(err, "writing %q to process", line).Error()) - } - } - }() - - // Use a waitgroup to block until our stdout/err copying is complete. - wg := new(sync.WaitGroup) - wg.Add(2) - // Asynchronously copy from stdout/err to outCh. - go func() { - s := bufio.NewScanner(stdout) - buf := []byte{} - s.Buffer(buf, BufioScannerBufferSize) - - for s.Scan() { - message := s.Text() - outCh <- Line{Line: message} - c.projectCmdOutputHandler.Send(ctx, message, false) - } - wg.Done() - }() - go func() { - s := bufio.NewScanner(stderr) - for s.Scan() { - message := s.Text() - outCh <- Line{Line: message} - c.projectCmdOutputHandler.Send(ctx, message, false) - } - wg.Done() - }() - - // Wait for our copying to complete. This *must* be done before - // calling cmd.Wait(). (see https://github.com/golang/go/issues/19685) - wg.Wait() - - // Wait for the command to complete. - err = cmd.Wait() +func (c *DefaultClient) RunCommandAsync(ctx command.ProjectContext, path string, args []string, customEnvVars map[string]string, v *version.Version, workspace string) (chan<- string, <-chan models.Line, error) { + cmd, envVars, err := c.prepCmd(ctx.Log, v, workspace, path, args) + if err != nil { + return nil, nil, err + } - // We're done now. Send an error if there was one. - if err != nil { - err = errors.Wrapf(err, "running %q in %q", tfCmd, path) - ctx.Log.Err(err.Error()) - outCh <- Line{Err: err} - } else { - ctx.Log.Info("successfully ran %q in %q", tfCmd, path) - } - }() + for key, val := range customEnvVars { + envVars = append(envVars, fmt.Sprintf("%s=%s", key, val)) + } - return inCh, outCh + runner := models.NewShellCommandRunner(cmd, envVars, path, c.projectCmdOutputHandler) + inCh, outCh := runner.RunCommandAsync(ctx) + return inCh, outCh, nil } // MustConstraint will parse one or more constraints from the given From ad6a9410a966548bd62068b680568a81bbb3f46e Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 12:10:22 -0700 Subject: [PATCH 11/22] Update signatures to propagate error finding version --- server/core/runtime/apply_step_runner_test.go | 12 ++++++------ server/core/runtime/plan_step_runner_test.go | 10 +++++----- .../terraform_client_internal_test.go | 18 ++++++++++++------ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/server/core/runtime/apply_step_runner_test.go b/server/core/runtime/apply_step_runner_test.go index eebd8ffe3b..1c719f8425 100644 --- a/server/core/runtime/apply_step_runner_test.go +++ b/server/core/runtime/apply_step_runner_test.go @@ -13,7 +13,7 @@ import ( . "github.com/petergtz/pegomock" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/core/runtime" - "github.com/runatlantis/atlantis/server/core/terraform" + runtimemodels "github.com/runatlantis/atlantis/server/core/runtime/models" "github.com/runatlantis/atlantis/server/core/terraform/mocks" matchers2 "github.com/runatlantis/atlantis/server/core/terraform/mocks/matchers" "github.com/runatlantis/atlantis/server/events/command" @@ -371,11 +371,11 @@ type remoteApplyMock struct { } // RunCommandAsync fakes out running terraform async. -func (r *remoteApplyMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan terraform.Line) { +func (r *remoteApplyMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line, error) { r.CalledArgs = args in := make(chan string) - out := make(chan terraform.Line) + out := make(chan runtimemodels.Line) // We use a wait group to ensure our sending and receiving routines have // completed. @@ -398,15 +398,15 @@ func (r *remoteApplyMock) RunCommandAsync(ctx command.ProjectContext, path strin // Asynchronously send the lines we're supposed to. go func() { for _, line := range strings.Split(r.LinesToSend, "\n") { - out <- terraform.Line{Line: line} + out <- runtimemodels.Line{Line: line} } if r.Err != nil { - out <- terraform.Line{Err: r.Err} + out <- runtimemodels.Line{Err: r.Err} } close(out) wg.Done() }() - return in, out + return in, out, nil } var preConfirmOutFmt = ` diff --git a/server/core/runtime/plan_step_runner_test.go b/server/core/runtime/plan_step_runner_test.go index 3db3514499..64bddddd37 100644 --- a/server/core/runtime/plan_step_runner_test.go +++ b/server/core/runtime/plan_step_runner_test.go @@ -8,13 +8,13 @@ import ( "testing" "github.com/hashicorp/go-version" - "github.com/runatlantis/atlantis/server/core/terraform" "github.com/runatlantis/atlantis/server/events/command" mocks2 "github.com/runatlantis/atlantis/server/events/mocks" . "github.com/petergtz/pegomock" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/core/runtime" + runtimemodels "github.com/runatlantis/atlantis/server/core/runtime/models" "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" @@ -885,18 +885,18 @@ type remotePlanMock struct { CalledArgs []string } -func (r *remotePlanMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan terraform.Line) { +func (r *remotePlanMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line, error) { r.CalledArgs = args in := make(chan string) - out := make(chan terraform.Line) + out := make(chan runtimemodels.Line) go func() { for _, line := range strings.Split(r.LinesToSend, "\n") { - out <- terraform.Line{Line: line} + out <- runtimemodels.Line{Line: line} } close(out) close(in) }() - return in, out + return in, out, nil } func stringSliceEquals(a, b []string) bool { diff --git a/server/core/terraform/terraform_client_internal_test.go b/server/core/terraform/terraform_client_internal_test.go index a99bc584bb..03e73a1722 100644 --- a/server/core/terraform/terraform_client_internal_test.go +++ b/server/core/terraform/terraform_client_internal_test.go @@ -8,6 +8,7 @@ import ( "testing" version "github.com/hashicorp/go-version" + runtimemodels "github.com/runatlantis/atlantis/server/core/runtime/models" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" jobmocks "github.com/runatlantis/atlantis/server/jobs/mocks" @@ -211,7 +212,8 @@ func TestDefaultClient_RunCommandAsync_Success(t *testing.T) { "ATLANTIS_TERRAFORM_VERSION=$ATLANTIS_TERRAFORM_VERSION", "DIR=$DIR", } - _, outCh := client.RunCommandAsync(ctx, tmp, args, map[string]string{}, nil, "workspace") + _, outCh, err := client.RunCommandAsync(ctx, tmp, args, map[string]string{}, nil, "workspace") + Ok(t, err) out, err := waitCh(outCh) Ok(t, err) @@ -260,7 +262,8 @@ func TestDefaultClient_RunCommandAsync_BigOutput(t *testing.T) { _, err = f.WriteString(s) Ok(t, err) } - _, outCh := client.RunCommandAsync(ctx, tmp, []string{filename}, map[string]string{}, nil, "workspace") + _, outCh, err := client.RunCommandAsync(ctx, tmp, []string{filename}, map[string]string{}, nil, "workspace") + Ok(t, err) out, err := waitCh(outCh) Ok(t, err) @@ -297,7 +300,8 @@ func TestDefaultClient_RunCommandAsync_StderrOutput(t *testing.T) { overrideTF: "echo", projectCmdOutputHandler: projectCmdOutputHandler, } - _, outCh := client.RunCommandAsync(ctx, tmp, []string{"stderr", ">&2"}, map[string]string{}, nil, "workspace") + _, outCh, err := client.RunCommandAsync(ctx, tmp, []string{"stderr", ">&2"}, map[string]string{}, nil, "workspace") + Ok(t, err) out, err := waitCh(outCh) Ok(t, err) @@ -334,7 +338,8 @@ func TestDefaultClient_RunCommandAsync_ExitOne(t *testing.T) { overrideTF: "echo", projectCmdOutputHandler: projectCmdOutputHandler, } - _, outCh := client.RunCommandAsync(ctx, tmp, []string{"dying", "&&", "exit", "1"}, map[string]string{}, nil, "workspace") + _, outCh, err := client.RunCommandAsync(ctx, tmp, []string{"dying", "&&", "exit", "1"}, map[string]string{}, nil, "workspace") + Ok(t, err) out, err := waitCh(outCh) ErrEquals(t, fmt.Sprintf(`running "echo dying && exit 1" in %q: exit status 1`, tmp), err) @@ -373,7 +378,8 @@ func TestDefaultClient_RunCommandAsync_Input(t *testing.T) { projectCmdOutputHandler: projectCmdOutputHandler, } - inCh, outCh := client.RunCommandAsync(ctx, tmp, []string{"a", "&&", "echo", "$a"}, map[string]string{}, nil, "workspace") + inCh, outCh, err := client.RunCommandAsync(ctx, tmp, []string{"a", "&&", "echo", "$a"}, map[string]string{}, nil, "workspace") + Ok(t, err) inCh <- "echo me\n" out, err := waitCh(outCh) @@ -381,7 +387,7 @@ func TestDefaultClient_RunCommandAsync_Input(t *testing.T) { Equals(t, "echo me", out) } -func waitCh(ch <-chan Line) (string, error) { +func waitCh(ch <-chan runtimemodels.Line) (string, error) { var ls []string for line := range ch { if line.Err != nil { From 4e894c48dd9b4dd3a4f2e33cabc87df86439ab25 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 12:15:45 -0700 Subject: [PATCH 12/22] Fix log output --- server/core/runtime/models/shell_command_runner.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/core/runtime/models/shell_command_runner.go b/server/core/runtime/models/shell_command_runner.go index f83e544337..e3c451b2c2 100644 --- a/server/core/runtime/models/shell_command_runner.go +++ b/server/core/runtime/models/shell_command_runner.go @@ -89,10 +89,10 @@ func (s *ShellCommandRunner) RunCommandAsync(ctx command.ProjectContext) (chan<- stderr, _ := s.cmd.StderrPipe() stdin, _ := s.cmd.StdinPipe() - ctx.Log.Debug("starting %q in %q", s.cmd, s.workingDir) + ctx.Log.Debug("starting %q in %q", s.command, s.workingDir) err := s.cmd.Start() if err != nil { - err = errors.Wrapf(err, "running %q in %q", s.cmd, s.workingDir) + err = errors.Wrapf(err, "running %q in %q", s.command, s.workingDir) ctx.Log.Err(err.Error()) outCh <- Line{Err: err} return @@ -144,11 +144,11 @@ func (s *ShellCommandRunner) RunCommandAsync(ctx command.ProjectContext) (chan<- // We're done now. Send an error if there was one. if err != nil { - err = errors.Wrapf(err, "running %q in %q", s.cmd, s.workingDir) + err = errors.Wrapf(err, "running %q in %q", s.command, s.workingDir) ctx.Log.Err(err.Error()) outCh <- Line{Err: err} } else { - ctx.Log.Info("successfully ran %q in %q", s.cmd, s.workingDir) + ctx.Log.Info("successfully ran %q in %q", s.command, s.workingDir) } }() From d87edd8af3ecdfc53e7d286d60acd43dd5c350c6 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 12:18:44 -0700 Subject: [PATCH 13/22] Fix error checking --- server/core/runtime/apply_step_runner.go | 4 ++-- server/core/runtime/plan_step_runner.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/core/runtime/apply_step_runner.go b/server/core/runtime/apply_step_runner.go index 9f01919d3e..41c483794b 100644 --- a/server/core/runtime/apply_step_runner.go +++ b/server/core/runtime/apply_step_runner.go @@ -91,7 +91,7 @@ func (a *ApplyStepRunner) cleanRemoteApplyOutput(out string) string { applyStartText := ` Terraform will perform the actions described above. Only 'yes' will be accepted to approve. - Enter a value: + Enter a value: ` applyStartIdx := strings.Index(out, applyStartText) if applyStartIdx < 0 { @@ -139,7 +139,7 @@ func (a *ApplyStepRunner) runRemoteApply( var runURL string var planChangedErr error - if err != nil { + if err == nil { for line := range outCh { if line.Err != nil { err = line.Err diff --git a/server/core/runtime/plan_step_runner.go b/server/core/runtime/plan_step_runner.go index 1f435124ae..59c88c625a 100644 --- a/server/core/runtime/plan_step_runner.go +++ b/server/core/runtime/plan_step_runner.go @@ -258,7 +258,7 @@ func (p *PlanStepRunner) runRemotePlan( nextLineIsRunURL := false var runURL string - if err != nil { + if err == nil { for line := range outCh { if line.Err != nil { err = line.Err From c9bc5df7e9fac1fe68f4b5ea4706a7104d492f57 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 12:21:30 -0700 Subject: [PATCH 14/22] Fix accidental whitespace stripping --- server/core/runtime/apply_step_runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/runtime/apply_step_runner.go b/server/core/runtime/apply_step_runner.go index 41c483794b..daeca3ebc3 100644 --- a/server/core/runtime/apply_step_runner.go +++ b/server/core/runtime/apply_step_runner.go @@ -91,7 +91,7 @@ func (a *ApplyStepRunner) cleanRemoteApplyOutput(out string) string { applyStartText := ` Terraform will perform the actions described above. Only 'yes' will be accepted to approve. - Enter a value: + Enter a value: ` applyStartIdx := strings.Index(out, applyStartText) if applyStartIdx < 0 { From 427c5def92535a736edcd3c6011ea28b94adfa04 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 12:25:55 -0700 Subject: [PATCH 15/22] Remove unused struct field --- server/core/runtime/models/shell_command_runner.go | 1 - 1 file changed, 1 deletion(-) diff --git a/server/core/runtime/models/shell_command_runner.go b/server/core/runtime/models/shell_command_runner.go index e3c451b2c2..14bd3e6e90 100644 --- a/server/core/runtime/models/shell_command_runner.go +++ b/server/core/runtime/models/shell_command_runner.go @@ -28,7 +28,6 @@ type Line struct { // `ProjectCommandOutputHandler`. type ShellCommandRunner struct { command string - environ []string workingDir string outputHandler jobs.ProjectCommandOutputHandler cmd *exec.Cmd From 7fe7e3e4514b5836d93f522601e398b5142d9e07 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 12:30:47 -0700 Subject: [PATCH 16/22] Fix error checking in terraform client --- server/core/terraform/terraform_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 6516491ffe..ccf012fd01 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -282,7 +282,7 @@ func (c *DefaultClient) RunCommandWithVersion(ctx command.ProjectContext, path s _, outCh, err := c.RunCommandAsync(ctx, path, args, customEnvVars, v, workspace) var lines []string - if err != nil { + if err == nil { for line := range outCh { if line.Err != nil { err = line.Err From d39b3b5445bbfd054c83e71a8ecbbc9bc50a39ff Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 13:03:41 -0700 Subject: [PATCH 17/22] Add unit tests to verify command output handler was called --- .../models/shell_command_runner_test.go | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 server/core/runtime/models/shell_command_runner_test.go diff --git a/server/core/runtime/models/shell_command_runner_test.go b/server/core/runtime/models/shell_command_runner_test.go new file mode 100644 index 0000000000..cc3f6dd920 --- /dev/null +++ b/server/core/runtime/models/shell_command_runner_test.go @@ -0,0 +1,61 @@ +package models_test + +import ( + "fmt" + "os" + "strings" + "testing" + + . "github.com/petergtz/pegomock" + "github.com/runatlantis/atlantis/server/core/runtime/models" + "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/jobs/mocks" + "github.com/runatlantis/atlantis/server/logging" + . "github.com/runatlantis/atlantis/testing" +) + +func TestShellCommandRunner_Run(t *testing.T) { + cases := []struct { + Command string + ExpLines []string + Environ map[string]string + }{ + { + Command: "echo $HELLO", + Environ: map[string]string{ + "HELLO": "world", + }, + ExpLines: []string{"world"}, + }, + { + Command: ">&2 echo this is an error", + ExpLines: []string{"this is an error"}, + }, + } + + for _, c := range cases { + t.Run(c.Command, func(t *testing.T) { + RegisterMockTestingT(t) + ctx := command.ProjectContext{ + Log: logging.NewNoopLogger(t), + Workspace: "default", + RepoRelDir: ".", + } + projectCmdOutputHandler := mocks.NewMockProjectCommandOutputHandler() + + cwd, err := os.Getwd() + Ok(t, err) + environ := []string{} + for k, v := range c.Environ { + environ = append(environ, fmt.Sprintf("%s=%s", k, v)) + } + runner := models.NewShellCommandRunner(c.Command, environ, cwd, projectCmdOutputHandler) + output, err := runner.Run(ctx) + Ok(t, err) + Equals(t, fmt.Sprintf("%s\n", strings.Join(c.ExpLines, "\n")), output) + for _, line := range c.ExpLines { + projectCmdOutputHandler.VerifyWasCalledOnce().Send(ctx, line, false) + } + }) + } +} From 795ac514c2fce9f00eabcbac5f4488abffc6ff8a Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 17:02:29 -0700 Subject: [PATCH 18/22] Remove err from async interface --- server/core/runtime/apply_step_runner.go | 72 +++++++++---------- server/core/runtime/apply_step_runner_test.go | 4 +- server/core/runtime/plan_step_runner.go | 37 +++++----- server/core/runtime/plan_step_runner_test.go | 4 +- server/core/runtime/runtime.go | 2 +- server/core/terraform/terraform_client.go | 28 +++++--- .../terraform_client_internal_test.go | 15 ++-- 7 files changed, 80 insertions(+), 82 deletions(-) diff --git a/server/core/runtime/apply_step_runner.go b/server/core/runtime/apply_step_runner.go index daeca3ebc3..67e1f58444 100644 --- a/server/core/runtime/apply_step_runner.go +++ b/server/core/runtime/apply_step_runner.go @@ -133,50 +133,48 @@ func (a *ApplyStepRunner) runRemoteApply( // Start the async command execution. ctx.Log.Debug("starting async tf remote operation") - inCh, outCh, err := a.AsyncTFExec.RunCommandAsync(ctx, filepath.Clean(path), applyArgs, envs, tfVersion, ctx.Workspace) + inCh, outCh := a.AsyncTFExec.RunCommandAsync(ctx, filepath.Clean(path), applyArgs, envs, tfVersion, ctx.Workspace) var lines []string nextLineIsRunURL := false var runURL string var planChangedErr error - if err == nil { - for line := range outCh { - if line.Err != nil { - err = line.Err - break - } - lines = append(lines, line.Line) - - // Here we're checking for the run url and updating the status - // if found. - if line.Line == lineBeforeRunURL { - nextLineIsRunURL = true - } else if nextLineIsRunURL { - runURL = strings.TrimSpace(line.Line) - ctx.Log.Debug("remote run url found, updating commit status") - updateStatusF(models.PendingCommitStatus, runURL) - nextLineIsRunURL = false - } + for line := range outCh { + if line.Err != nil { + err = line.Err + break + } + lines = append(lines, line.Line) + + // Here we're checking for the run url and updating the status + // if found. + if line.Line == lineBeforeRunURL { + nextLineIsRunURL = true + } else if nextLineIsRunURL { + runURL = strings.TrimSpace(line.Line) + ctx.Log.Debug("remote run url found, updating commit status") + updateStatusF(models.PendingCommitStatus, runURL) + nextLineIsRunURL = false + } - // If the plan is complete and it's waiting for us to verify the apply, - // check if the plan is the same and if so, input "yes". - if a.atConfirmApplyPrompt(lines) { - ctx.Log.Debug("remote apply is waiting for confirmation") - - // Check if the plan is as expected. - planChangedErr = a.remotePlanChanged(string(planfileBytes), strings.Join(lines, "\n"), tfVersion) - if planChangedErr != nil { - ctx.Log.Err("plan generated during apply does not match expected plan, aborting") - inCh <- "no\n" - // Need to continue so we read all the lines, otherwise channel - // sender (in TerraformClient) will block indefinitely waiting - // for us to read. - continue - } - - ctx.Log.Debug("plan generated during apply matches expected plan, continuing") - inCh <- "yes\n" + // If the plan is complete and it's waiting for us to verify the apply, + // check if the plan is the same and if so, input "yes". + if a.atConfirmApplyPrompt(lines) { + ctx.Log.Debug("remote apply is waiting for confirmation") + + // Check if the plan is as expected. + planChangedErr = a.remotePlanChanged(string(planfileBytes), strings.Join(lines, "\n"), tfVersion) + if planChangedErr != nil { + ctx.Log.Err("plan generated during apply does not match expected plan, aborting") + inCh <- "no\n" + // Need to continue so we read all the lines, otherwise channel + // sender (in TerraformClient) will block indefinitely waiting + // for us to read. + continue } + + ctx.Log.Debug("plan generated during apply matches expected plan, continuing") + inCh <- "yes\n" } } diff --git a/server/core/runtime/apply_step_runner_test.go b/server/core/runtime/apply_step_runner_test.go index 1c719f8425..23a4bbc6a3 100644 --- a/server/core/runtime/apply_step_runner_test.go +++ b/server/core/runtime/apply_step_runner_test.go @@ -371,7 +371,7 @@ type remoteApplyMock struct { } // RunCommandAsync fakes out running terraform async. -func (r *remoteApplyMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line, error) { +func (r *remoteApplyMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line) { r.CalledArgs = args in := make(chan string) @@ -406,7 +406,7 @@ func (r *remoteApplyMock) RunCommandAsync(ctx command.ProjectContext, path strin close(out) wg.Done() }() - return in, out, nil + return in, out } var preConfirmOutFmt = ` diff --git a/server/core/runtime/plan_step_runner.go b/server/core/runtime/plan_step_runner.go index 59c88c625a..a328c09daa 100644 --- a/server/core/runtime/plan_step_runner.go +++ b/server/core/runtime/plan_step_runner.go @@ -253,29 +253,28 @@ func (p *PlanStepRunner) runRemotePlan( // Start the async command execution. ctx.Log.Debug("starting async tf remote operation") - _, outCh, err := p.AsyncTFExec.RunCommandAsync(ctx, filepath.Clean(path), cmdArgs, envs, tfVersion, ctx.Workspace) + _, outCh := p.AsyncTFExec.RunCommandAsync(ctx, filepath.Clean(path), cmdArgs, envs, tfVersion, ctx.Workspace) var lines []string nextLineIsRunURL := false var runURL string + var err error - if err == nil { - for line := range outCh { - if line.Err != nil { - err = line.Err - break - } - lines = append(lines, line.Line) - - // Here we're checking for the run url and updating the status - // if found. - if line.Line == lineBeforeRunURL { - nextLineIsRunURL = true - } else if nextLineIsRunURL { - runURL = strings.TrimSpace(line.Line) - ctx.Log.Debug("remote run url found, updating commit status") - updateStatusF(models.PendingCommitStatus, runURL) - nextLineIsRunURL = false - } + for line := range outCh { + if line.Err != nil { + err = line.Err + break + } + lines = append(lines, line.Line) + + // Here we're checking for the run url and updating the status + // if found. + if line.Line == lineBeforeRunURL { + nextLineIsRunURL = true + } else if nextLineIsRunURL { + runURL = strings.TrimSpace(line.Line) + ctx.Log.Debug("remote run url found, updating commit status") + updateStatusF(models.PendingCommitStatus, runURL) + nextLineIsRunURL = false } } diff --git a/server/core/runtime/plan_step_runner_test.go b/server/core/runtime/plan_step_runner_test.go index 64bddddd37..e8fb35f538 100644 --- a/server/core/runtime/plan_step_runner_test.go +++ b/server/core/runtime/plan_step_runner_test.go @@ -885,7 +885,7 @@ type remotePlanMock struct { CalledArgs []string } -func (r *remotePlanMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line, error) { +func (r *remotePlanMock) RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line) { r.CalledArgs = args in := make(chan string) out := make(chan runtimemodels.Line) @@ -896,7 +896,7 @@ func (r *remotePlanMock) RunCommandAsync(ctx command.ProjectContext, path string close(out) close(in) }() - return in, out, nil + return in, out } func stringSliceEquals(a, b []string) bool { diff --git a/server/core/runtime/runtime.go b/server/core/runtime/runtime.go index 0f15d761f2..c82b9bc8f9 100644 --- a/server/core/runtime/runtime.go +++ b/server/core/runtime/runtime.go @@ -41,7 +41,7 @@ type AsyncTFExec interface { // Callers can use the input channel to pass stdin input to the command. // If any error is passed on the out channel, there will be no // further output (so callers are free to exit). - RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line, error) + RunCommandAsync(ctx command.ProjectContext, path string, args []string, envs map[string]string, v *version.Version, workspace string) (chan<- string, <-chan runtimemodels.Line) } // StatusUpdater brings the interface from CommitStatusUpdater into this package diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index ccf012fd01..42eef1185e 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -279,17 +279,16 @@ func (c *DefaultClient) EnsureVersion(log logging.SimpleLogging, v *version.Vers // See Client.RunCommandWithVersion. func (c *DefaultClient) RunCommandWithVersion(ctx command.ProjectContext, path string, args []string, customEnvVars map[string]string, v *version.Version, workspace string) (string, error) { if isAsyncEligibleCommand(args[0]) { - _, outCh, err := c.RunCommandAsync(ctx, path, args, customEnvVars, v, workspace) + _, outCh := c.RunCommandAsync(ctx, path, args, customEnvVars, v, workspace) var lines []string - if err == nil { - for line := range outCh { - if line.Err != nil { - err = line.Err - break - } - lines = append(lines, line.Line) + var err error + for line := range outCh { + if line.Err != nil { + err = line.Err + break } + lines = append(lines, line.Line) } output := strings.Join(lines, "\n") @@ -378,10 +377,17 @@ func (c *DefaultClient) prepCmd(log logging.SimpleLogging, v *version.Version, w // Callers can use the input channel to pass stdin input to the command. // If any error is passed on the out channel, there will be no // further output (so callers are free to exit). -func (c *DefaultClient) RunCommandAsync(ctx command.ProjectContext, path string, args []string, customEnvVars map[string]string, v *version.Version, workspace string) (chan<- string, <-chan models.Line, error) { +func (c *DefaultClient) RunCommandAsync(ctx command.ProjectContext, path string, args []string, customEnvVars map[string]string, v *version.Version, workspace string) (chan<- string, <-chan models.Line) { cmd, envVars, err := c.prepCmd(ctx.Log, v, workspace, path, args) if err != nil { - return nil, nil, err + outCh := make(chan models.Line) + inCh := make(chan string) + go func() { + outCh <- models.Line{Err: err} + close(outCh) + close(inCh) + }() + return inCh, outCh } for key, val := range customEnvVars { @@ -390,7 +396,7 @@ func (c *DefaultClient) RunCommandAsync(ctx command.ProjectContext, path string, runner := models.NewShellCommandRunner(cmd, envVars, path, c.projectCmdOutputHandler) inCh, outCh := runner.RunCommandAsync(ctx) - return inCh, outCh, nil + return inCh, outCh } // MustConstraint will parse one or more constraints from the given diff --git a/server/core/terraform/terraform_client_internal_test.go b/server/core/terraform/terraform_client_internal_test.go index 03e73a1722..647ee48e06 100644 --- a/server/core/terraform/terraform_client_internal_test.go +++ b/server/core/terraform/terraform_client_internal_test.go @@ -212,8 +212,7 @@ func TestDefaultClient_RunCommandAsync_Success(t *testing.T) { "ATLANTIS_TERRAFORM_VERSION=$ATLANTIS_TERRAFORM_VERSION", "DIR=$DIR", } - _, outCh, err := client.RunCommandAsync(ctx, tmp, args, map[string]string{}, nil, "workspace") - Ok(t, err) + _, outCh := client.RunCommandAsync(ctx, tmp, args, map[string]string{}, nil, "workspace") out, err := waitCh(outCh) Ok(t, err) @@ -262,8 +261,7 @@ func TestDefaultClient_RunCommandAsync_BigOutput(t *testing.T) { _, err = f.WriteString(s) Ok(t, err) } - _, outCh, err := client.RunCommandAsync(ctx, tmp, []string{filename}, map[string]string{}, nil, "workspace") - Ok(t, err) + _, outCh := client.RunCommandAsync(ctx, tmp, []string{filename}, map[string]string{}, nil, "workspace") out, err := waitCh(outCh) Ok(t, err) @@ -300,8 +298,7 @@ func TestDefaultClient_RunCommandAsync_StderrOutput(t *testing.T) { overrideTF: "echo", projectCmdOutputHandler: projectCmdOutputHandler, } - _, outCh, err := client.RunCommandAsync(ctx, tmp, []string{"stderr", ">&2"}, map[string]string{}, nil, "workspace") - Ok(t, err) + _, outCh := client.RunCommandAsync(ctx, tmp, []string{"stderr", ">&2"}, map[string]string{}, nil, "workspace") out, err := waitCh(outCh) Ok(t, err) @@ -338,8 +335,7 @@ func TestDefaultClient_RunCommandAsync_ExitOne(t *testing.T) { overrideTF: "echo", projectCmdOutputHandler: projectCmdOutputHandler, } - _, outCh, err := client.RunCommandAsync(ctx, tmp, []string{"dying", "&&", "exit", "1"}, map[string]string{}, nil, "workspace") - Ok(t, err) + _, outCh := client.RunCommandAsync(ctx, tmp, []string{"dying", "&&", "exit", "1"}, map[string]string{}, nil, "workspace") out, err := waitCh(outCh) ErrEquals(t, fmt.Sprintf(`running "echo dying && exit 1" in %q: exit status 1`, tmp), err) @@ -378,8 +374,7 @@ func TestDefaultClient_RunCommandAsync_Input(t *testing.T) { projectCmdOutputHandler: projectCmdOutputHandler, } - inCh, outCh, err := client.RunCommandAsync(ctx, tmp, []string{"a", "&&", "echo", "$a"}, map[string]string{}, nil, "workspace") - Ok(t, err) + inCh, outCh := client.RunCommandAsync(ctx, tmp, []string{"a", "&&", "echo", "$a"}, map[string]string{}, nil, "workspace") inCh <- "echo me\n" out, err := waitCh(outCh) From 58dd23e4a7c0e4aea88e589b38a4afad2ee135f3 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 17:19:05 -0700 Subject: [PATCH 19/22] Remove duplicative log now that shell command runner does it --- server/core/runtime/run_step_runner.go | 1 - 1 file changed, 1 deletion(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index e8e8f6c98f..af608ff7ef 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -73,6 +73,5 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str ctx.Log.Debug("error: %s", err) return "", err } - ctx.Log.Info("successfully ran %q in %q", command, path) return output, nil } From dddd74ec60a143b6ae9c28763b84a7cf8d149f46 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 17:28:22 -0700 Subject: [PATCH 20/22] Hide output in stream for env/multienv --- server/core/runtime/env_step_runner.go | 4 +++- .../runtime/models/shell_command_runner.go | 12 ++++++++--- .../models/shell_command_runner_test.go | 18 +++++++++++++++-- server/core/runtime/multienv_step_runner.go | 2 +- server/core/runtime/run_step_runner.go | 4 ++-- server/core/runtime/run_step_runner_test.go | 2 +- server/core/terraform/terraform_client.go | 2 +- .../events/mocks/mock_custom_step_runner.go | 20 +++++++++++-------- server/events/project_command_runner.go | 4 ++-- server/events/project_command_runner_test.go | 8 ++++---- 10 files changed, 51 insertions(+), 25 deletions(-) diff --git a/server/core/runtime/env_step_runner.go b/server/core/runtime/env_step_runner.go index ae7c27dd5d..6eced91ad1 100644 --- a/server/core/runtime/env_step_runner.go +++ b/server/core/runtime/env_step_runner.go @@ -18,7 +18,9 @@ func (r *EnvStepRunner) Run(ctx command.ProjectContext, command string, value st if value != "" { return value, nil } - res, err := r.RunStepRunner.Run(ctx, command, path, envs) + // Pass `false` for streamOutput because this isn't interesting to the user reading the build logs + // in the web UI. + res, err := r.RunStepRunner.Run(ctx, command, path, envs, false) // Trim newline from res to support running `echo env_value` which has // a newline. We don't recommend users run echo -n env_value to remove the // newline because -n doesn't work in the sh shell which is what we use diff --git a/server/core/runtime/models/shell_command_runner.go b/server/core/runtime/models/shell_command_runner.go index 14bd3e6e90..c30f97378d 100644 --- a/server/core/runtime/models/shell_command_runner.go +++ b/server/core/runtime/models/shell_command_runner.go @@ -30,10 +30,11 @@ type ShellCommandRunner struct { command string workingDir string outputHandler jobs.ProjectCommandOutputHandler + streamOutput bool cmd *exec.Cmd } -func NewShellCommandRunner(command string, environ []string, workingDir string, outputHandler jobs.ProjectCommandOutputHandler) *ShellCommandRunner { +func NewShellCommandRunner(command string, environ []string, workingDir string, streamOutput bool, outputHandler jobs.ProjectCommandOutputHandler) *ShellCommandRunner { cmd := exec.Command("sh", "-c", command) // #nosec cmd.Env = environ cmd.Dir = workingDir @@ -42,6 +43,7 @@ func NewShellCommandRunner(command string, environ []string, workingDir string, command: command, workingDir: workingDir, outputHandler: outputHandler, + streamOutput: streamOutput, cmd: cmd, } } @@ -120,7 +122,9 @@ func (s *ShellCommandRunner) RunCommandAsync(ctx command.ProjectContext) (chan<- for scanner.Scan() { message := scanner.Text() outCh <- Line{Line: message} - s.outputHandler.Send(ctx, message, false) + if s.streamOutput { + s.outputHandler.Send(ctx, message, false) + } } wg.Done() }() @@ -129,7 +133,9 @@ func (s *ShellCommandRunner) RunCommandAsync(ctx command.ProjectContext) (chan<- for scanner.Scan() { message := scanner.Text() outCh <- Line{Line: message} - s.outputHandler.Send(ctx, message, false) + if s.streamOutput { + s.outputHandler.Send(ctx, message, false) + } } wg.Done() }() diff --git a/server/core/runtime/models/shell_command_runner_test.go b/server/core/runtime/models/shell_command_runner_test.go index cc3f6dd920..84054b838a 100644 --- a/server/core/runtime/models/shell_command_runner_test.go +++ b/server/core/runtime/models/shell_command_runner_test.go @@ -7,6 +7,7 @@ import ( "testing" . "github.com/petergtz/pegomock" + "github.com/runatlantis/atlantis/server/core/runtime/mocks/matchers" "github.com/runatlantis/atlantis/server/core/runtime/models" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/jobs/mocks" @@ -49,13 +50,26 @@ func TestShellCommandRunner_Run(t *testing.T) { for k, v := range c.Environ { environ = append(environ, fmt.Sprintf("%s=%s", k, v)) } - runner := models.NewShellCommandRunner(c.Command, environ, cwd, projectCmdOutputHandler) + expectedOutput := fmt.Sprintf("%s\n", strings.Join(c.ExpLines, "\n")) + + // Run once with streaming enabled + runner := models.NewShellCommandRunner(c.Command, environ, cwd, true, projectCmdOutputHandler) output, err := runner.Run(ctx) Ok(t, err) - Equals(t, fmt.Sprintf("%s\n", strings.Join(c.ExpLines, "\n")), output) + Equals(t, expectedOutput, output) for _, line := range c.ExpLines { projectCmdOutputHandler.VerifyWasCalledOnce().Send(ctx, line, false) } + + // And again with streaming disabled. Everything should be the same except the + // command output handler should not have received anything + + projectCmdOutputHandler = mocks.NewMockProjectCommandOutputHandler() + runner = models.NewShellCommandRunner(c.Command, environ, cwd, false, projectCmdOutputHandler) + output, err = runner.Run(ctx) + Ok(t, err) + Equals(t, expectedOutput, output) + projectCmdOutputHandler.VerifyWasCalled(Never()).Send(matchers.AnyModelsProjectCommandContext(), AnyString(), EqBool(false)) }) } } diff --git a/server/core/runtime/multienv_step_runner.go b/server/core/runtime/multienv_step_runner.go index 69a9a2028a..fd659a4de4 100644 --- a/server/core/runtime/multienv_step_runner.go +++ b/server/core/runtime/multienv_step_runner.go @@ -15,7 +15,7 @@ type MultiEnvStepRunner struct { // Run runs the multienv step command. // The command must return a json string containing the array of name-value pairs that are being added as extra environment variables func (r *MultiEnvStepRunner) Run(ctx command.ProjectContext, command string, path string, envs map[string]string) (string, error) { - res, err := r.RunStepRunner.Run(ctx, command, path, envs) + res, err := r.RunStepRunner.Run(ctx, command, path, envs, false) if err == nil { envVars := strings.Split(res, ",") if len(envVars) > 0 { diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index af608ff7ef..c0a4d53efc 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -21,7 +21,7 @@ type RunStepRunner struct { ProjectCmdOutputHandler jobs.ProjectCommandOutputHandler } -func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path string, envs map[string]string) (string, error) { +func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path string, envs map[string]string, streamOutput bool) (string, error) { tfVersion := r.DefaultTFVersion if ctx.TerraformVersion != nil { tfVersion = ctx.TerraformVersion @@ -65,7 +65,7 @@ func (r *RunStepRunner) Run(ctx command.ProjectContext, command string, path str finalEnvVars = append(finalEnvVars, fmt.Sprintf("%s=%s", key, val)) } - runner := models.NewShellCommandRunner(command, finalEnvVars, path, r.ProjectCmdOutputHandler) + runner := models.NewShellCommandRunner(command, finalEnvVars, path, streamOutput, r.ProjectCmdOutputHandler) output, err := runner.Run(ctx) if err != nil { diff --git a/server/core/runtime/run_step_runner_test.go b/server/core/runtime/run_step_runner_test.go index 8ce664beeb..e6910c89f6 100644 --- a/server/core/runtime/run_step_runner_test.go +++ b/server/core/runtime/run_step_runner_test.go @@ -146,7 +146,7 @@ func TestRunStepRunner_Run(t *testing.T) { ProjectName: c.ProjectName, EscapedCommentArgs: []string{"-target=resource1", "-target=resource2"}, } - out, err := r.Run(ctx, c.Command, tmpDir, map[string]string{"test": "var"}) + out, err := r.Run(ctx, c.Command, tmpDir, map[string]string{"test": "var"}, true) if c.ExpErr != "" { ErrContains(t, c.ExpErr, err) return diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 42eef1185e..8e55b4e698 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -394,7 +394,7 @@ func (c *DefaultClient) RunCommandAsync(ctx command.ProjectContext, path string, envVars = append(envVars, fmt.Sprintf("%s=%s", key, val)) } - runner := models.NewShellCommandRunner(cmd, envVars, path, c.projectCmdOutputHandler) + runner := models.NewShellCommandRunner(cmd, envVars, path, true, c.projectCmdOutputHandler) inCh, outCh := runner.RunCommandAsync(ctx) return inCh, outCh } diff --git a/server/events/mocks/mock_custom_step_runner.go b/server/events/mocks/mock_custom_step_runner.go index 6490660663..8554651132 100644 --- a/server/events/mocks/mock_custom_step_runner.go +++ b/server/events/mocks/mock_custom_step_runner.go @@ -26,11 +26,11 @@ func NewMockCustomStepRunner(options ...pegomock.Option) *MockCustomStepRunner { func (mock *MockCustomStepRunner) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockCustomStepRunner) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockCustomStepRunner) Run(ctx command.ProjectContext, cmd string, path string, envs map[string]string) (string, error) { +func (mock *MockCustomStepRunner) Run(ctx command.ProjectContext, cmd string, path string, envs map[string]string, streamOutput bool) (string, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockCustomStepRunner().") } - params := []pegomock.Param{ctx, cmd, path, envs} + params := []pegomock.Param{ctx, cmd, path, envs, streamOutput} result := pegomock.GetGenericMockFrom(mock).Invoke("Run", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 string var ret1 error @@ -82,8 +82,8 @@ type VerifierMockCustomStepRunner struct { timeout time.Duration } -func (verifier *VerifierMockCustomStepRunner) Run(ctx command.ProjectContext, cmd string, path string, envs map[string]string) *MockCustomStepRunner_Run_OngoingVerification { - params := []pegomock.Param{ctx, cmd, path, envs} +func (verifier *VerifierMockCustomStepRunner) Run(ctx command.ProjectContext, cmd string, path string, envs map[string]string, streamOutput bool) *MockCustomStepRunner_Run_OngoingVerification { + params := []pegomock.Param{ctx, cmd, path, envs, streamOutput} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Run", params, verifier.timeout) return &MockCustomStepRunner_Run_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -93,12 +93,12 @@ type MockCustomStepRunner_Run_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockCustomStepRunner_Run_OngoingVerification) GetCapturedArguments() (command.ProjectContext, string, string, map[string]string) { - ctx, cmd, path, envs := c.GetAllCapturedArguments() - return ctx[len(ctx)-1], cmd[len(cmd)-1], path[len(path)-1], envs[len(envs)-1] +func (c *MockCustomStepRunner_Run_OngoingVerification) GetCapturedArguments() (command.ProjectContext, string, string, map[string]string, bool) { + ctx, cmd, path, envs, streamOutput := c.GetAllCapturedArguments() + return ctx[len(ctx)-1], cmd[len(cmd)-1], path[len(path)-1], envs[len(envs)-1], streamOutput[len(streamOutput)-1] } -func (c *MockCustomStepRunner_Run_OngoingVerification) GetAllCapturedArguments() (_param0 []command.ProjectContext, _param1 []string, _param2 []string, _param3 []map[string]string) { +func (c *MockCustomStepRunner_Run_OngoingVerification) GetAllCapturedArguments() (_param0 []command.ProjectContext, _param1 []string, _param2 []string, _param3 []map[string]string, _param4 []bool) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]command.ProjectContext, len(c.methodInvocations)) @@ -117,6 +117,10 @@ func (c *MockCustomStepRunner_Run_OngoingVerification) GetAllCapturedArguments() for u, param := range params[3] { _param3[u] = param.(map[string]string) } + _param4 = make([]bool, len(c.methodInvocations)) + for u, param := range params[4] { + _param4[u] = param.(bool) + } } return } diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index a52c9e2cdb..5b5c7976f0 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -62,7 +62,7 @@ type StepRunner interface { // CustomStepRunner runs custom run steps. type CustomStepRunner interface { // Run cmd in path. - Run(ctx command.ProjectContext, cmd string, path string, envs map[string]string) (string, error) + Run(ctx command.ProjectContext, cmd string, path string, envs map[string]string, streamOutput bool) (string, error) } //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_env_step_runner.go EnvStepRunner @@ -493,7 +493,7 @@ func (p *DefaultProjectCommandRunner) runSteps(steps []valid.Step, ctx command.P case "version": out, err = p.VersionStepRunner.Run(ctx, step.ExtraArgs, absPath, envs) case "run": - out, err = p.RunStepRunner.Run(ctx, step.RunCommand, absPath, envs) + out, err = p.RunStepRunner.Run(ctx, step.RunCommand, absPath, envs, true) case "env": out, err = p.EnvStepRunner.Run(ctx, step.RunCommand, step.EnvVarValue, absPath, envs) envs[step.EnvVarName] = out diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go index b0bb776668..6c75f3beea 100644 --- a/server/events/project_command_runner_test.go +++ b/server/events/project_command_runner_test.go @@ -112,7 +112,7 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) { When(mockInit.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("init", nil) When(mockPlan.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("plan", nil) When(mockApply.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("apply", nil) - When(mockRun.Run(ctx, "", repoDir, expEnvs)).ThenReturn("run", nil) + When(mockRun.Run(ctx, "", repoDir, expEnvs, true)).ThenReturn("run", nil) res := runner.Plan(ctx) Assert(t, res.PlanSuccess != nil, "exp plan success") @@ -128,7 +128,7 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) { case "apply": mockApply.VerifyWasCalledOnce().Run(ctx, nil, repoDir, expEnvs) case "run": - mockRun.VerifyWasCalledOnce().Run(ctx, "", repoDir, expEnvs) + mockRun.VerifyWasCalledOnce().Run(ctx, "", repoDir, expEnvs, true) } } } @@ -458,7 +458,7 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { When(mockInit.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("init", nil) When(mockPlan.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("plan", nil) When(mockApply.Run(ctx, nil, repoDir, expEnvs)).ThenReturn("apply", nil) - When(mockRun.Run(ctx, "", repoDir, expEnvs)).ThenReturn("run", nil) + When(mockRun.Run(ctx, "", repoDir, expEnvs, true)).ThenReturn("run", nil) When(mockEnv.Run(ctx, "", "value", repoDir, make(map[string]string))).ThenReturn("value", nil) res := runner.Apply(ctx) @@ -474,7 +474,7 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { case "apply": mockApply.VerifyWasCalledOnce().Run(ctx, nil, repoDir, expEnvs) case "run": - mockRun.VerifyWasCalledOnce().Run(ctx, "", repoDir, expEnvs) + mockRun.VerifyWasCalledOnce().Run(ctx, "", repoDir, expEnvs, true) case "env": mockEnv.VerifyWasCalledOnce().Run(ctx, "", "value", repoDir, expEnvs) } From 86c5eddaa5e2a104c1c77a907bd5ccfe591c4cc4 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 17:41:54 -0700 Subject: [PATCH 21/22] Add comment explaining goroutines --- server/core/terraform/terraform_client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/core/terraform/terraform_client.go b/server/core/terraform/terraform_client.go index 8e55b4e698..b359b841e4 100644 --- a/server/core/terraform/terraform_client.go +++ b/server/core/terraform/terraform_client.go @@ -380,6 +380,9 @@ func (c *DefaultClient) prepCmd(log logging.SimpleLogging, v *version.Version, w func (c *DefaultClient) RunCommandAsync(ctx command.ProjectContext, path string, args []string, customEnvVars map[string]string, v *version.Version, workspace string) (chan<- string, <-chan models.Line) { cmd, envVars, err := c.prepCmd(ctx.Log, v, workspace, path, args) if err != nil { + // The signature of `RunCommandAsync` doesn't provide for returning an immediate error, only one + // once reading the output. Since we won't be spawning a process, simulate that by sending the + // errorcustomEnvVars to the output channel. outCh := make(chan models.Line) inCh := make(chan string) go func() { From bdcbcd0f5c03481ef0d55118fd4e92e0ff203f47 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 17 May 2022 17:45:16 -0700 Subject: [PATCH 22/22] Use printf for better macOS compatibility --- server/core/runtime/run_step_runner_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/runtime/run_step_runner_test.go b/server/core/runtime/run_step_runner_test.go index e6910c89f6..d6e235dcfc 100644 --- a/server/core/runtime/run_step_runner_test.go +++ b/server/core/runtime/run_step_runner_test.go @@ -46,7 +46,7 @@ func TestRunStepRunner_Run(t *testing.T) { ExpOut: "your main.tf file does not provide default region.\ncheck\n", }, { - Command: `echo "\e[0;32mgreen"`, + Command: `printf '\e[32mgreen'`, ExpOut: "green\n", }, {