From 9a62ab301450a294d92964249d9593d96cb9bd70 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 5 Nov 2019 17:20:26 -0800 Subject: [PATCH] command: "terraform show" renders plans like "terraform plan" During the Terraform 0.12 work we briefly had a partial update of the old Terraform 0.11 (and prior) diff renderer that could work with the new plan structure, but could produce only partial results. We switched to the new plan implementation prior to release, but the "terraform show" command was left calling into the old partial implementation, and thus produced incomplete results when rendering a saved plan. Here we instead use the plan rendering logic from the "terraform plan" command, making the output of both identical. Unfortunately, due to the current backend architecture that logic lives inside the local backend package, and it contains some business logic around state and schema wrangling that would make it inappropriate to move wholesale into the command/format package. To allow for a low-risk fix to the "terraform show" output, here we avoid some more severe refactoring by just exporting the rendering functionality in a way that allows the "terraform show" command to call into it. In future we'd like to move all of the code that actually writes to the output into the "command" package so that the roles of these components are better segregated, but that is too big a change to block fixing this issue. --- backend/local/backend_plan.go | 32 +++++++++++++++++++++++++------- command/show.go | 13 +++++++++++-- command/show_test.go | 8 +++++++- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index 0a7f28dac..247076d31 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -8,6 +8,9 @@ import ( "sort" "strings" + "github.com/mitchellh/cli" + "github.com/mitchellh/colorstring" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/command/format" @@ -190,6 +193,21 @@ func (b *Local) opPlan( } func (b *Local) renderPlan(plan *plans.Plan, state *states.State, schemas *terraform.Schemas) { + RenderPlan(plan, state, schemas, b.CLI, b.Colorize()) +} + +// RenderPlan renders the given plan to the given UI. +// +// This is exported only so that the "terraform show" command can re-use it. +// Ideally it would be somewhere outside of this backend code so that both +// can call into it, but we're leaving it here for now in order to avoid +// disruptive refactoring. +// +// If you find yourself wanting to call this function from a third callsite, +// please consider whether it's time to do the more disruptive refactoring +// so that something other than the local backend package is offering this +// functionality. +func RenderPlan(plan *plans.Plan, state *states.State, schemas *terraform.Schemas, ui cli.Ui, colorize *colorstring.Colorize) { counts := map[plans.Action]int{} var rChanges []*plans.ResourceInstanceChangeSrc for _, change := range plan.Changes.Resources { @@ -223,9 +241,9 @@ func (b *Local) renderPlan(plan *plans.Plan, state *states.State, schemas *terra fmt.Fprintf(headerBuf, "%s read (data resources)\n", format.DiffActionSymbol(plans.Read)) } - b.CLI.Output(b.Colorize().Color(headerBuf.String())) + ui.Output(colorize.Color(headerBuf.String())) - b.CLI.Output("Terraform will perform the following actions:\n") + ui.Output("Terraform will perform the following actions:\n") // Note: we're modifying the backing slice of this plan object in-place // here. The ordering of resource changes in a plan is not significant, @@ -247,13 +265,13 @@ func (b *Local) renderPlan(plan *plans.Plan, state *states.State, schemas *terra providerSchema := schemas.ProviderSchema(rcs.ProviderAddr.ProviderConfig.Type) if providerSchema == nil { // Should never happen - b.CLI.Output(fmt.Sprintf("(schema missing for %s)\n", rcs.ProviderAddr)) + ui.Output(fmt.Sprintf("(schema missing for %s)\n", rcs.ProviderAddr)) continue } rSchema, _ := providerSchema.SchemaForResourceAddr(rcs.Addr.Resource.Resource) if rSchema == nil { // Should never happen - b.CLI.Output(fmt.Sprintf("(schema missing for %s)\n", rcs.Addr)) + ui.Output(fmt.Sprintf("(schema missing for %s)\n", rcs.Addr)) continue } @@ -267,11 +285,11 @@ func (b *Local) renderPlan(plan *plans.Plan, state *states.State, schemas *terra } } - b.CLI.Output(format.ResourceChange( + ui.Output(format.ResourceChange( rcs, tainted, rSchema, - b.CLIColor, + colorize, )) } @@ -288,7 +306,7 @@ func (b *Local) renderPlan(plan *plans.Plan, state *states.State, schemas *terra stats[change.Action]++ } } - b.CLI.Output(b.Colorize().Color(fmt.Sprintf( + ui.Output(colorize.Color(fmt.Sprintf( "[reset][bold]Plan:[reset] "+ "%d to add, %d to change, %d to destroy.", stats[plans.Create], stats[plans.Update], stats[plans.Delete], diff --git a/command/show.go b/command/show.go index 2c75f9301..d7b5f7ef8 100644 --- a/command/show.go +++ b/command/show.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/hashicorp/terraform/backend" + localBackend "github.com/hashicorp/terraform/backend/local" "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/command/jsonplan" "github.com/hashicorp/terraform/command/jsonstate" @@ -152,8 +153,16 @@ func (c *ShowCommand) Run(args []string) int { c.Ui.Output(string(jsonPlan)) return 0 } - dispPlan := format.NewPlan(plan.Changes) - c.Ui.Output(dispPlan.Format(c.Colorize())) + + // FIXME: We currently call into the local backend for this, since + // the "terraform plan" logic lives there and our package call graph + // means we can't orient this dependency the other way around. In + // future we'll hopefully be able to refactor the backend architecture + // a little so that CLI UI rendering always happens in this "command" + // package rather than in the backends themselves, but for now we're + // accepting this oddity because "terraform show" is a less commonly + // used way to render a plan than "terraform plan" is. + localBackend.RenderPlan(plan, stateFile.State, schemas, c.Ui, c.Colorize()) return 0 } diff --git a/command/show_test.go b/command/show_test.go index b66a17168..30da8c572 100644 --- a/command/show_test.go +++ b/command/show_test.go @@ -79,7 +79,7 @@ func TestShow_noArgsNoState(t *testing.T) { func TestShow_plan(t *testing.T) { planPath := testPlanFileNoop(t) - ui := new(cli.MockUi) + ui := cli.NewMockUi() c := &ShowCommand{ Meta: Meta{ testingOverrides: metaOverridesForProvider(testProvider()), @@ -93,6 +93,12 @@ func TestShow_plan(t *testing.T) { if code := c.Run(args); code != 0 { t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) } + + want := `Terraform will perform the following actions` + got := ui.OutputWriter.String() + if !strings.Contains(got, want) { + t.Errorf("missing expected output\nwant: %s\ngot:\n%s", want, got) + } } func TestShow_plan_json(t *testing.T) {