Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backend/local: Show resource instance drift as part of the plan #26921

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion backend/local/backend_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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("")
}

Expand Down
99 changes: 95 additions & 4 deletions backend/local/backend_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we use this approach for rendering drift, there are two non-ideal consequences:

  • terraform plan -out=saved.tfplan and terraform show saved.tfplan will not render the same result (the drift will be dropped)
  • terraform show -json saved.tfplan does not have enough information to render drift

There are a couple of ways that I can think of handling this (either or both of which may be flawed):

  1. Extend the plan file format to store a set of objects similar to ResourceInstanceChange for the drifted resources, computing those from the diff between the two states, and then correspondingly update the plan renderer and JSON plan output
  2. Store the base state in the plan file along with the refreshed state, use those to render the plan

As a potential consumer of the JSON plan, I would much rather see option 1 if possible, so long as the resource_drift objects are semantically equivalent to the existing resource_changes objects. I can then reuse the same logic to do whatever I want with that information. If only option 2 is possible, I'd have to write new logic to diff the state files myself, which is possible but feels like more work.


counts := map[plans.Action]int{}
var rChanges []*plans.ResourceInstanceChangeSrc
for _, change := range plan.Changes.Resources {
Expand Down Expand Up @@ -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:
Expand Down
131 changes: 131 additions & 0 deletions command/format/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down
2 changes: 1 addition & 1 deletion command/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
20 changes: 20 additions & 0 deletions states/state_equal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}