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

This reverts commit 7301feb.

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"), tfVersion)
+			planChangedErr = a.remotePlanChanged(string(planfileBytes), strings.Join(lines, "\n"))
 			if planChangedErr != nil {
 				ctx.Log.Err("plan generated during apply does not match expected plan, aborting")
 				inCh <- "no\n"
@@ -198,15 +198,19 @@ 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, tfVersion *version.Version) error {
-	output := StripRefreshingFromPlanOutput(applyOut, tfVersion)
+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)
+	}
 
-	// Strip plan output after the prompt to execute the plan.
-	planEndIdx := strings.Index(output, "Do you want to perform these actions in workspace \"")
+	// ...and the prompt to execute the plan.
+	planEndIdx := strings.Index(applyOut, "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(output[: planEndIdx])
+	currPlan := strings.TrimSpace(applyOut[planStartIdx+len(refreshSeparator) : 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,7 +15,8 @@ import (
 
 const (
 	defaultWorkspace = "default"
-	refreshKeyword = "Refreshing state..."
+	// refreshSeparator is what separates the refresh stage from the calculated
+	// plan during a terraform plan.
 	refreshSeparator = "------------------------------------------------------------------------\n"
 )
 
@@ -54,7 +55,7 @@ func (p *PlanStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []strin
 	if err != nil {
 		return output, err
 	}
-	return p.fmtPlanOutput(output, tfVersion), nil
+	return p.fmtPlanOutput(output), nil
 }
 
 // isRemoteOpsErr returns true if there was an error caused due to this
@@ -88,7 +89,11 @@ 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 := StripRefreshingFromPlanOutput(output, tfVersion)
+	planOutput := output
+	sepIdx := strings.Index(planOutput, refreshSeparator)
+	if sepIdx > -1 {
+		planOutput = planOutput[sepIdx+len(refreshSeparator):]
+	}
 
 	// We also prepend our own remote ops header to the file so during apply we
 	// know this is a remote apply.
@@ -97,7 +102,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, tfVersion), nil
+	return p.fmtPlanOutput(output), nil
 }
 
 // switchWorkspace changes the terraform workspace if necessary and will create
@@ -223,8 +228,14 @@ 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, tfVersion *version.Version) string {
-	output = StripRefreshingFromPlanOutput(output, tfVersion)
+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):]
+	}
+
 	output = plusDiffRegex.ReplaceAllString(output, "+")
 	output = tildeDiffRegex.ReplaceAllString(output, "~")
 	return minusDiffRegex.ReplaceAllString(output, "-")
@@ -288,32 +299,6 @@ 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,71 +814,6 @@ 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