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

Only show external changes which contributed to the plan #30486

Merged
merged 19 commits into from Mar 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 39 additions & 0 deletions internal/addrs/parse_ref.go
Expand Up @@ -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
Expand All @@ -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.
Expand Down
21 changes: 2 additions & 19 deletions internal/command/format/diff.go
Expand Up @@ -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,
Expand Down Expand Up @@ -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))
Expand Down
20 changes: 5 additions & 15 deletions internal/command/format/diff_test.go
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand Down
79 changes: 54 additions & 25 deletions internal/command/jsonplan/plan.go
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
//
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
52 changes: 50 additions & 2 deletions internal/command/views/operation_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down