Commit f90e5082 authored by Martin Atkins's avatar Martin Atkins
Browse files

Prototype of de-emphasizing less relevant refresh-detected changes

parent d211b53e
Showing with 244 additions and 31 deletions
+244 -31
......@@ -5,7 +5,9 @@ import (
"fmt"
"log"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/backend"
"github.com/hashicorp/terraform/internal/lang/globalref"
"github.com/hashicorp/terraform/internal/plans"
"github.com/hashicorp/terraform/internal/plans/planfile"
"github.com/hashicorp/terraform/internal/states/statefile"
......@@ -155,3 +157,36 @@ func (b *Local) opPlan(
op.View.PlanNextStep(op.PlanOutPath)
}
}
// findResourcesPossiblyContributingToPlan is a heuristic that tries to
// identify which upstream resources may have led to the changes proposed
// in the given plan.
//
// We use this as a hint for the human-oriented renderer of detected changes
// outside of Terraform (from "refresh") so it can use a more compact
// presentation for external changes that seem unrelated to the changes
// Terraform is proposing to make.
func findResourcesPossiblyContributingToPlan(azr *globalref.Analyzer, plan *plans.Plan) []addrs.AbsResource {
// For the moment we're using a relatively coarse heuristic here: a
// resource "possibly contributes" if we can find a possibly-indirect
// path to that resource through references from any resource that has a
// proposed change in the plan.
//
// This doesn't consider exactly which attributes are proposed to be
// changed or that have had external changes detected. Perhaps we'll make
// this more precise later, if that seems warranted.
// Collect up all of the references inside the configuration for any
// resource instance that has a proposed change.
var refs []globalref.Reference
for _, change := range plan.Changes.Resources {
if change.Action == plans.NoOp {
continue
}
instAddr := change.Addr
moreRefs := azr.ReferencesFromResourceInstance(instAddr)
refs = append(refs, moreRefs...)
}
return azr.ContributingResources(refs...)
}
......@@ -280,6 +280,53 @@ func ResourceInstanceDrift(
return buf.String()
}
// ResourceInstanceDriftSummary is similar to ResourceInstanceDrift but it
// reduces the reported information only to a single-line report containing
// only the change type icon and the resource instance address.
//
// This is intended for concisely reporting changes we've detected but which
// don't seem to have contributed to any proposed changes in the plan.
func ResourceInstanceDriftSummary(
addr addrs.AbsResourceInstance,
before, after *states.ResourceInstance,
schema *configschema.Block,
color *colorstring.Colorize,
) string {
// HACK: ResourceInstanceDrift includes some logic to deal with legacy
// SDK misbehavior, some of which it only detects while actually
// _rendering_ the diff, and so we'll actually render a full diff
// here and throw away the result just to make sure that we always
// make the same decision in the summary case. Gross!
if diff := ResourceInstanceDrift(addr, before, after, schema, nil); diff == "" {
return ""
}
if color == nil {
color = &colorstring.Colorize{
Colors: colorstring.DefaultColors,
Disable: true,
Reset: false,
}
}
dispAddr := addr.String()
action := plans.Update
switch {
case before == nil || before.Current == nil:
// before should never be nil, but before.Current can be if the
// instance was deposed. There is nothing to render for a deposed
// instance, since we intend to remove it.
return ""
case after == nil || after.Current == nil:
action = plans.Delete
}
symbol := color.Color(DiffActionSymbol(action))
return fmt.Sprintf("%s %s\n", symbol, dispAddr)
}
// OutputChanges returns a string representation of a set of changes to output
// values for inclusion in user-facing plan output.
//
......
......@@ -97,21 +97,7 @@ func (v *PlanJSON) HelpPrompt() {
// The plan renderer is used by the Operation view (for plan and apply
// commands) and the Show view (for the show command).
func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) {
haveRefreshChanges := renderChangesDetectedByRefresh(plan.PrevRunState, plan.PriorState, schemas, view)
if haveRefreshChanges {
switch plan.UIMode {
case plans.RefreshOnlyMode:
view.streams.Println(format.WordWrap(
"\nThis is a refresh-only plan, so Terraform will not take any actions to undo these. If you were expecting these changes then you can apply this plan to record the updated values in the Terraform state without changing any remote objects.",
view.outputColumns(),
))
default:
view.streams.Println(format.WordWrap(
"\nUnless you have made equivalent changes to your configuration, or ignored the relevant attributes using ignore_changes, the following plan may include actions to undo or respond to these changes.",
view.outputColumns(),
))
}
}
haveRefreshChanges := renderChangesDetectedByRefresh(plan.UIMode, plan.PrevRunState, plan.PriorState, plan.RelevantResources, schemas, view)
counts := map[plans.Action]int{}
var rChanges []*plans.ResourceInstanceChangeSrc
......@@ -338,7 +324,7 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) {
// renderChangesDetectedByRefresh returns true if it produced at least one
// line of output, and guarantees to always produce whole lines terminated
// by newline characters.
func renderChangesDetectedByRefresh(before, after *states.State, schemas *terraform.Schemas, view *View) bool {
func renderChangesDetectedByRefresh(mode plans.Mode, before, after *states.State, relevant []addrs.AbsResource, schemas *terraform.Schemas, view *View) bool {
// ManagedResourceEqual checks that the state is exactly equal for all
// managed resources; but semantically equivalent states, or changes to
// deposed instances may not actually represent changes we need to present
......@@ -349,6 +335,7 @@ func renderChangesDetectedByRefresh(before, after *states.State, schemas *terraf
}
var diffs []string
var summaries []string
for _, bms := range before.Modules {
for _, brs := range bms.Resources {
......@@ -382,14 +369,41 @@ func renderChangesDetectedByRefresh(before, after *states.State, schemas *terraf
pis = prs.Instance(key)
}
diff := format.ResourceInstanceDrift(
addr.Instance(key),
bis, pis,
rSchema,
view.colorize,
)
if diff != "" {
diffs = append(diffs, diff)
isRelevant := false
switch mode {
case plans.RefreshOnlyMode:
// All changes are "relevant" in refresh-only mode, because
// detecting changes is the point of that mode.
isRelevant = true
default:
for _, candidate := range relevant {
if addr.Equal(candidate) {
isRelevant = true
break
}
}
}
if isRelevant {
diff := format.ResourceInstanceDrift(
addr.Instance(key),
bis, pis,
rSchema,
view.colorize,
)
if diff != "" {
diffs = append(diffs, diff)
}
} else {
summary := format.ResourceInstanceDriftSummary(
addr.Instance(key),
bis, pis,
rSchema,
view.colorize,
)
if summary != "" {
summaries = append(summaries, summary)
}
}
}
}
......@@ -398,18 +412,62 @@ func renderChangesDetectedByRefresh(before, after *states.State, schemas *terraf
// If we only have changes regarding deposed instances, or the diff
// renderer is suppressing irrelevant changes from the legacy SDK, there
// may not have been anything to display to the user.
if len(diffs) > 0 {
if len(diffs) > 0 || len(summaries) > 0 {
view.streams.Print(
view.colorize.Color("[reset]\n[bold][cyan]Note:[reset][bold] Objects have changed outside of Terraform[reset]\n\n"),
)
view.streams.Print(format.WordWrap(
"Terraform detected the following changes made outside of Terraform since the last \"terraform apply\":\n\n",
view.outputColumns(),
))
if len(diffs) > 0 {
if len(summaries) > 0 {
view.streams.Print(format.WordWrap(
"Terraform detected the following changes made outside of Terraform since the last \"terraform apply\", which may have caused proposed changes below:\n\n",
view.outputColumns(),
))
} else {
view.streams.Print(format.WordWrap(
"Terraform detected the following changes made outside of Terraform since the last \"terraform apply\":\n\n",
view.outputColumns(),
))
}
for _, diff := range diffs {
view.streams.Print(diff)
}
switch mode {
case plans.RefreshOnlyMode:
view.streams.Println(format.WordWrap(
"\nThis is a refresh-only plan, so Terraform will not take any actions to undo these. If you were expecting these changes then you can apply this plan to record the updated values in the Terraform state without changing any remote objects.",
view.outputColumns(),
))
default:
view.streams.Println(format.WordWrap(
"\nUnless you have made equivalent changes to your configuration, or ignored the relevant attributes using ignore_changes, the following plan may include actions to undo or respond to these changes.",
view.outputColumns(),
))
}
for _, diff := range diffs {
view.streams.Print(diff)
}
if len(summaries) > 0 {
if len(diffs) > 0 {
view.streams.Print(format.WordWrap(
"\nTerraform also changes to some other resource instances which don't seem to affect the plan:\n",
view.outputColumns(),
))
} else {
view.streams.Print(format.WordWrap(
"Terraform detected changes to the following resource instances made outside of Terraform, but they don't require Terraform to take any corrective action:\n",
view.outputColumns(),
))
}
for _, summary := range summaries {
view.streams.Print(summary)
}
if len(diffs) == 0 {
view.streams.Print(format.WordWrap(
"\nFor more detail on these detected changes, create a refresh-only plan.\n",
view.outputColumns(),
))
}
}
return true
}
......
......@@ -36,6 +36,21 @@ type Plan struct {
ProviderSHA256s map[string][]byte
Backend Backend
// RelevantResources is a set of resource addresses that are either
// directly affected by proposed changes or may have indirectly contributed
// to them via references in expressions.
//
// This is the result of a heuristic and is intended only as a hint to
// the UI layer in case it wants to emphasize or de-emphasize certain
// resources. Don't use this to drive any non-cosmetic behavior, especially
// including anything that would be subject to compatibility constraints.
//
// FIXME: This result currently doesn't survive round-tripping through a
// saved plan file, and so it'll be populated only for a freshly-created
// plan that has only existed in memory so far. When reloading a saved
// plan it will always appear as if there are no "relevant resources".
RelevantResources []addrs.AbsResource
// PrevRunState and PriorState both describe the situation that the plan
// was derived from:
//
......
......@@ -12,6 +12,7 @@ import (
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/lang"
"github.com/hashicorp/terraform/internal/lang/globalref"
"github.com/hashicorp/terraform/internal/plans"
"github.com/hashicorp/terraform/internal/providers"
"github.com/hashicorp/terraform/internal/provisioners"
......@@ -684,6 +685,7 @@ func (c *Context) plan() (*plans.Plan, tfdiags.Diagnostics) {
// to Apply work as expected.
c.state = refreshedState
plan.RelevantResources = c.relevantResourcesForPlan(plan)
return plan, diags
}
......@@ -746,6 +748,7 @@ func (c *Context) destroyPlan() (*plans.Plan, tfdiags.Diagnostics) {
destroyPlan.UIMode = plans.DestroyMode
destroyPlan.Changes = c.changes
destroyPlan.RelevantResources = c.relevantResourcesForPlan(destroyPlan)
return destroyPlan, diags
}
......@@ -802,9 +805,57 @@ func (c *Context) refreshOnlyPlan() (*plans.Plan, tfdiags.Diagnostics) {
// mutate
c.state = refreshedState
// We don't populate RelevantResources for a refresh-only plan, because
// they never have any planned actions and so no resource can ever be
// "relevant" per the intended meaning of that field.
return plan, diags
}
// relevantResourcesForPlan implements the heuristic we use to populate the
// RelevantResources field of returned plans.
func (c *Context) relevantResourcesForPlan(plan *plans.Plan) []addrs.AbsResource {
azr := c.ReferenceAnalyzer()
// Our current strategy is that a resource is relevant if it either has
// a proposed change action directly, or if its attributes are used as
// any part of a resource that has a proposed change action. We don't
// consider individual changed attributes for now, because we can't
// really reason about any rules that providers might have about changes
// to one attribute implying a change to another.
// We'll use the string representation of a resource address as a unique
// key so we can dedupe our results.
relevant := make(map[string]addrs.AbsResource)
var refs []globalref.Reference
for _, change := range plan.Changes.Resources {
if change.Action == plans.NoOp {
continue
}
instAddr := change.Addr
addr := instAddr.ContainingResource()
relevant[addr.String()] = addr
moreRefs := azr.ReferencesFromResourceInstance(instAddr)
refs = append(refs, moreRefs...)
}
contributors := azr.ContributingResources(refs...)
for _, addr := range contributors {
relevant[addr.String()] = addr
}
if len(relevant) == 0 {
return nil
}
ret := make([]addrs.AbsResource, 0, len(relevant))
for _, addr := range relevant {
ret = append(ret, addr)
}
return ret
}
// Refresh goes through all the resources in the state and refreshes them
// to their latest state. This is done by executing a plan, and retaining the
// state while discarding the change set.
......@@ -906,6 +957,13 @@ func (c *Context) SetVariable(k string, v cty.Value) {
}
}
// ReferenceAnalyzer returns a globalref.Analyzer object to help with
// global analysis of references within the configuration that's attached
// to the receiving context.
func (c *Context) ReferenceAnalyzer() *globalref.Analyzer {
return globalref.NewAnalyzer(c.Config(), c.Schemas().Providers)
}
func (c *Context) acquireRun(phase string) func() {
// With the run lock held, grab the context lock to make changes
// to the run context.
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment