Fix race when using ResolvedImports (#1277)

Signed-off-by: RLRabinowitz <rlrabinowitz2@gmail.com>

internal/tofu/context_import.go

@@ -7,6 +7,7 @@ package tofu
 
 import (
 	"log"
+	"sync"
 
 	"github.com/hashicorp/hcl/v2"
 
@@ -95,20 +96,67 @@ func (i *ImportTarget) ResolvedAddr() (address addrs.AbsResourceInstance, evalua
 
 // ResolvedConfigImportsKey is a key for a map of ImportTargets originating from the configuration
 // It is used as a one-to-one representation of an EvaluatedConfigImportTarget.
-// Used in ResolvedImports to maintain a map of all resolved imports when walking the graph
+// Used in ImportResolver to maintain a map of all resolved imports when walking the graph
 type ResolvedConfigImportsKey struct {
 	// An address string is one-to-one with addrs.AbsResourceInstance
 	AddrStr string
 	ID      string
 }
 
-// ResolvedImports is a struct that maintains a map of all imports as they are being resolved.
+// ImportResolver is a struct that maintains a map of all imports as they are being resolved.
 // This is specifically for imports originating from configuration.
 // Import targets' addresses are not fully known from the get-go, and could only be resolved later when walking
 // the graph. This struct helps keep track of the resolved imports, mostly for validation that all imports
 // have been addressed and point to an actual configuration
-type ResolvedImports struct {
-	imports map[ResolvedConfigImportsKey]bool
+type ImportResolver struct {
+	mu      sync.RWMutex
+	imports map[ResolvedConfigImportsKey]EvaluatedConfigImportTarget
+}
+
+func NewImportResolver() *ImportResolver {
+	return &ImportResolver{imports: make(map[ResolvedConfigImportsKey]EvaluatedConfigImportTarget)}
+}
+
+// ResolveImport resolves the ID and address (soon, when it will be necessary) of an ImportTarget originating
+// from an import block, when we have the context necessary to resolve them. The resolved import target would be an
+// EvaluatedConfigImportTarget.
+// This function mutates the EvalContext's ImportResolver, adding the resolved import target
+// The function errors if we failed to evaluate the ID or the address (soon)
+func (ri *ImportResolver) ResolveImport(importTarget *ImportTarget, ctx EvalContext) error {
+	importId, evalDiags := evaluateImportIdExpression(importTarget.Config.ID, ctx)
+	if evalDiags.HasErrors() {
+		return evalDiags.Err()
+	}
+
+	// TODO - Change once an import target's address is more dynamic
+	importAddress := importTarget.Config.To
+
+	ri.mu.Lock()
+	defer ri.mu.Unlock()
+
+	resolvedImportKey := ResolvedConfigImportsKey{
+		AddrStr: importAddress.String(),
+		ID:      importId,
+	}
+
+	ri.imports[resolvedImportKey] = EvaluatedConfigImportTarget{
+		Config: importTarget.Config,
+		ID:     importId,
+	}
+
+	return nil
+}
+
+// GetAllImports returns all resolved imports
+func (ri *ImportResolver) GetAllImports() []EvaluatedConfigImportTarget {
+	ri.mu.RLock()
+	defer ri.mu.RUnlock()
+
+	var allImports []EvaluatedConfigImportTarget
+	for _, importTarget := range ri.imports {
+		allImports = append(allImports, importTarget)
+	}
+	return allImports
 }
 
 // Import takes already-created external resources and brings them

internal/tofu/context_plan.go

@@ -532,9 +532,9 @@ func (c *Context) postPlanValidateMoves(config *configs.Config, stmts []refactor
 // All import target addresses with a key must already exist in config.
 // When we are able to generate config for expanded resources, this rule can be
 // relaxed.
-func (c *Context) postPlanValidateImports(resolvedImports *ResolvedImports, allInst instances.Set) tfdiags.Diagnostics {
+func (c *Context) postPlanValidateImports(importResolver *ImportResolver, allInst instances.Set) tfdiags.Diagnostics {
 	var diags tfdiags.Diagnostics
-	for resolvedImport := range resolvedImports.imports {
+	for resolvedImport := range importResolver.imports {
 		// We only care about import target addresses that have a key.
 		// If the address does not have a key, we don't need it to be in config
 		// because are able to generate config.
@@ -621,7 +621,7 @@ func (c *Context) planWalk(config *configs.Config, prevRunState *states.State, o
 
 	allInsts := walker.InstanceExpander.AllInstances()
 
-	importValidateDiags := c.postPlanValidateImports(walker.ResolvedImports, allInsts)
+	importValidateDiags := c.postPlanValidateImports(walker.ImportResolver, allInsts)
 	if importValidateDiags.HasErrors() {
 		return nil, importValidateDiags
 	}

internal/tofu/context_walk.go

@@ -149,7 +149,7 @@ func (c *Context) graphWalker(operation walkOperation, opts *graphWalkOpts) *Con
 		Checks:           checkState,
 		InstanceExpander: instances.NewExpander(),
 		MoveResults:      opts.MoveResults,
-		ResolvedImports:  &ResolvedImports{imports: make(map[ResolvedConfigImportsKey]bool)},
+		ImportResolver:   NewImportResolver(),
 		Operation:        operation,
 		StopContext:      c.runContext,
 		PlanTimestamp:    opts.PlanTimeTimestamp,

internal/tofu/eval_context.go

@@ -203,13 +203,13 @@ type EvalContext interface {
 	// objects accessible through it.
 	MoveResults() refactoring.MoveResults
 
-	// ResolvedImports returns a map describing the resolved imports
+	// ImportResolver returns a map describing the resolved imports
 	// after evaluating the dynamic address of the import targets
 	//
 	// This data is created during the graph walk, as import target addresses are being resolved
 	// Its primary use is for validation at the end of a plan - To make sure all imports have been satisfied
 	// and have a configuration
-	ResolvedImports() *ResolvedImports
+	ImportResolver() *ImportResolver
 
 	// WithPath returns a copy of the context with the internal path set to the
 	// path argument.

internal/tofu/eval_context_builtin.go

@@ -75,7 +75,7 @@ type BuiltinEvalContext struct {
 	PrevRunStateValue     *states.SyncState
 	InstanceExpanderValue *instances.Expander
 	MoveResultsValue      refactoring.MoveResults
-	ResolvedImportsValue  *ResolvedImports
+	ImportResolverValue   *ImportResolver
 }
 
 // BuiltinEvalContext implements EvalContext
@@ -512,6 +512,6 @@ func (ctx *BuiltinEvalContext) MoveResults() refactoring.MoveResults {
 	return ctx.MoveResultsValue
 }
 
-func (ctx *BuiltinEvalContext) ResolvedImports() *ResolvedImports {
-	return ctx.ResolvedImportsValue
+func (ctx *BuiltinEvalContext) ImportResolver() *ImportResolver {
+	return ctx.ImportResolverValue
 }

internal/tofu/eval_context_mock.go

@@ -151,8 +151,8 @@ type MockEvalContext struct {
 	MoveResultsCalled  bool
 	MoveResultsResults refactoring.MoveResults
 
-	ResolvedImportsCalled  bool
-	ResolvedImportsResults *ResolvedImports
+	ImportResolverCalled  bool
+	ImportResolverResults *ImportResolver
 
 	InstanceExpanderCalled   bool
 	InstanceExpanderExpander *instances.Expander
@@ -403,9 +403,9 @@ func (c *MockEvalContext) MoveResults() refactoring.MoveResults {
 	return c.MoveResultsResults
 }
 
-func (c *MockEvalContext) ResolvedImports() *ResolvedImports {
-	c.ResolvedImportsCalled = true
-	return c.ResolvedImportsResults
+func (c *MockEvalContext) ImportResolver() *ImportResolver {
+	c.ImportResolverCalled = true
+	return c.ImportResolverResults
 }
 
 func (c *MockEvalContext) InstanceExpander() *instances.Expander {

internal/tofu/graph_walk_context.go

@@ -38,7 +38,7 @@ type ContextGraphWalker struct {
 	Changes            *plans.ChangesSync      // Used for safe concurrent writes to changes
 	Checks             *checks.State           // Used for safe concurrent writes of checkable objects and their check results
 	InstanceExpander   *instances.Expander     // Tracks our gradual expansion of module and resource instances
-	ResolvedImports    *ResolvedImports        // Tracks import targets as they are being resolved
+	ImportResolver     *ImportResolver         // Tracks import targets as they are being resolved
 	MoveResults        refactoring.MoveResults // Read-only record of earlier processing of move statements
 	Operation          walkOperation
 	StopContext        context.Context
@@ -103,7 +103,7 @@ func (w *ContextGraphWalker) EvalContext() EvalContext {
 		InstanceExpanderValue: w.InstanceExpander,
 		Plugins:               w.Context.plugins,
 		MoveResultsValue:      w.MoveResults,
-		ResolvedImportsValue:  w.ResolvedImports,
+		ImportResolverValue:   w.ImportResolver,
 		ProviderCache:         w.providerCache,
 		ProviderInputConfig:   w.Context.providerInputConfig,
 		ProviderLock:          &w.providerLock,

internal/tofu/node_resource_plan.go

@@ -303,29 +303,17 @@ func (n *nodeExpandPlannableResource) resourceInstanceSubgraph(ctx EvalContext,
 	var diags tfdiags.Diagnostics
 
 	var commandLineImportTargets []CommandLineImportTarget
-	var evaluatedConfigImportTargets []EvaluatedConfigImportTarget
+	importResolver := ctx.ImportResolver()
 	// FIXME - Deal with cases of duplicate addresses
 
 	for _, importTarget := range n.importTargets {
 		if importTarget.IsFromImportCommandLine() {
 			commandLineImportTargets = append(commandLineImportTargets, *importTarget.CommandLineImportTarget)
 		} else {
-			importId, evalDiags := evaluateImportIdExpression(importTarget.Config.ID, ctx)
-			if evalDiags.HasErrors() {
-				return nil, evalDiags.Err()
+			err := importResolver.ResolveImport(importTarget, ctx)
+			if err != nil {
+				return nil, err
 			}
-
-			evaluatedConfigImportTargets = append(evaluatedConfigImportTargets, EvaluatedConfigImportTarget{
-				Config: importTarget.Config,
-				ID:     importId,
-			})
-
-			resolvedImports := ctx.ResolvedImports().imports
-			resolvedImports[ResolvedConfigImportsKey{
-				AddrStr: importTarget.Config.To.String(),
-				ID:      importId,
-			}] = true
-
 		}
 	}
 
@@ -373,7 +361,7 @@ func (n *nodeExpandPlannableResource) resourceInstanceSubgraph(ctx EvalContext,
 			forceReplace:             n.forceReplace,
 		}
 
-		for _, evaluatedConfigImportTarget := range evaluatedConfigImportTargets {
+		for _, evaluatedConfigImportTarget := range ctx.ImportResolver().GetAllImports() {
 			// TODO - Change this code once Config.To is not a static address, to actually evaluate it
 			if evaluatedConfigImportTarget.Config.To.Equal(a.Addr) {
 				// If we get here, we're definitely not in legacy import mode,