Merge pull request #30486 from hashicorp/jbardin/drift

Only show external changes which contributed to the plan

internal/addrs/parse_ref.go

@@ -2,10 +2,12 @@ package addrs
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 	"github.com/hashicorp/terraform/internal/tfdiags"
+	"github.com/zclconf/go-cty/cty"
 )
 
 // Reference describes a reference to an address with source location
@@ -16,6 +18,43 @@ type Reference struct {
 	Remaining   hcl.Traversal
 }
 
+// DisplayString returns a string that approximates the subject and remaining
+// traversal of the reciever in a way that resembles the Terraform language
+// syntax that could've produced it.
+//
+// It's not guaranteed to actually be a valid Terraform language expression,
+// since the intended use here is primarily for UI messages such as
+// diagnostics.
+func (r *Reference) DisplayString() string {
+	if len(r.Remaining) == 0 {
+		// Easy case: we can just return the subject's string.
+		return r.Subject.String()
+	}
+
+	var ret strings.Builder
+	ret.WriteString(r.Subject.String())
+	for _, step := range r.Remaining {
+		switch tStep := step.(type) {
+		case hcl.TraverseRoot:
+			ret.WriteString(tStep.Name)
+		case hcl.TraverseAttr:
+			ret.WriteByte('.')
+			ret.WriteString(tStep.Name)
+		case hcl.TraverseIndex:
+			ret.WriteByte('[')
+			switch tStep.Key.Type() {
+			case cty.String:
+				ret.WriteString(fmt.Sprintf("%q", tStep.Key.AsString()))
+			case cty.Number:
+				bf := tStep.Key.AsBigFloat()
+				ret.WriteString(bf.Text('g', 10))
+			}
+			ret.WriteByte(']')
+		}
+	}
+	return ret.String()
+}
+
 // ParseRef attempts to extract a referencable address from the prefix of the
 // given traversal, which must be an absolute traversal or this function
 // will panic.

internal/command/format/diff.go

@@ -45,7 +45,7 @@ const (
 // If "color" is non-nil, it will be used to color the result. Otherwise,
 // no color codes will be included.
 func ResourceChange(
-	change *plans.ResourceInstanceChangeSrc,
+	change *plans.ResourceInstanceChange,
 	schema *configschema.Block,
 	color *colorstring.Colorize,
 	language DiffLanguage,
@@ -187,24 +187,7 @@ func ResourceChange(
 	// structures.
 	path := make(cty.Path, 0, 3)
 
-	changeV, err := change.Decode(schema.ImpliedType())
-	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 plan for %s while rendering diff: %s", addr, err))
-	}
-
-	// 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.)
-	changeV.Change.Before = objchange.NormalizeObjectFromLegacySDK(changeV.Change.Before, schema)
-	changeV.Change.After = objchange.NormalizeObjectFromLegacySDK(changeV.Change.After, schema)
-
-	result := p.writeBlockBodyDiff(schema, changeV.Before, changeV.After, 6, path)
+	result := p.writeBlockBodyDiff(schema, change.Before, change.After, 6, path)
 	if result.bodyWritten {
 		buf.WriteString("\n")
 		buf.WriteString(strings.Repeat(" ", 4))

internal/command/format/diff_test.go

@@ -4857,10 +4857,6 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
 			case !beforeVal.IsKnown():
 				beforeVal = cty.UnknownVal(ty) // allow mistyped unknowns
 			}
-			before, err := plans.NewDynamicValue(beforeVal, ty)
-			if err != nil {
-				t.Fatal(err)
-			}
 
 			afterVal := tc.After
 			switch { // Some fixups to make the test cases a little easier to write
@@ -4869,10 +4865,6 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
 			case !afterVal.IsKnown():
 				afterVal = cty.UnknownVal(ty) // allow mistyped unknowns
 			}
-			after, err := plans.NewDynamicValue(afterVal, ty)
-			if err != nil {
-				t.Fatal(err)
-			}
 
 			addr := addrs.Resource{
 				Mode: tc.Mode,
@@ -4887,20 +4879,18 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
 				prevRunAddr = addr
 			}
 
-			change := &plans.ResourceInstanceChangeSrc{
+			change := &plans.ResourceInstanceChange{
 				Addr:        addr,
 				PrevRunAddr: prevRunAddr,
 				DeposedKey:  tc.DeposedKey,
 				ProviderAddr: addrs.AbsProviderConfig{
 					Provider: addrs.NewDefaultProvider("test"),
 					Module:   addrs.RootModule,
 				},
-				ChangeSrc: plans.ChangeSrc{
-					Action:         tc.Action,
-					Before:         before,
-					After:          after,
-					BeforeValMarks: tc.BeforeValMarks,
-					AfterValMarks:  tc.AfterValMarks,
+				Change: plans.Change{
+					Action: tc.Action,
+					Before: beforeVal.MarkWithPaths(tc.BeforeValMarks),
+					After:  afterVal.MarkWithPaths(tc.AfterValMarks),
 				},
 				ActionReason:    tc.ActionReason,
 				RequiredReplace: tc.RequiredReplace,

internal/command/jsonplan/plan.go

@@ -33,11 +33,12 @@ type plan struct {
 	PlannedValues    stateValues `json:"planned_values,omitempty"`
 	// ResourceDrift and ResourceChanges are sorted in a user-friendly order
 	// that is undefined at this time, but consistent.
-	ResourceDrift   []resourceChange  `json:"resource_drift,omitempty"`
-	ResourceChanges []resourceChange  `json:"resource_changes,omitempty"`
-	OutputChanges   map[string]change `json:"output_changes,omitempty"`
-	PriorState      json.RawMessage   `json:"prior_state,omitempty"`
-	Config          json.RawMessage   `json:"configuration,omitempty"`
+	ResourceDrift      []resourceChange  `json:"resource_drift,omitempty"`
+	ResourceChanges    []resourceChange  `json:"resource_changes,omitempty"`
+	OutputChanges      map[string]change `json:"output_changes,omitempty"`
+	PriorState         json.RawMessage   `json:"prior_state,omitempty"`
+	Config             json.RawMessage   `json:"configuration,omitempty"`
+	RelevantAttributes []resourceAttr    `json:"relevant_attributes,omitempty"`
 }
 
 func newPlan() *plan {
@@ -46,6 +47,13 @@ func newPlan() *plan {
 	}
 }
 
+// resourceAttr contains the address and attribute of an external for the
+// RelevantAttributes in the plan.
+type resourceAttr struct {
+	Resource string          `json:"resource"`
+	Attr     json.RawMessage `json:"attribute"`
+}
+
 // Change is the representation of a proposed change for an object.
 type change struct {
 	// Actions are the actions that will be taken on the object selected by the
@@ -151,6 +159,10 @@ func Marshal(
 		}
 	}
 
+	if err := output.marshalRelevantAttrs(p); err != nil {
+		return nil, fmt.Errorf("error marshaling relevant attributes for external changes: %s", err)
+	}
+
 	// output.ResourceChanges
 	if p.Changes != nil {
 		output.ResourceChanges, err = output.marshalResourceChanges(p.Changes.Resources, schemas)
@@ -482,6 +494,19 @@ func (p *plan) marshalPlannedValues(changes *plans.Changes, schemas *terraform.S
 	return nil
 }
 
+func (p *plan) marshalRelevantAttrs(plan *plans.Plan) error {
+	for _, ra := range plan.RelevantAttributes {
+		addr := ra.Resource.String()
+		path, err := encodePath(ra.Attr)
+		if err != nil {
+			return err
+		}
+
+		p.RelevantAttributes = append(p.RelevantAttributes, resourceAttr{addr, path})
+	}
+	return nil
+}
+
 // omitUnknowns recursively walks the src cty.Value and returns a new cty.Value,
 // omitting any unknowns.
 //
@@ -655,26 +680,7 @@ func encodePaths(pathSet cty.PathSet) (json.RawMessage, error) {
 	jsonPaths := make([]json.RawMessage, 0, len(pathList))
 
 	for _, path := range pathList {
-		steps := make([]json.RawMessage, 0, len(path))
-		for _, step := range path {
-			switch s := step.(type) {
-			case cty.IndexStep:
-				key, err := ctyjson.Marshal(s.Key, s.Key.Type())
-				if err != nil {
-					return nil, fmt.Errorf("Failed to marshal index step key %#v: %s", s.Key, err)
-				}
-				steps = append(steps, key)
-			case cty.GetAttrStep:
-				name, err := json.Marshal(s.Name)
-				if err != nil {
-					return nil, fmt.Errorf("Failed to marshal get attr step name %#v: %s", s.Name, err)
-				}
-				steps = append(steps, name)
-			default:
-				return nil, fmt.Errorf("Unsupported path step %#v (%t)", step, step)
-			}
-		}
-		jsonPath, err := json.Marshal(steps)
+		jsonPath, err := encodePath(path)
 		if err != nil {
 			return nil, err
 		}
@@ -683,3 +689,26 @@ func encodePaths(pathSet cty.PathSet) (json.RawMessage, error) {
 
 	return json.Marshal(jsonPaths)
 }
+
+func encodePath(path cty.Path) (json.RawMessage, error) {
+	steps := make([]json.RawMessage, 0, len(path))
+	for _, step := range path {
+		switch s := step.(type) {
+		case cty.IndexStep:
+			key, err := ctyjson.Marshal(s.Key, s.Key.Type())
+			if err != nil {
+				return nil, fmt.Errorf("Failed to marshal index step key %#v: %s", s.Key, err)
+			}
+			steps = append(steps, key)
+		case cty.GetAttrStep:
+			name, err := json.Marshal(s.Name)
+			if err != nil {
+				return nil, fmt.Errorf("Failed to marshal get attr step name %#v: %s", s.Name, err)
+			}
+			steps = append(steps, name)
+		default:
+			return nil, fmt.Errorf("Unsupported path step %#v (%t)", step, step)
+		}
+	}
+	return json.Marshal(steps)
+}

internal/command/views/operation_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/command/arguments"
+	"github.com/hashicorp/terraform/internal/lang/globalref"
 	"github.com/hashicorp/terraform/internal/plans"
 	"github.com/hashicorp/terraform/internal/states"
 	"github.com/hashicorp/terraform/internal/states/statefile"
@@ -107,7 +108,7 @@ func TestOperation_planNoChanges(t *testing.T) {
 			},
 			"No objects need to be destroyed.",
 		},
-		"drift detected in normal mode": {
+		"no drift detected in normal noop": {
 			func(schemas *terraform.Schemas) *plans.Plan {
 				addr := addrs.Resource{
 					Mode: addrs.ManagedResourceMode,
@@ -146,7 +147,54 @@ func TestOperation_planNoChanges(t *testing.T) {
 					DriftedResources: drs,
 				}
 			},
-			"to update the Terraform state to match, create and apply a refresh-only plan",
+			"No changes",
+		},
+		"drift detected in normal mode": {
+			func(schemas *terraform.Schemas) *plans.Plan {
+				addr := addrs.Resource{
+					Mode: addrs.ManagedResourceMode,
+					Type: "test_resource",
+					Name: "somewhere",
+				}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance)
+				schema, _ := schemas.ResourceTypeConfig(
+					addrs.NewDefaultProvider("test"),
+					addr.Resource.Resource.Mode,
+					addr.Resource.Resource.Type,
+				)
+				ty := schema.ImpliedType()
+				rc := &plans.ResourceInstanceChange{
+					Addr:        addr,
+					PrevRunAddr: addr,
+					ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(
+						addrs.NewDefaultProvider("test"),
+					),
+					Change: plans.Change{
+						Action: plans.Update,
+						Before: cty.NullVal(ty),
+						After: cty.ObjectVal(map[string]cty.Value{
+							"id":  cty.StringVal("1234"),
+							"foo": cty.StringVal("bar"),
+						}),
+					},
+				}
+				rcs, err := rc.Encode(ty)
+				if err != nil {
+					panic(err)
+				}
+				drs := []*plans.ResourceInstanceChangeSrc{rcs}
+				changes := plans.NewChanges()
+				changes.Resources = drs
+				return &plans.Plan{
+					UIMode:           plans.NormalMode,
+					Changes:          changes,
+					DriftedResources: drs,
+					RelevantAttributes: []globalref.ResourceAttr{{
+						Resource: addr,
+						Attr:     cty.GetAttrPath("id"),
+					}},
+				}
+			},
+			"Objects have changed outside of Terraform",
 		},
 		"drift detected in refresh-only mode": {
 			func(schemas *terraform.Schemas) *plans.Plan {