core: Propagate check results accurately from plan to apply

In an earlier commit we changed the states.CheckResults model to
explicitly model the config object vs. dynamic checkable object hierarchy,
but neglected to update the logic in Terraform Core to take that into
account when propagating the object expansion decisions from the plan
phase to the apply phase. That meant that we were incorrectly classifying
zero-instance resources always as having an unknown number of instances,
rather than possibly being known to have zero instances.

This now follows the two-level heirarchy of the data structure, which has
the nice side-effect that we can remove some of the special-case methods
from checks.State that we were using to bulk-load data: the data is now
shaped in the appropriate way to reload the data using the same method
the plan phase would've used to record the results in the first place.

internal/checks/state_report.go

@@ -51,43 +51,6 @@ func (c *State) ReportCheckableObjects(configAddr addrs.ConfigCheckable, objectA
 	}
 }
 
-// ReportRestoredCheckableObjects is the interface by which Terraform Core
-// should reload all of the checkable object addresses it previously determined
-// during the planning phase in order to re-evaluate their results during
-// the apply phase using updated data.
-//
-// If any of the given objects don't have a corresponding configuration object
-// known to this check state then this function will panic.
-func (c *State) ReportRestoredCheckableObjects(objectAddrs addrs.Set[addrs.Checkable]) {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
-	// This is a similar principle as for ReportCheckableObjects except that
-	// we don't do the consistency checks that would normally ensure we only
-	// get one ReportCheckableObjects call per ConfigCheckable.
-
-	for _, objectAddr := range objectAddrs {
-		configAddr := objectAddr.ConfigCheckable()
-		st, ok := c.statuses.GetOk(configAddr)
-		if !ok {
-			panic(fmt.Sprintf("checkable objects report for unknown configuration object %s", configAddr))
-		}
-
-		checks := make(map[addrs.CheckType][]Status, len(st.checkTypes))
-		for checkType, count := range st.checkTypes {
-			// NOTE: This is intentionally a slice of count of the zero value
-			// of Status, which is StatusUnknown to represent that we don't
-			// yet have a report for that particular check.
-			checks[checkType] = make([]Status, count)
-		}
-
-		if st.objects.Elems == nil {
-			st.objects = addrs.MakeMap[addrs.Checkable, map[addrs.CheckType][]Status]()
-		}
-		st.objects.Put(objectAddr, checks)
-	}
-}
-
 // ReportCheckResult is the interface by which Terraform Core should tell the
 // State object the result of a specific check for an object that was
 // previously registered with ReportCheckableObjects.

internal/states/checks.go

@@ -111,21 +111,6 @@ func NewCheckResults(source *checks.State) *CheckResults {
 	return ret
 }
 
-// AllCheckedObjects returns a set of all of the objects that have at least
-// one check in the set of results.
-func (r *CheckResults) AllCheckedObjects() addrs.Set[addrs.Checkable] {
-	if r == nil || len(r.ConfigResults.Elems) == 0 {
-		return nil
-	}
-	ret := addrs.MakeSet[addrs.Checkable]()
-	for _, configElem := range r.ConfigResults.Elems {
-		for _, objElem := range configElem.Value.ObjectResults.Elems {
-			ret.Add(objElem.Key)
-		}
-	}
-	return ret
-}
-
 // GetObjectResult looks up the result for a single object, or nil if there
 // is no such object.
 //
@@ -180,3 +165,18 @@ func (r *CheckResults) DeepCopy() *CheckResults {
 
 	return ret
 }
+
+// ObjectAddrsKnown determines whether the set of objects recorded in this
+// aggregate is accurate (true) or if it's incomplete as a result of the
+// run being interrupted before instance expansion.
+func (r *CheckResultAggregate) ObjectAddrsKnown() bool {
+	if r.ObjectResults.Len() != 0 {
+		// If there are any object results at all then we definitely know.
+		return true
+	}
+
+	// If we don't have any object addresses then we distinguish a known
+	// empty set of objects from an unknown set of objects by the aggregate
+	// status being unknown.
+	return r.Status != checks.StatusUnknown
+}

internal/terraform/context_apply.go

@@ -36,10 +36,10 @@ func (c *Context) Apply(plan *plans.Plan, config *configs.Config) (*states.State
 		InputState: workingState,
 		Changes:    plan.Changes,
 
-		// We need to propagate the results of deciding which objects will
-		// have checks from the plan phase, since we won't re-run the
-		// instance expansion logic during the apply phase.
-		KnownCheckableObjects: plan.Checks.AllCheckedObjects(),
+		// We need to propagate the check results from the plan phase,
+		// because that will tell us which checkable objects we're expecting
+		// to see updated results from during the apply step.
+		PlanTimeCheckResults: plan.Checks,
 	})
 	diags = diags.Append(walker.NonFatalDiagnostics)
 	diags = diags.Append(walkDiags)

internal/terraform/context_walk.go

@@ -3,7 +3,6 @@ package terraform
 import (
 	"log"
 
-	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/checks"
 	"github.com/hashicorp/terraform/internal/configs"
 	"github.com/hashicorp/terraform/internal/instances"
@@ -25,12 +24,13 @@ type graphWalkOpts struct {
 	Changes    *plans.Changes
 	Config     *configs.Config
 
-	// KnownCheckableObjects is a set of objects that we know ahead of time
-	// ought to have checks run against them during this graph walk.
-	// Use this to propagate the decision about which objects are checkable
-	// from the plan phase into the apply phase, so that the apply phase
-	// doesn't need to recalcuate it.
-	KnownCheckableObjects addrs.Set[addrs.Checkable]
+	// PlanTimeCheckResults should be populated during the apply phase with
+	// the snapshot of check results that was generated during the plan step.
+	//
+	// This then propagates the decisions about which checkable objects exist
+	// from the plan phase into the apply phase without having to re-compute
+	// the module and resource expansion.
+	PlanTimeCheckResults *states.CheckResults
 
 	MoveResults refactoring.MoveResults
 }
@@ -116,7 +116,17 @@ func (c *Context) graphWalker(operation walkOperation, opts *graphWalkOpts) *Con
 	}
 
 	checkState := checks.NewState(opts.Config)
-	checkState.ReportRestoredCheckableObjects(opts.KnownCheckableObjects)
+	if opts.PlanTimeCheckResults != nil {
+		// We'll re-report all of the same objects we determined during the
+		// plan phase so that we can repeat the checks during the apply
+		// phase to finalize them.
+		for _, configElem := range opts.PlanTimeCheckResults.ConfigResults.Elems {
+			if configElem.Value.ObjectAddrsKnown() {
+				configAddr := configElem.Key
+				checkState.ReportCheckableObjects(configAddr, configElem.Value.ObjectResults.Keys())
+			}
+		}
+	}
 
 	return &ContextGraphWalker{
 		Context:          c,