diff --git a/states/module.go b/states/module.go index b6727b75d..ea5093754 100644 --- a/states/module.go +++ b/states/module.go @@ -173,12 +173,12 @@ func (ms *Module) ForgetResourceInstanceDeposed(addr addrs.ResourceInstance, key // deposeResourceInstanceObject is the real implementation of // SyncState.DeposeResourceInstanceObject. -func (ms *Module) deposeResourceInstanceObject(addr addrs.ResourceInstance) DeposedKey { +func (ms *Module) deposeResourceInstanceObject(addr addrs.ResourceInstance, forceKey DeposedKey) DeposedKey { is := ms.ResourceInstance(addr) if is == nil { return NotDeposed } - return is.deposeCurrentObject() + return is.deposeCurrentObject(forceKey) } // SetOutputValue writes an output value into the state, overwriting any diff --git a/states/resource.go b/states/resource.go index 2f0f48ac1..e2a2b8588 100644 --- a/states/resource.go +++ b/states/resource.go @@ -104,12 +104,19 @@ func (i *ResourceInstance) HasObjects() bool { // SyncState.DeposeResourceInstanceObject. The exported method uses a lock // to ensure that we can safely allocate an unused deposed key without // collision. -func (i *ResourceInstance) deposeCurrentObject() DeposedKey { +func (i *ResourceInstance) deposeCurrentObject(forceKey DeposedKey) DeposedKey { if !i.HasCurrent() { return NotDeposed } - key := i.findUnusedDeposedKey() + key := forceKey + if key == NotDeposed { + key = i.findUnusedDeposedKey() + } else { + if _, exists := i.Deposed[key]; exists { + panic(fmt.Sprintf("forced key %s is already in use", forceKey)) + } + } i.Deposed[key] = i.Current i.Current = nil return key @@ -134,6 +141,17 @@ func (i *ResourceInstance) GetGeneration(gen Generation) *ResourceInstanceObject panic(fmt.Sprintf("get invalid Generation %#v", gen)) } +// FindUnusedDeposedKey generates a unique DeposedKey that is guaranteed not to +// already be in use for this instance at the time of the call. +// +// Note that the validity of this result may change if new deposed keys are +// allocated before it is used. To avoid this risk, instead use the +// DeposeResourceInstanceObject method on the SyncState wrapper type, which +// allocates a key and uses it atomically. +func (i *ResourceInstance) FindUnusedDeposedKey() DeposedKey { + return i.findUnusedDeposedKey() +} + // findUnusedDeposedKey generates a unique DeposedKey that is guaranteed not to // already be in use for this instance. func (i *ResourceInstance) findUnusedDeposedKey() DeposedKey { diff --git a/states/state.go b/states/state.go index 7a8d72987..67dfe891d 100644 --- a/states/state.go +++ b/states/state.go @@ -61,6 +61,9 @@ func (s *State) Empty() bool { // Module returns the state for the module with the given address, or nil if // the requested module is not tracked in the state. func (s *State) Module(addr addrs.ModuleInstance) *Module { + if s == nil { + panic("State.Module on nil *State") + } return s.Modules[addr.String()] } @@ -127,6 +130,9 @@ func (s *State) Resource(addr addrs.AbsResource) *Resource { // ResourceInstance returns the state for the resource instance with the given // address, or nil if no such resource is tracked in the state. func (s *State) ResourceInstance(addr addrs.AbsResourceInstance) *ResourceInstance { + if s == nil { + panic("State.ResourceInstance on nil *State") + } ms := s.Module(addr.Module) if ms == nil { return nil diff --git a/states/sync.go b/states/sync.go index 9f36c42c3..00f2fd317 100644 --- a/states/sync.go +++ b/states/sync.go @@ -323,7 +323,28 @@ func (s *SyncState) DeposeResourceInstanceObject(addr addrs.AbsResourceInstance) return NotDeposed } - return ms.deposeResourceInstanceObject(addr.Resource) + return ms.deposeResourceInstanceObject(addr.Resource, NotDeposed) +} + +// DeposeResourceInstanceObjectForceKey is like DeposeResourceInstanceObject +// but uses a pre-allocated key. It's the caller's responsibility to ensure +// that there aren't any races to use a particular key; this method will panic +// if the given key is already in use. +func (s *SyncState) DeposeResourceInstanceObjectForceKey(addr addrs.AbsResourceInstance, forcedKey DeposedKey) { + s.lock.Lock() + defer s.lock.Unlock() + + if forcedKey == NotDeposed { + // Usage error: should use DeposeResourceInstanceObject in this case + panic("DeposeResourceInstanceObjectForceKey called without forced key") + } + + ms := s.state.Module(addr.Module) + if ms == nil { + return // Nothing to do, since there can't be any current object either. + } + + ms.deposeResourceInstanceObject(addr.Resource, forcedKey) } // ForgetResourceInstanceDeposed removes the record of the deposed object with diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 8bf1e8570..dae07dc68 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -8375,32 +8375,35 @@ func TestContext2Apply_createBefore_depends(t *testing.T) { }) if p, diags := ctx.Plan(); diags.HasErrors() { - t.Fatalf("diags: %s", diags.Err()) + logDiagnostics(t, diags) + t.Fatal("plan failed") } else { - t.Logf("plan: %s", legacyDiffComparisonString(p.Changes)) + t.Logf("plan:\n%s", legacyDiffComparisonString(p.Changes)) } h.Active = true state, diags := ctx.Apply() if diags.HasErrors() { - t.Fatalf("diags: %s", diags.Err()) + logDiagnostics(t, diags) + t.Fatal("apply failed") } mod := state.RootModule() if len(mod.Resources) < 2 { - t.Fatalf("bad: %#v", mod.Resources) + t.Logf("state after apply:\n%s", state.String()) + t.Fatalf("only %d resources in root module; want at least 2", len(mod.Resources)) } - actual := strings.TrimSpace(state.String()) - expected := strings.TrimSpace(testTerraformApplyDependsCreateBeforeStr) - if actual != expected { - t.Fatalf("bad: \n%s\n\n%s", actual, expected) + got := strings.TrimSpace(state.String()) + want := strings.TrimSpace(testTerraformApplyDependsCreateBeforeStr) + if got != want { + t.Fatalf("wrong final state\ngot:\n%s\n\nwant:\n%s", got, want) } // Test that things were managed _in the right order_ order := h.States diffs := h.Diffs - if order[0].GetAttr("id").AsString() != "" || diffs[0].Action == plans.Delete { + if !order[0].IsNull() || diffs[0].Action == plans.Delete { t.Fatalf("should create new instance first: %#v", order) } diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 756e0e5da..4be930bf8 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -224,6 +224,10 @@ func (n *EvalApplyPre) Eval(ctx EvalContext) (interface{}, error) { 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 diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 4d2f70cfd..1573f3d38 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -310,6 +310,12 @@ func (n *EvalWriteStateDeposed) Eval(ctx EvalContext) (interface{}, error) { type EvalDeposeState struct { Addr addrs.ResourceInstance + // ForceKey, if a value other than states.NotDeposed, will be used as the + // key for the newly-created deposed object that results from this action. + // If set to states.NotDeposed (the zero value), a new unique key will be + // allocated. + ForceKey states.DeposedKey + // OutputKey, if non-nil, will be written with the deposed object key that // was generated for the object. This can then be passed to // EvalUndeposeState.Key so it knows which deposed instance to forget. @@ -321,7 +327,13 @@ func (n *EvalDeposeState) Eval(ctx EvalContext) (interface{}, error) { absAddr := n.Addr.Absolute(ctx.Path()) state := ctx.State() - key := state.DeposeResourceInstanceObject(absAddr) + 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 { diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index ede1ec3e2..0db65fe62 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -95,6 +95,8 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // ConfigTransformer above. &DiffTransformer{ Concrete: concreteResourceInstance, + Config: b.Config, + State: b.State, Changes: b.Changes, }, diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index 78f914bb9..413be7213 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -1,12 +1,13 @@ package terraform import ( + "fmt" "strings" "testing" - "github.com/hashicorp/terraform/plans" - "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/states" ) func TestApplyGraphBuilder_impl(t *testing.T) { @@ -105,20 +106,42 @@ func TestApplyGraphBuilder_depCbd(t *testing.T) { t.Fatalf("wrong path %q", g.Path.String()) } + // We're going to go hunting for our deposed instance node here, so we + // can find out its key to use in the assertions below. + var dk states.DeposedKey + for _, v := range g.Vertices() { + tv, ok := v.(*NodeDestroyDeposedResourceInstanceObject) + if !ok { + continue + } + if dk != states.NotDeposed { + t.Fatalf("more than one deposed instance node in the graph; want only one") + } + dk = tv.DeposedKey + } + if dk == states.NotDeposed { + t.Fatalf("no deposed instance node in the graph; want one") + } + + destroyName := fmt.Sprintf("test_object.A (deposed %s)", dk) + // Create A, Modify B, Destroy A testGraphHappensBefore( t, g, "test_object.A", - "test_object.A (destroy)") + destroyName, + ) testGraphHappensBefore( t, g, "test_object.A", - "test_object.B") + "test_object.B", + ) testGraphHappensBefore( t, g, "test_object.B", - "test_object.A (destroy)") + destroyName, + ) } // This tests the ordering of two resources that are both CBD that diff --git a/terraform/graph_builder_destroy_plan.go b/terraform/graph_builder_destroy_plan.go index 464e4d2fe..e86af161a 100644 --- a/terraform/graph_builder_destroy_plan.go +++ b/terraform/graph_builder_destroy_plan.go @@ -51,6 +51,12 @@ func (b *DestroyPlanGraphBuilder) Steps() []GraphTransformer { NodeAbstractResourceInstance: a, } } + concreteResourceInstanceDeposed := func(a *NodeAbstractResourceInstance, key states.DeposedKey) dag.Vertex { + return &NodePlanDeposedResourceInstanceObject{ + NodeAbstractResourceInstance: a, + DeposedKey: key, + } + } concreteProvider := func(a *NodeAbstractProvider) dag.Vertex { return &NodeApplyableProvider{ @@ -61,8 +67,9 @@ func (b *DestroyPlanGraphBuilder) Steps() []GraphTransformer { steps := []GraphTransformer{ // Creates nodes for the resource instances tracked in the state. &StateTransformer{ - Concrete: concreteResourceInstance, - State: b.State, + ConcreteCurrent: concreteResourceInstance, + ConcreteDeposed: concreteResourceInstanceDeposed, + State: b.State, }, // Attach the configuration to any resources diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index 881fa5677..7b9cda1c4 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -69,6 +69,13 @@ func (b *PlanGraphBuilder) Build(path addrs.ModuleInstance) (*Graph, tfdiags.Dia func (b *PlanGraphBuilder) Steps() []GraphTransformer { b.once.Do(b.init) + concreteResourceInstanceDeposed := func(a *NodeAbstractResourceInstance, key states.DeposedKey) dag.Vertex { + return &NodePlanDeposedResourceInstanceObject{ + NodeAbstractResourceInstance: a, + DeposedKey: key, + } + } + steps := []GraphTransformer{ // Creates all the resources represented in the config &ConfigTransformer{ @@ -89,6 +96,15 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { Config: b.Config, }, + // We also need nodes for any deposed instance objects present in the + // state, so we can plan to destroy them. (This intentionally + // skips creating nodes for _current_ objects, since ConfigTransformer + // created nodes that will do that during DynamicExpand.) + &StateTransformer{ + ConcreteDeposed: concreteResourceInstanceDeposed, + State: b.State, + }, + // Create orphan output nodes &OrphanOutputTransformer{ Config: b.Config, diff --git a/terraform/graph_builder_refresh.go b/terraform/graph_builder_refresh.go index 84975314f..a3b3de4f8 100644 --- a/terraform/graph_builder_refresh.go +++ b/terraform/graph_builder_refresh.go @@ -78,6 +78,14 @@ func (b *RefreshGraphBuilder) Steps() []GraphTransformer { } } + concreteResourceInstanceDeposed := func(a *NodeAbstractResourceInstance, key states.DeposedKey) dag.Vertex { + // The "Plan" node type also handles refreshing behavior. + return &NodePlanDeposedResourceInstanceObject{ + NodeAbstractResourceInstance: a, + DeposedKey: key, + } + } + concreteDataResource := func(a *NodeAbstractResource) dag.Vertex { return &NodeRefreshableDataResource{ NodeAbstractResource: a, @@ -121,6 +129,15 @@ func (b *RefreshGraphBuilder) Steps() []GraphTransformer { Config: b.Config, }, + // We also need nodes for any deposed instance objects present in the + // state, so we can check if they still exist. (This intentionally + // skips creating nodes for _current_ objects, since ConfigTransformer + // created nodes that will do that during DynamicExpand.) + &StateTransformer{ + ConcreteDeposed: concreteResourceInstanceDeposed, + State: b.State, + }, + // Attach the state &AttachStateTransformer{State: b.State}, diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index 402594fb6..7b4123c23 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -23,6 +23,7 @@ type NodeApplyableResourceInstance struct { *NodeAbstractResourceInstance destroyNode GraphNodeDestroyerCBD + graphNodeDeposer // implementation of GraphNodeDeposer } var ( @@ -30,6 +31,7 @@ var ( _ GraphNodeResourceInstance = (*NodeApplyableResourceInstance)(nil) _ GraphNodeCreator = (*NodeApplyableResourceInstance)(nil) _ GraphNodeReferencer = (*NodeApplyableResourceInstance)(nil) + _ GraphNodeDeposer = (*NodeApplyableResourceInstance)(nil) _ GraphNodeEvalable = (*NodeApplyableResourceInstance)(nil) ) @@ -248,6 +250,7 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe }, Then: &EvalDeposeState{ Addr: addr.Resource, + ForceKey: n.PreallocatedDeposedKey, OutputKey: &deposedKey, }, }, diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index fec49da44..ede2ba217 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -37,6 +37,9 @@ var ( ) func (n *NodeDestroyResourceInstance) Name() string { + if n.DeposedKey != states.NotDeposed { + return fmt.Sprintf("%s (destroy deposed %s)", n.ResourceInstanceAddr(), n.DeposedKey) + } return n.ResourceInstanceAddr().String() + " (destroy)" } @@ -115,44 +118,6 @@ func (n *NodeDestroyResourceInstance) References() []*addrs.Reference { return nil } -// GraphNodeDynamicExpandable -func (n *NodeDestroyResourceInstance) DynamicExpand(ctx EvalContext) (*Graph, error) { - if n.DeposedKey != states.NotDeposed { - return nil, fmt.Errorf("NodeDestroyResourceInstance not yet updated to deal with explicit DeposedKey") - } - - // Our graph transformers require direct access to read the entire state - // structure, so we'll lock the whole state for the duration of this work. - state := ctx.State().Lock() - defer ctx.State().Unlock() - - // Start creating the steps - steps := make([]GraphTransformer, 0, 5) - - // We want deposed resources in the state to be destroyed - steps = append(steps, &DeposedTransformer{ - State: state, - InstanceAddr: n.ResourceInstanceAddr(), - ResolvedProvider: n.ResolvedProvider, - }) - - // Target - steps = append(steps, &TargetsTransformer{ - Targets: n.Targets, - }) - - // Always end with the root being added - steps = append(steps, &RootTransformer{}) - - // Build the graph - b := &BasicGraphBuilder{ - Steps: steps, - Name: "NodeResourceDestroy", - } - g, diags := b.Build(ctx.Path()) - return g, diags.ErrWithWarnings() -} - // GraphNodeEvalable func (n *NodeDestroyResourceInstance) EvalTree() EvalNode { addr := n.ResourceInstanceAddr() diff --git a/terraform/node_resource_destroy_deposed.go b/terraform/node_resource_destroy_deposed.go new file mode 100644 index 000000000..c85657975 --- /dev/null +++ b/terraform/node_resource_destroy_deposed.go @@ -0,0 +1,297 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/providers" + "github.com/hashicorp/terraform/states" +) + +// ConcreteResourceInstanceDeposedNodeFunc is a callback type used to convert +// an abstract resource instance to a concrete one of some type that has +// an associated deposed object key. +type ConcreteResourceInstanceDeposedNodeFunc func(*NodeAbstractResourceInstance, states.DeposedKey) dag.Vertex + +// NodePlanDeposedResourceInstanceObject represents deposed resource +// instance objects during plan. These are distinct from the primary object +// for each resource instance since the only valid operation to do with them +// is to destroy them. +// +// This node type is also used during the refresh walk to ensure that the +// record of a deposed object is up-to-date before we plan to destroy it. +type NodePlanDeposedResourceInstanceObject struct { + *NodeAbstractResourceInstance + DeposedKey states.DeposedKey +} + +var ( + _ GraphNodeResource = (*NodePlanDeposedResourceInstanceObject)(nil) + _ GraphNodeResourceInstance = (*NodePlanDeposedResourceInstanceObject)(nil) + _ GraphNodeReferenceable = (*NodePlanDeposedResourceInstanceObject)(nil) + _ GraphNodeReferencer = (*NodePlanDeposedResourceInstanceObject)(nil) + _ GraphNodeEvalable = (*NodePlanDeposedResourceInstanceObject)(nil) + _ GraphNodeProviderConsumer = (*NodePlanDeposedResourceInstanceObject)(nil) + _ GraphNodeProvisionerConsumer = (*NodePlanDeposedResourceInstanceObject)(nil) +) + +func (n *NodePlanDeposedResourceInstanceObject) Name() string { + return fmt.Sprintf("%s (deposed %s)", n.Addr.String(), n.DeposedKey) +} + +// GraphNodeReferenceable implementation, overriding the one from NodeAbstractResourceInstance +func (n *NodePlanDeposedResourceInstanceObject) ReferenceableAddrs() []addrs.Referenceable { + // Deposed objects don't participate in references. + return nil +} + +// GraphNodeReferencer implementation, overriding the one from NodeAbstractResourceInstance +func (n *NodePlanDeposedResourceInstanceObject) References() []*addrs.Reference { + // We don't evaluate configuration for deposed objects, so they effectively + // make no references. + return nil +} + +// GraphNodeEvalable impl. +func (n *NodePlanDeposedResourceInstanceObject) EvalTree() EvalNode { + addr := n.ResourceInstanceAddr() + + var provider providers.Interface + var providerSchema *ProviderSchema + var state *states.ResourceInstanceObject + + seq := &EvalSequence{Nodes: make([]EvalNode, 0, 5)} + + // During the refresh walk we will ensure that our record of the deposed + // object is up-to-date. If it was already deleted outside of Terraform + // then this will remove it from state and thus avoid us planning a + // destroy for it during the subsequent plan walk. + seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + Ops: []walkOperation{walkRefresh}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + &EvalGetProvider{ + Addr: n.ResolvedProvider, + Output: &provider, + Schema: &providerSchema, + }, + &EvalReadStateDeposed{ + Addr: addr.Resource, + ProviderSchema: &providerSchema, + Key: n.DeposedKey, + Output: &state, + }, + &EvalRefresh{ + Addr: addr.Resource, + ProviderAddr: n.ResolvedProvider, + Provider: &provider, + ProviderSchema: &providerSchema, + State: &state, + Output: &state, + }, + &EvalWriteStateDeposed{ + Addr: addr.Resource, + Key: n.DeposedKey, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + State: &state, + }, + }, + }, + }) + + // 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 + seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + Ops: []walkOperation{walkPlan, walkPlanDestroy}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + &EvalGetProvider{ + Addr: n.ResolvedProvider, + Output: &provider, + Schema: &providerSchema, + }, + &EvalReadStateDeposed{ + Addr: addr.Resource, + Output: &state, + Key: n.DeposedKey, + Provider: &provider, + ProviderSchema: &providerSchema, + }, + &EvalDiffDestroy{ + Addr: addr.Resource, + DeposedKey: n.DeposedKey, + State: &state, + Output: &change, + }, + &EvalWriteDiff{ + Addr: addr.Resource, + DeposedKey: n.DeposedKey, + ProviderSchema: &providerSchema, + Change: &change, + }, + // Since deposed objects cannot be referenced by expressions + // elsewhere, we don't need to also record the planned new + // state in this case. + }, + }, + }) + + return seq +} + + +// NodeDestroyDeposedResourceInstanceObject represents deposed resource +// instance objects during apply. Nodes of this type are inserted by +// DiffTransformer when the planned changeset contains "delete" changes for +// deposed instance objects, and its only supported operation is to destroy +// and then forget the associated object. +type NodeDestroyDeposedResourceInstanceObject struct { + *NodeAbstractResourceInstance + DeposedKey states.DeposedKey +} + +var ( + _ GraphNodeResource = (*NodeDestroyDeposedResourceInstanceObject)(nil) + _ GraphNodeResourceInstance = (*NodeDestroyDeposedResourceInstanceObject)(nil) + _ GraphNodeDestroyer = (*NodeDestroyDeposedResourceInstanceObject)(nil) + _ GraphNodeDestroyerCBD = (*NodeDestroyDeposedResourceInstanceObject)(nil) + _ GraphNodeReferenceable = (*NodeDestroyDeposedResourceInstanceObject)(nil) + _ GraphNodeReferencer = (*NodeDestroyDeposedResourceInstanceObject)(nil) + _ GraphNodeEvalable = (*NodeDestroyDeposedResourceInstanceObject)(nil) + _ GraphNodeProviderConsumer = (*NodeDestroyDeposedResourceInstanceObject)(nil) + _ GraphNodeProvisionerConsumer = (*NodeDestroyDeposedResourceInstanceObject)(nil) +) + +func (n *NodeDestroyDeposedResourceInstanceObject) Name() string { + return fmt.Sprintf("%s (destroy deposed %s)", n.Addr.String(), n.DeposedKey) +} + +// GraphNodeReferenceable implementation, overriding the one from NodeAbstractResourceInstance +func (n *NodeDestroyDeposedResourceInstanceObject) ReferenceableAddrs() []addrs.Referenceable { + // Deposed objects don't participate in references. + return nil +} + +// GraphNodeReferencer implementation, overriding the one from NodeAbstractResourceInstance +func (n *NodeDestroyDeposedResourceInstanceObject) References() []*addrs.Reference { + // We don't evaluate configuration for deposed objects, so they effectively + // make no references. + return nil +} + +// GraphNodeDestroyer +func (n *NodeDestroyDeposedResourceInstanceObject) DestroyAddr() *addrs.AbsResourceInstance { + addr := n.ResourceInstanceAddr() + return &addr +} + +// GraphNodeDestroyerCBD +func (n *NodeDestroyDeposedResourceInstanceObject) CreateBeforeDestroy() bool { + // A deposed instance is always CreateBeforeDestroy by definition, since + // we use deposed only to handle create-before-destroy. + return true +} + +// GraphNodeDestroyerCBD +func (n *NodeDestroyDeposedResourceInstanceObject) ModifyCreateBeforeDestroy(v bool) error { + if !v { + // Should never happen: deposed instances are _always_ create_before_destroy. + return fmt.Errorf("can't deactivate create_before_destroy for a deposed instance") + } + return nil +} + +// GraphNodeEvalable impl. +func (n *NodeDestroyDeposedResourceInstanceObject) EvalTree() EvalNode { + addr := n.ResourceInstanceAddr() + + var provider providers.Interface + var providerSchema *ProviderSchema + var state *states.ResourceInstanceObject + var change *plans.ResourceInstanceChange + var err error + + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalGetProvider{ + Addr: n.ResolvedProvider, + Output: &provider, + Schema: &providerSchema, + }, + &EvalReadStateDeposed{ + Addr: addr.Resource, + Output: &state, + Key: n.DeposedKey, + Provider: &provider, + ProviderSchema: &providerSchema, + }, + &EvalDiffDestroy{ + Addr: addr.Resource, + State: &state, + Output: &change, + }, + // Call pre-apply hook + &EvalApplyPre{ + Addr: addr.Resource, + State: &state, + Change: &change, + }, + &EvalApply{ + Addr: addr.Resource, + Config: nil, // No configuration because we are destroying + State: &state, + Change: &change, + Provider: &provider, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + Output: &state, + Error: &err, + }, + // Always write the resource back to the state deposed... if it + // was successfully destroyed it will be pruned. If it was not, it will + // be caught on the next run. + &EvalWriteStateDeposed{ + Addr: addr.Resource, + Key: n.DeposedKey, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + State: &state, + }, + &EvalApplyPost{ + Addr: addr.Resource, + State: &state, + Error: &err, + }, + &EvalReturnError{ + Error: &err, + }, + &EvalUpdateStateHook{}, + }, + } +} + +// GraphNodeDeposer is an optional interface implemented by graph nodes that +// might create a single new deposed object for a specific associated resource +// instance, allowing a caller to optionally pre-allocate a DeposedKey for +// it. +type GraphNodeDeposer interface { + // SetPreallocatedDeposedKey will be called during graph construction + // if a particular node must use a pre-allocated deposed key if/when it + // "deposes" the current object of its associated resource instance. + SetPreallocatedDeposedKey(key states.DeposedKey) +} + +// graphNodeDeposer is an embeddable implementation of GraphNodeDeposer. +// Embed it in a node type to get automatic support for it, and then access +// the field PreallocatedDeposedKey to access any pre-allocated key. +type graphNodeDeposer struct { + PreallocatedDeposedKey states.DeposedKey +} + +func (n *graphNodeDeposer) SetPreallocatedDeposedKey(key states.DeposedKey) { + n.PreallocatedDeposedKey = key +} diff --git a/terraform/node_resource_destroy_test.go b/terraform/node_resource_destroy_test.go deleted file mode 100644 index 25b363d62..000000000 --- a/terraform/node_resource_destroy_test.go +++ /dev/null @@ -1,68 +0,0 @@ -package terraform - -import ( - "strings" - "testing" - - "github.com/hashicorp/terraform/addrs" -) - -func TestNodeDestroyResourceDynamicExpand_deposedCount(t *testing.T) { - state := mustShimLegacyState(&State{ - Modules: []*ModuleState{ - &ModuleState{ - Path: rootModulePath, - Resources: map[string]*ResourceState{ - "aws_instance.bar.0": &ResourceState{ - Type: "aws_instance", - Deposed: []*InstanceState{ - &InstanceState{ - ID: "foo", - }, - }, - Provider: "provider.aws", - }, - "aws_instance.bar.1": &ResourceState{ - Type: "aws_instance", - Deposed: []*InstanceState{ - &InstanceState{ - ID: "bar", - }, - }, - Provider: "provider.aws", - }, - }, - }, - }, - }) - - m := testModule(t, "apply-cbd-count") - n := &NodeDestroyResourceInstance{ - NodeAbstractResourceInstance: &NodeAbstractResourceInstance{ - NodeAbstractResource: NodeAbstractResource{ - Addr: addrs.RootModuleInstance.Resource( - addrs.ManagedResourceMode, "aws_instance", "bar", - ), - Config: m.Module.ManagedResources["aws_instance.bar"], - }, - InstanceKey: addrs.IntKey(0), - ResourceState: state.Modules[""].Resources["aws_instance.bar[0]"], - }, - } - - g, err := n.DynamicExpand(&MockEvalContext{ - PathPath: addrs.RootModuleInstance, - StateState: state.SyncWrapper(), - }) - if err != nil { - t.Fatalf("err: %s", err) - } - - got := strings.TrimSpace(g.String()) - want := strings.TrimSpace(` -aws_instance.bar[0] (deposed 00000001) -`) - if got != want { - t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", got, want) - } -} diff --git a/terraform/test-fixtures/apply-depends-create-before/main.tf b/terraform/test-fixtures/apply-depends-create-before/main.tf index 89436a000..63478d893 100644 --- a/terraform/test-fixtures/apply-depends-create-before/main.tf +++ b/terraform/test-fixtures/apply-depends-create-before/main.tf @@ -1,10 +1,10 @@ resource "aws_instance" "web" { - require_new = "ami-new" - lifecycle { - create_before_destroy = true - } + require_new = "ami-new" + lifecycle { + create_before_destroy = true + } } resource "aws_instance" "lb" { - instance = "${aws_instance.web.id}" + instance = aws_instance.web.id } diff --git a/terraform/transform_deposed.go b/terraform/transform_deposed.go deleted file mode 100644 index 681e4d8eb..000000000 --- a/terraform/transform_deposed.go +++ /dev/null @@ -1,190 +0,0 @@ -package terraform - -import ( - "fmt" - - "github.com/hashicorp/terraform/addrs" - "github.com/hashicorp/terraform/plans" - "github.com/hashicorp/terraform/providers" - "github.com/hashicorp/terraform/states" -) - -// DeposedTransformer is a GraphTransformer that adds nodes to the graph for -// the deposed objects associated with a given resource instance. -type DeposedTransformer struct { - // State is the global state, from which we'll retrieve the state for - // the instance given in InstanceAddr. - State *states.State - - // InstanceAddr is the address of the instance whose deposed objects will - // have graph nodes created. - InstanceAddr addrs.AbsResourceInstance - - // The provider used by the resourced which were deposed - ResolvedProvider addrs.AbsProviderConfig -} - -func (t *DeposedTransformer) Transform(g *Graph) error { - rs := t.State.Resource(t.InstanceAddr.ContainingResource()) - if rs == nil { - // If the resource has no state then there can't be deposed objects. - return nil - } - is := rs.Instances[t.InstanceAddr.Resource.Key] - if is == nil { - // If the instance has no state then there can't be deposed objects. - return nil - } - - providerAddr := rs.ProviderConfig - - for k := range is.Deposed { - g.Add(&graphNodeDeposedResource{ - Addr: t.InstanceAddr, - DeposedKey: k, - RecordedProvider: providerAddr, - ResolvedProvider: t.ResolvedProvider, - }) - } - - return nil -} - -// graphNodeDeposedResource is the graph vertex representing a deposed resource. -type graphNodeDeposedResource struct { - Addr addrs.AbsResourceInstance - DeposedKey states.DeposedKey - RecordedProvider addrs.AbsProviderConfig - ResolvedProvider addrs.AbsProviderConfig -} - -var ( - _ GraphNodeProviderConsumer = (*graphNodeDeposedResource)(nil) - _ GraphNodeEvalable = (*graphNodeDeposedResource)(nil) -) - -func (n *graphNodeDeposedResource) Name() string { - return fmt.Sprintf("%s (deposed %s)", n.Addr.String(), n.DeposedKey) -} - -func (n *graphNodeDeposedResource) ProvidedBy() (addrs.AbsProviderConfig, bool) { - return n.RecordedProvider, true -} - -func (n *graphNodeDeposedResource) SetProvider(addr addrs.AbsProviderConfig) { - // Because our ProvidedBy returns exact=true, this is actually rather - // pointless and should always just be the address we asked for. - n.RecordedProvider = addr -} - -// GraphNodeEvalable impl. -func (n *graphNodeDeposedResource) EvalTree() EvalNode { - addr := n.Addr - - var provider providers.Interface - var providerSchema *ProviderSchema - var state *states.ResourceInstanceObject - - seq := &EvalSequence{Nodes: make([]EvalNode, 0, 5)} - - // Refresh the resource - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ - Ops: []walkOperation{walkRefresh}, - Node: &EvalSequence{ - Nodes: []EvalNode{ - &EvalGetProvider{ - Addr: n.ResolvedProvider, - Output: &provider, - Schema: &providerSchema, - }, - &EvalReadStateDeposed{ - Addr: addr.Resource, - ProviderSchema: &providerSchema, - Key: n.DeposedKey, - Output: &state, - }, - &EvalRefresh{ - Addr: addr.Resource, - ProviderAddr: n.ResolvedProvider, - Provider: &provider, - ProviderSchema: &providerSchema, - State: &state, - Output: &state, - }, - &EvalWriteStateDeposed{ - Addr: addr.Resource, - Key: n.DeposedKey, - ProviderAddr: n.ResolvedProvider, - ProviderSchema: &providerSchema, - State: &state, - }, - }, - }, - }) - - // Apply - var change *plans.ResourceInstanceChange - var err error - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ - Ops: []walkOperation{walkApply, walkDestroy}, - Node: &EvalSequence{ - Nodes: []EvalNode{ - &EvalGetProvider{ - Addr: n.ResolvedProvider, - Output: &provider, - Schema: &providerSchema, - }, - &EvalReadStateDeposed{ - Addr: addr.Resource, - Output: &state, - Key: n.DeposedKey, - Provider: &provider, - ProviderSchema: &providerSchema, - }, - &EvalDiffDestroy{ - Addr: addr.Resource, - State: &state, - Output: &change, - }, - // Call pre-apply hook - &EvalApplyPre{ - Addr: addr.Resource, - State: &state, - Change: &change, - }, - &EvalApply{ - Addr: addr.Resource, - Config: nil, // No configuration because we are destroying - State: &state, - Change: &change, - Provider: &provider, - ProviderAddr: n.ResolvedProvider, - ProviderSchema: &providerSchema, - Output: &state, - Error: &err, - }, - // Always write the resource back to the state deposed... if it - // was successfully destroyed it will be pruned. If it was not, it will - // be caught on the next run. - &EvalWriteStateDeposed{ - Addr: addr.Resource, - Key: n.DeposedKey, - ProviderAddr: n.ResolvedProvider, - ProviderSchema: &providerSchema, - State: &state, - }, - &EvalApplyPost{ - Addr: addr.Resource, - State: &state, - Error: &err, - }, - &EvalReturnError{ - Error: &err, - }, - &EvalUpdateStateHook{}, - }, - }, - }) - - return seq -} diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index 4daca54cf..1d991770a 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -4,15 +4,19 @@ import ( "fmt" "log" + "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/tfdiags" ) // DiffTransformer is a GraphTransformer that adds graph nodes representing // each of the resource changes described in the given Changes object. type DiffTransformer struct { Concrete ConcreteResourceInstanceNodeFunc + Config *configs.Config + State *states.State Changes *plans.Changes } @@ -25,6 +29,11 @@ func (t *DiffTransformer) Transform(g *Graph) error { // Go through all the modules in the diff. log.Printf("[TRACE] DiffTransformer starting") + var diags tfdiags.Diagnostics + config := t.Config + state := t.State + changes := t.Changes + // DiffTransformer creates resource _instance_ nodes. If there are any // whole-resource nodes already in the graph, we must ensure that they // get evaluated before any of the corresponding instances by creating @@ -47,14 +56,22 @@ func (t *DiffTransformer) Transform(g *Graph) error { resourceNodes[addr] = append(resourceNodes[addr], rn) } - for _, rc := range t.Changes.Resources { + for _, rc := range changes.Resources { addr := rc.Addr dk := rc.DeposedKey + var rCfg *configs.Resource + + log.Printf("[TRACE] DiffTransformer: found %s change for %s %s", rc.Action, addr, dk) + + modCfg := config.DescendentForInstance(addr.Module) + if modCfg != nil { + rCfg = modCfg.Module.ResourceByAddr(addr.Resource.Resource) + } // Depending on the action we'll need some different combinations of // nodes, because destroying uses a special node type separate from // other actions. - var update, delete bool + var update, delete, createBeforeDestroy bool switch rc.Action { case plans.NoOp: continue @@ -67,6 +84,56 @@ func (t *DiffTransformer) Transform(g *Graph) error { update = true } + if dk != states.NotDeposed && update { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid planned change for deposed object", + fmt.Sprintf("The plan contains a non-delete change for %s deposed object %s. The only valid action for a deposed object is to destroy it, so this is a bug in Terraform.", addr, dk), + )) + continue + } + + if rCfg != nil && rCfg.Managed != nil && rCfg.Managed.CreateBeforeDestroy { + createBeforeDestroy = true + } + + // If we're going to do a create_before_destroy Replace operation then + // we need to allocate a DeposedKey to use to retain the + // not-yet-destroyed prior object, so that the delete node can destroy + // _that_ rather than the newly-created node, which will be current + // by the time the delete node is visited. + if update && delete && createBeforeDestroy { + // In this case, variable dk will be the _pre-assigned_ DeposedKey + // that must be used if the update graph node deposes the current + // instance, which will then align with the same key we pass + // into the destroy node to ensure we destroy exactly the deposed + // object we expect. + if state != nil { + ris := state.ResourceInstance(addr) + if ris == nil { + // Should never happen, since we don't plan to replace an + // instance that doesn't exist yet. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid planned change", + fmt.Sprintf("The plan contains a replace change for %s, which doesn't exist yet. This is a bug in Terraform.", addr), + )) + continue + } + + // Allocating a deposed key separately from using it can be racy + // in general, but we assume here that nothing except the apply + // node we instantiate below will actually make new deposed objects + // in practice, and so the set of already-used keys will not change + // between now and then. + dk = ris.FindUnusedDeposedKey() + } else { + // If we have no state at all yet then we can use _any_ + // DeposedKey. + dk = states.NewDeposedKey() + } + } + if update { // All actions except destroying the node type chosen by t.Concrete abstract := NewNodeAbstractResourceInstance(addr) @@ -75,14 +142,19 @@ func (t *DiffTransformer) Transform(g *Graph) error { node = f(abstract) } - if dk != states.NotDeposed { - // The only valid action for deposed objects is to destroy them. - // Entering this branch suggests a bug in the plan phase that - // proposed this change. - return fmt.Errorf("invalid %s action for deposed object on %s: only Delete is allowed", rc.Action, addr) + if createBeforeDestroy { + // We'll attach our pre-allocated DeposedKey to the node if + // it supports that. NodeApplyableResourceInstance is the + // specific concrete node type we are looking for here really, + // since that's the only node type that might depose objects. + if dn, ok := node.(GraphNodeDeposer); ok { + dn.SetPreallocatedDeposedKey(dk) + } + log.Printf("[TRACE] DiffTransformer: %s will be represented by %s, deposing prior object to %s", addr, dag.VertexName(node), dk) + } else { + log.Printf("[TRACE] DiffTransformer: %s will be represented by %s", addr, dag.VertexName(node)) } - log.Printf("[TRACE] DiffTransformer: %s will be represented by %s", addr, dag.VertexName(node)) g.Add(node) rsrcAddr := addr.ContainingResource().String() for _, rsrcNode := range resourceNodes[rsrcAddr] { @@ -91,11 +163,21 @@ func (t *DiffTransformer) Transform(g *Graph) error { } if delete { - // Destroying always uses this destroy-specific node type. + // Destroying always uses a destroy-specific node type, though + // which one depends on whether we're destroying a current object + // or a deposed object. + var node GraphNodeResourceInstance abstract := NewNodeAbstractResourceInstance(addr) - node := &NodeDestroyResourceInstance{ - NodeAbstractResourceInstance: abstract, - DeposedKey: dk, + if dk == states.NotDeposed { + node = &NodeDestroyResourceInstance{ + NodeAbstractResourceInstance: abstract, + DeposedKey: dk, + } + } else { + node = &NodeDestroyDeposedResourceInstanceObject{ + NodeAbstractResourceInstance: abstract, + DeposedKey: dk, + } } if dk == states.NotDeposed { log.Printf("[TRACE] DiffTransformer: %s will be represented for destruction by %s", addr, dag.VertexName(node)) @@ -117,5 +199,5 @@ func (t *DiffTransformer) Transform(g *Graph) error { log.Printf("[TRACE] DiffTransformer complete") - return nil + return diags.Err() } diff --git a/terraform/transform_provisioner_test.go b/terraform/transform_provisioner_test.go index 1c2c4f772..dd2714f33 100644 --- a/terraform/transform_provisioner_test.go +++ b/terraform/transform_provisioner_test.go @@ -93,8 +93,8 @@ func TestMissingProvisionerTransformer_module(t *testing.T) { }) tf := &StateTransformer{ - Concrete: concreteResource, - State: state, + ConcreteCurrent: concreteResource, + State: state, } if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) diff --git a/terraform/transform_state.go b/terraform/transform_state.go index bd6f0d163..0b52347df 100644 --- a/terraform/transform_state.go +++ b/terraform/transform_state.go @@ -3,7 +3,6 @@ package terraform import ( "log" - "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/states" ) @@ -13,7 +12,15 @@ import ( // This transform is used for example by the DestroyPlanGraphBuilder to ensure // that only resources that are in the state are represented in the graph. type StateTransformer struct { - Concrete ConcreteResourceInstanceNodeFunc + // ConcreteCurrent and ConcreteDeposed are used to specialize the abstract + // resource instance nodes that this transformer will create. + // + // If either of these is nil, the objects of that type will be skipped and + // not added to the graph at all. It doesn't make sense to use this + // transformer without setting at least one of these, since that would + // skip everything and thus be a no-op. + ConcreteCurrent ConcreteResourceInstanceNodeFunc + ConcreteDeposed ConcreteResourceInstanceDeposedNodeFunc State *states.State } @@ -24,24 +31,41 @@ func (t *StateTransformer) Transform(g *Graph) error { return nil } - log.Printf("[TRACE] StateTransformer: starting") + switch { + case t.ConcreteCurrent != nil && t.ConcreteDeposed != nil: + log.Printf("[TRACE] StateTransformer: creating nodes for both current and deposed instance objects") + case t.ConcreteCurrent != nil: + log.Printf("[TRACE] StateTransformer: creating nodes for current instance objects only") + case t.ConcreteDeposed != nil: + log.Printf("[TRACE] StateTransformer: creating nodes for deposed instance objects only") + default: + log.Printf("[TRACE] StateTransformer: pointless no-op call, creating no nodes at all") + } + for _, ms := range t.State.Modules { moduleAddr := ms.Addr for _, rs := range ms.Resources { resourceAddr := rs.Addr.Absolute(moduleAddr) - for key := range rs.Instances { + for key, is := range rs.Instances { addr := resourceAddr.Instance(key) - abstract := NewNodeAbstractResourceInstance(addr) - var node dag.Vertex = abstract - if f := t.Concrete; f != nil { - node = f(abstract) + if obj := is.Current; obj != nil && t.ConcreteCurrent != nil { + abstract := NewNodeAbstractResourceInstance(addr) + node := t.ConcreteCurrent(abstract) + g.Add(node) + log.Printf("[TRACE] StateTransformer: added %T for %s current object", node, addr) } - g.Add(node) - log.Printf("[TRACE] StateTransformer: added %T for %s", node, addr) + if t.ConcreteDeposed != nil { + for dk := range is.Deposed { + abstract := NewNodeAbstractResourceInstance(addr) + node := t.ConcreteDeposed(abstract, dk) + g.Add(node) + log.Printf("[TRACE] StateTransformer: added %T for %s deposed object %s", node, addr, dk) + } + } } } }