states: separate types for encoded and decoded state objects

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.

states/instance_object.go

@@ -2,6 +2,7 @@ package states
 
 import (
 	"github.com/zclconf/go-cty/cty"
+	ctyjson "github.com/zclconf/go-cty/cty/json"
 
 	"github.com/hashicorp/terraform/addrs"
 )
@@ -14,43 +15,9 @@ import (
 // It is not valid to mutate a ResourceInstanceObject once it has been created.
 // Instead, create a new object and replace the existing one.
 type ResourceInstanceObject struct {
-	// SchemaVersion identifies which version of the resource type schema the
+	// Value is the object-typed value representing the remote object within
-	// Attrs or AttrsFlat value conforms to. If this is less than the schema
+	// Terraform.
-	// version number given by the current provider version then the value
+	Value cty.Value
-	// must be upgraded to the latest version before use. If it is greater
-	// than the current version number then the provider must be upgraded
-	// before any operations can be performed.
-	SchemaVersion uint64
-
-	// AttrsJSON is a JSON-encoded representation of the object attributes,
-	// encoding the value (of the object type implied by the associated resource
-	// type schema) that represents this remote object in Terraform Language
-	// expressions, and is compared with configuration when producing a diff.
-	//
-	// This is retained in JSON format here because it may require preprocessing
-	// before decoding if, for example, the stored attributes are for an older
-	// schema version which the provider must upgrade before use. If the
-	// version is current, it is valid to simply decode this using the
-	// type implied by the current schema, without the need for the provider
-	// to perform an upgrade first.
-	//
-	// When writing a ResourceInstanceObject into the state, AttrsJSON should
-	// always be conformant to the current schema version and the current
-	// schema version should be recorded in the SchemaVersion field.
-	AttrsJSON []byte
-
-	// AttrsFlat is a legacy form of attributes used in older state file
-	// formats, and in the new state format for objects that haven't yet been
-	// upgraded. This attribute is mutually exclusive with Attrs: for any
-	// ResourceInstanceObject, only one of these attributes may be populated
-	// and the other must be nil.
-	//
-	// An instance object with this field populated should be upgraded to use
-	// Attrs at the earliest opportunity, since this legacy flatmap-based
-	// format will be phased out over time. AttrsFlat should not be used when
-	// writing new or updated objects to state; instead, callers must follow
-	// the recommendations in the AttrsJSON documentation above.
-	AttrsFlat map[string]string
 
 	// Internal is an opaque value set by the provider when this object was
 	// last created or updated. Terraform Core does not use this value in
@@ -85,3 +52,32 @@ const (
 	// ObjectRead state, a tainted object must be replaced.
 	ObjectTainted ObjectStatus = 'T'
 )
+
+// Encode marshals the value within the receiver to produce a
+// ResourceInstanceObjectSrc ready to be written to a state file.
+//
+// The given type must be the implied type of the resource type schema, and
+// the given value must conform to it. It is important to pass the schema
+// type and not the object's own type so that dynamically-typed attributes
+// will be stored correctly. The caller must also provide the version number
+// of the schema that the given type was derived from, which will be recorded
+// in the source object so it can be used to detect when schema migration is
+// required on read.
+//
+// The returned object may share internal references with the receiver and
+// so the caller must not mutate the receiver any further once once this
+// method is called.
+func (o *ResourceInstanceObject) Encode(val cty.Value, ty cty.Type, schemaVersion uint64) (*ResourceInstanceObjectSrc, error) {
+	src, err := ctyjson.Marshal(val, ty)
+	if err != nil {
+		return nil, err
+	}
+
+	return &ResourceInstanceObjectSrc{
+		SchemaVersion: schemaVersion,
+		AttrsJSON:     src,
+		Private:       o.Private,
+		Status:        o.Status,
+		Dependencies:  o.Dependencies,
+	}, nil
+}

states/instance_object_src.go

@@ -0,0 +1,91 @@
+package states
+
+import (
+	"github.com/zclconf/go-cty/cty"
+	ctyjson "github.com/zclconf/go-cty/cty/json"
+
+	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/config/hcl2shim"
+)
+
+// ResourceInstanceObjectSrc is a not-fully-decoded version of
+// ResourceInstanceObject. Decoding of it can be completed by first handling
+// any schema migration steps to get to the latest schema version and then
+// calling method Decode with the implied type of the latest schema.
+type ResourceInstanceObjectSrc struct {
+	// SchemaVersion is the resource-type-specific schema version number that
+	// was current when either AttrsJSON or AttrsFlat was encoded. Migration
+	// steps are required if this is less than the current version number
+	// reported by the corresponding provider.
+	SchemaVersion uint64
+
+	// AttrsJSON is a JSON-encoded representation of the object attributes,
+	// encoding the value (of the object type implied by the associated resource
+	// type schema) that represents this remote object in Terraform Language
+	// expressions, and is compared with configuration when producing a diff.
+	//
+	// This is retained in JSON format here because it may require preprocessing
+	// before decoding if, for example, the stored attributes are for an older
+	// schema version which the provider must upgrade before use. If the
+	// version is current, it is valid to simply decode this using the
+	// type implied by the current schema, without the need for the provider
+	// to perform an upgrade first.
+	//
+	// When writing a ResourceInstanceObject into the state, AttrsJSON should
+	// always be conformant to the current schema version and the current
+	// schema version should be recorded in the SchemaVersion field.
+	AttrsJSON []byte
+
+	// AttrsFlat is a legacy form of attributes used in older state file
+	// formats, and in the new state format for objects that haven't yet been
+	// upgraded. This attribute is mutually exclusive with Attrs: for any
+	// ResourceInstanceObject, only one of these attributes may be populated
+	// and the other must be nil.
+	//
+	// An instance object with this field populated should be upgraded to use
+	// Attrs at the earliest opportunity, since this legacy flatmap-based
+	// format will be phased out over time. AttrsFlat should not be used when
+	// writing new or updated objects to state; instead, callers must follow
+	// the recommendations in the AttrsJSON documentation above.
+	AttrsFlat map[string]string
+
+	// These fields all correspond to the fields of the same name on
+	// ResourceInstanceObject.
+	Private      cty.Value
+	Status       ObjectStatus
+	Dependencies []addrs.Referenceable
+}
+
+// Decode unmarshals the raw representation of the object attributes. Pass the
+// implied type of the corresponding resource type schema for correct operation.
+//
+// Before calling Decode, the caller must check that the SchemaVersion field
+// exactly equals the version number of the schema whose implied type is being
+// passed, or else the result is undefined.
+//
+// The returned object may share internal references with the receiver and
+// so the caller must not mutate the receiver any further once once this
+// method is called.
+func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObject, error) {
+	var val cty.Value
+	var err error
+	if os.AttrsFlat != nil {
+		// Legacy mode. We'll do our best to unpick this from the flatmap.
+		val, err = hcl2shim.HCL2ValueFromFlatmap(os.AttrsFlat, ty)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		val, err = ctyjson.Unmarshal(os.AttrsJSON, ty)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return &ResourceInstanceObject{
+		Value:        val,
+		Status:       os.Status,
+		Dependencies: os.Dependencies,
+		Private:      os.Private,
+	}, nil
+}

states/module.go

@@ -89,7 +89,7 @@ func (ms *Module) RemoveResource(addr addrs.Resource) {
 // The provider address and "each mode" are resource-wide settings and so they
 // are updated for all other instances of the same resource as a side-effect of
 // this call.
-func (ms *Module) SetResourceInstanceCurrent(addr addrs.ResourceInstance, obj *ResourceInstanceObject, provider addrs.AbsProviderConfig) {
+func (ms *Module) SetResourceInstanceCurrent(addr addrs.ResourceInstance, obj *ResourceInstanceObjectSrc, provider addrs.AbsProviderConfig) {
 	ms.SetResourceMeta(addr.Resource, eachModeForInstanceKey(addr.Key), provider)
 
 	rs := ms.Resource(addr.Resource)
@@ -125,7 +125,7 @@ func (ms *Module) SetResourceInstanceCurrent(addr addrs.ResourceInstance, obj *R
 // is overwritten. Set obj to nil to remove the deposed object altogether. If
 // the instance is left with no objects after this operation then it will
 // be removed from its containing resource altogether.
-func (ms *Module) SetResourceInstanceDeposed(addr addrs.ResourceInstance, key DeposedKey, obj *ResourceInstanceObject) {
+func (ms *Module) SetResourceInstanceDeposed(addr addrs.ResourceInstance, key DeposedKey, obj *ResourceInstanceObjectSrc) {
 	rs := ms.Resource(addr.Resource)
 	if rs == nil {
 		panic(fmt.Sprintf("attempt to register deposed instance object for non-existent resource %s", addr.Resource.Absolute(ms.Addr)))

states/resource.go

@@ -57,20 +57,20 @@ func (rs *Resource) EnsureInstance(key addrs.InstanceKey) *ResourceInstance {
 type ResourceInstance struct {
 	// Current, if non-nil, is the remote object that is currently represented
 	// by the corresponding resource instance.
-	Current *ResourceInstanceObject
+	Current *ResourceInstanceObjectSrc
 
 	// Deposed, if len > 0, contains any remote objects that were previously
 	// represented by the corresponding resource instance but have been
 	// replaced and are pending destruction due to the create_before_destroy
 	// lifecycle mode.
-	Deposed map[DeposedKey]*ResourceInstanceObject
+	Deposed map[DeposedKey]*ResourceInstanceObjectSrc
 }
 
 // NewResourceInstance constructs and returns a new ResourceInstance, ready to
 // use.
 func NewResourceInstance() *ResourceInstance {
 	return &ResourceInstance{
-		Deposed: map[DeposedKey]*ResourceInstanceObject{},
+		Deposed: map[DeposedKey]*ResourceInstanceObjectSrc{},
 	}
 }
 
@@ -119,7 +119,7 @@ func (i *ResourceInstance) deposeCurrentObject() DeposedKey {
 // ResourceInstance, or returns nil if there is no such object.
 //
 // If the given generation is nil or invalid, this method will panic.
-func (i *ResourceInstance) GetGeneration(gen Generation) *ResourceInstanceObject {
+func (i *ResourceInstance) GetGeneration(gen Generation) *ResourceInstanceObjectSrc {
 	if gen == CurrentGen {
 		return i.Current
 	}

states/state_deepcopy.go

@@ -107,7 +107,7 @@ func (is *ResourceInstance) DeepCopy() *ResourceInstance {
 		return nil
 	}
 
-	deposed := make(map[DeposedKey]*ResourceInstanceObject, len(is.Deposed))
+	deposed := make(map[DeposedKey]*ResourceInstanceObjectSrc, len(is.Deposed))
 	for k, obj := range is.Deposed {
 		deposed[k] = obj.DeepCopy()
 	}
@@ -126,7 +126,7 @@ func (is *ResourceInstance) DeepCopy() *ResourceInstance {
 // is the caller's responsibility to ensure mutual exclusion for the duration
 // of the operation, but may then freely modify the receiver and the returned
 // copy independently once this method returns.
-func (obj *ResourceInstanceObject) DeepCopy() *ResourceInstanceObject {
+func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc {
 	if obj == nil {
 		return nil
 	}
@@ -149,7 +149,7 @@ func (obj *ResourceInstanceObject) DeepCopy() *ResourceInstanceObject {
 	// we treat them as immutable by convention and so we don't deep-copy here.
 	dependencies := make([]addrs.Referenceable, len(obj.Dependencies))
 
-	return &ResourceInstanceObject{
+	return &ResourceInstanceObjectSrc{
 		Status:        obj.Status,
 		SchemaVersion: obj.SchemaVersion,
 		Private:       obj.Private,

states/statefile/version4.go

@@ -139,7 +139,7 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) {
 
 			instAddr := rAddr.Instance(key)
 
-			obj := &states.ResourceInstanceObject{
+			obj := &states.ResourceInstanceObjectSrc{
 				SchemaVersion: isV4.SchemaVersion,
 			}
 
@@ -455,7 +455,7 @@ func writeStateV4(file *File, w io.Writer) tfdiags.Diagnostics {
 	return diags
 }
 
-func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstance, key addrs.InstanceKey, obj *states.ResourceInstanceObject, deposed states.DeposedKey, isV4s []instanceObjectStateV4) ([]instanceObjectStateV4, tfdiags.Diagnostics) {
+func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstance, key addrs.InstanceKey, obj *states.ResourceInstanceObjectSrc, deposed states.DeposedKey, isV4s []instanceObjectStateV4) ([]instanceObjectStateV4, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
 	var status string

states/sync.go

@@ -160,7 +160,7 @@ func (s *SyncState) ResourceInstance(addr addrs.AbsResourceInstance) *ResourceIn
 //
 // The return value is a pointer to a copy of the object, which the caller may
 // then freely access and mutate.
-func (s *SyncState) ResourceInstanceObject(addr addrs.AbsResourceInstance, gen Generation) *ResourceInstanceObject {
+func (s *SyncState) ResourceInstanceObject(addr addrs.AbsResourceInstance, gen Generation) *ResourceInstanceObjectSrc {
 	s.lock.RLock()
 	defer s.lock.RUnlock()
 
@@ -215,7 +215,7 @@ func (s *SyncState) RemoveResource(addr addrs.AbsResource) {
 //
 // If the containing module for this resource or the resource itself are not
 // already tracked in state then they will be added as a side-effect.
-func (s *SyncState) SetResourceInstanceCurrent(addr addrs.AbsResourceInstance, obj *ResourceInstanceObject, provider addrs.AbsProviderConfig) {
+func (s *SyncState) SetResourceInstanceCurrent(addr addrs.AbsResourceInstance, obj *ResourceInstanceObjectSrc, provider addrs.AbsProviderConfig) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
@@ -246,7 +246,7 @@ func (s *SyncState) SetResourceInstanceCurrent(addr addrs.AbsResourceInstance, o
 //
 // If the containing module for this resource or the resource itself are not
 // already tracked in state then they will be added as a side-effect.
-func (s *SyncState) SetResourceInstanceDeposed(addr addrs.AbsResourceInstance, key DeposedKey, obj *ResourceInstanceObject) {
+func (s *SyncState) SetResourceInstanceDeposed(addr addrs.AbsResourceInstance, key DeposedKey, obj *ResourceInstanceObjectSrc) {
 	s.lock.Lock()
 	defer s.lock.Unlock()