Eval(): refactor EvalWriteState() (#27145)

* fix inaccurate log

* terraform: refactor EvalWriteState

EvalWriteState is refactored into a method on
NodeAbstractResourceInstance and renamed writeResourceInstanceState.

Import, my nemesis, gave me pause. I did not expect to find
EvalWriteState in an transform node, and so I decided to copy the
function inline rather than rethink my entire refactor for one function
that's likely to be (heavily) refactored in the future.

terraform/eval_provider.go

@@ -49,7 +49,7 @@ func buildProviderConfig(ctx EvalContext, addr addrs.AbsProviderConfig, config *
 func GetProvider(ctx EvalContext, addr addrs.AbsProviderConfig) (providers.Interface, *ProviderSchema, error) {
 	if addr.Provider.Type == "" {
 		// Should never happen
-		panic("EvalGetProvider used with uninitialized provider configuration address")
+		panic("GetProvider used with uninitialized provider configuration address")
 	}
 	provider := ctx.Provider(addr)
 	if provider == nil {

terraform/eval_state.go

@@ -34,98 +34,6 @@ func UpdateStateHook(ctx EvalContext) error {
 	return err
 }
 
-// EvalWriteState is an EvalNode implementation that saves the given object
-// as the current object for the selected resource instance.
-type EvalWriteState struct {
-	// Addr is the address of the instance to read state for.
-	Addr addrs.ResourceInstance
-
-	// State is the object state to save.
-	State **states.ResourceInstanceObject
-
-	// ProviderSchema is the schema for the provider given in ProviderAddr.
-	ProviderSchema **ProviderSchema
-
-	// ProviderAddr is the address of the provider configuration that
-	// produced the given object.
-	ProviderAddr addrs.AbsProviderConfig
-
-	// Dependencies are the inter-resource dependencies to be stored in the
-	// state.
-	Dependencies *[]addrs.ConfigResource
-
-	// targetState determines which context state we're writing to during plan.
-	// The default is the global working state.
-	targetState phaseState
-}
-
-func (n *EvalWriteState) Eval(ctx EvalContext) tfdiags.Diagnostics {
-	var diags tfdiags.Diagnostics
-
-	if n.State == nil {
-		// Note that a pointer _to_ nil is valid here, indicating the total
-		// absense of an object as we'd see during destroy.
-		panic("EvalWriteState used with no ResourceInstanceObject")
-	}
-
-	absAddr := n.Addr.Absolute(ctx.Path())
-
-	var state *states.SyncState
-	switch n.targetState {
-	case refreshState:
-		log.Printf("[TRACE] EvalWriteState: using RefreshState for %s", absAddr)
-		state = ctx.RefreshState()
-	default:
-		state = ctx.State()
-	}
-
-	if n.ProviderAddr.Provider.Type == "" {
-		diags = diags.Append(fmt.Errorf("failed to write state for %s: missing provider type", absAddr))
-		return diags
-	}
-	obj := *n.State
-	if obj == nil || obj.Value.IsNull() {
-		// No need to encode anything: we'll just write it directly.
-		state.SetResourceInstanceCurrent(absAddr, nil, n.ProviderAddr)
-		log.Printf("[TRACE] EvalWriteState: removing state object for %s", absAddr)
-		return diags
-	}
-
-	// store the new deps in the state
-	if n.Dependencies != nil {
-		log.Printf("[TRACE] EvalWriteState: recording %d dependencies for %s", len(*n.Dependencies), absAddr)
-		obj.Dependencies = *n.Dependencies
-	}
-
-	if n.ProviderSchema == nil || *n.ProviderSchema == nil {
-		// Should never happen, unless our state object is nil
-		panic("EvalWriteState used with pointer to nil ProviderSchema object")
-	}
-
-	if obj != nil {
-		log.Printf("[TRACE] EvalWriteState: writing current state object for %s", absAddr)
-	} else {
-		log.Printf("[TRACE] EvalWriteState: removing current state object for %s", absAddr)
-	}
-
-	schema, currentVersion := (*n.ProviderSchema).SchemaForResourceAddr(n.Addr.ContainingResource())
-	if schema == nil {
-		// It shouldn't be possible to get this far in any real scenario
-		// without a schema, but we might end up here in contrived tests that
-		// fail to set up their world properly.
-		diags = diags.Append(fmt.Errorf("failed to encode %s in state: no resource type schema available", absAddr))
-		return diags
-	}
-	src, err := obj.Encode(schema.ImpliedType(), currentVersion)
-	if err != nil {
-		diags = diags.Append(fmt.Errorf("failed to encode %s in state: %s", absAddr, err))
-		return diags
-	}
-
-	state.SetResourceInstanceCurrent(absAddr, src, n.ProviderAddr)
-	return diags
-}
-
 // EvalWriteStateDeposed is an EvalNode implementation that writes
 // an InstanceState out to the Deposed list of a resource in the state.
 type EvalWriteStateDeposed struct {

terraform/eval_state_test.go

@@ -11,56 +11,6 @@ import (
 	"github.com/hashicorp/terraform/states"
 )
 
-func TestReadResourceInstanceState(t *testing.T) {
-
-}
-
-func TestEvalWriteState(t *testing.T) {
-	state := states.NewState()
-	ctx := new(MockEvalContext)
-	ctx.StateState = state.SyncWrapper()
-	ctx.PathPath = addrs.RootModuleInstance
-
-	mockProvider := mockProviderWithResourceTypeSchema("aws_instance", &configschema.Block{
-		Attributes: map[string]*configschema.Attribute{
-			"id": {
-				Type:     cty.String,
-				Optional: true,
-			},
-		},
-	})
-	providerSchema := mockProvider.GetSchemaReturn
-
-	obj := &states.ResourceInstanceObject{
-		Value: cty.ObjectVal(map[string]cty.Value{
-			"id": cty.StringVal("i-abc123"),
-		}),
-		Status: states.ObjectReady,
-	}
-	node := &EvalWriteState{
-		Addr: addrs.Resource{
-			Mode: addrs.ManagedResourceMode,
-			Type: "aws_instance",
-			Name: "foo",
-		}.Instance(addrs.NoKey),
-
-		State: &obj,
-
-		ProviderSchema: &providerSchema,
-		ProviderAddr:   addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("aws")),
-	}
-	diags := node.Eval(ctx)
-	if diags.HasErrors() {
-		t.Fatalf("Got err: %#v", diags.ErrWithWarnings())
-	}
-
-	checkStateString(t, state, `
-aws_instance.foo:
-  ID = i-abc123
-  provider = provider["registry.terraform.io/hashicorp/aws"]
-	`)
-}
-
 func TestEvalWriteStateDeposed(t *testing.T) {
 	state := states.NewState()
 	ctx := new(MockEvalContext)

terraform/node_resource_abstract_instance.go

@@ -244,3 +244,69 @@ func (n *NodeAbstractResourceInstance) PostApplyHook(ctx EvalContext, state *sta
 
 	return diags
 }
+
+// writeResourceInstanceState saves the given object as the current object for
+// the selected resource instance.
+//
+// dependencies is a parameter, instead of those directly attacted to the
+// NodeAbstractResourceInstance, because we don't write dependencies for
+// datasources.
+//
+// targetState determines which context state we're writing to during plan. The
+// default is the global working state.
+func (n *NodeAbstractResourceInstance) writeResourceInstanceState(ctx EvalContext, obj *states.ResourceInstanceObject, dependencies []addrs.ConfigResource, targetState phaseState) error {
+	absAddr := n.Addr
+	_, providerSchema, err := GetProvider(ctx, n.ResolvedProvider)
+	if err != nil {
+		return err
+	}
+
+	var state *states.SyncState
+	switch targetState {
+	case refreshState:
+		log.Printf("[TRACE] writeResourceInstanceState: using RefreshState for %s", absAddr)
+		state = ctx.RefreshState()
+	default:
+		state = ctx.State()
+	}
+
+	if obj == nil || obj.Value.IsNull() {
+		// No need to encode anything: we'll just write it directly.
+		state.SetResourceInstanceCurrent(absAddr, nil, n.ResolvedProvider)
+		log.Printf("[TRACE] writeResourceInstanceState: removing state object for %s", absAddr)
+		return nil
+	}
+
+	// store the new deps in the state.
+	// We check for nil here because don't want to override existing dependencies on orphaned nodes.
+	if dependencies != nil {
+		obj.Dependencies = dependencies
+	}
+
+	if providerSchema == nil {
+		// Should never happen, unless our state object is nil
+		panic("writeResourceInstanceState used with nil ProviderSchema")
+	}
+
+	if obj != nil {
+		log.Printf("[TRACE] writeResourceInstanceState: writing current state object for %s", absAddr)
+	} else {
+		log.Printf("[TRACE] writeResourceInstanceState: removing current state object for %s", absAddr)
+	}
+
+	schema, currentVersion := (*providerSchema).SchemaForResourceAddr(absAddr.ContainingResource().Resource)
+	if schema == nil {
+		// It shouldn't be possible to get this far in any real scenario
+		// without a schema, but we might end up here in contrived tests that
+		// fail to set up their world properly.
+		return fmt.Errorf("failed to encode %s in state: no resource type schema available", absAddr)
+	}
+
+	src, err := obj.Encode(schema.ImpliedType(), currentVersion)
+	if err != nil {
+		return fmt.Errorf("failed to encode %s in state: %s", absAddr, err)
+	}
+
+	state.SetResourceInstanceCurrent(absAddr, src, n.ResolvedProvider)
+	return nil
+}

terraform/node_resource_abstract_instance_test.go

@@ -6,6 +6,9 @@ import (
 
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
+	"github.com/hashicorp/terraform/configs/configschema"
+	"github.com/hashicorp/terraform/states"
+	"github.com/zclconf/go-cty/cty"
 )
 
 func TestNodeAbstractResourceInstanceProvider(t *testing.T) {
@@ -109,3 +112,47 @@ func TestNodeAbstractResourceInstanceProvider(t *testing.T) {
 		})
 	}
 }
+
+func TestNodeAbstractResourceInstance_WriteResourceInstanceState(t *testing.T) {
+	state := states.NewState()
+	ctx := new(MockEvalContext)
+	ctx.StateState = state.SyncWrapper()
+	ctx.PathPath = addrs.RootModuleInstance
+
+	mockProvider := mockProviderWithResourceTypeSchema("aws_instance", &configschema.Block{
+		Attributes: map[string]*configschema.Attribute{
+			"id": {
+				Type:     cty.String,
+				Optional: true,
+			},
+		},
+	})
+
+	obj := &states.ResourceInstanceObject{
+		Value: cty.ObjectVal(map[string]cty.Value{
+			"id": cty.StringVal("i-abc123"),
+		}),
+		Status: states.ObjectReady,
+	}
+
+	node := &NodeAbstractResourceInstance{
+		Addr: mustResourceInstanceAddr("aws_instance.foo"),
+		// instanceState:        obj,
+		NodeAbstractResource: NodeAbstractResource{
+			ResolvedProvider: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
+		},
+	}
+	ctx.ProviderProvider = mockProvider
+	ctx.ProviderSchemaSchema = mockProvider.GetSchemaReturn
+
+	err := node.writeResourceInstanceState(ctx, obj, nil, workingState)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err.Error())
+	}
+
+	checkStateString(t, state, `
+aws_instance.foo:
+  ID = i-abc123
+  provider = provider["registry.terraform.io/hashicorp/aws"]
+	`)
+}

terraform/node_resource_apply_instance.go

@@ -173,13 +173,8 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) (di
 		return diags
 	}
 
-	writeState := &EvalWriteState{
-		Addr:           addr,
-		ProviderAddr:   n.ResolvedProvider,
-		ProviderSchema: &providerSchema,
-		State:          &state,
-	}
-	diags = diags.Append(writeState.Eval(ctx))
+	// We don't write dependencies for datasources
+	diags = diags.Append(n.writeResourceInstanceState(ctx, state, nil, workingState))
 	if diags.HasErrors() {
 		return diags
 	}
@@ -362,14 +357,7 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 		return diags
 	}
 
-	writeState := &EvalWriteState{
-		Addr:           addr,
-		ProviderAddr:   n.ResolvedProvider,
-		ProviderSchema: &providerSchema,
-		State:          &state,
-		Dependencies:   &n.Dependencies,
-	}
-	diags = diags.Append(writeState.Eval(ctx))
+	diags = diags.Append(n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState))
 	if diags.HasErrors() {
 		return diags
 	}
@@ -398,14 +386,7 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 		return diags
 	}
 
-	writeState = &EvalWriteState{
-		Addr:           addr,
-		ProviderAddr:   n.ResolvedProvider,
-		ProviderSchema: &providerSchema,
-		State:          &state,
-		Dependencies:   &n.Dependencies,
-	}
-	diags = diags.Append(writeState.Eval(ctx))
+	diags = diags.Append(n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState))
 	if diags.HasErrors() {
 		return diags
 	}

terraform/node_resource_destroy.go

@@ -224,14 +224,7 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 		if diags.HasErrors() {
 			return diags
 		}
-
-		evalWriteState := &EvalWriteState{
-			Addr:           addr.Resource,
-			ProviderAddr:   n.ResolvedProvider,
-			ProviderSchema: &providerSchema,
-			State:          &state,
-		}
-		diags = diags.Append(evalWriteState.Eval(ctx))
+		diags = diags.Append(n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState))
 		if diags.HasErrors() {
 			return diags
 		}

terraform/node_resource_plan_instance.go

@@ -95,25 +95,11 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) (di
 
 	// write the data source into both the refresh state and the
 	// working state
-	writeRefreshState := &EvalWriteState{
-		Addr:           addr.Resource,
-		ProviderAddr:   n.ResolvedProvider,
-		ProviderSchema: &providerSchema,
-		State:          &state,
-		targetState:    refreshState,
-	}
-	diags = diags.Append(writeRefreshState.Eval(ctx))
+	diags = diags.Append(n.writeResourceInstanceState(ctx, state, nil, refreshState))
 	if diags.HasErrors() {
 		return diags
 	}
-
-	writeState := &EvalWriteState{
-		Addr:           addr.Resource,
-		ProviderAddr:   n.ResolvedProvider,
-		ProviderSchema: &providerSchema,
-		State:          &state,
-	}
-	diags = diags.Append(writeState.Eval(ctx))
+	diags = diags.Append(n.writeResourceInstanceState(ctx, state, nil, workingState))
 	if diags.HasErrors() {
 		return diags
 	}
@@ -183,15 +169,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
 			return diags
 		}
 
-		writeRefreshState := &EvalWriteState{
-			Addr:           addr.Resource,
-			ProviderAddr:   n.ResolvedProvider,
-			ProviderSchema: &providerSchema,
-			State:          &instanceRefreshState,
-			targetState:    refreshState,
-			Dependencies:   &n.Dependencies,
-		}
-		diags = diags.Append(writeRefreshState.Eval(ctx))
+		diags = diags.Append(n.writeResourceInstanceState(ctx, instanceRefreshState, n.Dependencies, refreshState))
 		if diags.HasErrors() {
 			return diags
 		}
@@ -220,13 +198,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
 		return diags
 	}
 
-	writeState := &EvalWriteState{
-		Addr:           addr.Resource,
-		ProviderAddr:   n.ResolvedProvider,
-		State:          &instancePlanState,
-		ProviderSchema: &providerSchema,
-	}
-	diags = diags.Append(writeState.Eval(ctx))
+	diags = diags.Append(n.writeResourceInstanceState(ctx, instancePlanState, n.Dependencies, workingState))
 	if diags.HasErrors() {
 		return diags
 	}

terraform/node_resource_plan_orphan.go

@@ -97,14 +97,7 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon
 			return diags
 		}
 
-		writeRefreshState := &EvalWriteState{
-			Addr:           addr.Resource,
-			ProviderAddr:   n.ResolvedProvider,
-			ProviderSchema: &providerSchema,
-			State:          &state,
-			targetState:    refreshState,
-		}
-		diags = diags.Append(writeRefreshState.Eval(ctx))
+		diags = diags.Append(n.writeResourceInstanceState(ctx, state, n.Dependencies, refreshState))
 		if diags.HasErrors() {
 			return diags
 		}
@@ -137,12 +130,6 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon
 		return diags
 	}
 
-	writeState := &EvalWriteState{
-		Addr:           addr.Resource,
-		ProviderAddr:   n.ResolvedProvider,
-		ProviderSchema: &providerSchema,
-		State:          &state,
-	}
-	diags = diags.Append(writeState.Eval(ctx))
+	diags = diags.Append(n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState))
 	return diags
 }

terraform/transform_import_state.go

@@ -300,13 +300,20 @@ func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) (di
 		return diags
 	}
 
-	//EvalWriteState
-	evalWriteState := &EvalWriteState{
-		Addr:           n.TargetAddr.Resource,
-		ProviderAddr:   n.ResolvedProvider,
-		ProviderSchema: &providerSchema,
-		State:          &state,
+	schema, currentVersion := providerSchema.SchemaForResourceAddr(n.TargetAddr.ContainingResource().Resource)
+	if schema == nil {
+		// It shouldn't be possible to get this far in any real scenario
+		// without a schema, but we might end up here in contrived tests that
+		// fail to set up their world properly.
+		diags = diags.Append(fmt.Errorf("failed to encode %s in state: no resource type schema available", n.TargetAddr.Resource))
+		return diags
 	}
-	diags = diags.Append(evalWriteState.Eval(ctx))
+	src, err := state.Encode(schema.ImpliedType(), currentVersion)
+	if err != nil {
+		diags = diags.Append(fmt.Errorf("failed to encode %s in state: %s", n.TargetAddr.Resource, err))
+		return diags
+	}
+	ctx.State().SetResourceInstanceCurrent(n.TargetAddr, src, n.ResolvedProvider)
+
 	return diags
 }