From f0cf4235f9e8eafe1d13a6a6e0720f0f0bc67e7e Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Mon, 13 Sep 2021 17:30:16 -0400 Subject: [PATCH] cli: Refactor resource drift rendering --- internal/command/format/diff.go | 175 +++------------ internal/command/format/diff_test.go | 2 +- .../command/format/difflanguage_string.go | 29 +++ internal/command/jsonplan/plan.go | 187 +++------------- .../testdata/show-json/drift/output.json | 1 + .../multi-resource-update/output.json | 3 +- internal/command/views/operation_test.go | 209 +++++++----------- internal/command/views/plan.go | 118 ++++------ 8 files changed, 209 insertions(+), 515 deletions(-) create mode 100644 internal/command/format/difflanguage_string.go diff --git a/internal/command/format/diff.go b/internal/command/format/diff.go index e111485e0..0028d0038 100644 --- a/internal/command/format/diff.go +++ b/internal/command/format/diff.go @@ -20,6 +20,21 @@ import ( "github.com/hashicorp/terraform/internal/states" ) +// DiffLanguage controls the description of the resource change reasons. +type DiffLanguage rune + +//go:generate go run golang.org/x/tools/cmd/stringer -type=DiffLanguage diff.go + +const ( + // DiffLanguageProposedChange indicates that the change is one which is + // planned to be applied. + DiffLanguageProposedChange DiffLanguage = 'P' + + // DiffLanguageDetectedDrift indicates that the change is detected drift + // from the configuration. + DiffLanguageDetectedDrift DiffLanguage = 'D' +) + // ResourceChange returns a string representation of a change to a particular // resource, for inclusion in user-facing plan output. // @@ -33,6 +48,7 @@ func ResourceChange( change *plans.ResourceInstanceChangeSrc, schema *configschema.Block, color *colorstring.Colorize, + language DiffLanguage, ) string { addr := change.Addr var buf bytes.Buffer @@ -56,7 +72,14 @@ func ResourceChange( case plans.Read: buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] will be read during apply\n # (config refers to values not yet known)", dispAddr))) case plans.Update: - buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] will be updated in-place", dispAddr))) + switch language { + case DiffLanguageProposedChange: + buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] will be updated in-place", dispAddr))) + case DiffLanguageDetectedDrift: + buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] has changed", dispAddr))) + default: + buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] update (unknown reason %s)", dispAddr, language))) + } case plans.CreateThenDelete, plans.DeleteThenCreate: switch change.ActionReason { case plans.ResourceInstanceReplaceBecauseTainted: @@ -67,7 +90,14 @@ func ResourceChange( buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] must be [bold][red]replaced", dispAddr))) } case plans.Delete: - buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] will be [bold][red]destroyed", dispAddr))) + switch language { + case DiffLanguageProposedChange: + buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] will be [bold][red]destroyed", dispAddr))) + case DiffLanguageDetectedDrift: + buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] has been deleted", dispAddr))) + default: + buf.WriteString(color.Color(fmt.Sprintf("[bold] # %s[reset] delete (unknown reason %s)", dispAddr, language))) + } if change.DeposedKey != states.NotDeposed { // Some extra context about this unusual situation. buf.WriteString(color.Color("\n # (left over from a partially-failed replacement of this instance)")) @@ -154,147 +184,6 @@ 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 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: - // 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, - } - - // 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 { - // We shouldn't encounter errors here because Terraform Core should've - // made sure that the previous run object conforms to the current - // schema by having the provider upgrade it, but we'll be robust here - // in case there are some edges we didn't find yet. - return fmt.Sprintf(" # %s previous run state doesn't conform to current schema; this is a Terraform bug\n # %s\n", addr, err) - } - if after != nil && after.Current != nil { - newObj, err = after.Current.Decode(ty) - if err != nil { - // We shouldn't encounter errors here because Terraform Core should've - // made sure that the prior state object conforms to the current - // schema by having the provider upgrade it, even if we skipped - // refreshing on this run, but we'll be robust here in case there are - // some edges we didn't find yet. - return fmt.Sprintf(" # %s refreshed state doesn't conform to current schema; this is a Terraform bug\n # %s\n", 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/internal/command/format/diff_test.go b/internal/command/format/diff_test.go index 7ca289c79..b59a3af56 100644 --- a/internal/command/format/diff_test.go +++ b/internal/command/format/diff_test.go @@ -4599,7 +4599,7 @@ func runTestCases(t *testing.T, testCases map[string]testCase) { RequiredReplace: tc.RequiredReplace, } - output := ResourceChange(change, tc.Schema, color) + output := ResourceChange(change, tc.Schema, color, DiffLanguageProposedChange) if diff := cmp.Diff(output, tc.ExpectedOutput); diff != "" { t.Errorf("wrong output\n%s", diff) } diff --git a/internal/command/format/difflanguage_string.go b/internal/command/format/difflanguage_string.go new file mode 100644 index 000000000..8399cddc4 --- /dev/null +++ b/internal/command/format/difflanguage_string.go @@ -0,0 +1,29 @@ +// Code generated by "stringer -type=DiffLanguage diff.go"; DO NOT EDIT. + +package format + +import "strconv" + +func _() { + // An "invalid array index" compiler error signifies that the constant values have changed. + // Re-run the stringer command to generate them again. + var x [1]struct{} + _ = x[DiffLanguageProposedChange-80] + _ = x[DiffLanguageDetectedDrift-68] +} + +const ( + _DiffLanguage_name_0 = "DiffLanguageDetectedDrift" + _DiffLanguage_name_1 = "DiffLanguageProposedChange" +) + +func (i DiffLanguage) String() string { + switch { + case i == 68: + return _DiffLanguage_name_0 + case i == 80: + return _DiffLanguage_name_1 + default: + return "DiffLanguage(" + strconv.FormatInt(int64(i), 10) + ")" + } +} diff --git a/internal/command/jsonplan/plan.go b/internal/command/jsonplan/plan.go index 1b90daf2d..1d7eff1ff 100644 --- a/internal/command/jsonplan/plan.go +++ b/internal/command/jsonplan/plan.go @@ -130,15 +130,17 @@ func Marshal( } // output.ResourceDrift - err = output.marshalResourceDrift(p.PrevRunState, p.PriorState, schemas) + output.ResourceDrift, err = output.marshalResourceChanges(p.DriftedResources, schemas) if err != nil { - return nil, fmt.Errorf("error in marshalResourceDrift: %s", err) + return nil, fmt.Errorf("error in marshaling resource drift: %s", err) } // output.ResourceChanges - err = output.marshalResourceChanges(p.Changes, schemas) - if err != nil { - return nil, fmt.Errorf("error in marshalResourceChanges: %s", err) + if p.Changes != nil { + output.ResourceChanges, err = output.marshalResourceChanges(p.Changes.Resources, schemas) + if err != nil { + return nil, fmt.Errorf("error in marshaling resource changes: %s", err) + } } // output.OutputChanges @@ -188,149 +190,10 @@ func (p *plan) marshalPlanVariables(vars map[string]plans.DynamicValue, schemas return nil } -func (p *plan) marshalResourceDrift(oldState, newState *states.State, schemas *terraform.Schemas) error { - // Our goal here is to build a data structure of the same shape as we use - // to describe planned resource changes, but in this case we'll be - // taking the old and new values from different state snapshots rather - // than from a real "Changes" object. - // - // In doing this we make an assumption that drift detection can only - // ever show objects as updated or removed, and will never show anything - // as created because we only refresh objects we were already tracking - // after the previous run. This means we can use oldState as our baseline - // for what resource instances we might include, and check for each item - // whether it's present in newState. If we ever have some mechanism to - // detect "additive drift" later then we'll need to take a different - // approach here, but we have no plans for that at the time of writing. - // - // We also assume that both states have had all managed resource objects - // upgraded to match the current schemas given in schemas, so we shouldn't - // need to contend with oldState having old-shaped objects even if the - // user changed provider versions since the last run. +func (p *plan) marshalResourceChanges(resources []*plans.ResourceInstanceChangeSrc, schemas *terraform.Schemas) ([]resourceChange, error) { + var ret []resourceChange - if newState.ManagedResourcesEqual(oldState) { - // Nothing to do, because we only detect and report drift for managed - // resource instances. - return nil - } - for _, ms := range oldState.Modules { - for _, rs := range ms.Resources { - if rs.Addr.Resource.Mode != addrs.ManagedResourceMode { - // Drift reporting is only for managed resources - continue - } - - provider := rs.ProviderConfig.Provider - for key, oldIS := range rs.Instances { - if oldIS.Current == nil { - // Not interested in instances that only have deposed objects - continue - } - addr := rs.Addr.Instance(key) - newIS := newState.ResourceInstance(addr) - - schema, _ := schemas.ResourceTypeConfig( - provider, - addr.Resource.Resource.Mode, - addr.Resource.Resource.Type, - ) - if schema == nil { - return fmt.Errorf("no schema found for %s (in provider %s)", addr, provider) - } - ty := schema.ImpliedType() - - oldObj, err := oldIS.Current.Decode(ty) - if err != nil { - return fmt.Errorf("failed to decode previous run data for %s: %s", addr, err) - } - - var newObj *states.ResourceInstanceObject - if newIS != nil && newIS.Current != nil { - newObj, err = newIS.Current.Decode(ty) - if err != nil { - return fmt.Errorf("failed to decode refreshed data for %s: %s", addr, err) - } - } - - var oldVal, newVal cty.Value - oldVal = oldObj.Value - if newObj != nil { - newVal = newObj.Value - } else { - newVal = cty.NullVal(ty) - } - - if oldVal.RawEquals(newVal) { - // No drift if the two values are semantically equivalent - continue - } - - oldSensitive := jsonstate.SensitiveAsBool(oldVal) - newSensitive := jsonstate.SensitiveAsBool(newVal) - oldVal, _ = oldVal.UnmarkDeep() - newVal, _ = newVal.UnmarkDeep() - - var before, after []byte - var beforeSensitive, afterSensitive []byte - before, err = ctyjson.Marshal(oldVal, oldVal.Type()) - if err != nil { - return fmt.Errorf("failed to encode previous run data for %s as JSON: %s", addr, err) - } - after, err = ctyjson.Marshal(newVal, oldVal.Type()) - if err != nil { - return fmt.Errorf("failed to encode refreshed data for %s as JSON: %s", addr, err) - } - beforeSensitive, err = ctyjson.Marshal(oldSensitive, oldSensitive.Type()) - if err != nil { - return fmt.Errorf("failed to encode previous run data sensitivity for %s as JSON: %s", addr, err) - } - afterSensitive, err = ctyjson.Marshal(newSensitive, newSensitive.Type()) - if err != nil { - return fmt.Errorf("failed to encode refreshed data sensitivity for %s as JSON: %s", addr, err) - } - - // We can only detect updates and deletes as drift. - action := plans.Update - if newVal.IsNull() { - action = plans.Delete - } - - change := resourceChange{ - Address: addr.String(), - ModuleAddress: addr.Module.String(), - Mode: "managed", // drift reporting is only for managed resources - Name: addr.Resource.Resource.Name, - Type: addr.Resource.Resource.Type, - ProviderName: provider.String(), - - Change: change{ - Actions: actionString(action.String()), - Before: json.RawMessage(before), - BeforeSensitive: json.RawMessage(beforeSensitive), - After: json.RawMessage(after), - AfterSensitive: json.RawMessage(afterSensitive), - // AfterUnknown is never populated here because - // values in a state are always fully known. - }, - } - p.ResourceDrift = append(p.ResourceDrift, change) - } - } - } - - sort.Slice(p.ResourceChanges, func(i, j int) bool { - return p.ResourceChanges[i].Address < p.ResourceChanges[j].Address - }) - - return nil -} - -func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform.Schemas) error { - if changes == nil { - // Nothing to do! - return nil - } - for _, rc := range changes.Resources { + for _, rc := range resources { var r resourceChange addr := rc.Addr r.Address = addr.String() @@ -349,12 +212,12 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform addr.Resource.Resource.Type, ) if schema == nil { - return fmt.Errorf("no schema found for %s (in provider %s)", r.Address, rc.ProviderAddr.Provider) + return nil, fmt.Errorf("no schema found for %s (in provider %s)", r.Address, rc.ProviderAddr.Provider) } changeV, err := rc.Decode(schema.ImpliedType()) if err != nil { - return err + return nil, err } // We drop the marks from the change, as decoding is only an // intermediate step to re-encode the values as json @@ -368,7 +231,7 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform if changeV.Before != cty.NilVal { before, err = ctyjson.Marshal(changeV.Before, changeV.Before.Type()) if err != nil { - return err + return nil, err } marks := rc.BeforeValMarks if schema.ContainsSensitive() { @@ -377,14 +240,14 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform bs := jsonstate.SensitiveAsBool(changeV.Before.MarkWithPaths(marks)) beforeSensitive, err = ctyjson.Marshal(bs, bs.Type()) if err != nil { - return err + return nil, err } } if changeV.After != cty.NilVal { if changeV.After.IsWhollyKnown() { after, err = ctyjson.Marshal(changeV.After, changeV.After.Type()) if err != nil { - return err + return nil, err } afterUnknown = cty.EmptyObjectVal } else { @@ -394,7 +257,7 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform } else { after, err = ctyjson.Marshal(filteredAfter, filteredAfter.Type()) if err != nil { - return err + return nil, err } } afterUnknown = unknownAsBool(changeV.After) @@ -406,17 +269,17 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform as := jsonstate.SensitiveAsBool(changeV.After.MarkWithPaths(marks)) afterSensitive, err = ctyjson.Marshal(as, as.Type()) if err != nil { - return err + return nil, err } } a, err := ctyjson.Marshal(afterUnknown, afterUnknown.Type()) if err != nil { - return err + return nil, err } replacePaths, err := encodePaths(rc.RequiredReplace) if err != nil { - return err + return nil, err } r.Change = change{ @@ -444,7 +307,7 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform case addrs.DataResourceMode: r.Mode = "data" default: - return fmt.Errorf("resource %s has an unsupported mode %s", r.Address, addr.Resource.Resource.Mode.String()) + return nil, fmt.Errorf("resource %s has an unsupported mode %s", r.Address, addr.Resource.Resource.Mode.String()) } r.ModuleAddress = addr.Module.String() r.Name = addr.Resource.Resource.Name @@ -461,18 +324,18 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform case plans.ResourceInstanceReplaceByRequest: r.ActionReason = "replace_by_request" default: - return fmt.Errorf("resource %s has an unsupported action reason %s", r.Address, rc.ActionReason) + return nil, fmt.Errorf("resource %s has an unsupported action reason %s", r.Address, rc.ActionReason) } - p.ResourceChanges = append(p.ResourceChanges, r) + ret = append(ret, r) } - sort.Slice(p.ResourceChanges, func(i, j int) bool { - return p.ResourceChanges[i].Address < p.ResourceChanges[j].Address + sort.Slice(ret, func(i, j int) bool { + return ret[i].Address < ret[j].Address }) - return nil + return ret, nil } func (p *plan) marshalOutputChanges(changes *plans.Changes) error { diff --git a/internal/command/testdata/show-json/drift/output.json b/internal/command/testdata/show-json/drift/output.json index 7badb45e5..79e702161 100644 --- a/internal/command/testdata/show-json/drift/output.json +++ b/internal/command/testdata/show-json/drift/output.json @@ -52,6 +52,7 @@ "id": "placeholder" }, "after_sensitive": {}, + "after_unknown": {}, "before_sensitive": {} } } diff --git a/internal/command/testdata/show-json/multi-resource-update/output.json b/internal/command/testdata/show-json/multi-resource-update/output.json index d84bc5b08..a7a6a3053 100644 --- a/internal/command/testdata/show-json/multi-resource-update/output.json +++ b/internal/command/testdata/show-json/multi-resource-update/output.json @@ -62,7 +62,8 @@ }, "after": null, "before_sensitive": {}, - "after_sensitive": false + "after_sensitive": false, + "after_unknown": {} } } ], diff --git a/internal/command/views/operation_test.go b/internal/command/views/operation_test.go index fd56350c5..b2e4f8f8e 100644 --- a/internal/command/views/operation_test.go +++ b/internal/command/views/operation_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform/internal/states/statefile" "github.com/hashicorp/terraform/internal/terminal" "github.com/hashicorp/terraform/internal/terraform" + "github.com/zclconf/go-cty/cty" ) func TestOperation_stopping(t *testing.T) { @@ -82,10 +83,8 @@ func TestOperation_planNoChanges(t *testing.T) { "nothing at all in normal mode": { func(schemas *terraform.Schemas) *plans.Plan { return &plans.Plan{ - UIMode: plans.NormalMode, - Changes: plans.NewChanges(), - PrevRunState: states.NewState(), - PriorState: states.NewState(), + UIMode: plans.NormalMode, + Changes: plans.NewChanges(), } }, "no differences, so no changes are needed.", @@ -93,10 +92,8 @@ func TestOperation_planNoChanges(t *testing.T) { "nothing at all in refresh-only mode": { func(schemas *terraform.Schemas) *plans.Plan { return &plans.Plan{ - UIMode: plans.RefreshOnlyMode, - Changes: plans.NewChanges(), - PrevRunState: states.NewState(), - PriorState: states.NewState(), + UIMode: plans.RefreshOnlyMode, + Changes: plans.NewChanges(), } }, "Terraform has checked that the real remote objects still match", @@ -104,148 +101,90 @@ func TestOperation_planNoChanges(t *testing.T) { "nothing at all in destroy mode": { func(schemas *terraform.Schemas) *plans.Plan { return &plans.Plan{ - UIMode: plans.DestroyMode, - Changes: plans.NewChanges(), - PrevRunState: states.NewState(), - PriorState: states.NewState(), + UIMode: plans.DestroyMode, + Changes: plans.NewChanges(), } }, "No objects need to be destroyed.", }, - "no drift to display with only deposed instances": { - // changes in deposed instances will cause a change in state, but - // have nothing to display to the user - func(schemas *terraform.Schemas) *plans.Plan { - return &plans.Plan{ - UIMode: plans.NormalMode, - Changes: plans.NewChanges(), - PrevRunState: states.BuildState(func(state *states.SyncState) { - state.SetResourceInstanceDeposed( - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_resource", - Name: "somewhere", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - states.NewDeposedKey(), - &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{"foo": "ok", "bars":[]}`), - }, - addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")), - ) - }), - PriorState: states.NewState(), - } - }, - "no differences, so no changes are needed.", - }, "drift detected in normal mode": { func(schemas *terraform.Schemas) *plans.Plan { - return &plans.Plan{ - UIMode: plans.NormalMode, - Changes: plans.NewChanges(), - PrevRunState: states.BuildState(func(state *states.SyncState) { - state.SetResourceInstanceCurrent( - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_resource", - Name: "somewhere", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{}`), - }, - addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")), - ) - }), - PriorState: states.NewState(), + 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"), + }), + }, } - }, - "to update the Terraform state to match, create and apply a refresh-only plan", - }, - "drift detected with deposed": { - func(schemas *terraform.Schemas) *plans.Plan { + rcs, err := rc.Encode(ty) + if err != nil { + panic(err) + } + drs := []*plans.ResourceInstanceChangeSrc{rcs} return &plans.Plan{ - UIMode: plans.NormalMode, - Changes: plans.NewChanges(), - PrevRunState: states.BuildState(func(state *states.SyncState) { - state.SetResourceInstanceCurrent( - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_resource", - Name: "changes", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{"foo":"b"}`), - }, - addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")), - ) - state.SetResourceInstanceDeposed( - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_resource", - Name: "broken", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - states.NewDeposedKey(), - &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{"foo":"c"}`), - }, - addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")), - ) - }), - PriorState: states.BuildState(func(state *states.SyncState) { - state.SetResourceInstanceCurrent( - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_resource", - Name: "changed", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{"foo":"b"}`), - }, - addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")), - ) - state.SetResourceInstanceDeposed( - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_resource", - Name: "broken", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - states.NewDeposedKey(), - &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{"foo":"d"}`), - }, - addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")), - ) - }), + UIMode: plans.NormalMode, + Changes: plans.NewChanges(), + DriftedResources: drs, } }, "to update the Terraform state to match, create and apply a refresh-only plan", }, "drift detected in refresh-only 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} return &plans.Plan{ - UIMode: plans.RefreshOnlyMode, - Changes: plans.NewChanges(), - PrevRunState: states.BuildState(func(state *states.SyncState) { - state.SetResourceInstanceCurrent( - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_resource", - Name: "somewhere", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{}`), - }, - addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("test")), - ) - }), - PriorState: states.NewState(), + UIMode: plans.RefreshOnlyMode, + Changes: plans.NewChanges(), + DriftedResources: drs, } }, "If you were expecting these changes then you can apply this plan", diff --git a/internal/command/views/plan.go b/internal/command/views/plan.go index ff794933b..175c26369 100644 --- a/internal/command/views/plan.go +++ b/internal/command/views/plan.go @@ -10,7 +10,6 @@ import ( "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/format" "github.com/hashicorp/terraform/internal/plans" - "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -97,8 +96,9 @@ 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) + haveRefreshChanges := len(plan.DriftedResources) > 0 if haveRefreshChanges { + renderChangesDetectedByRefresh(plan.DriftedResources, schemas, view) switch plan.UIMode { case plans.RefreshOnlyMode: view.streams.Println(format.WordWrap( @@ -292,6 +292,7 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) { rcs, rSchema, view.colorize, + format.DiffLanguageProposedChange, )) } @@ -344,82 +345,53 @@ 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 { - // 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 - // to the user, so for now this only serves as a short-circuit to skip - // attempting to render the diffs below. - if after.ManagedResourcesEqual(before) { - return false - } +func renderChangesDetectedByRefresh(drs []*plans.ResourceInstanceChangeSrc, schemas *terraform.Schemas, view *View) { + 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(), + )) - var diffs []string - - for _, bms := range before.Modules { - for _, brs := range bms.Resources { - if brs.Addr.Resource.Mode != addrs.ManagedResourceMode { - continue // only managed resources can "drift" - } - addr := brs.Addr - prs := after.Resource(brs.Addr) - - provider := brs.ProviderConfig.Provider - providerSchema := schemas.ProviderSchema(provider) - if providerSchema == nil { - // Should never happen - view.streams.Printf("(schema missing for %s)\n", provider) - continue - } - rSchema, _ := providerSchema.SchemaForResourceAddr(addr.Resource) - if rSchema == nil { - // Should never happen - view.streams.Printf("(schema missing for %s)\n", addr) - continue - } - - for key, bis := range brs.Instances { - if bis.Current == nil { - // No current instance to render here - continue - } - var pis *states.ResourceInstance - if prs != nil { - pis = prs.Instance(key) - } - - diff := format.ResourceInstanceDrift( - addr.Instance(key), - bis, pis, - rSchema, - view.colorize, - ) - if diff != "" { - diffs = append(diffs, diff) - } - } + // Note: we're modifying the backing slice of this plan object in-place + // here. The ordering of resource changes in a plan is not significant, + // but we can only do this safely here because we can assume that nobody + // is concurrently modifying our changes while we're trying to print it. + sort.Slice(drs, func(i, j int) bool { + iA := drs[i].Addr + jA := drs[j].Addr + if iA.String() == jA.String() { + return drs[i].DeposedKey < drs[j].DeposedKey } - } + return iA.Less(jA) + }) - // 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 { - 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(), + for _, rcs := range drs { + if rcs.Action == plans.NoOp && !rcs.Moved() { + continue + } + + providerSchema := schemas.ProviderSchema(rcs.ProviderAddr.Provider) + if providerSchema == nil { + // Should never happen + view.streams.Printf("(schema missing for %s)\n\n", rcs.ProviderAddr) + continue + } + rSchema, _ := providerSchema.SchemaForResourceAddr(rcs.Addr.Resource.Resource) + if rSchema == nil { + // Should never happen + view.streams.Printf("(schema missing for %s)\n\n", rcs.Addr) + continue + } + + view.streams.Println(format.ResourceChange( + rcs, + rSchema, + view.colorize, + format.DiffLanguageDetectedDrift, )) - - for _, diff := range diffs { - view.streams.Print(diff) - } - return true } - - return false } const planHeaderIntro = `