diff --git a/server/events/runtime/apply_step_runner.go b/server/events/runtime/apply_step_runner.go index a96238302..a847381eb 100644 --- a/server/events/runtime/apply_step_runner.go +++ b/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 diff --git a/server/events/runtime/plan_step_runner.go b/server/events/runtime/plan_step_runner.go index 35b13d994..3796070ad 100644 --- a/server/events/runtime/plan_step_runner.go +++ b/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! diff --git a/server/events/runtime/plan_step_runner_test.go b/server/events/runtime/plan_step_runner_test.go index 1830196dd..5ef5809a1 100644 --- a/server/events/runtime/plan_step_runner_test.go +++ b/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