From b32a9db1a3a4beaabc8c07eac0b15e4a9338bf53 Mon Sep 17 00:00:00 2001 From: Ben Zhang Date: Fri, 19 May 2023 23:50:00 +0000 Subject: [PATCH] Complete implementation of PreventRemoval Resolves https://github.com/hashicorp/terraform/issues/17599 --- internal/states/instance_object.go | 8 ++++++ internal/states/instance_object_src.go | 2 ++ internal/states/state_deepcopy.go | 2 ++ internal/states/statefile/version4.go | 4 +++ .../node_resource_abstract_instance.go | 26 ++++++++++++++++--- .../terraform/node_resource_plan_instance.go | 1 + 6 files changed, 39 insertions(+), 4 deletions(-) diff --git a/internal/states/instance_object.go b/internal/states/instance_object.go index 0e790bba1a66..54294251e02a 100644 --- a/internal/states/instance_object.go +++ b/internal/states/instance_object.go @@ -44,6 +44,13 @@ type ResourceInstanceObject struct { // destroy operations, we need to record the status to ensure a resource // removed from the config will still be destroyed in the same manner. CreateBeforeDestroy bool + + // PreventRemoval is a lifecycle option that can be set on a resource + // instance to prevent it from being removed from state. This is useful + // for preventing a stateful resource from being removed unexpectedly, + // regardless of whether it is still present in configuration. + // This can be thought of as a stronger version of prevent_destroy. + PreventRemoval bool } // ObjectStatus represents the status of a RemoteObject. @@ -130,6 +137,7 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res Status: o.Status, Dependencies: dependencies, CreateBeforeDestroy: o.CreateBeforeDestroy, + PreventRemoval: o.PreventRemoval, }, nil } diff --git a/internal/states/instance_object_src.go b/internal/states/instance_object_src.go index a564e0d90778..4f727a938dbf 100644 --- a/internal/states/instance_object_src.go +++ b/internal/states/instance_object_src.go @@ -59,6 +59,7 @@ type ResourceInstanceObjectSrc struct { Status ObjectStatus Dependencies []addrs.ConfigResource CreateBeforeDestroy bool + PreventRemoval bool } // Decode unmarshals the raw representation of the object attributes. Pass the @@ -97,6 +98,7 @@ func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObjec Dependencies: os.Dependencies, Private: os.Private, CreateBeforeDestroy: os.CreateBeforeDestroy, + PreventRemoval: os.PreventRemoval, }, nil } diff --git a/internal/states/state_deepcopy.go b/internal/states/state_deepcopy.go index b8498d53ff11..493178e8aa03 100644 --- a/internal/states/state_deepcopy.go +++ b/internal/states/state_deepcopy.go @@ -174,6 +174,7 @@ func (os *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc { AttrSensitivePaths: attrPaths, Dependencies: dependencies, CreateBeforeDestroy: os.CreateBeforeDestroy, + PreventRemoval: os.PreventRemoval, } } @@ -210,6 +211,7 @@ func (o *ResourceInstanceObject) DeepCopy() *ResourceInstanceObject { Private: private, Dependencies: dependencies, CreateBeforeDestroy: o.CreateBeforeDestroy, + PreventRemoval: o.PreventRemoval, } } diff --git a/internal/states/statefile/version4.go b/internal/states/statefile/version4.go index 2cdde4be7107..2f44e2b368cf 100644 --- a/internal/states/statefile/version4.go +++ b/internal/states/statefile/version4.go @@ -136,6 +136,7 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) { obj := &states.ResourceInstanceObjectSrc{ SchemaVersion: isV4.SchemaVersion, CreateBeforeDestroy: isV4.CreateBeforeDestroy, + PreventRemoval: isV4.PreventRemoval, } { @@ -507,6 +508,7 @@ func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstanc PrivateRaw: privateRaw, Dependencies: deps, CreateBeforeDestroy: obj.CreateBeforeDestroy, + PreventRemoval: obj.PreventRemoval, }), diags } @@ -708,6 +710,8 @@ type instanceObjectStateV4 struct { Dependencies []string `json:"dependencies,omitempty"` CreateBeforeDestroy bool `json:"create_before_destroy,omitempty"` + + PreventRemoval bool `json:"prevent_removal,omitempty"` } type checkResultsV4 struct { diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index d14d0d92ff76..ab66a7304027 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -157,22 +157,37 @@ func (n *NodeAbstractResourceInstance) readDiff(ctx EvalContext, providerSchema } func (n *NodeAbstractResourceInstance) checkPreventDestroy(change *plans.ResourceInstanceChange) error { - if change == nil || n.Config == nil || n.Config.Managed == nil { + if change == nil { return nil } - preventDestroy := n.Config.Managed.PreventDestroy + preventDestroy := false + + // if prevent_destroy is set, then we should prevent destroy + if n.Config != nil && n.Config.Managed != nil { + preventDestroy = preventDestroy || n.Config.Managed.PreventDestroy + } + + // if there's a prevent_removal is true in the state, then we should prevent destroy + if n.instanceState != nil && n.instanceState.Current != nil { + preventDestroy = preventDestroy || n.instanceState.Current.PreventRemoval + } if (change.Action == plans.Delete || change.Action.IsReplace()) && preventDestroy { + var subject *hcl.Range + if n.Config != nil { + subject = &n.Config.DeclRange + } + var diags tfdiags.Diagnostics diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Instance cannot be destroyed", Detail: fmt.Sprintf( - "Resource %s has lifecycle.prevent_destroy set, but the plan calls for this resource to be destroyed. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy or reduce the scope of the plan using the -target flag.", + "Resource %s has lifecycle.prevent_destroy or had lifecycle.prevent_removal set, but the plan calls for this resource to be destroyed. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy and lifecycle.prevent_removal, or reduce the scope of the plan using the -target flag.", n.Addr.String(), ), - Subject: &n.Config.DeclRange, + Subject: subject, }) return diags.Err() } @@ -2152,6 +2167,7 @@ func (n *NodeAbstractResourceInstance) apply( if change.Action == plans.Update && eq && !marksEqual(beforePaths, afterPaths) { // Copy the previous state, changing only the value newState := &states.ResourceInstanceObject{ + PreventRemoval: state.PreventRemoval, CreateBeforeDestroy: state.CreateBeforeDestroy, Dependencies: state.Dependencies, Private: state.Private, @@ -2364,6 +2380,7 @@ func (n *NodeAbstractResourceInstance) apply( Value: newVal, Private: resp.Private, CreateBeforeDestroy: createBeforeDestroy, + PreventRemoval: state.PreventRemoval, } // if the resource was being deleted, the dependencies are not going to @@ -2381,6 +2398,7 @@ func (n *NodeAbstractResourceInstance) apply( Value: newVal, Private: resp.Private, CreateBeforeDestroy: createBeforeDestroy, + PreventRemoval: applyConfig.Managed.PreventRemoval, } return newState, diags diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index aac13bedc82d..246b7bc53267 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -173,6 +173,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) } else { if instanceRefreshState != nil { instanceRefreshState.CreateBeforeDestroy = n.Config.Managed.CreateBeforeDestroy || n.ForceCreateBeforeDestroy + instanceRefreshState.PreventRemoval = n.Config.Managed.PreventRemoval } }