Merge pull request #367 from runatlantis/output-bug

Fix pre plan output being removed.

server/events/markdown_renderer.go

@@ -16,7 +16,6 @@ package events
 import (
 	"bytes"
 	"fmt"
-	"regexp"
 	"strings"
 	"text/template"
 
@@ -32,12 +31,6 @@ const (
 	maxUnwrappedLines = 12
 )
 
-var (
-	plusDiffRegex  = regexp.MustCompile(`(?m)^  \+`)
-	tildeDiffRegex = regexp.MustCompile(`(?m)^  ~`)
-	minusDiffRegex = regexp.MustCompile(`(?m)^  -`)
-)
-
 // MarkdownRenderer renders responses as markdown.
 type MarkdownRenderer struct {
 	// GitlabSupportsCommonMark is true if the version of GitLab we're
@@ -121,7 +114,6 @@ func (m *MarkdownRenderer) renderProjectResults(results []ProjectResult, common
 				Failure: result.Failure,
 			})
 		} else if result.PlanSuccess != nil {
-			result.PlanSuccess.TerraformOutput = m.fmtPlan(result.PlanSuccess.TerraformOutput)
 			if m.shouldUseWrappedTmpl(vcsHost, result.PlanSuccess.TerraformOutput) {
 				resultData.Rendered = m.renderTemplate(planSuccessWrappedTmpl, *result.PlanSuccess)
 			} else {
@@ -176,26 +168,6 @@ func (m *MarkdownRenderer) shouldUseWrappedTmpl(vcsHost models.VCSHostType, outp
 	return strings.Count(output, "\n") > maxUnwrappedLines
 }
 
-// fmtPlan uses regex's to remove any leading whitespace in front of the
-// terraform output so that the diff syntax highlighting works. Example:
-// "  - aws_security_group_rule.allow_all" =>
-// "- aws_security_group_rule.allow_all"
-// We do it for +, ~ and -.
-// It also removes the "Refreshing..." preamble.
-func (m *MarkdownRenderer) fmtPlan(tfOutput string) string {
-	// Plan output contains a lot of "Refreshing..." lines followed by a
-	// separator. We want to remove everything before that separator.
-	refreshSeparator := "------------------------------------------------------------------------\n"
-	sepIdx := strings.Index(tfOutput, refreshSeparator)
-	if sepIdx > -1 {
-		tfOutput = tfOutput[sepIdx+len(refreshSeparator):]
-	}
-
-	tfOutput = plusDiffRegex.ReplaceAllString(tfOutput, "+")
-	tfOutput = tildeDiffRegex.ReplaceAllString(tfOutput, "~")
-	return minusDiffRegex.ReplaceAllString(tfOutput, "-")
-}
-
 func (m *MarkdownRenderer) renderTemplate(tmpl *template.Template, data interface{}) string {
 	buf := &bytes.Buffer{}
 	if err := tmpl.Execute(buf, data); err != nil {

server/events/markdown_renderer_test.go

@@ -486,98 +486,6 @@ $$$
 	}
 }
 
-// Test that we format the terraform plan output so it shows up properly
-// when using diff syntax highlighting.
-func TestRenderProjectResults_DiffSyntax(t *testing.T) {
-	r := events.MarkdownRenderer{}
-	result := r.Render(
-		events.CommandResult{
-			ProjectResults: []events.ProjectResult{
-				{
-					RepoRelDir: ".",
-					Workspace:  "default",
-					PlanSuccess: &events.PlanSuccess{
-						TerraformOutput: `Refreshing Terraform state in-memory prior to plan...
-The refreshed state will be used to calculate this plan, but will not be
-persisted to local or remote state storage.
-
-
-------------------------------------------------------------------------
-
-An execution plan has been generated and is shown below.
-Resource actions are indicated with the following symbols:
-  + create
-  ~ update in-place
-  - destroy
-
-Terraform will perform the following actions:
-
-+ null_resource.test[0]
-      id: <computed>
-
-  + null_resource.test[1]
-      id: <computed>
-
-  ~ aws_security_group_rule.allow_all
-      description: "" => "test3"
-
-  - aws_security_group_rule.allow_all
-`,
-						LockURL:   "lock-url",
-						RePlanCmd: "atlantis plan -d .",
-						ApplyCmd:  "atlantis apply -d .",
-					},
-				},
-			},
-		},
-		events.PlanCommand,
-		"log",
-		false,
-		models.Github,
-	)
-
-	exp := `Ran Plan in dir: $.$ workspace: $default$
-
-<details><summary>Show Output</summary>
-
-$$$diff
-
-An execution plan has been generated and is shown below.
-Resource actions are indicated with the following symbols:
-+ create
-~ update in-place
-- destroy
-
-Terraform will perform the following actions:
-
-+ null_resource.test[0]
-      id: <computed>
-
-+ null_resource.test[1]
-      id: <computed>
-
-~ aws_security_group_rule.allow_all
-      description: "" => "test3"
-
-- aws_security_group_rule.allow_all
-
-$$$
-
-* :arrow_forward: To **apply** this plan, comment:
-    * $atlantis apply -d .$
-* :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
-* :repeat: To **plan** this project again, comment:
-    * $atlantis plan -d .$
-</details>
-
----
-* :fast_forward: To **apply** all unapplied plans from this pull request, comment:
-    * $atlantis apply$
-`
-	expWithBackticks := strings.Replace(exp, "$", "`", -1)
-	Equals(t, expWithBackticks, result)
-}
-
 // Test that if the output is longer than 12 lines, it gets wrapped on the right
 // VCS hosts during an error.
 func TestRenderProjectResults_WrappedErr(t *testing.T) {

server/events/runtime/plan_step_runner.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	"github.com/hashicorp/go-version"
@@ -12,6 +13,12 @@ import (
 
 const defaultWorkspace = "default"
 
+var (
+	plusDiffRegex  = regexp.MustCompile(`(?m)^ {2}\+`)
+	tildeDiffRegex = regexp.MustCompile(`(?m)^ {2}~`)
+	minusDiffRegex = regexp.MustCompile(`(?m)^ {2}-`)
+)
+
 type PlanStepRunner struct {
 	TerraformExecutor TerraformExec
 	DefaultTFVersion  *version.Version
@@ -30,7 +37,11 @@ func (p *PlanStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []strin
 	}
 
 	planCmd := p.buildPlanCmd(ctx, extraArgs, path)
-	return p.TerraformExecutor.RunCommandWithVersion(ctx.Log, filepath.Clean(path), planCmd, tfVersion, ctx.Workspace)
+	output, err := p.TerraformExecutor.RunCommandWithVersion(ctx.Log, filepath.Clean(path), planCmd, tfVersion, ctx.Workspace)
+	if err != nil {
+		return "", err
+	}
+	return p.fmtPlanOutput(output), nil
 }
 
 // switchWorkspace changes the terraform workspace if necessary and will create
@@ -140,3 +151,23 @@ func (p *PlanStepRunner) flatten(slices [][]string) []string {
 	}
 	return flattened
 }
+
+// fmtPlanOutput uses regex's to remove any leading whitespace in front of the
+// terraform output so that the diff syntax highlighting works. Example:
+// "  - aws_security_group_rule.allow_all" =>
+// "- aws_security_group_rule.allow_all"
+// We do it for +, ~ and -.
+// It also removes the "Refreshing..." preamble.
+func (p *PlanStepRunner) fmtPlanOutput(output string) string {
+	// Plan output contains a lot of "Refreshing..." lines followed by a
+	// separator. We want to remove everything before that separator.
+	refreshSeparator := "------------------------------------------------------------------------\n"
+	sepIdx := strings.Index(output, refreshSeparator)
+	if sepIdx > -1 {
+		output = output[sepIdx+len(refreshSeparator):]
+	}
+
+	output = plusDiffRegex.ReplaceAllString(output, "+")
+	output = tildeDiffRegex.ReplaceAllString(output, "~")
+	return minusDiffRegex.ReplaceAllString(output, "-")
+}

server/events/runtime/plan_step_runner_test.go

@@ -494,3 +494,69 @@ func TestRun_UsesDiffPathForProject(t *testing.T) {
 	Ok(t, err)
 	Equals(t, "output", output)
 }
+
+// Test that we format the plan output for better rendering.
+func TestRun_PlanFmt(t *testing.T) {
+	rawOutput := `Refreshing Terraform state in-memory prior to plan...
+The refreshed state will be used to calculate this plan, but will not be
+persisted to local or remote state storage.
+
+
+------------------------------------------------------------------------
+
+An execution plan has been generated and is shown below.
+Resource actions are indicated with the following symbols:
+  + create
+  ~ update in-place
+  - destroy
+
+Terraform will perform the following actions:
+
++ null_resource.test[0]
+      id: <computed>
+
+  + null_resource.test[1]
+      id: <computed>
+
+  ~ aws_security_group_rule.allow_all
+      description: "" => "test3"
+
+  - aws_security_group_rule.allow_all
+`
+	RegisterMockTestingT(t)
+	terraform := mocks.NewMockClient()
+	tfVersion, _ := version.NewVersion("0.10.0")
+	s := runtime.PlanStepRunner{
+		TerraformExecutor: terraform,
+		DefaultTFVersion:  tfVersion,
+	}
+	When(terraform.RunCommandWithVersion(
+		matchers.AnyPtrToLoggingSimpleLogger(),
+		AnyString(),
+		AnyStringSlice(),
+		matchers2.AnyPtrToGoVersionVersion(),
+		AnyString())).
+		ThenReturn(rawOutput, nil)
+	actOutput, err := s.Run(models.ProjectCommandContext{}, nil, "")
+	Ok(t, err)
+	Equals(t, `
+An execution plan has been generated and is shown below.
+Resource actions are indicated with the following symbols:
++ create
+~ update in-place
+- destroy
+
+Terraform will perform the following actions:
+
++ null_resource.test[0]
+      id: <computed>
+
++ null_resource.test[1]
+      id: <computed>
+
+~ aws_security_group_rule.allow_all
+      description: "" => "test3"
+
+- aws_security_group_rule.allow_all
+`, actOutput)
+}

server/testfixtures/test-repos/simple-yaml/atlantis.yaml

@@ -11,14 +11,19 @@ workflows:
     # Only specify plan so should use default apply workflow.
     plan:
       steps:
+      - run: echo preinit
       - init
       - plan:
           extra_args: [-var, var=fromconfig]
+      - run: echo postplan
   staging:
     plan:
       steps:
       - init
       - plan:
           extra_args: [-var-file, staging.tfvars]
     apply:
-      steps: [apply]
+      steps:
+      - run: echo preapply
+      - apply
+      - run: echo postapply

server/testfixtures/test-repos/simple-yaml/exp-output-apply-all.txt

@@ -18,7 +18,11 @@ workspace = default
 
 ---
 ### 2. workspace: `staging` dir: `.`
+<details><summary>Show Output</summary>
+
 ```diff
+preapply
+
 null_resource.simple:
 null_resource.simple:
 
@@ -29,7 +33,10 @@ Outputs:
 var = fromfile
 workspace = staging
 
+postapply
+
 ```
+</details>
 
 ---
 

server/testfixtures/test-repos/simple-yaml/exp-output-apply-staging.txt

@@ -1,6 +1,10 @@
 Ran Apply in dir: `.` workspace: `staging`
 
+<details><summary>Show Output</summary>
+
 ```diff
+preapply
+
 null_resource.simple:
 null_resource.simple:
 
@@ -11,5 +15,8 @@ Outputs:
 var = fromfile
 workspace = staging
 
+postapply
+
 ```
+</details>
 

server/testfixtures/test-repos/simple-yaml/exp-output-autoplan.txt

@@ -3,7 +3,11 @@ Ran Plan for 2 projects:
 1. workspace: `staging` dir: `.`
 
 ### 1. workspace: `default` dir: `.`
+<details><summary>Show Output</summary>
+
 ```diff
+preinit
+
 
 An execution plan has been generated and is shown below.
 Resource actions are indicated with the following symbols:
@@ -15,13 +19,16 @@ Terraform will perform the following actions:
       id: <computed>
 Plan: 1 to add, 0 to change, 0 to destroy.
 
+postplan
+
 ```
 
 * :arrow_forward: To **apply** this plan, comment:
     * `atlantis apply -d .`
 * :put_litter_in_its_place: To **delete** this plan click [here](lock-url)
 * :repeat: To **plan** this project again, comment:
     * `atlantis plan -d .`
+</details>
 
 ---
 ### 2. workspace: `staging` dir: `.`