fix(output): Remove Refreshing state... from output (#1352)

* fix(output): Remove Refreshing state... from output

Since Terraform 0.14.0 there are no separator between refreshing plan and the plan.

* Fix comment typo

Author: mathcantin

server/events/runtime/apply_step_runner.go

@@ -90,7 +90,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 {
@@ -162,7 +162,7 @@ func (a *ApplyStepRunner) runRemoteApply(
 			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"))
+			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"
@@ -198,19 +198,15 @@ func (a *ApplyStepRunner) runRemoteApply(
 // the one we're about to apply in the apply phase.
 // If the plans don't match, it returns an error with a diff of the two plans
 // that can be printed to the pull request.
-func (a *ApplyStepRunner) remotePlanChanged(planfileContents string, applyOut string) error {
-	// The plan is between the refresh separator...
-	planStartIdx := strings.Index(applyOut, refreshSeparator)
-	if planStartIdx < 0 {
-		return fmt.Errorf("Couldn't find refresh separator when parsing apply output:\n%q", applyOut)
-	}
+func (a *ApplyStepRunner) remotePlanChanged(planfileContents string, applyOut string, tfVersion *version.Version) error {
+	output := StripRefreshingFromPlanOutput(applyOut, tfVersion)
 
-	// ...and the prompt to execute the plan.
-	planEndIdx := strings.Index(applyOut, "Do you want to perform these actions in workspace \"")
+	// Strip plan output after the prompt to execute the plan.
+	planEndIdx := strings.Index(output, "Do you want to perform these actions in workspace \"")
 	if planEndIdx < 0 {
 		return fmt.Errorf("Couldn't find plan end when parsing apply output:\n%q", applyOut)
 	}
-	currPlan := strings.TrimSpace(applyOut[planStartIdx+len(refreshSeparator) : planEndIdx])
+	currPlan := strings.TrimSpace(output[: planEndIdx])
 
 	// Ensure we strip the remoteOpsHeader from the plan contents so the
 	// comparison is fair. We add this header in the plan phase so we can

server/events/runtime/plan_step_runner.go

@@ -15,8 +15,7 @@ import (
 
 const (
 	defaultWorkspace = "default"
-	// refreshSeparator is what separates the refresh stage from the calculated
-	// plan during a terraform plan.
+	refreshKeyword = "Refreshing state..."
 	refreshSeparator = "------------------------------------------------------------------------\n"
 )
 
@@ -55,7 +54,7 @@ func (p *PlanStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []strin
 	if err != nil {
 		return output, err
 	}
-	return p.fmtPlanOutput(output), nil
+	return p.fmtPlanOutput(output, tfVersion), nil
 }
 
 // isRemoteOpsErr returns true if there was an error caused due to this
@@ -89,11 +88,7 @@ func (p *PlanStepRunner) remotePlan(ctx models.ProjectCommandContext, extraArgs
 	// plan. To ensure that what gets applied is the plan we printed to the PR,
 	// during the apply phase, we diff the output we stored in the fake
 	// planfile with the pending apply output.
-	planOutput := output
-	sepIdx := strings.Index(planOutput, refreshSeparator)
-	if sepIdx > -1 {
-		planOutput = planOutput[sepIdx+len(refreshSeparator):]
-	}
+	planOutput := StripRefreshingFromPlanOutput(output, tfVersion)
 
 	// We also prepend our own remote ops header to the file so during apply we
 	// know this is a remote apply.
@@ -102,7 +97,7 @@ func (p *PlanStepRunner) remotePlan(ctx models.ProjectCommandContext, extraArgs
 		return output, errors.Wrap(err, "unable to create planfile for remote ops")
 	}
 
-	return p.fmtPlanOutput(output), nil
+	return p.fmtPlanOutput(output, tfVersion), nil
 }
 
 // switchWorkspace changes the terraform workspace if necessary and will create
@@ -228,14 +223,8 @@ func (p *PlanStepRunner) flatten(slices [][]string) []string {
 // "- 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.
-	sepIdx := strings.Index(output, refreshSeparator)
-	if sepIdx > -1 {
-		output = output[sepIdx+len(refreshSeparator):]
-	}
-
+func (p *PlanStepRunner) fmtPlanOutput(output string, tfVersion *version.Version) string {
+	output = StripRefreshingFromPlanOutput(output, tfVersion)
 	output = plusDiffRegex.ReplaceAllString(output, "+")
 	output = tildeDiffRegex.ReplaceAllString(output, "~")
 	return minusDiffRegex.ReplaceAllString(output, "-")
@@ -299,6 +288,32 @@ func (p *PlanStepRunner) runRemotePlan(
 	return output, err
 }
 
+func StripRefreshingFromPlanOutput(output string, tfVersion *version.Version) string {
+	if tfVersion.GreaterThanOrEqual(version.Must(version.NewVersion("0.14.0"))) {
+		// Plan output contains a lot of "Refreshing..." lines, remove it
+		lines := strings.Split(output, "\n")
+		finalIndex := 0
+		for i, line := range lines {
+			if strings.Contains(line, refreshKeyword) {
+				finalIndex = i
+			}
+		}
+
+		if finalIndex != 0 {
+			output = strings.Join(lines[finalIndex + 1:], "\n")
+		}
+		return output
+	} else {
+		// Plan output contains a lot of "Refreshing..." lines followed by a
+		// separator. We want to remove everything before that separator.
+		sepIdx := strings.Index(output, refreshSeparator)
+		if sepIdx > -1 {
+			output = output[sepIdx+len(refreshSeparator):]
+		}
+		return output
+	}
+}
+
 // remoteOpsErr01114 is the error terraform plan will return if this project is
 // using TFE remote operations in TF 0.11.14.
 var remoteOpsErr01114 = `Error: Saving a generated plan is currently not supported!

server/events/runtime/plan_step_runner_test.go

@@ -814,6 +814,71 @@ Plan: 0 to add, 0 to change, 1 to destroy.`, string(bytes))
 	}
 }
 
+// Test striping output method
+func TestStripRefreshingFromPlanOutput(t *testing.T) {
+	tfVersion_0135, _ := version.NewVersion("0.13.5")
+	tfVersion_0140, _ := version.NewVersion("0.14.0")
+	cases := []struct {
+		out string
+		tfVersion *version.Version
+	}{
+		{
+			remotePlanOutput,
+			tfVersion_0135,
+		},
+		{
+			`Running plan in the remote backend. Output will stream here. Pressing Ctrl-C
+will stop streaming the logs, but will not stop the plan running remotely.
+
+Preparing the remote plan...
+
+To view this run in a browser, visit:
+https://app.terraform.io/app/lkysow-enterprises/atlantis-tfe-test/runs/run-is4oVvJfrkud1KvE
+
+Waiting for the plan to start...
+
+Terraform v0.14.0
+
+Configuring remote state backend...
+Initializing Terraform configuration...
+2019/02/20 22:40:52 [DEBUG] Using modified User-Agent: Terraform/0.14.0TFE/202eeff
+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.
+
+null_resource.hi: Refreshing state... (ID: 217661332516885645)
+null_resource.hi[1]: Refreshing state... (ID: 6064510335076839362)
+
+An execution plan has been generated and is shown below.
+Resource actions are indicated with the following symbols:
+  - destroy
+
+Terraform will perform the following actions:
+
+  - null_resource.hi[1]
+
+
+Plan: 0 to add, 0 to change, 1 to destroy.`,
+			tfVersion_0140,
+		},
+	}
+
+	for _, c := range cases {
+		output := runtime.StripRefreshingFromPlanOutput(c.out, c.tfVersion)
+		Equals(t, `
+An execution plan has been generated and is shown below.
+Resource actions are indicated with the following symbols:
+  - destroy
+
+Terraform will perform the following actions:
+
+  - null_resource.hi[1]
+
+
+Plan: 0 to add, 0 to change, 1 to destroy.`, output)
+	}
+}
+
 type remotePlanMock struct {
 	// LinesToSend will be sent on the channel.
 	LinesToSend string