From 51aa71e4cbcde03b288e4a3ee625407fb1728495 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 13 Nov 2020 17:06:39 -0800 Subject: [PATCH] backend/local: Show resource instance drift as part of the plan When a user edits Terraform-managed objects outside of Terraform it can often prove confusing because Terraform indicates the need to change something even though the configuration hasn't changed. We retain enough information in the state that we can actually detect when an object has "drifted" since the last apply, so now we'll include that information in the plan output both just to generally make it more visible (in case the drift wasn't actually intentional and so investigation is warranted) and also to potentially serve as an explanation for other changes that appear in the plan that follows. --- backend/local/backend_apply.go | 4 +- backend/local/backend_plan.go | 99 ++++++++++++++++++++++++- command/format/diff.go | 131 +++++++++++++++++++++++++++++++++ command/show.go | 2 +- states/state_equal.go | 20 +++++ 5 files changed, 250 insertions(+), 6 deletions(-) diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index 7e177eac85e1..c0ff42dbf45e 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -70,6 +70,8 @@ func (b *Local) opApply( // If we weren't given a plan, then we refresh/plan if op.PlanFile == nil { + initialState := tfCtx.State().DeepCopy() + // Perform the plan log.Printf("[INFO] backend/local: apply calling Plan") plan, planDiags := tfCtx.Plan() @@ -104,7 +106,7 @@ func (b *Local) opApply( if !trivialPlan { // Display the plan of what we are going to apply/destroy. - b.renderPlan(plan, runningOp.State, tfCtx.Schemas()) + b.renderPlan(plan, initialState, plan.State, tfCtx.Schemas()) b.CLI.Output("") } diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index a5733473ce59..bb913e1053fd 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -83,6 +83,7 @@ func (b *Local) opPlan( } }() + initialState := tfCtx.State().DeepCopy() runningOp.State = tfCtx.State() // Perform the plan in a goroutine so we can be interrupted @@ -155,7 +156,7 @@ func (b *Local) opPlan( return } - b.renderPlan(plan, plan.State, schemas) + b.renderPlan(plan, initialState, 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 @@ -182,8 +183,8 @@ func (b *Local) opPlan( } } -func (b *Local) renderPlan(plan *plans.Plan, baseState *states.State, schemas *terraform.Schemas) { - RenderPlan(plan, baseState, schemas, b.CLI, b.Colorize()) +func (b *Local) renderPlan(plan *plans.Plan, baseState, priorState *states.State, schemas *terraform.Schemas) { + RenderPlan(plan, baseState, priorState, schemas, b.CLI, b.Colorize()) } // RenderPlan renders the given plan to the given UI. @@ -206,7 +207,12 @@ func (b *Local) renderPlan(plan *plans.Plan, baseState *states.State, schemas *t // 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, schemas *terraform.Schemas, ui cli.Ui, colorize *colorstring.Colorize) { +func RenderPlan(plan *plans.Plan, baseState, priorState *states.State, schemas *terraform.Schemas, ui cli.Ui, colorize *colorstring.Colorize) { + if statesShowDrift(baseState, priorState) { + // We've detected some drift by refreshing during our planning, then. + renderDrift(baseState, priorState, schemas, ui, colorize) + } + counts := map[plans.Action]int{} var rChanges []*plans.ResourceInstanceChangeSrc for _, change := range plan.Changes.Resources { @@ -329,6 +335,91 @@ func RenderPlan(plan *plans.Plan, baseState *states.State, schemas *terraform.Sc } } +// statesShowDrift reports whether the two given states have any differences +// that suggest "drift", in the sense of managed resources whose state +// data has changed. The result makes sense only if priorState is the result +// of running refresh operations against baseState. +func statesShowDrift(baseState, priorState *states.State) bool { + if baseState == nil || priorState == nil { + return false + } + for _, bms := range baseState.Modules { + for _, brs := range bms.Resources { + if brs.Addr.Resource.Mode != addrs.ManagedResourceMode { + continue // only managed resources can "drift" + } + prs := priorState.Resource(brs.Addr) + if prs == nil { + // Refreshing detected that the remote object has been deleted + return true + } + if !prs.Equal(brs) { + // Refreshing detected that the remote object has changed. + return true + } + } + } + return false +} + +func renderDrift(baseState, priorState *states.State, schemas *terraform.Schemas, ui cli.Ui, colorize *colorstring.Colorize) { + // The cyan "Note:" here is following the color we use for the <= "read" + // action in the plan output, because this is reporting the result of + // all of the resource reading we've done. This isn't a "warning" because + // intentionally creating drift is an intentional part of some workflows + // (albeit less common ones, and some that may not be so advisable). + // The aim is just to draw attention to the changes as an explanation + // for possible unexpected changes in the plan, not to assume or suggest + // that such changes are "bad". + ui.Output("\n------------------------------------------------------------------------") + ui.Output(colorize.Color("\n[reset][bold][cyan]Note:[reset][bold] Objects have changed outside of Terraform[reset]")) + ui.Output("\nTerraform detected the following changes made outside Terraform since\nthe most recent \"terraform apply\":\n") + + for _, bms := range baseState.Modules { + for _, brs := range bms.Resources { + if brs.Addr.Resource.Mode != addrs.ManagedResourceMode { + continue // only managed resources can "drift" + } + addr := brs.Addr + prs := priorState.Resource(brs.Addr) + + provider := brs.ProviderConfig.Provider + providerSchema := schemas.ProviderSchema(provider) + if providerSchema == nil { + // Should never happen + ui.Output(fmt.Sprintf("(schema missing for %s)\n", provider)) + continue + } + rSchema, _ := providerSchema.SchemaForResourceAddr(addr.Resource) + if rSchema == nil { + // Should never happen + ui.Output(fmt.Sprintf("(schema missing for %s)\n", addr)) + continue + } + + for key, bis := range brs.Instances { + var pis *states.ResourceInstance + if prs != nil { + pis = prs.Instance(key) + } + + diff := format.ResourceInstanceDrift( + addr.Instance(key), + bis, pis, + rSchema, + colorize, + ) + if diff != "" { + ui.Output(diff) + } + } + } + } + + ui.Output("Unless you have made equivalent changes to your configuration, or\nignored the relevant attributes using ignore_changes, the following plan\nmay include actions to undo or respond to these changes.") + ui.Output("\n------------------------------------------------------------------------") +} + const planHeaderIntro = ` An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: diff --git a/command/format/diff.go b/command/format/diff.go index 7a3df5f09409..4782f48251c2 100644 --- a/command/format/diff.go +++ b/command/format/diff.go @@ -143,6 +143,137 @@ func ResourceChange( return buf.String() } +// ResourceInstanceDrift returns a string representation of a change to a +// particular resource instance that was made outside of Terraform, for +// reporting a change that has already happened rather than one that is planned. +// +// The the two resource instances have equal current objects then the result +// will be an empty string to indicate that there is no drift to render. +// +// The resource schema must be provided along with the change so that the +// formatted change can reflect the configuration structure for the associated +// resource. +// +// If "color" is non-nil, it will be used to color the result. Otherwise, +// no color codes will be included. +func ResourceInstanceDrift( + addr addrs.AbsResourceInstance, + before, after *states.ResourceInstance, + schema *configschema.Block, + color *colorstring.Colorize, +) string { + var buf bytes.Buffer + + if color == nil { + color = &colorstring.Colorize{ + Colors: colorstring.DefaultColors, + Disable: true, + Reset: false, + } + } + + dispAddr := addr.String() + action := plans.Update + + switch { + case after == nil || after.Current == nil: + // The object was deleted + buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] has been deleted", dispAddr))) + action = plans.Delete + default: + // The object was changed + buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] has been changed", dispAddr))) + } + + buf.WriteString(color.Color("[reset]\n")) + + buf.WriteString(color.Color(DiffActionSymbol(action)) + " ") + + switch addr.Resource.Resource.Mode { + case addrs.ManagedResourceMode: + buf.WriteString(fmt.Sprintf( + "resource %q %q", + addr.Resource.Resource.Type, + addr.Resource.Resource.Name, + )) + case addrs.DataResourceMode: + buf.WriteString(fmt.Sprintf( + "data %q %q ", + addr.Resource.Resource.Type, + addr.Resource.Resource.Name, + )) + default: + // should never happen, since the above is exhaustive + buf.WriteString(addr.String()) + } + + buf.WriteString(" {") + + p := blockBodyDiffPrinter{ + buf: &buf, + color: color, + action: action, + concise: experiment.Enabled(experiment.X_concise_diff), + } + + // Most commonly-used resources have nested blocks that result in us + // going at least three traversals deep while we recurse here, so we'll + // start with that much capacity and then grow as needed for deeper + // structures. + path := make(cty.Path, 0, 3) + + ty := schema.ImpliedType() + + var err error + var oldObj, newObj *states.ResourceInstanceObject + oldObj, err = before.Current.Decode(ty) + if err != nil { + // Should never happen in here, since we've already been through + // loads of layers of encode/decode of the planned changes before now. + panic(fmt.Sprintf("failed to decode old object for %s while rendering diff: %s", addr, err)) + } + if after != nil && after.Current != nil { + newObj, err = after.Current.Decode(ty) + if err != nil { + // Should never happen in here, since we've already been through + // loads of layers of encode/decode of the planned changes before now. + panic(fmt.Sprintf("failed to decode new object for %s while rendering diff: %s", addr, err)) + } + } + + oldVal := oldObj.Value + var newVal cty.Value + if newObj != nil { + newVal = newObj.Value + } else { + newVal = cty.NullVal(ty) + } + + if newVal.RawEquals(oldVal) { + // Nothing to show, then. + return "" + } + + // We currently have an opt-out that permits the legacy SDK to return values + // that defy our usual conventions around handling of nesting blocks. To + // avoid the rendering code from needing to handle all of these, we'll + // normalize first. + // (Ideally we'd do this as part of the SDK opt-out implementation in core, + // but we've added it here for now to reduce risk of unexpected impacts + // on other code in core.) + oldVal = objchange.NormalizeObjectFromLegacySDK(oldVal, schema) + newVal = objchange.NormalizeObjectFromLegacySDK(newVal, schema) + + result := p.writeBlockBodyDiff(schema, oldVal, newVal, 6, path) + if result.bodyWritten { + buf.WriteString("\n") + buf.WriteString(strings.Repeat(" ", 4)) + } + buf.WriteString("}\n") + + return buf.String() +} + // OutputChanges returns a string representation of a set of changes to output // values for inclusion in user-facing plan output. // diff --git a/command/show.go b/command/show.go index 56d0e34b9e86..f3551b3a1c54 100644 --- a/command/show.go +++ b/command/show.go @@ -163,7 +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. - localBackend.RenderPlan(plan, stateFile.State, schemas, c.Ui, c.Colorize()) + localBackend.RenderPlan(plan, stateFile.State, stateFile.State, schemas, c.Ui, c.Colorize()) return 0 } diff --git a/states/state_equal.go b/states/state_equal.go index ea20967e5b94..57d847aecbba 100644 --- a/states/state_equal.go +++ b/states/state_equal.go @@ -16,3 +16,23 @@ func (s *State) Equal(other *State) bool { // more sophisticated comparisons. return reflect.DeepEqual(s, other) } + +// Equal returns true if the receiver is functionally equivalent to other, +// including any ephemeral portions of the state that would not be included +// if the state were saved to files. +func (s *Resource) Equal(other *Resource) bool { + // For the moment this is sufficient, but we may need to do something + // more elaborate in future if we have any portions of state that require + // more sophisticated comparisons. + return reflect.DeepEqual(s, other) +} + +// Equal returns true if the receiver is functionally equivalent to other, +// including any ephemeral portions of the state that would not be included +// if the state were saved to files. +func (s *ResourceInstance) Equal(other *ResourceInstance) bool { + // For the moment this is sufficient, but we may need to do something + // more elaborate in future if we have any portions of state that require + // more sophisticated comparisons. + return reflect.DeepEqual(s, other) +}