From 7370a98ab738d19288427dc0cc54dc06c9546aaf Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Fri, 4 Dec 2020 09:16:26 -0500 Subject: [PATCH] Eval() Refactor (#27087) * terraform: refactor EvalPreApply and EvalPostApply EvalPreApply and EvalPostApply have been refactored as methods on NodeAbstractResourceInstance. * terraform: remove EvalReadState and EvalReadStateDeposed These two functions had already been re-implemented as functions on NodeAbstractResource, so this commit finished the process of removing the Evals and refactoring the tests. * terraform: remove EvalRefreshLifecycle EvalRefreshLifecycle was only used in one node, NodePlannableResourceInstance, so the functionality has been moved directly inline. * terraform: remove EvalDeposeState EvalDeposeState was only used in one function, so it has been removed and the logic placed in-line in NodeApplyableResourceInstance.managedResourceExecute. * terraform: remove EvalMaybeRestoreDeposedObject EvalMaybeRestoreDeposedObject was only used in one place, so I've removed it in favor of in-line code. --- terraform/eval_apply.go | 64 ----- terraform/eval_state.go | 271 ------------------- terraform/eval_state_test.go | 145 +--------- terraform/node_resource_abstract.go | 45 +++ terraform/node_resource_abstract_instance.go | 45 +++ terraform/node_resource_abstract_test.go | 129 +++++++++ terraform/node_resource_apply_instance.go | 91 ++++--- terraform/node_resource_destroy.go | 21 +- terraform/node_resource_destroy_deposed.go | 42 +-- terraform/node_resource_plan_instance.go | 19 +- 10 files changed, 290 insertions(+), 582 deletions(-) diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 7d14ad13a..34f7e2745 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -375,70 +375,6 @@ func (n *EvalApply) Eval(ctx EvalContext) tfdiags.Diagnostics { return diags } -// EvalApplyPre is an EvalNode implementation that does the pre-Apply work -type EvalApplyPre struct { - Addr addrs.ResourceInstance - Gen states.Generation - State **states.ResourceInstanceObject - Change **plans.ResourceInstanceChange -} - -// TODO: test -func (n *EvalApplyPre) Eval(ctx EvalContext) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics - change := *n.Change - absAddr := n.Addr.Absolute(ctx.Path()) - - if change == nil { - panic(fmt.Sprintf("EvalApplyPre for %s called with nil Change", absAddr)) - } - - if resourceHasUserVisibleApply(n.Addr) { - priorState := change.Before - plannedNewState := change.After - - diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { - return h.PreApply(absAddr, n.Gen, change.Action, priorState, plannedNewState) - })) - if diags.HasErrors() { - return diags - } - } - - return nil -} - -// EvalApplyPost is an EvalNode implementation that does the post-Apply work -type EvalApplyPost struct { - Addr addrs.ResourceInstance - Gen states.Generation - State **states.ResourceInstanceObject - Error *error -} - -// TODO: test -func (n *EvalApplyPost) Eval(ctx EvalContext) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics - state := *n.State - - if resourceHasUserVisibleApply(n.Addr) { - absAddr := n.Addr.Absolute(ctx.Path()) - var newState cty.Value - if state != nil { - newState = state.Value - } else { - newState = cty.NullVal(cty.DynamicPseudoType) - } - diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostApply(absAddr, n.Gen, newState, *n.Error) - })) - } - - diags = diags.Append(*n.Error) - - return diags -} - // EvalMaybeTainted is an EvalNode that takes the planned change, new value, // and possible error from an apply operation and produces a new instance // object marked as tainted if it appears that a create operation has failed. diff --git a/terraform/eval_state.go b/terraform/eval_state.go index d8b79eb8a..2fd6eca85 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -5,9 +5,6 @@ import ( "log" "github.com/hashicorp/terraform/addrs" - "github.com/hashicorp/terraform/configs" - "github.com/hashicorp/terraform/plans" - "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" ) @@ -19,145 +16,6 @@ const ( refreshState ) -// EvalReadState is an EvalNode implementation that reads the -// current object for a specific instance in the state. -type EvalReadState struct { - // Addr is the address of the instance to read state for. - Addr addrs.ResourceInstance - - // ProviderSchema is the schema for the provider given in Provider. - ProviderSchema **ProviderSchema - - // Provider is the provider that will subsequently perform actions on - // the the state object. This is used to perform any schema upgrades - // that might be required to prepare the stored data for use. - Provider *providers.Interface - - // Output will be written with a pointer to the retrieved object. - Output **states.ResourceInstanceObject -} - -func (n *EvalReadState) Eval(ctx EvalContext) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics - - if n.Provider == nil || *n.Provider == nil { - panic("EvalReadState used with no Provider object") - } - if n.ProviderSchema == nil || *n.ProviderSchema == nil { - panic("EvalReadState used with no ProviderSchema object") - } - - absAddr := n.Addr.Absolute(ctx.Path()) - log.Printf("[TRACE] EvalReadState: reading state for %s", absAddr) - - src := ctx.State().ResourceInstanceObject(absAddr, states.CurrentGen) - if src == nil { - // Presumably we only have deposed objects, then. - log.Printf("[TRACE] EvalReadState: no state present for %s", absAddr) - return nil - } - - schema, currentVersion := (*n.ProviderSchema).SchemaForResourceAddr(n.Addr.ContainingResource()) - if schema == nil { - // Shouldn't happen since we should've failed long ago if no schema is present - diags = diags.Append(fmt.Errorf("no schema available for %s while reading state; this is a bug in Terraform and should be reported", absAddr)) - return diags - } - - src, diags = UpgradeResourceState(absAddr, *n.Provider, src, schema, currentVersion) - if diags.HasErrors() { - // Note that we don't have any channel to return warnings here. We'll - // accept that for now since warnings during a schema upgrade would - // be pretty weird anyway, since this operation is supposed to seem - // invisible to the user. - return diags - } - - obj, err := src.Decode(schema.ImpliedType()) - if err != nil { - diags = diags.Append(err) - return diags - } - - if n.Output != nil { - *n.Output = obj - } - return diags -} - -// EvalReadStateDeposed is an EvalNode implementation that reads the -// deposed InstanceState for a specific resource out of the state -type EvalReadStateDeposed struct { - // Addr is the address of the instance to read state for. - Addr addrs.ResourceInstance - - // Key identifies which deposed object we will read. - Key states.DeposedKey - - // ProviderSchema is the schema for the provider given in Provider. - ProviderSchema **ProviderSchema - - // Provider is the provider that will subsequently perform actions on - // the the state object. This is used to perform any schema upgrades - // that might be required to prepare the stored data for use. - Provider *providers.Interface - - // Output will be written with a pointer to the retrieved object. - Output **states.ResourceInstanceObject -} - -func (n *EvalReadStateDeposed) Eval(ctx EvalContext) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics - - if n.Provider == nil || *n.Provider == nil { - panic("EvalReadStateDeposed used with no Provider object") - } - if n.ProviderSchema == nil || *n.ProviderSchema == nil { - panic("EvalReadStateDeposed used with no ProviderSchema object") - } - - key := n.Key - if key == states.NotDeposed { - diags = diags.Append(fmt.Errorf("EvalReadStateDeposed used with no instance key; this is a bug in Terraform and should be reported")) - return diags - } - absAddr := n.Addr.Absolute(ctx.Path()) - log.Printf("[TRACE] EvalReadStateDeposed: reading state for %s deposed object %s", absAddr, n.Key) - - src := ctx.State().ResourceInstanceObject(absAddr, key) - if src == nil { - // Presumably we only have deposed objects, then. - log.Printf("[TRACE] EvalReadStateDeposed: no state present for %s deposed object %s", absAddr, n.Key) - return diags - } - - schema, currentVersion := (*n.ProviderSchema).SchemaForResourceAddr(n.Addr.ContainingResource()) - if schema == nil { - // Shouldn't happen since we should've failed long ago if no schema is present - diags = diags.Append(fmt.Errorf("no schema available for %s while reading state; this is a bug in Terraform and should be reported", absAddr)) - return diags - } - - src, diags = UpgradeResourceState(absAddr, *n.Provider, src, schema, currentVersion) - if diags.HasErrors() { - // Note that we don't have any channel to return warnings here. We'll - // accept that for now since warnings during a schema upgrade would - // be pretty weird anyway, since this operation is supposed to seem - // invisible to the user. - return diags - } - - obj, err := src.Decode(schema.ImpliedType()) - if err != nil { - diags = diags.Append(err) - return diags - } - if n.Output != nil { - *n.Output = obj - } - return diags -} - // UpdateStateHook calls the PostStateUpdate hook with the current state. func UpdateStateHook(ctx EvalContext) error { // In principle we could grab the lock here just long enough to take a @@ -358,132 +216,3 @@ type EvalDeposeState struct { // EvalUndeposeState.Key so it knows which deposed instance to forget. OutputKey *states.DeposedKey } - -// TODO: test -func (n *EvalDeposeState) Eval(ctx EvalContext) tfdiags.Diagnostics { - absAddr := n.Addr.Absolute(ctx.Path()) - state := ctx.State() - - var key states.DeposedKey - if n.ForceKey == states.NotDeposed { - key = state.DeposeResourceInstanceObject(absAddr) - } else { - key = n.ForceKey - state.DeposeResourceInstanceObjectForceKey(absAddr, key) - } - log.Printf("[TRACE] EvalDeposeState: prior object for %s now deposed with key %s", absAddr, key) - - if n.OutputKey != nil { - *n.OutputKey = key - } - - return nil -} - -// EvalMaybeRestoreDeposedObject is an EvalNode implementation that will -// restore a particular deposed object of the specified resource instance -// to be the "current" object if and only if the instance doesn't currently -// have a current object. -// -// This is intended for use when the create leg of a create before destroy -// fails with no partial new object: if we didn't take any action, the user -// would be left in the unfortunate situation of having no current object -// and the previously-workign object now deposed. This EvalNode causes a -// better outcome by restoring things to how they were before the replace -// operation began. -// -// The create operation may have produced a partial result even though it -// failed and it's important that we don't "forget" that state, so in that -// situation the prior object remains deposed and the partial new object -// remains the current object, allowing the situation to hopefully be -// improved in a subsequent run. -type EvalMaybeRestoreDeposedObject struct { - Addr addrs.ResourceInstance - - // PlannedChange might be the action we're performing that includes - // the possiblity of restoring a deposed object. However, it might also - // be nil. It's here only for use in error messages and must not be - // used for business logic. - PlannedChange **plans.ResourceInstanceChange - - // Key is a pointer to the deposed object key that should be forgotten - // from the state, which must be non-nil. - Key *states.DeposedKey -} - -// TODO: test -func (n *EvalMaybeRestoreDeposedObject) Eval(ctx EvalContext) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics - - absAddr := n.Addr.Absolute(ctx.Path()) - dk := *n.Key - state := ctx.State() - - if dk == states.NotDeposed { - // This should never happen, and so it always indicates a bug. - // We should evaluate this node only if we've previously deposed - // an object as part of the same operation. - if n.PlannedChange != nil && *n.PlannedChange != nil { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Attempt to restore non-existent deposed object", - fmt.Sprintf( - "Terraform has encountered a bug where it would need to restore a deposed object for %s without knowing a deposed object key for that object. This occurred during a %s action. This is a bug in Terraform; please report it!", - absAddr, (*n.PlannedChange).Action, - ), - )) - } else { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Attempt to restore non-existent deposed object", - fmt.Sprintf( - "Terraform has encountered a bug where it would need to restore a deposed object for %s without knowing a deposed object key for that object. This is a bug in Terraform; please report it!", - absAddr, - ), - )) - } - return diags - } - - restored := state.MaybeRestoreResourceInstanceDeposed(absAddr, dk) - if restored { - log.Printf("[TRACE] EvalMaybeRestoreDeposedObject: %s deposed object %s was restored as the current object", absAddr, dk) - } else { - log.Printf("[TRACE] EvalMaybeRestoreDeposedObject: %s deposed object %s remains deposed", absAddr, dk) - } - - return diags -} - -// EvalRefreshLifecycle is an EvalNode implementation that updates -// the status of the lifecycle options stored in the state. -// This currently only applies to create_before_destroy. -type EvalRefreshLifecycle struct { - Addr addrs.AbsResourceInstance - - Config *configs.Resource - // Prior State - State **states.ResourceInstanceObject - // ForceCreateBeforeDestroy indicates a create_before_destroy resource - // depends on this resource. - ForceCreateBeforeDestroy bool -} - -func (n *EvalRefreshLifecycle) Eval(ctx EvalContext) tfdiags.Diagnostics { - state := *n.State - if state == nil { - // no existing state - return nil - } - - // In 0.13 we could be refreshing a resource with no config. - // We should be operating on managed resource, but check here to be certain - if n.Config == nil || n.Config.Managed == nil { - log.Printf("[WARN] EvalRefreshLifecycle: no Managed config value found in instance state for %q", n.Addr) - return nil - } - - state.CreateBeforeDestroy = n.Config.Managed.CreateBeforeDestroy || n.ForceCreateBeforeDestroy - - return nil -} diff --git a/terraform/eval_state_test.go b/terraform/eval_state_test.go index 718d2051b..3466e1f3d 100644 --- a/terraform/eval_state_test.go +++ b/terraform/eval_state_test.go @@ -8,154 +8,11 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs/configschema" - "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" ) -func TestEvalReadState(t *testing.T) { - var output *states.ResourceInstanceObject - mockProvider := mockProviderWithResourceTypeSchema("aws_instance", &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "id": { - Type: cty.String, - Optional: true, - }, - }, - }) - providerSchema := mockProvider.GetSchemaReturn - provider := providers.Interface(mockProvider) +func TestReadResourceInstanceState(t *testing.T) { - cases := map[string]struct { - State *states.State - Node *EvalReadState - ExpectedInstanceId string - }{ - "ReadState gets primary instance state": { - State: states.BuildState(func(s *states.SyncState) { - providerAddr := addrs.AbsProviderConfig{ - Provider: addrs.NewDefaultProvider("aws"), - Module: addrs.RootModule, - } - oneAddr := addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "aws_instance", - Name: "bar", - }.Absolute(addrs.RootModuleInstance) - s.SetResourceProvider(oneAddr, providerAddr) - s.SetResourceInstanceCurrent(oneAddr.Instance(addrs.NoKey), &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{"id":"i-abc123"}`), - }, providerAddr) - }), - - Node: &EvalReadState{ - Addr: addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "aws_instance", - Name: "bar", - }.Instance(addrs.NoKey), - Provider: &provider, - ProviderSchema: &providerSchema, - - Output: &output, - }, - ExpectedInstanceId: "i-abc123", - }, - } - - for k, c := range cases { - t.Run(k, func(t *testing.T) { - ctx := new(MockEvalContext) - ctx.StateState = c.State.SyncWrapper() - ctx.PathPath = addrs.RootModuleInstance - - diags := c.Node.Eval(ctx) - if diags.HasErrors() { - t.Fatalf("[%s] Got err: %#v", k, diags.ErrWithWarnings()) - } - - expected := c.ExpectedInstanceId - - if !(output != nil && output.Value.GetAttr("id") == cty.StringVal(expected)) { - t.Fatalf("[%s] Expected output with ID %#v, got: %#v", k, expected, output) - } - - output = nil - }) - } -} - -func TestEvalReadStateDeposed(t *testing.T) { - var output *states.ResourceInstanceObject - mockProvider := mockProviderWithResourceTypeSchema("aws_instance", &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "id": { - Type: cty.String, - Optional: true, - }, - }, - }) - providerSchema := mockProvider.GetSchemaReturn - provider := providers.Interface(mockProvider) - - cases := map[string]struct { - State *states.State - Node *EvalReadStateDeposed - ExpectedInstanceId string - }{ - "ReadStateDeposed gets deposed instance": { - State: states.BuildState(func(s *states.SyncState) { - providerAddr := addrs.AbsProviderConfig{ - Provider: addrs.NewDefaultProvider("aws"), - Module: addrs.RootModule, - } - oneAddr := addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "aws_instance", - Name: "bar", - }.Absolute(addrs.RootModuleInstance) - s.SetResourceProvider(oneAddr, providerAddr) - s.SetResourceInstanceDeposed(oneAddr.Instance(addrs.NoKey), states.DeposedKey("00000001"), &states.ResourceInstanceObjectSrc{ - Status: states.ObjectReady, - AttrsJSON: []byte(`{"id":"i-abc123"}`), - }, providerAddr) - }), - - Node: &EvalReadStateDeposed{ - Addr: addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "aws_instance", - Name: "bar", - }.Instance(addrs.NoKey), - Key: states.DeposedKey("00000001"), // shim from legacy state assigns 0th deposed index this key - Provider: &provider, - ProviderSchema: &providerSchema, - - Output: &output, - }, - ExpectedInstanceId: "i-abc123", - }, - } - for k, c := range cases { - t.Run(k, func(t *testing.T) { - ctx := new(MockEvalContext) - ctx.StateState = c.State.SyncWrapper() - ctx.PathPath = addrs.RootModuleInstance - - diags := c.Node.Eval(ctx) - if diags.HasErrors() { - t.Fatalf("[%s] Got err: %#v", k, diags.ErrWithWarnings()) - } - - expected := c.ExpectedInstanceId - - if !(output != nil && output.Value.GetAttr("id") == cty.StringVal(expected)) { - t.Fatalf("[%s] Expected output with ID %#v, got: %#v", k, expected, output) - } - - output = nil - }) - } } func TestEvalWriteState(t *testing.T) { diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index d378e97a7..56e904aaf 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -385,6 +385,51 @@ func (n *NodeAbstractResource) ReadResourceInstanceState(ctx EvalContext, addr a return obj, nil } +// ReadResourceInstanceStateDeposed reads the deposed object for a specific +// instance in the state. +func (n *NodeAbstractResource) ReadResourceInstanceStateDeposed(ctx EvalContext, addr addrs.AbsResourceInstance, key states.DeposedKey) (*states.ResourceInstanceObject, error) { + provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) + if err != nil { + return nil, err + } + + if key == states.NotDeposed { + return nil, fmt.Errorf("EvalReadStateDeposed used with no instance key; this is a bug in Terraform and should be reported") + } + + log.Printf("[TRACE] EvalReadStateDeposed: reading state for %s deposed object %s", addr, key) + + src := ctx.State().ResourceInstanceObject(addr, key) + if src == nil { + // Presumably we only have deposed objects, then. + log.Printf("[TRACE] EvalReadStateDeposed: no state present for %s deposed object %s", addr, key) + return nil, nil + } + + schema, currentVersion := (providerSchema).SchemaForResourceAddr(addr.Resource.ContainingResource()) + if schema == nil { + // Shouldn't happen since we should've failed long ago if no schema is present + return nil, fmt.Errorf("no schema available for %s while reading state; this is a bug in Terraform and should be reported", addr) + + } + + src, diags := UpgradeResourceState(addr, provider, src, schema, currentVersion) + if diags.HasErrors() { + // Note that we don't have any channel to return warnings here. We'll + // accept that for now since warnings during a schema upgrade would + // be pretty weird anyway, since this operation is supposed to seem + // invisible to the user. + return nil, diags.Err() + } + + obj, err := src.Decode(schema.ImpliedType()) + if err != nil { + return nil, err + } + + return obj, nil +} + // graphNodesAreResourceInstancesInDifferentInstancesOfSameModule is an // annoyingly-task-specific helper function that returns true if and only if // the following conditions hold: diff --git a/terraform/node_resource_abstract_instance.go b/terraform/node_resource_abstract_instance.go index 4739679ce..dc1d2d386 100644 --- a/terraform/node_resource_abstract_instance.go +++ b/terraform/node_resource_abstract_instance.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" + "github.com/zclconf/go-cty/cty" ) // NodeAbstractResourceInstance represents a resource instance with no @@ -199,3 +200,47 @@ func (n *NodeAbstractResourceInstance) checkPreventDestroy(change *plans.Resourc return nil } + +// PreApplyHook calls the pre-Apply hook +func (n *NodeAbstractResourceInstance) PreApplyHook(ctx EvalContext, change *plans.ResourceInstanceChange) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + if change == nil { + panic(fmt.Sprintf("PreApplyHook for %s called with nil Change", n.Addr)) + } + + if resourceHasUserVisibleApply(n.Addr.Resource) { + priorState := change.Before + plannedNewState := change.After + + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreApply(n.Addr, nil, change.Action, priorState, plannedNewState) + })) + if diags.HasErrors() { + return diags + } + } + + return nil +} + +// PostApplyHook calls the post-Apply hook +func (n *NodeAbstractResourceInstance) PostApplyHook(ctx EvalContext, state *states.ResourceInstanceObject, err *error) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + if resourceHasUserVisibleApply(n.Addr.Resource) { + var newState cty.Value + if state != nil { + newState = state.Value + } else { + newState = cty.NullVal(cty.DynamicPseudoType) + } + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostApply(n.Addr, nil, newState, *err) + })) + } + + diags = diags.Append(*err) + + return diags +} diff --git a/terraform/node_resource_abstract_test.go b/terraform/node_resource_abstract_test.go index 6faafe1e8..f0655a276 100644 --- a/terraform/node_resource_abstract_test.go +++ b/terraform/node_resource_abstract_test.go @@ -6,6 +6,10 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/configs/configschema" + "github.com/hashicorp/terraform/providers" + "github.com/hashicorp/terraform/states" + "github.com/zclconf/go-cty/cty" ) func TestNodeAbstractResourceProvider(t *testing.T) { @@ -107,3 +111,128 @@ func TestNodeAbstractResourceProvider(t *testing.T) { }) } } + +func TestNodeAbstractResource_ReadResourceInstanceState(t *testing.T) { + mockProvider := mockProviderWithResourceTypeSchema("aws_instance", &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Optional: true, + }, + }, + }) + + tests := map[string]struct { + State *states.State + Node *NodeAbstractResource + ExpectedInstanceId string + }{ + "ReadState gets primary instance state": { + State: states.BuildState(func(s *states.SyncState) { + providerAddr := addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("aws"), + Module: addrs.RootModule, + } + oneAddr := addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "bar", + }.Absolute(addrs.RootModuleInstance) + s.SetResourceProvider(oneAddr, providerAddr) + s.SetResourceInstanceCurrent(oneAddr.Instance(addrs.NoKey), &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"i-abc123"}`), + }, providerAddr) + }), + Node: &NodeAbstractResource{ + Addr: mustConfigResourceAddr("aws_instance.bar"), + ResolvedProvider: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + }, + ExpectedInstanceId: "i-abc123", + }, + } + + for k, test := range tests { + t.Run(k, func(t *testing.T) { + ctx := new(MockEvalContext) + ctx.StateState = test.State.SyncWrapper() + ctx.PathPath = addrs.RootModuleInstance + ctx.ProviderSchemaSchema = mockProvider.GetSchemaReturn + ctx.ProviderProvider = providers.Interface(mockProvider) + + got, err := test.Node.ReadResourceInstanceState(ctx, test.Node.Addr.Resource.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance)) + if err != nil { + t.Fatalf("[%s] Got err: %#v", k, err.Error()) + } + + expected := test.ExpectedInstanceId + + if !(got != nil && got.Value.GetAttr("id") == cty.StringVal(expected)) { + t.Fatalf("[%s] Expected output with ID %#v, got: %#v", k, expected, got) + } + }) + } +} + +func TestNodeAbstractResource_ReadResourceInstanceStateDeposed(t *testing.T) { + mockProvider := mockProviderWithResourceTypeSchema("aws_instance", &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Optional: true, + }, + }, + }) + + tests := map[string]struct { + State *states.State + Node *NodeAbstractResource + ExpectedInstanceId string + }{ + "ReadStateDeposed gets deposed instance": { + State: states.BuildState(func(s *states.SyncState) { + providerAddr := addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("aws"), + Module: addrs.RootModule, + } + oneAddr := addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "bar", + }.Absolute(addrs.RootModuleInstance) + s.SetResourceProvider(oneAddr, providerAddr) + s.SetResourceInstanceDeposed(oneAddr.Instance(addrs.NoKey), states.DeposedKey("00000001"), &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"i-abc123"}`), + }, providerAddr) + }), + Node: &NodeAbstractResource{ + Addr: mustConfigResourceAddr("aws_instance.bar"), + ResolvedProvider: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + }, + ExpectedInstanceId: "i-abc123", + }, + } + for k, test := range tests { + t.Run(k, func(t *testing.T) { + ctx := new(MockEvalContext) + ctx.StateState = test.State.SyncWrapper() + ctx.PathPath = addrs.RootModuleInstance + ctx.ProviderSchemaSchema = mockProvider.GetSchemaReturn + ctx.ProviderProvider = providers.Interface(mockProvider) + + key := states.DeposedKey("00000001") // shim from legacy state assigns 0th deposed index this key + + got, err := test.Node.ReadResourceInstanceStateDeposed(ctx, test.Node.Addr.Resource.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), key) + if err != nil { + t.Fatalf("[%s] Got err: %#v", k, err.Error()) + } + + expected := test.ExpectedInstanceId + + if !(got != nil && got.Value.GetAttr("id") == cty.StringVal(expected)) { + t.Fatalf("[%s] Expected output with ID %#v, got: %#v", k, expected, got) + } + }) + } +} diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index bd6a52896..c4a2ee38d 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "log" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" @@ -234,25 +235,18 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) } if createBeforeDestroyEnabled { - deposeState := &EvalDeposeState{ - Addr: addr, - ForceKey: n.PreallocatedDeposedKey, - OutputKey: &deposedKey, - } - diags = diags.Append(deposeState.Eval(ctx)) - if diags.HasErrors() { - return diags + state := ctx.State() + if n.PreallocatedDeposedKey == states.NotDeposed { + deposedKey = state.DeposeResourceInstanceObject(n.Addr) + } else { + deposedKey = n.PreallocatedDeposedKey + state.DeposeResourceInstanceObjectForceKey(n.Addr, deposedKey) } + log.Printf("[TRACE] managedResourceExecute: prior object for %s now deposed with key %s", n.Addr, deposedKey) } - readState := &EvalReadState{ - Addr: addr, - Provider: &provider, - ProviderSchema: &providerSchema, - - Output: &state, - } - diags = diags.Append(readState.Eval(ctx)) + state, err = n.ReadResourceInstanceState(ctx, n.ResourceInstanceAddr()) + diags = diags.Append(err) if diags.HasErrors() { return diags } @@ -296,14 +290,8 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) return diags } - readState = &EvalReadState{ - Addr: addr, - Provider: &provider, - ProviderSchema: &providerSchema, - - Output: &state, - } - diags = diags.Append(readState.Eval(ctx)) + state, err = n.ReadResourceInstanceState(ctx, n.ResourceInstanceAddr()) + diags = diags.Append(err) if diags.HasErrors() { return diags } @@ -326,12 +314,7 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) return diags } - evalApplyPre := &EvalApplyPre{ - Addr: addr, - State: &state, - Change: &diffApply, - } - diags = diags.Append(evalApplyPre.Eval(ctx)) + diags = diags.Append(n.PreApplyHook(ctx, diffApply)) if diags.HasErrors() { return diags } @@ -428,23 +411,43 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) } if createBeforeDestroyEnabled && applyError != nil { - maybeRestoreDesposedObject := &EvalMaybeRestoreDeposedObject{ - Addr: addr, - PlannedChange: &diffApply, - Key: &deposedKey, - } - diags := diags.Append(maybeRestoreDesposedObject.Eval(ctx)) - if diags.HasErrors() { - return diags + if deposedKey == states.NotDeposed { + // This should never happen, and so it always indicates a bug. + // We should evaluate this node only if we've previously deposed + // an object as part of the same operation. + if diffApply != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Attempt to restore non-existent deposed object", + fmt.Sprintf( + "Terraform has encountered a bug where it would need to restore a deposed object for %s without knowing a deposed object key for that object. This occurred during a %s action. This is a bug in Terraform; please report it!", + addr, diffApply.Action, + ), + )) + } else { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Attempt to restore non-existent deposed object", + fmt.Sprintf( + "Terraform has encountered a bug where it would need to restore a deposed object for %s without knowing a deposed object key for that object. This is a bug in Terraform; please report it!", + addr, + ), + )) + } + } else { + restored := ctx.State().MaybeRestoreResourceInstanceDeposed(addr.Absolute(ctx.Path()), deposedKey) + if restored { + log.Printf("[TRACE] EvalMaybeRestoreDeposedObject: %s deposed object %s was restored as the current object", addr, deposedKey) + } else { + log.Printf("[TRACE] EvalMaybeRestoreDeposedObject: %s deposed object %s remains deposed", addr, deposedKey) + } } } + if diags.HasErrors() { + return diags + } - applyPost := &EvalApplyPost{ - Addr: addr, - State: &state, - Error: &applyError, - } - diags = diags.Append(applyPost.Eval(ctx)) + diags = diags.Append(n.PostApplyHook(ctx, state, &applyError)) if diags.HasErrors() { return diags } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 163aa24da..87a646da4 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -177,12 +177,7 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) return diags } - evalApplyPre := &EvalApplyPre{ - Addr: addr.Resource, - State: &state, - Change: &changeApply, - } - diags = diags.Append(evalApplyPre.Eval(ctx)) + diags = diags.Append(n.PreApplyHook(ctx, changeApply)) if diags.HasErrors() { return diags } @@ -203,12 +198,7 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) if provisionerErr != nil { // If we have a provisioning error, then we just call // the post-apply hook now. - evalApplyPost := &EvalApplyPost{ - Addr: addr.Resource, - State: &state, - Error: &provisionerErr, - } - diags = diags.Append(evalApplyPost.Eval(ctx)) + diags = diags.Append(n.PostApplyHook(ctx, state, &provisionerErr)) if diags.HasErrors() { return diags } @@ -251,12 +241,7 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) state.SetResourceInstanceCurrent(n.Addr, nil, n.ResolvedProvider) } - evalApplyPost := &EvalApplyPost{ - Addr: addr.Resource, - State: &state, - Error: &provisionerErr, - } - diags = diags.Append(evalApplyPost.Eval(ctx)) + diags = diags.Append(n.PostApplyHook(ctx, state, &provisionerErr)) if diags.HasErrors() { return diags } diff --git a/terraform/node_resource_destroy_deposed.go b/terraform/node_resource_destroy_deposed.go index 8fbfc2b9e..63a284566 100644 --- a/terraform/node_resource_destroy_deposed.go +++ b/terraform/node_resource_destroy_deposed.go @@ -67,7 +67,7 @@ func (n *NodePlanDeposedResourceInstanceObject) References() []*addrs.Reference func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { addr := n.ResourceInstanceAddr() - provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) + _, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) diags = diags.Append(err) if diags.HasErrors() { return diags @@ -76,16 +76,10 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk // During the plan walk we always produce a planned destroy change, because // destroying is the only supported action for deposed objects. var change *plans.ResourceInstanceChange - var state *states.ResourceInstanceObject - readStateDeposed := &EvalReadStateDeposed{ - Addr: addr.Resource, - Output: &state, - Key: n.DeposedKey, - Provider: &provider, - ProviderSchema: &providerSchema, - } - diags = diags.Append(readStateDeposed.Eval(ctx)) + // Read the state for the deposed resource instance + state, err := n.ReadResourceInstanceStateDeposed(ctx, n.Addr, n.DeposedKey) + diags = diags.Append(err) if diags.HasErrors() { return diags } @@ -181,8 +175,6 @@ func (n *NodeDestroyDeposedResourceInstanceObject) ModifyCreateBeforeDestroy(v b // GraphNodeExecutable impl. func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { addr := n.ResourceInstanceAddr().Resource - - var state *states.ResourceInstanceObject var change *plans.ResourceInstanceChange var applyError error @@ -192,14 +184,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w return diags } - readStateDeposed := &EvalReadStateDeposed{ - Addr: addr, - Output: &state, - Key: n.DeposedKey, - Provider: &provider, - ProviderSchema: &providerSchema, - } - diags = diags.Append(readStateDeposed.Eval(ctx)) + // Read the state for the deposed resource instance + state, err := n.ReadResourceInstanceStateDeposed(ctx, n.Addr, n.DeposedKey) + diags = diags.Append(err) if diags.HasErrors() { return diags } @@ -216,12 +203,7 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w } // Call pre-apply hook - applyPre := &EvalApplyPre{ - Addr: addr, - State: &state, - Change: &change, - } - diags = diags.Append(applyPre.Eval(ctx)) + diags = diags.Append(n.PreApplyHook(ctx, change)) if diags.HasErrors() { return diags } @@ -257,15 +239,11 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w return diags } - applyPost := &EvalApplyPost{ - Addr: addr, - State: &state, - Error: &applyError, - } - diags = diags.Append(applyPost.Eval(ctx)) + diags = diags.Append(n.PostApplyHook(ctx, state, &applyError)) if diags.HasErrors() { return diags } + if applyError != nil { diags = diags.Append(applyError) return diags diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index e870e3050..abee6cd34 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "log" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" @@ -155,15 +156,15 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) if diags.HasErrors() { return diags } - refreshLifecycle := &EvalRefreshLifecycle{ - Addr: addr, - Config: n.Config, - State: &instanceRefreshState, - ForceCreateBeforeDestroy: n.ForceCreateBeforeDestroy, - } - diags = diags.Append(refreshLifecycle.Eval(ctx)) - if diags.HasErrors() { - return diags + + // In 0.13 we could be refreshing a resource with no config. + // We should be operating on managed resource, but check here to be certain + if n.Config == nil || n.Config.Managed == nil { + log.Printf("[WARN] managedResourceExecute: no Managed config value found in instance state for %q", n.Addr) + } else { + if instanceRefreshState != nil { + instanceRefreshState.CreateBeforeDestroy = n.Config.Managed.CreateBeforeDestroy || n.ForceCreateBeforeDestroy + } } // Refresh, maybe