diff --git a/server/events/markdown_renderer.go b/server/events/markdown_renderer.go index ccbbd6887c..638ae9d9ea 100644 --- a/server/events/markdown_renderer.go +++ b/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 { diff --git a/server/events/markdown_renderer_test.go b/server/events/markdown_renderer_test.go index 3d8d791e19..3fb1c3a6ed 100644 --- a/server/events/markdown_renderer_test.go +++ b/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: - - + null_resource.test[1] - id: - - ~ 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$ - -
Show Output - -$$$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: - -+ null_resource.test[1] - id: - -~ 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 .$ -
- ---- -* :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) { diff --git a/server/events/runtime/plan_step_runner.go b/server/events/runtime/plan_step_runner.go index 90215af824..3a393cdc5e 100644 --- a/server/events/runtime/plan_step_runner.go +++ b/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, "-") +} diff --git a/server/events/runtime/plan_step_runner_test.go b/server/events/runtime/plan_step_runner_test.go index a91e075481..d2f5c41bf2 100644 --- a/server/events/runtime/plan_step_runner_test.go +++ b/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: + + + null_resource.test[1] + id: + + ~ 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: + ++ null_resource.test[1] + id: + +~ aws_security_group_rule.allow_all + description: "" => "test3" + +- aws_security_group_rule.allow_all +`, actOutput) +} diff --git a/server/testfixtures/test-repos/simple-yaml/atlantis.yaml b/server/testfixtures/test-repos/simple-yaml/atlantis.yaml index 62e6047617..2ffd1cba86 100644 --- a/server/testfixtures/test-repos/simple-yaml/atlantis.yaml +++ b/server/testfixtures/test-repos/simple-yaml/atlantis.yaml @@ -11,9 +11,11 @@ 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: @@ -21,4 +23,7 @@ workflows: - plan: extra_args: [-var-file, staging.tfvars] apply: - steps: [apply] + steps: + - run: echo preapply + - apply + - run: echo postapply diff --git a/server/testfixtures/test-repos/simple-yaml/exp-output-apply-all.txt b/server/testfixtures/test-repos/simple-yaml/exp-output-apply-all.txt index 4de00a6ae4..8b8b4f450c 100644 --- a/server/testfixtures/test-repos/simple-yaml/exp-output-apply-all.txt +++ b/server/testfixtures/test-repos/simple-yaml/exp-output-apply-all.txt @@ -18,7 +18,11 @@ workspace = default --- ### 2. workspace: `staging` dir: `.` +
Show Output + ```diff +preapply + null_resource.simple: null_resource.simple: @@ -29,7 +33,10 @@ Outputs: var = fromfile workspace = staging +postapply + ``` +
--- diff --git a/server/testfixtures/test-repos/simple-yaml/exp-output-apply-staging.txt b/server/testfixtures/test-repos/simple-yaml/exp-output-apply-staging.txt index 4e38e9600b..e18fd05b4d 100644 --- a/server/testfixtures/test-repos/simple-yaml/exp-output-apply-staging.txt +++ b/server/testfixtures/test-repos/simple-yaml/exp-output-apply-staging.txt @@ -1,6 +1,10 @@ Ran Apply in dir: `.` workspace: `staging` +
Show Output + ```diff +preapply + null_resource.simple: null_resource.simple: @@ -11,5 +15,8 @@ Outputs: var = fromfile workspace = staging +postapply + ``` +
diff --git a/server/testfixtures/test-repos/simple-yaml/exp-output-autoplan.txt b/server/testfixtures/test-repos/simple-yaml/exp-output-autoplan.txt index 96ea52f956..99e843eb4c 100644 --- a/server/testfixtures/test-repos/simple-yaml/exp-output-autoplan.txt +++ b/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: `.` +
Show Output + ```diff +preinit + An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: @@ -15,6 +19,8 @@ Terraform will perform the following actions: id: Plan: 1 to add, 0 to change, 0 to destroy. +postplan + ``` * :arrow_forward: To **apply** this plan, comment: @@ -22,6 +28,7 @@ Plan: 1 to add, 0 to change, 0 to destroy. * :put_litter_in_its_place: To **delete** this plan click [here](lock-url) * :repeat: To **plan** this project again, comment: * `atlantis plan -d .` +
--- ### 2. workspace: `staging` dir: `.`