core: Prune placeholder objects from state after refresh

Prior to our refactoring here, we were relying on a lucky coincidence for
correct behavior of the plan walk following a refresh in the same run:

- The refresh phase created placeholder objects in the state to represent
  any resource instance pending creation, to allow the interpolator to
  read attributes from them when evaluating "provider" and "data" blocks.
  In effect, the refresh walk is creating a partial plan that only covers
  creation actions, but was immediately discarding the actual diff entries
  and storing only the planned new state.

- It happened that objects pending creation showed up in state with an
  empty ID value, since that only gets assigned by the provider during
  apply.

- The Refresh function concluded by calling terraform.State.Prune, which
  deletes from the state any objects that have an empty ID value, which
  therefore prevented these temporary objects from surviving into the
  plan phase.

After refactoring, we no longer have this special ID field on instance
object state, and we instead rely on the Status field for tracking such
things. We also no longer have an explicit "prune" step on state, since
the state mutation methods themselves keep the structure pruned.

To address this, here we introduce a new instance object status "planned",
which is equivalent to having an empty ID value in the old world. We also
introduce a new method on states.SyncState that deletes from the state
any planned objects, which therefore replaces that portion of the old
State.prune operation just for this refresh use-case.

Finally, we are now expecting the expression evaluator to pull pending
objects from the planned changeset rather than from the state directly,
and so for correct results these placeholder resource creation changes
must also be reported in a throwaway changeset during the refresh walk.

The addition of states.ObjectPlanned also permits a previously-missing
safety check in the expression evaluator to prevent us from relying on the
incomplete value stored in state for a pending object, in the event that
some bug prevents the real pending object from being written into the
planned changeset.

states/instance_object.go

@@ -51,6 +51,18 @@ const (
 	// update, or delete operation. Since it cannot be moved into the
 	// ObjectRead state, a tainted object must be replaced.
 	ObjectTainted ObjectStatus = 'T'
+
+	// ObjectPlanned is a special object status used only for the transient
+	// placeholder objects we place into state during the refresh and plan
+	// walks to stand in for objects that will be created during apply.
+	//
+	// Any object of this status must have a corresponding change recorded
+	// in the current plan, whose value must then be used in preference to
+	// the value stored in state when evaluating expressions. A planned
+	// object stored in state will be incomplete if any of its attributes are
+	// not yet known, and the plan must be consulted in order to "see" those
+	// unknown values, because the state is not able to represent them.
+	ObjectPlanned ObjectStatus = 'P'
 )
 
 // Encode marshals the value within the receiver to produce a

states/sync.go

@@ -336,6 +336,59 @@ func (s *SyncState) ForgetResourceInstanceDeposed(addr addrs.AbsResourceInstance
 	ms.ForgetResourceInstanceDeposed(addr.Resource, key)
 }
 
+// RemovePlannedResourceInstanceObjects removes from the state any resource
+// instance objects that have the status ObjectPlanned, indiciating that they
+// are just transient placeholders created during planning.
+//
+// Note that this does not restore any "ready" or "tainted" object that might
+// have been present before the planned object was written. The only real use
+// for this method is in preparing the state created during a refresh walk,
+// where we run the planning step for certain instances just to create enough
+// information to allow correct expression evaluation within provider and
+// data resource blocks. Discarding planned instances in that case is okay
+// because the refresh phase only creates planned objects to stand in for
+// objects that don't exist yet, and thus the planned object must have been
+// absent before by definition.
+func (s *SyncState) RemovePlannedResourceInstanceObjects() {
+	// TODO: Merge together the refresh and plan phases into a single walk,
+	// so we can remove the need to create this "partial plan" during refresh
+	// that we then need to clean up before proceeding.
+
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	for _, ms := range s.state.Modules {
+		moduleAddr := ms.Addr
+
+		for _, rs := range ms.Resources {
+			resAddr := rs.Addr
+
+			for ik, is := range rs.Instances {
+				instAddr := resAddr.Instance(ik)
+
+				if is.Current != nil && is.Current.Status == ObjectPlanned {
+					// Setting the current instance to nil removes it from the
+					// state altogether if there are not also deposed instances.
+					ms.SetResourceInstanceCurrent(instAddr, nil, rs.ProviderConfig)
+				}
+
+				for dk, obj := range is.Deposed {
+					// Deposed objects should never be "planned", but we'll
+					// do this anyway for the sake of completeness.
+					if obj.Status == ObjectPlanned {
+						ms.ForgetResourceInstanceDeposed(instAddr, dk)
+					}
+				}
+			}
+		}
+
+		// We may have deleted some objects, which means that we may have
+		// left a module empty, and so we must prune to preserve the invariant
+		// that only the root module is allowed to be empty.
+		s.maybePruneModule(moduleAddr)
+	}
+}
+
 // Lock acquires an explicit lock on the state, allowing direct read and write
 // access to the returned state object. The caller must call Unlock once
 // access is no longer needed, and then immediately discard the state pointer

terraform/context.go

@@ -544,6 +544,15 @@ func (c *Context) Refresh() (*states.State, tfdiags.Diagnostics) {
 	// Copy our own state
 	c.state = c.state.DeepCopy()
 
+	// Refresh builds a partial changeset as part of its work because it must
+	// create placeholder stubs for any resource instances that'll be created
+	// in subsequent plan so that provider configurations and data resources
+	// can interpolate from them. This plan is always thrown away after
+	// the operation completes, restoring any existing changeset.
+	oldChanges := c.changes
+	defer func() { c.changes = oldChanges }()
+	c.changes = plans.NewChanges()
+
 	// Build the graph.
 	graph, diags := c.Graph(GraphTypeRefresh, nil)
 	if diags.HasErrors() {
@@ -557,6 +566,17 @@ func (c *Context) Refresh() (*states.State, tfdiags.Diagnostics) {
 		return nil, diags
 	}
 
+	// During our walk we will have created planned object placeholders in
+	// state for resource instances that are in configuration but not yet
+	// created. These were created only to allow expression evaluation to
+	// work properly in provider and data blocks during the walk and must
+	// now be discarded, since a subsequent plan walk is responsible for
+	// creating these "for real".
+	// TODO: Consolidate refresh and plan into a single walk, so that the
+	// refresh walk doesn't need to emulate various aspects of the plan
+	// walk in order to properly evaluate provider and data blocks.
+	c.state.SyncWrapper().RemovePlannedResourceInstanceObjects()
+
 	return c.state, diags
 }
 

terraform/eval_diff.go

@@ -322,7 +322,13 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 	// Update the state if we care
 	if n.OutputState != nil {
 		*n.OutputState = &states.ResourceInstanceObject{
-			Status: states.ObjectReady,
+			// We use the special "planned" status here to note that this
+			// object's value is not yet complete. Objects with this status
+			// cannot be used during expression evaluation, so the caller
+			// must _also_ record the returned change in the active plan,
+			// which the expression evaluator will use in preference to this
+			// incomplete value recorded in the state.
+			Status: states.ObjectPlanned,
 			Value:  plannedNewVal,
 		}
 	}

terraform/evaluate.go

@@ -563,6 +563,19 @@ func (d *evaluationStateData) getResourceInstanceSingle(addr addrs.ResourceInsta
 		return val, diags
 	}
 
+	if is.Current.Status == states.ObjectPlanned {
+		// If the object is in planned status then we should not
+		// get here, since we should've found a pending value
+		// in the plan above instead.
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Missing pending object in plan",
+			Detail:   fmt.Sprintf("Instance %s is marked as having a change pending but that change is not recorded in the plan. This is a bug in Terraform; please report it.", addr),
+			Subject:  &config.DeclRange,
+		})
+		return cty.UnknownVal(ty), diags
+	}
+
 	ios, err := is.Current.Decode(ty)
 	if err != nil {
 		// This shouldn't happen, since by the time we get here
@@ -634,6 +647,19 @@ func (d *evaluationStateData) getResourceInstancesAll(addr addrs.Resource, rng t
 					continue
 				}
 
+				if is.Current.Status == states.ObjectPlanned {
+					// If the object is in planned status then we should not
+					// get here, since we should've found a pending value
+					// in the plan above instead.
+					diags = diags.Append(&hcl.Diagnostic{
+						Severity: hcl.DiagError,
+						Summary:  "Missing pending object in plan",
+						Detail:   fmt.Sprintf("Instance %s is marked as having a change pending but that change is not recorded in the plan. This is a bug in Terraform; please report it.", instAddr),
+						Subject:  &config.DeclRange,
+					})
+					continue
+				}
+
 				ios, err := is.Current.Decode(ty)
 				if err != nil {
 					// This shouldn't happen, since by the time we get here

terraform/node_resource_refresh.go

@@ -270,6 +270,20 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResourceNoState(
 				ProviderSchema: &providerSchema,
 				State:          &state,
 			},
+
+			// We must also save the planned change, so that expressions in
+			// other nodes, such as provider configurations and data resources,
+			// can work with the planned new value.
+			//
+			// This depends on the fact that Context.Refresh creates a
+			// temporary new empty changeset for the duration of its graph
+			// walk, and so this recorded change will be discarded immediately
+			// after the refresh walk completes.
+			&EvalWriteDiff{
+				Addr:           addr.Resource,
+				Change:         &change,
+				ProviderSchema: &providerSchema,
+			},
 		},
 	}
 }