diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index d754573d4..7e177eac8 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -66,16 +66,6 @@ func (b *Local) opApply( } }() - // Before we do anything else we'll take a snapshot of the prior state - // so we can use it for some fixups to our detection of whether the plan - // includes externally-visible side-effects that need to be applied. - // (We should be able to remove this once we complete the planned work - // described in the comment for func planHasSideEffects in backend_plan.go .) - // We go directly to the state manager here because the state inside - // tfCtx was already implicitly changed by a validation walk inside - // the b.context method. - priorState := opState.State().DeepCopy() - runningOp.State = tfCtx.State() // If we weren't given a plan, then we refresh/plan @@ -89,7 +79,7 @@ func (b *Local) opApply( return } - trivialPlan := !planHasSideEffects(priorState, plan.Changes) + trivialPlan := plan.Changes.Empty() hasUI := op.UIOut != nil && op.UIIn != nil mustConfirm := hasUI && ((op.Destroy && (!op.DestroyForce && !op.AutoApprove)) || (!op.Destroy && !op.AutoApprove && !trivialPlan)) if mustConfirm { @@ -114,7 +104,7 @@ func (b *Local) opApply( if !trivialPlan { // Display the plan of what we are going to apply/destroy. - b.renderPlan(plan, runningOp.State, priorState, tfCtx.Schemas()) + b.renderPlan(plan, runningOp.State, tfCtx.Schemas()) b.CLI.Output("") } diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index da82e3116..0fbb0b58a 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -10,13 +10,11 @@ import ( "github.com/mitchellh/cli" "github.com/mitchellh/colorstring" - "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/plans" - "github.com/hashicorp/terraform/plans/objchange" "github.com/hashicorp/terraform/plans/planfile" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/states/statemgr" @@ -85,16 +83,6 @@ func (b *Local) opPlan( } }() - // Before we do anything else we'll take a snapshot of the prior state - // so we can use it for some fixups to our detection of whether the plan - // includes externally-visible side-effects that need to be applied. - // (We should be able to remove this once we complete the planned work - // described in the comment for func planHasSideEffects below.) - // We go directly to the state manager here because the state inside - // tfCtx was already implicitly changed by a validation walk inside - // the b.context method. - priorState := opState.State().DeepCopy() - runningOp.State = tfCtx.State() // Perform the plan in a goroutine so we can be interrupted @@ -123,7 +111,7 @@ func (b *Local) opPlan( } // Record whether this plan includes any side-effects that could be applied. - runningOp.PlanEmpty = !planHasSideEffects(priorState, plan.Changes) + runningOp.PlanEmpty = plan.Changes.Empty() // Save the plan to disk if path := op.PlanOutPath; path != "" { @@ -167,7 +155,7 @@ func (b *Local) opPlan( return } - b.renderPlan(plan, plan.State, priorState, schemas) + b.renderPlan(plan, plan.State, schemas) // If we've accumulated any warnings along the way then we'll show them // here just before we show the summary and next steps. If we encountered @@ -194,8 +182,8 @@ func (b *Local) opPlan( } } -func (b *Local) renderPlan(plan *plans.Plan, baseState *states.State, priorState *states.State, schemas *terraform.Schemas) { - RenderPlan(plan, baseState, priorState, schemas, b.CLI, b.Colorize()) +func (b *Local) renderPlan(plan *plans.Plan, baseState *states.State, schemas *terraform.Schemas) { + RenderPlan(plan, baseState, schemas, b.CLI, b.Colorize()) } // RenderPlan renders the given plan to the given UI. @@ -218,7 +206,7 @@ func (b *Local) renderPlan(plan *plans.Plan, baseState *states.State, priorState // output values will not currently be rendered because their prior values // are currently stored only in the prior state. (see the docstring for // func planHasSideEffects for why this is and when that might change) -func RenderPlan(plan *plans.Plan, baseState *states.State, priorState *states.State, schemas *terraform.Schemas, ui cli.Ui, colorize *colorstring.Colorize) { +func RenderPlan(plan *plans.Plan, baseState *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 { @@ -325,159 +313,12 @@ func RenderPlan(plan *plans.Plan, baseState *states.State, priorState *states.St ))) // If there is at least one planned change to the root module outputs - // then we'll render a summary of those too. This is easier said than done - // because currently output changes are not accurately recorded in - // plan.Changes.Outputs (see the func planHasSideEffects docstring for why) - // and so we must use priorState to produce an actually-accurate changeset - // to display. - // - // Some callers (i.e. "terraform show") only have the plan and therefore - // can't provide the prior state. In that case we'll skip showing the - // outputs for now, until we can make plan.Changes.Outputs itself be - // accurate and self-contained. - if priorState != nil { - var synthOutputChanges []*plans.OutputChangeSrc - outputChangeCount := 0 - for _, addr := range allRootModuleOutputs(priorState, plan.Changes) { - before := cty.NullVal(cty.DynamicPseudoType) - after := cty.NullVal(cty.DynamicPseudoType) - sensitive := false - if changeSrc := plan.Changes.OutputValue(addr); changeSrc != nil { - sensitive = sensitive || changeSrc.Sensitive - change, err := changeSrc.Decode() - if err != nil { - // It would be very strange to get here because changeSrc was - // presumably just created by Terraform Core and so should never - // be invalid. - panic(fmt.Sprintf("failed to decode change for %s: %s", addr, err)) - } - after = change.After - } - if priorOutputState := priorState.OutputValue(addr); priorOutputState != nil { - sensitive = sensitive || priorOutputState.Sensitive - before = priorOutputState.Value - } - - // We'll now construct ourselves a new, accurate change. - change := &plans.OutputChange{ - Addr: addr, - Sensitive: sensitive, - Change: plans.Change{ - Action: objchange.ActionForChange(before, after), - Before: before, - After: after, - }, - } - if change.Action == plans.NoOp { - continue // ignore non-changes - } - outputChangeCount++ - newChangeSrc, err := change.Encode() - if err != nil { - // Again, it would be very strange to see an error here because - // we've literally just created this value in memory above. - panic(fmt.Sprintf("failed to encode change for %s: %s", addr, err)) - } - synthOutputChanges = append(synthOutputChanges, newChangeSrc) - } - if outputChangeCount > 0 { - ui.Output(colorize.Color("[reset]\n[bold]Changes to Outputs:[reset]" + format.OutputChanges(synthOutputChanges, colorize))) - } + // then we'll render a summary of those too. + if len(plan.Changes.Outputs) > 0 { + ui.Output(colorize.Color("[reset]\n[bold]Changes to Outputs:[reset]" + format.OutputChanges(plan.Changes.Outputs, colorize))) } } -// planHasSideEffects determines whether the given planned changeset has -// externally-visible side-effects that warrant giving the user an opportunity -// to apply the plan. If planHasSideEffects returns false, the caller should -// return a "No changes" message and not offer to apply the plan. -// -// This is currently implemented here, rather than in the "terraform" package, -// because with the current separation of the refresh vs. plan walks there is -// never any single point in the "terraform" package where both the prior and -// planned new values for outputs are available at once. We have this out here -// as a temporary workaround for that design problem, with the intent of moving -// this down into the "terraform" package once we've completed some work to -// combine the refresh and plan walks together into a single walk and thus -// that walk will be able to see both the prior and new values for outputs. -func planHasSideEffects(priorState *states.State, changes *plans.Changes) bool { - if !changes.Empty() { - // At the time of writing, changes.Empty considers only resource - // changes because the planned changes for outputs are inaccurate. - // If we have at least one resource change then we know we have - // side-effects though, regardless of outputs. - return true - } - - // If we get here then there are definitely no resource changes in the plan - // but we may have some changes to outputs that "changes" hasn't properly - // captured, because it treats all outputs as being either created or - // deleted regardless of their prior values. To work around that for now, - // we'll use priorState to see if those planned changes really are changes. - for _, addr := range allRootModuleOutputs(priorState, changes) { - before := cty.NullVal(cty.DynamicPseudoType) - after := cty.NullVal(cty.DynamicPseudoType) - if changeSrc := changes.OutputValue(addr); changeSrc != nil { - change, err := changeSrc.Decode() - if err != nil { - // It would be very strange to get here because changeSrc was - // presumably just created by Terraform Core and so should never - // be invalid. In this unlikely event, we'll just conservatively - // assume there is a change. - return true - } - after = change.After - } - if priorState != nil { - if priorOutputState := priorState.OutputValue(addr); priorOutputState != nil { - before = priorOutputState.Value - } - } - if objchange.ActionForChange(before, after) != plans.NoOp { - return true - } - } - - // If we fall out here then we didn't find any effective changes in the - // outputs, and we already showed that there were no resource changes, so - // this plan has no side-effects. - return false -} - -// allRootModuleOutputs is a helper function to produce the union of all -// root module output values across both the given prior state and the given -// changeset. This is to compensate for the fact that the outputs portion of -// a plans.Changes is currently incomplete and inaccurate due to limitations of -// Terraform Core's design; we need to use information from the prior state -// to compensate for those limitations when making decisions based on the -// effective output changes. -func allRootModuleOutputs(priorState *states.State, changes *plans.Changes) []addrs.AbsOutputValue { - m := make(map[string]addrs.AbsOutputValue) - if priorState != nil { - for _, os := range priorState.RootModule().OutputValues { - m[os.Addr.String()] = os.Addr - } - } - if changes != nil { - for _, oc := range changes.Outputs { - if !oc.Addr.Module.IsRoot() { - continue - } - m[oc.Addr.String()] = oc.Addr - } - } - if len(m) == 0 { - return nil - } - ret := make([]addrs.AbsOutputValue, 0, len(m)) - for _, addr := range m { - ret = append(ret, addr) - } - sort.Slice(ret, func(i, j int) bool { - return ret[i].OutputValue.Name < ret[j].OutputValue.Name - }) - return ret -} - const planHeaderIntro = ` An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: diff --git a/command/show.go b/command/show.go index 17f4a3d7a..3bf034c9e 100644 --- a/command/show.go +++ b/command/show.go @@ -163,10 +163,7 @@ func (c *ShowCommand) Run(args []string) int { // 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. - // We're setting priorState to null because a saved plan file only - // records the base state (possibly updated by refresh), not the - // prior state (direct result of the previous apply). - localBackend.RenderPlan(plan, stateFile.State, nil, schemas, c.Ui, c.Colorize()) + localBackend.RenderPlan(plan, stateFile.State, schemas, c.Ui, c.Colorize()) return 0 } diff --git a/helper/resource/testing_config.go b/helper/resource/testing_config.go index 04194dddf..74739c8a0 100644 --- a/helper/resource/testing_config.go +++ b/helper/resource/testing_config.go @@ -115,9 +115,17 @@ func testStep(opts terraform.ContextOpts, state *terraform.State, step TestStep) if stepDiags.HasErrors() { return state, newOperationError("second follow-up plan", stepDiags) } - empty := p.Changes.Empty() + empty := true newState := p.State + // the legacy tests never took outputs into account + for _, c := range p.Changes.Resources { + if c.Action != plans.NoOp { + empty = false + break + } + } + if !empty { if step.ExpectNonEmptyPlan { log.Printf("[INFO] Got non-empty plan, as expected:\n\n%s", legacyPlanComparisonString(newState, p.Changes)) diff --git a/plans/changes.go b/plans/changes.go index 239ceb714..414099488 100644 --- a/plans/changes.go +++ b/plans/changes.go @@ -37,6 +37,13 @@ func (c *Changes) Empty() bool { return false } } + + for _, out := range c.Outputs { + if out.Action != NoOp { + return false + } + } + return true }