plans: separate types for encoded and decoded changes

The types here were originally written to allow us to defer decoding of
object values until schemas are available, but it turns out that this was
forcing us to defer decoding longer than necessary and potentially decode
the same value multiple times.

To avoid this, we create pairs of types to represent the encoded and
decoded versions and methods for moving between them. These types are
identical to one another apart from how the dynamic values are
represented.

plans/changes.go

@@ -3,6 +3,7 @@ package plans
 import (
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/states"
+	"github.com/zclconf/go-cty/cty"
 )
 
 // Changes describes various actions that Terraform will attempt to take if
@@ -11,8 +12,15 @@ import (
 // A Changes object can be rendered into a visual diff (by the caller, using
 // code in another package) for display to the user.
 type Changes struct {
-	Resources   []*ResourceInstanceChange
+	Resources   []*ResourceInstanceChangeSrc
-	RootOutputs map[string]*OutputChange
+	RootOutputs map[string]*OutputChangeSrc
+}
+
+// NewChanges returns a valid Changes object that describes no changes.
+func NewChanges() *Changes {
+	return &Changes{
+		RootOutputs: make(map[string]*OutputChangeSrc),
+	}
 }
 
 // ResourceInstanceChange describes a change to a particular resource instance
@@ -41,6 +49,22 @@ type ResourceInstanceChange struct {
 	Change
 }
 
+// Encode produces a variant of the reciever that has its change values
+// serialized so it can be written to a plan file. Pass the implied type of the
+// corresponding resource type schema for correct operation.
+func (rc *ResourceInstanceChange) Encode(ty cty.Type) (*ResourceInstanceChangeSrc, error) {
+	cs, err := rc.Change.Encode(ty)
+	if err != nil {
+		return nil, err
+	}
+	return &ResourceInstanceChangeSrc{
+		Addr:         rc.Addr,
+		DeposedKey:   rc.DeposedKey,
+		ProviderAddr: rc.ProviderAddr,
+		ChangeSrc:    *cs,
+	}, err
+}
+
 // OutputChange describes a change to an output value.
 type OutputChange struct {
 	// Change is an embedded description of the change.
@@ -56,6 +80,19 @@ type OutputChange struct {
 	Sensitive bool
 }
 
+// Encode produces a variant of the reciever that has its change values
+// serialized so it can be written to a plan file.
+func (oc *OutputChange) Encode() (*OutputChangeSrc, error) {
+	cs, err := oc.Change.Encode(cty.DynamicPseudoType)
+	if err != nil {
+		return nil, err
+	}
+	return &OutputChangeSrc{
+		ChangeSrc: *cs,
+		Sensitive: oc.Sensitive,
+	}, err
+}
+
 // Change describes a single change with a given action.
 type Change struct {
 	// Action defines what kind of change is being made.
@@ -75,9 +112,30 @@ type Change struct {
 	// Unknown values may appear anywhere within the Before and After values,
 	// either as the values themselves or as nested elements within known
 	// collections/structures.
-	//
+	Before, After cty.Value
-	// A plan contains only raw (not yet decoded) values. The caller must use
+}
-	// schema information obtained out-of-band to decode dynamic values before
+
-	// they can be used.
+// Encode produces a variant of the reciever that has its change values
-	Before, After DynamicValue
+// serialized so it can be written to a plan file. Pass the type constraint
+// that the values are expected to conform to; to properly decode the values
+// later an identical type constraint must be provided at that time.
+//
+// Where a Change is embedded in some other struct, it's generally better
+// to call the corresponding Encode method of that struct rather than working
+// directly with its embedded Change.
+func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) {
+	beforeDV, err := NewDynamicValue(c.Before, ty)
+	if err != nil {
+		return nil, err
+	}
+	afterDV, err := NewDynamicValue(c.After, ty)
+	if err != nil {
+		return nil, err
+	}
+
+	return &ChangeSrc{
+		Action: c.Action,
+		Before: beforeDV,
+		After:  afterDV,
+	}, nil
 }

plans/changes_src.go

@@ -0,0 +1,114 @@
+package plans
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/states"
+	"github.com/zclconf/go-cty/cty"
+)
+
+// ResourceInstanceChangeSrc is a not-yet-decoded ResourceInstanceChange.
+// Pass the associated resource type's schema type to method Decode to
+// obtain a ResourceInstancChange.
+type ResourceInstanceChangeSrc struct {
+	// Addr is the absolute address of the resource instance that the change
+	// will apply to.
+	Addr addrs.AbsResourceInstance
+
+	// DeposedKey is the identifier for a deposed object associated with the
+	// given instance, or states.NotDeposed if this change applies to the
+	// current object.
+	//
+	// A Replace change for a resource with create_before_destroy set will
+	// create a new DeposedKey temporarily during replacement. In that case,
+	// DeposedKey in the plan is always states.NotDeposed, representing that
+	// the current object is being replaced with the deposed.
+	DeposedKey states.DeposedKey
+
+	// Provider is the address of the provider configuration that was used
+	// to plan this change, and thus the configuration that must also be
+	// used to apply it.
+	ProviderAddr addrs.AbsProviderConfig
+
+	// ChangeSrc is an embedded description of the not-yet-decoded change.
+	ChangeSrc
+}
+
+// Decode unmarshals the raw representation of the instance object being
+// changed. Pass the implied type of the corresponding resource type schema
+// for correct operation.
+func (rcs *ResourceInstanceChangeSrc) Decode(ty cty.Type) (*ResourceInstanceChange, error) {
+	change, err := rcs.ChangeSrc.Decode(ty)
+	if err != nil {
+		return nil, err
+	}
+	return &ResourceInstanceChange{
+		Addr:         rcs.Addr,
+		DeposedKey:   rcs.DeposedKey,
+		ProviderAddr: rcs.ProviderAddr,
+		Change:       *change,
+	}, nil
+}
+
+// OutputChange describes a change to an output value.
+type OutputChangeSrc struct {
+	// ChangeSrc is an embedded description of the not-yet-decoded change.
+	//
+	// For output value changes, the type constraint for the DynamicValue
+	// instances is always cty.DynamicPseudoType.
+	ChangeSrc
+
+	// Sensitive, if true, indicates that either the old or new value in the
+	// change is sensitive and so a rendered version of the plan in the UI
+	// should elide the actual values while still indicating the action of the
+	// change.
+	Sensitive bool
+}
+
+// Decode unmarshals the raw representation of the output value being
+// changed.
+func (ocs *OutputChangeSrc) Decode() (*OutputChange, error) {
+	change, err := ocs.ChangeSrc.Decode(cty.DynamicPseudoType)
+	if err != nil {
+		return nil, err
+	}
+	return &OutputChange{
+		Change:    *change,
+		Sensitive: ocs.Sensitive,
+	}, nil
+}
+
+// ChangeSrc is a not-yet-decoded Change.
+type ChangeSrc struct {
+	// Action defines what kind of change is being made.
+	Action Action
+
+	// Before and After correspond to the fields of the same name in Change,
+	// but have not yet been decoded from the serialized value used for
+	// storage.
+	Before, After DynamicValue
+}
+
+// Decode unmarshals the raw representations of the before and after values
+// to produce a Change object. Pass the type constraint that the result must
+// conform to.
+//
+// Where a ChangeSrc is embedded in some other struct, it's generally better
+// to call the corresponding Decode method of that struct rather than working
+// directly with its embedded Change.
+func (cs *ChangeSrc) Decode(ty cty.Type) (*Change, error) {
+	before, err := cs.Before.Decode(ty)
+	if err != nil {
+		return nil, fmt.Errorf("error decoding 'before' value: %s", err)
+	}
+	after, err := cs.After.Decode(ty)
+	if err != nil {
+		return nil, fmt.Errorf("error decoding 'after' value: %s", err)
+	}
+	return &Change{
+		Action: cs.Action,
+		Before: before,
+		After:  after,
+	}, nil
+}

plans/planfile/tfplan.go

@@ -52,8 +52,8 @@ func readTfplan(r io.Reader) (*plans.Plan, error) {
 	plan := &plans.Plan{
 		VariableValues: map[string]plans.DynamicValue{},
 		Changes: &plans.Changes{
-			RootOutputs: map[string]*plans.OutputChange{},
+			RootOutputs: map[string]*plans.OutputChangeSrc{},
-			Resources:   []*plans.ResourceInstanceChange{},
+			Resources:   []*plans.ResourceInstanceChangeSrc{},
 		},
 
 		ProviderSHA256s: map[string][]byte{},
@@ -66,8 +66,8 @@ func readTfplan(r io.Reader) (*plans.Plan, error) {
 			return nil, fmt.Errorf("invalid plan for output %q: %s", name, err)
 		}
 
-		plan.Changes.RootOutputs[name] = &plans.OutputChange{
+		plan.Changes.RootOutputs[name] = &plans.OutputChangeSrc{
-			Change:    *change,
+			ChangeSrc: *change,
 			Sensitive: rawOC.Sensitive,
 		}
 	}
@@ -123,14 +123,14 @@ func readTfplan(r io.Reader) (*plans.Plan, error) {
 	return plan, nil
 }
 
-func resourceChangeFromTfplan(rawChange *planproto.ResourceInstanceChange) (*plans.ResourceInstanceChange, error) {
+func resourceChangeFromTfplan(rawChange *planproto.ResourceInstanceChange) (*plans.ResourceInstanceChangeSrc, error) {
 	if rawChange == nil {
 		// Should never happen in practice, since protobuf can't represent
 		// a nil value in a list.
 		return nil, fmt.Errorf("resource change object is absent")
 	}
 
-	ret := &plans.ResourceInstanceChange{}
+	ret := &plans.ResourceInstanceChangeSrc{}
 
 	moduleAddr := addrs.RootModuleInstance
 	if rawChange.ModulePath != "" {
@@ -190,17 +190,17 @@ func resourceChangeFromTfplan(rawChange *planproto.ResourceInstanceChange) (*pla
 		return nil, fmt.Errorf("invalid plan for resource %s: %s", ret.Addr, err)
 	}
 
-	ret.Change = *change
+	ret.ChangeSrc = *change
 
 	return ret, nil
 }
 
-func changeFromTfplan(rawChange *planproto.Change) (*plans.Change, error) {
+func changeFromTfplan(rawChange *planproto.Change) (*plans.ChangeSrc, error) {
 	if rawChange == nil {
 		return nil, fmt.Errorf("change object is absent")
 	}
 
-	ret := &plans.Change{}
+	ret := &plans.ChangeSrc{}
 
 	// -1 indicates that there is no index. We'll customize these below
 	// depending on the change action, and then decode.
@@ -288,7 +288,7 @@ func writeTfplan(plan *plans.Plan, w io.Writer) error {
 		// Writing outputs as cty.DynamicPseudoType forces the stored values
 		// to also contain dynamic type information, so we can recover the
 		// original type when we read the values back in readTFPlan.
-		protoChange, err := changeToTfplan(&oc.Change)
+		protoChange, err := changeToTfplan(&oc.ChangeSrc)
 		if err != nil {
 			return fmt.Errorf("cannot write output value %q: %s", name, err)
 		}
@@ -341,7 +341,7 @@ func writeTfplan(plan *plans.Plan, w io.Writer) error {
 	return nil
 }
 
-func resourceChangeToTfplan(change *plans.ResourceInstanceChange) (*planproto.ResourceInstanceChange, error) {
+func resourceChangeToTfplan(change *plans.ResourceInstanceChangeSrc) (*planproto.ResourceInstanceChange, error) {
 	ret := &planproto.ResourceInstanceChange{}
 
 	ret.ModulePath = change.Addr.Module.String()
@@ -376,7 +376,7 @@ func resourceChangeToTfplan(change *plans.ResourceInstanceChange) (*planproto.Re
 	ret.DeposedKey = string(change.DeposedKey)
 	ret.Provider = change.ProviderAddr.String()
 
-	valChange, err := changeToTfplan(&change.Change)
+	valChange, err := changeToTfplan(&change.ChangeSrc)
 	if err != nil {
 		return nil, fmt.Errorf("failed to serialize resource %s change: %s", relAddr, err)
 	}
@@ -385,7 +385,7 @@ func resourceChangeToTfplan(change *plans.ResourceInstanceChange) (*planproto.Re
 	return ret, nil
 }
 
-func changeToTfplan(change *plans.Change) (*planproto.Change, error) {
+func changeToTfplan(change *plans.ChangeSrc) (*planproto.Change, error) {
 	ret := &planproto.Change{}
 
 	before := valueToTfplan(change.Before)