Merge pull request #23475 from hashicorp/pselle/setMetaCleanup

Cleanup SetResourceInstanceCurrent to be clearer and more reliable

states/module.go

@@ -88,23 +88,59 @@ func (ms *Module) RemoveResource(addr addrs.Resource) {
 // 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 *ResourceInstanceObjectSrc, provider addrs.AbsProviderConfig) {
-	ms.SetResourceMeta(addr.Resource, eachModeForInstanceKey(addr.Key), provider)
-
 	rs := ms.Resource(addr.Resource)
-	is := rs.EnsureInstance(addr.Key)
+	// if the resource is nil and the object is nil, don't do anything!
-
+	// you'll probably just cause issues
+	if obj == nil && rs == nil {
+		return
+	}
+	if obj == nil && rs != nil {
+		// does the resource have any other objects?
+		// if not then delete the whole resource
+		// When deleting the resource, ensure that its EachMode is NoEach,
+		// as a resource with EachList or EachMap can have 0 instances and be valid
+		if rs.EachMode == NoEach && len(rs.Instances) == 0 {
+			delete(ms.Resources, addr.Resource.String())
+			return
+		}
+		// check for an existing resource, now that we've ensured that rs.Instances is more than 0/not nil
+		is := rs.Instance(addr.Key)
+		if is == nil {
+			// if there is no instance, but the resource exists and has other instances,
+			// be chill, just return
+			return
+		}
+		// if we have an instance, update the current
+		is.Current = obj
+		if !is.HasObjects() {
+			// If we have no objects at all then we'll clean up.
+			delete(rs.Instances, addr.Key)
+			// Delete the resource if it has no instances, but only if NoEach
+			if rs.EachMode == NoEach && len(rs.Instances) == 0 {
+				delete(ms.Resources, addr.Resource.String())
+				return
+			}
+		}
+		// Nothing more to do here, so return!
+		return
+	}
+	if rs == nil && obj != nil {
+		// We don't have have a resource so make one, which is a side effect of setResourceMeta
+		ms.SetResourceMeta(addr.Resource, eachModeForInstanceKey(addr.Key), provider)
+		// now we have a resource! so update the rs value to point to it
+		rs = ms.Resource(addr.Resource)
+	}
+	// Get our instance from the resource; it could be there or not at this point
+	is := rs.Instance(addr.Key)
+	if is == nil {
+		// if we don't have a resource, create one and add to the instances
+		is = rs.CreateInstance(addr.Key)
+		// update the resource meta because we have a new instance, so EachMode may have changed
+		ms.SetResourceMeta(addr.Resource, eachModeForInstanceKey(addr.Key), provider)
+	}
+	// Update the resource's ProviderConfig, in case the provider has updated
+	rs.ProviderConfig = provider
 	is.Current = obj
-
-	if !is.HasObjects() {
-		// If we have no objects at all then we'll clean up.
-		delete(rs.Instances, addr.Key)
-	}
-	if rs.EachMode == NoEach && len(rs.Instances) == 0 {
-		// Also clean up if we only expect to have one instance anyway
-		// and there are none. We leave the resource behind if an each mode
-		// is active because an empty list or map of instances is a valid state.
-		delete(ms.Resources, addr.Resource.String())
-	}
 }
 
 // SetResourceInstanceDeposed saves the given instance object as a deposed

states/resource.go

@@ -39,6 +39,13 @@ func (rs *Resource) Instance(key addrs.InstanceKey) *ResourceInstance {
 	return rs.Instances[key]
 }
 
+// CreateInstance creates an instance and adds it to the resource
+func (rs *Resource) CreateInstance(key addrs.InstanceKey) *ResourceInstance {
+	is := NewResourceInstance()
+	rs.Instances[key] = is
+	return is
+}
+
 // EnsureInstance returns the state for the instance with the given key,
 // creating a new empty state for it if one doesn't already exist.
 //

terraform/eval_diff.go

@@ -1,7 +1,6 @@
 package terraform
 
 import (
-	"bytes"
 	"fmt"
 	"log"
 	"strings"
@@ -567,49 +566,6 @@ func processIgnoreChangesIndividual(prior, proposed cty.Value, ignoreChanges []h
 	return ret, diags
 }
 
-// legacyFlagmapKeyForTraversal constructs a key string compatible with what
-// the flatmap package would generate for an attribute addressable by the given
-// traversal.
-//
-// This is used only to shim references to attributes within the diff and
-// state structures, which have not (at the time of writing) yet been updated
-// to use the newer HCL-based representations.
-func legacyFlatmapKeyForTraversal(traversal hcl.Traversal) string {
-	var buf bytes.Buffer
-	first := true
-	for _, step := range traversal {
-		if !first {
-			buf.WriteByte('.')
-		}
-		switch ts := step.(type) {
-		case hcl.TraverseRoot:
-			buf.WriteString(ts.Name)
-		case hcl.TraverseAttr:
-			buf.WriteString(ts.Name)
-		case hcl.TraverseIndex:
-			val := ts.Key
-			switch val.Type() {
-			case cty.Number:
-				bf := val.AsBigFloat()
-				buf.WriteString(bf.String())
-			case cty.String:
-				s := val.AsString()
-				buf.WriteString(s)
-			default:
-				// should never happen, since no other types appear in
-				// traversals in practice.
-				buf.WriteByte('?')
-			}
-		default:
-			// should never happen, since we've covered all of the types
-			// that show up in parsed traversals in practice.
-			buf.WriteByte('?')
-		}
-		first = false
-	}
-	return buf.String()
-}
-
 // a group of key-*ResourceAttrDiff pairs from the same flatmapped container
 type flatAttrDiff map[string]*ResourceAttrDiff
 
@@ -630,33 +586,6 @@ func (f flatAttrDiff) keepDiff(ignoreChanges map[string]bool) bool {
 	return false
 }
 
-// sets, lists and maps need to be compared for diff inclusion as a whole, so
-// group the flatmapped keys together for easier comparison.
-func groupContainers(d *InstanceDiff) map[string]flatAttrDiff {
-	isIndex := multiVal.MatchString
-	containers := map[string]flatAttrDiff{}
-	attrs := d.CopyAttributes()
-	// we need to loop once to find the index key
-	for k := range attrs {
-		if isIndex(k) {
-			// add the key, always including the final dot to fully qualify it
-			containers[k[:len(k)-1]] = flatAttrDiff{}
-		}
-	}
-
-	// loop again to find all the sub keys
-	for prefix, values := range containers {
-		for k, attrDiff := range attrs {
-			// we include the index value as well, since it could be part of the diff
-			if strings.HasPrefix(k, prefix) {
-				values[k] = attrDiff
-			}
-		}
-	}
-
-	return containers
-}
-
 // EvalDiffDestroy is an EvalNode implementation that returns a plain
 // destroy diff.
 type EvalDiffDestroy struct {