From 5158c5e1dfd50ae43399f41df1e6e0b0eb12427c Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 2 Jun 2026 15:51:26 -0400 Subject: [PATCH 1/7] EvalActionBlock --- internal/lang/eval.go | 30 +++++++++++ .../action_trigger_instance_apply.go | 5 +- .../terraform/context_apply_action_test.go | 3 +- internal/terraform/node_action.go | 52 ++++++++++++------- internal/terraform/node_action_invoke.go | 5 +- .../terraform/node_resource_apply_instance.go | 29 ++--------- .../terraform/node_resource_plan_instance.go | 2 +- 7 files changed, 77 insertions(+), 49 deletions(-) diff --git a/internal/lang/eval.go b/internal/lang/eval.go index 7e55f1b849..b420ef2550 100644 --- a/internal/lang/eval.go +++ b/internal/lang/eval.go @@ -79,6 +79,34 @@ func (s *Scope) EvalBlock(body hcl.Body, schema *configschema.Block) (cty.Value, return val, diags } +// EvalActionBlock is a special case for action blocks that allows the caller to +// directly pass in the "caller" value. This allows for the evaluation of a +// resource value which may be different from what's expected in the global +// context, like for example when a destroy action needs to evaluate the +// "before" value of the resource change. +func (s *Scope) EvalActionBlock(body hcl.Body, schema *configschema.Block, caller cty.Value) (val cty.Value, diags tfdiags.Diagnostics) { + + spec := schema.DecoderSpec() + + refs, diags := langrefs.ReferencesInBlock(s.ParseRef, body, schema) + + ctx, ctxDiags := s.EvalContext(refs) + diags = diags.Append(ctxDiags) + + if diags.HasErrors() { + // We'll stop early if we found problems in the references, because + // it's likely evaluation will produce redundant copies of the same errors. + return cty.UnknownVal(schema.ImpliedType()), diags + } + + ctx.Variables["caller"] = caller + + val, evalDiags := hcldec.Decode(body, spec, ctx) + diags = diags.Append(CheckForUnknownFunctionDiags(evalDiags, s.IgnoreUnknownProviderFunctions)) + + return val, diags +} + // EvalSelfBlock evaluates the given body only within the scope of the provided // object and instance key data. References to the object must use self, and the // key data will only contain count.index or each.key. The static values for @@ -479,6 +507,8 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl if self != cty.NilVal { vals["self"] = self + // "caller" is used directly when an action in invoked from the CLI, + // because we need to automatically retrieve the resource from state vals["caller"] = self } diff --git a/internal/terraform/action_trigger_instance_apply.go b/internal/terraform/action_trigger_instance_apply.go index f562aaa30c..06902593e1 100644 --- a/internal/terraform/action_trigger_instance_apply.go +++ b/internal/terraform/action_trigger_instance_apply.go @@ -7,6 +7,7 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/plans" @@ -29,7 +30,7 @@ var ( _ GraphNodeProviderConsumer = (*actionTriggerApplyInstance)(nil) ) -func (n *actionTriggerApplyInstance) invoke(ctx EvalContext, caller addrs.Referenceable) tfdiags.Diagnostics { +func (n *actionTriggerApplyInstance) invoke(ctx EvalContext, caller addrs.Referenceable, callerVal cty.Value) tfdiags.Diagnostics { var diags tfdiags.Diagnostics provider, _, err := getProvider(ctx, n.ActionInvocation.ProviderAddr) @@ -72,7 +73,7 @@ func (n *actionTriggerApplyInstance) invoke(ctx EvalContext, caller addrs.Refere // } // configValue := inv.ConfigValue - configValue, actionDiags := n.actionNode.EvalInstance(ctx, n.ActionInvocation.Addr, nil, caller) + configValue, actionDiags := n.actionNode.EvalInstance(ctx, n.ActionInvocation.Addr, nil, caller, callerVal) diags = diags.Append(actionDiags) if diags.HasErrors() { return diags diff --git a/internal/terraform/context_apply_action_test.go b/internal/terraform/context_apply_action_test.go index df6b7f4071..e707968046 100644 --- a/internal/terraform/context_apply_action_test.go +++ b/internal/terraform/context_apply_action_test.go @@ -1438,7 +1438,7 @@ resource "test_object" "a" { expectInvokeActionCalled: true, }, - "before_create references caller": { + "before_update references caller": { module: map[string]string{ "main.tf": ` action "action_example" "test" { @@ -1451,7 +1451,6 @@ resource "test_object" "a" { lifecycle { action_trigger { events = [before_update] - condition = self.test_string == "new" actions = [action.action_example.test] } } diff --git a/internal/terraform/node_action.go b/internal/terraform/node_action.go index 8985195daa..8b6333401e 100644 --- a/internal/terraform/node_action.go +++ b/internal/terraform/node_action.go @@ -169,7 +169,9 @@ func (n *NodeActionConfig) Validate(ctx EvalContext, caller addrs.Referenceable) config = hcl.EmptyBody() } - configVal, _, valDiags := ctx.EvaluateBlock(config, n.Schema.ConfigSchema, caller, keyData) + scope := ctx.EvaluationScope(caller, nil, keyData) + configVal, valDiags := scope.EvalActionBlock(config, n.Schema.ConfigSchema, cty.DynamicVal) + // configVal, _, valDiags := ctx.EvaluateBlock(config, n.Schema.ConfigSchema, caller, keyData) if valDiags.HasErrors() { // If there was no config block at all, we'll add a Context range to the returned diagnostic if n.Config.Config == nil { @@ -291,7 +293,7 @@ func (n *NodeActionConfig) AttachDependencies(deps []addrs.ConfigResource) { // addrs.NoKey This function uses addrs.ActionInstance even though it only needs // the key because we need to use use a full instance addr for the resulting map // keys anyway. -func (n *NodeActionConfig) EvalInstances(ctx EvalContext, addr addrs.ActionInstance, callRange *hcl.Range, caller addrs.Referenceable) (addrs.Map[addrs.ActionInstance, cty.Value], tfdiags.Diagnostics) { +func (n *NodeActionConfig) EvalInvokedInstances(ctx EvalContext, addr addrs.ActionInstance, callRange *hcl.Range, caller addrs.Referenceable) (addrs.Map[addrs.ActionInstance, cty.Value], tfdiags.Diagnostics) { var diags tfdiags.Diagnostics all := addrs.MakeMap[addrs.ActionInstance, cty.Value]() @@ -310,7 +312,7 @@ func (n *NodeActionConfig) EvalInstances(ctx EvalContext, addr addrs.ActionInsta for _, instAddr := range instances { repData := expander.GetActionInstanceRepetitionData(instAddr) - val, evalDiags := n.evalInstance(ctx, repData, callRange, caller) + val, evalDiags := n.evalInstance(ctx, repData, callRange, caller, cty.NilVal) diags = diags.Append(evalDiags) if diags.HasErrors() { return all, diags @@ -403,7 +405,7 @@ func (n *NodeActionConfig) validateInstanceKey(addr addrs.AbsActionInstance, cal } // EvalInstance returns the value from the expanded action block -func (n *NodeActionConfig) EvalInstance(ctx EvalContext, inst addrs.AbsActionInstance, callRange *hcl.Range, caller addrs.Referenceable) (cty.Value, tfdiags.Diagnostics) { +func (n *NodeActionConfig) EvalInstance(ctx EvalContext, inst addrs.AbsActionInstance, callRange *hcl.Range, caller addrs.Referenceable, callerVal cty.Value) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics diags = diags.Append(n.validateInstanceKey(inst, callRange)) @@ -434,13 +436,13 @@ func (n *NodeActionConfig) EvalInstance(ctx EvalContext, inst addrs.AbsActionIns repData := expander.GetActionInstanceRepetitionData(instAddr) - return n.evalInstance(ctx, repData, callRange, caller) + return n.evalInstance(ctx, repData, callRange, caller, callerVal) } // Eval one or more instances of the action. This function expects that the key // is already validated for the the calling context, and will not produce // diagnostics for incorrect key types. -func (n *NodeActionConfig) evalInstance(ctx EvalContext, repData instances.RepetitionData, callRange *hcl.Range, caller addrs.Referenceable) (cty.Value, tfdiags.Diagnostics) { +func (n *NodeActionConfig) evalInstance(ctx EvalContext, repData instances.RepetitionData, callRange *hcl.Range, caller addrs.Referenceable, callerVal cty.Value) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics // This should have been caught already @@ -449,20 +451,34 @@ func (n *NodeActionConfig) evalInstance(ctx EvalContext, repData instances.Repet } configVal := cty.NullVal(n.Schema.ConfigSchema.ImpliedType()) - if n.Config.Config != nil { - var configDiags tfdiags.Diagnostics - configVal, _, configDiags = ctx.EvaluateBlock(n.Config.Config, n.Schema.ConfigSchema.DeepCopy(), caller, repData) - diags = diags.Append(configDiags) - if configDiags.HasErrors() { + if n.Config.Config == nil { + return configVal, nil + } + + // For invoke we have no callerVal, but can use the normal self evaluation + // mechanisms because the instance must be in state already. + var evalDiags tfdiags.Diagnostics + if callerVal == cty.NilVal { + configVal, _, evalDiags = ctx.EvaluateBlock(n.Config.Config, n.Schema.ConfigSchema, caller, repData) + diags = diags.Append(evalDiags) + if diags.HasErrors() { + return configVal, diags + } + } else { + scope := ctx.EvaluationScope(caller, nil, repData) + configVal, evalDiags = scope.EvalActionBlock(n.Config.Config, n.Schema.ConfigSchema, callerVal) + diags = diags.Append(evalDiags) + if diags.HasErrors() { return configVal, diags } - - valDiags := validateResourceForbiddenEphemeralValues(ctx, configVal, n.Schema.ConfigSchema) - diags = diags.Append(valDiags.InConfigBody(n.Config.Config, n.Addr.String())) - - var deprecationDiags tfdiags.Diagnostics - configVal, deprecationDiags = ctx.Deprecations().ValidateAndUnmarkConfig(configVal, n.Schema.ConfigSchema, n.ModulePath()) - diags = diags.Append(deprecationDiags.InConfigBody(n.Config.Config, n.Addr.String())) } + + valDiags := validateResourceForbiddenEphemeralValues(ctx, configVal, n.Schema.ConfigSchema) + diags = diags.Append(valDiags.InConfigBody(n.Config.Config, n.Addr.String())) + + var deprecationDiags tfdiags.Diagnostics + configVal, deprecationDiags = ctx.Deprecations().ValidateAndUnmarkConfig(configVal, n.Schema.ConfigSchema, n.ModulePath()) + diags = diags.Append(deprecationDiags.InConfigBody(n.Config.Config, n.Addr.String())) + return configVal, diags } diff --git a/internal/terraform/node_action_invoke.go b/internal/terraform/node_action_invoke.go index 8ae3222e32..09e6de02c6 100644 --- a/internal/terraform/node_action_invoke.go +++ b/internal/terraform/node_action_invoke.go @@ -170,7 +170,7 @@ func (n *nodeActionPlanInvoke) planActions(ctx EvalContext) tfdiags.Diagnostics // We're relying on the given addr derived from the action target to // determine which action instance to evaluate. If the address has no key // and the action is expanded, we will plan all instances. - actionVals, actionDiags := n.ActionConfig.EvalInstances(ctx, n.Addr.Action, nil, n.Caller) + actionVals, actionDiags := n.ActionConfig.EvalInvokedInstances(ctx, n.Addr.Action, nil, n.Caller) diags = diags.Append(actionDiags) if diags.HasErrors() { return diags @@ -257,7 +257,8 @@ func (n *nodeActionInvokeApplyInstance) Name() string { } func (n *nodeActionInvokeApplyInstance) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { - return n.invoke(ctx, n.ActionInvocation.Caller) + // FIXME: caller! + return n.invoke(ctx, n.ActionInvocation.Caller, cty.NilVal) } func (n *nodeActionInvokeApplyInstance) References() []*addrs.Reference { diff --git a/internal/terraform/node_resource_apply_instance.go b/internal/terraform/node_resource_apply_instance.go index ac13a3cb4b..12e11a29c5 100644 --- a/internal/terraform/node_resource_apply_instance.go +++ b/internal/terraform/node_resource_apply_instance.go @@ -232,7 +232,7 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) // Make a new diff, in case we've learned new values in the state // during apply which we can now incorporate. - diffApply, instancePlannedState, deferred, planDiags := n.plan(ctx, diff, state, false, n.forceReplace, repData) + diffApply, _, deferred, planDiags := n.plan(ctx, diff, state, false, n.forceReplace, repData) diags = diags.Append(planDiags) if diags.HasErrors() { return diags @@ -275,27 +275,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) return diags.Append(n.managedResourcePostconditions(ctx, repData)) } - // In order to ensure the action can be evaluated, we need to update the - // stored change and state in case it provides some newly known values. We - // hedge against any unexpected changes in diff handling for existing - // configurations by only updating these when we have actions to evaluate. if n.hasBeforeActions() { - changes := ctx.Changes() - changes.RemoveResourceInstanceChange(n.Addr, deposedKey) - changes.AppendResourceInstanceChange(diffApply) - - // we also have to update the state to reflect the pending changes - // again, or else evaluation will return the old state. Since before - // actions are the first time we've encountered this eval-before-applied - // situation, all instances in the state were previously just left as - // ObjectReady. - diags = diags.Append(n.writeResourceInstanceState(ctx, instancePlannedState, workingState)) - if diags.HasErrors() { - return diags - } - log.Printf("[DEBUG] NodeApplyableResourceInstance: invoking before actions for %s", n.Addr) - diags = diags.Append(n.invokeActions(ctx, repData, configs.BeforeEvents)) + diags = diags.Append(n.invokeActions(ctx, repData, configs.BeforeEvents, diffApply.After)) if diags.HasErrors() { return diags } @@ -391,7 +373,7 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) // until the user makes the condition succeed. diags = diags.Append(n.managedResourcePostconditions(ctx, repData)) - diags = diags.Append(n.invokeActions(ctx, repData, configs.AfterEvents)) + diags = diags.Append(n.invokeActions(ctx, repData, configs.AfterEvents, state.Value)) return diags } @@ -412,9 +394,8 @@ func (n *NodeApplyableResourceInstance) hasBeforeActions() bool { // invokeActions invokes any actions triggered for the listed events. Condition // expressions are reevaluated here when they exist, and failing conditions are // skipped. -func (n *NodeApplyableResourceInstance) invokeActions(ctx EvalContext, repData instances.RepetitionData, forEvents []configs.ActionTriggerEvent) tfdiags.Diagnostics { +func (n *NodeApplyableResourceInstance) invokeActions(ctx EvalContext, repData instances.RepetitionData, forEvents []configs.ActionTriggerEvent, callerVal cty.Value) tfdiags.Diagnostics { var diags tfdiags.Diagnostics - for _, trigger := range n.actionTriggers { event := trigger.ActionInvocation.ActionTrigger.TriggerEvent() if !slices.Contains(forEvents, event) { @@ -432,7 +413,7 @@ func (n *NodeApplyableResourceInstance) invokeActions(ctx EvalContext, repData i continue } - diags = diags.Append(trigger.invoke(ctx, n.Addr.Resource)) + diags = diags.Append(trigger.invoke(ctx, n.Addr.Resource, callerVal)) if diags.HasErrors() { break } diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index c0dcdb8305..2736b48eb9 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -696,7 +696,7 @@ func (n *NodePlannableResourceInstance) planActionTrigger(ctx EvalContext, resRe return } - actionVal, actionDiags := actionRef.actionNode.EvalInstance(ctx, actionInst.Absolute(ctx.Path()), actionRef.configRef.Expr.Range().Ptr(), n.Addr.Resource) + actionVal, actionDiags := actionRef.actionNode.EvalInstance(ctx, actionInst.Absolute(ctx.Path()), actionRef.configRef.Expr.Range().Ptr(), n.Addr.Resource, n.change.After) diags = diags.Append(actionDiags) if diags.HasErrors() { return From 7bf04f71601204d95aea3419843b93009afb1a5f Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 2 Jun 2026 09:23:48 -0400 Subject: [PATCH 2/7] add before and after destroy config events --- internal/configs/action.go | 6 ++++- internal/configs/action_test.go | 2 +- .../terraform/context_plan_actions_test.go | 22 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/internal/configs/action.go b/internal/configs/action.go index 832deaf6e1..ddb3c5ece3 100644 --- a/internal/configs/action.go +++ b/internal/configs/action.go @@ -115,11 +115,15 @@ func decodeActionTriggerBlock(block *hcl.Block) (*ActionTrigger, hcl.Diagnostics containsBefore = true case "after_update": event = AfterUpdate + case "before_destroy": + event = BeforeDestroy + case "after_destroy": + event = AfterDestroy default: diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: fmt.Sprintf("Invalid \"event\" value %s", hcl.ExprAsKeyword(expr)), - Detail: "The \"event\" argument supports the following values: before_create, after_create, before_update, after_update.", + Detail: "The \"event\" argument supports the following values: before_create, after_create, before_update, after_update, before_destroy, after_destroy.", Subject: expr.Range().Ptr(), }) continue diff --git a/internal/configs/action_test.go b/internal/configs/action_test.go index 1da92a6a9b..39cb3f748d 100644 --- a/internal/configs/action_test.go +++ b/internal/configs/action_test.go @@ -198,7 +198,7 @@ func TestDecodeActionTriggerBlock(t *testing.T) { }, }, []string{ - "MockExprTraversal:0,0-12: Invalid \"event\" value not_an_event; The \"event\" argument supports the following values: before_create, after_create, before_update, after_update.", + "MockExprTraversal:0,0-12: Invalid \"event\" value not_an_event; The \"event\" argument supports the following values: before_create, after_create, before_update, after_update, before_destroy, after_destroy.", ":0,0-0: No events specified; At least one event must be specified for an action_trigger.", }, }, diff --git a/internal/terraform/context_plan_actions_test.go b/internal/terraform/context_plan_actions_test.go index 6ae613c4d4..d4fdcf10a0 100644 --- a/internal/terraform/context_plan_actions_test.go +++ b/internal/terraform/context_plan_actions_test.go @@ -3338,6 +3338,28 @@ resource "test_object" "a" { expectPlanActionCalled: true, }, + "after_destroy": { + module: map[string]string{ + "main.tf": ` +action "test_action" "test" { + config { + attr = caller.name + } +} +resource "test_object" "a" { + name = "new" + lifecycle { + action_trigger { + events = [after_destroy] + actions = [action.test_action.test] + } + } +} +`, + }, + expectPlanActionCalled: true, + }, + "non-boolean condition": { module: map[string]string{ "main.tf": ` From 93c5f2e16c75ef8ffd0f2e7176ab99925a832240 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 2 Jun 2026 16:16:20 -0400 Subject: [PATCH 3/7] Handle destroy actions Destroy actions can now be used in resources, with some constraints in place. To avoid dependency cycles we must fully plan the action. This means that the action config and the triggering condition are completely known at plan time, so that the stored values can be used during apply. This also means that ephemeral values cannot be used in destroy action configuration, because they are not guaranteed to be the same during apply. --- internal/configs/action.go | 17 +- .../action_trigger_instance_apply.go | 57 +++-- .../terraform/context_apply_action_test.go | 51 ++++ .../terraform/context_plan_actions_test.go | 239 ++++++++++++++++-- internal/terraform/node_action.go | 1 + internal/terraform/node_action_invoke.go | 2 +- .../node_resource_abstract_instance.go | 113 +++++++++ .../terraform/node_resource_apply_instance.go | 90 ------- internal/terraform/node_resource_destroy.go | 11 + .../node_resource_destroy_deposed.go | 12 + .../terraform/node_resource_plan_instance.go | 46 +++- internal/terraform/transform_action_diff.go | 5 +- 12 files changed, 498 insertions(+), 146 deletions(-) diff --git a/internal/configs/action.go b/internal/configs/action.go index ddb3c5ece3..ae43de3d59 100644 --- a/internal/configs/action.go +++ b/internal/configs/action.go @@ -67,9 +67,22 @@ const ( Invoke ) +func (e ActionTriggerEvent) IsBefore() bool { + return slices.Contains(BeforeEvents, e) +} + +func (e ActionTriggerEvent) IsAfter() bool { + return slices.Contains(AfterEvents, e) +} + +func (e ActionTriggerEvent) IsDestroy() bool { + return slices.Contains(DestroyEvents, e) +} + var ( - BeforeEvents = []ActionTriggerEvent{BeforeCreate, BeforeUpdate, BeforeDestroy} - AfterEvents = []ActionTriggerEvent{AfterCreate, AfterUpdate, AfterDestroy} + BeforeEvents = []ActionTriggerEvent{BeforeCreate, BeforeUpdate, BeforeDestroy} + AfterEvents = []ActionTriggerEvent{AfterCreate, AfterUpdate, AfterDestroy} + DestroyEvents = []ActionTriggerEvent{BeforeDestroy, AfterDestroy} ) // ActionRef represents a reference to a configured Action diff --git a/internal/terraform/action_trigger_instance_apply.go b/internal/terraform/action_trigger_instance_apply.go index 06902593e1..4da9bd9b60 100644 --- a/internal/terraform/action_trigger_instance_apply.go +++ b/internal/terraform/action_trigger_instance_apply.go @@ -30,10 +30,10 @@ var ( _ GraphNodeProviderConsumer = (*actionTriggerApplyInstance)(nil) ) -func (n *actionTriggerApplyInstance) invoke(ctx EvalContext, caller addrs.Referenceable, callerVal cty.Value) tfdiags.Diagnostics { +func (n *actionTriggerApplyInstance) Invoke(ctx EvalContext, caller addrs.Referenceable, callerVal cty.Value, fromPlan bool) tfdiags.Diagnostics { var diags tfdiags.Diagnostics - provider, _, err := getProvider(ctx, n.ActionInvocation.ProviderAddr) + provider, actionProviderSchema, err := getProvider(ctx, n.ActionInvocation.ProviderAddr) if err != nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, @@ -54,32 +54,37 @@ func (n *actionTriggerApplyInstance) invoke(ctx EvalContext, caller addrs.Refere return diags } - // TODO: we will need to decode the saved config value for our initial attempt at destroy actions - // - // actionSchema, ok := actionProviderSchema.Actions[n.ActionInvocation.Addr.Action.Action.Type] - // if !ok { - // // This should have been caught earlier, but we don't want to panic - // diags = diags.Append(&hcl.Diagnostic{ - // Severity: hcl.DiagError, - // Summary: fmt.Sprintf("Action %s not found in provider schema", n.ActionInvocation.Addr), - // Detail: fmt.Sprintf("The action %s was not found in the provider schema for %s", n.ActionInvocation.Addr, n.ActionInvocation.ProviderAddr), - // Subject: n.actionNode.Config.DeclRange.Ptr(), - // }) - // return diags - // } - // inv, err := n.ActionInvocation.Decode(&actionSchema) - // if err != nil { - // return diags.Append(err) - // } - // configValue := inv.ConfigValue + configVal := cty.DynamicVal - configValue, actionDiags := n.actionNode.EvalInstance(ctx, n.ActionInvocation.Addr, nil, caller, callerVal) - diags = diags.Append(actionDiags) - if diags.HasErrors() { - return diags + // fromPlan indicates we can use the entire planned value for the action, + // and should not attempt to reevaluate the config. + if fromPlan { + actionSchema, ok := actionProviderSchema.Actions[n.ActionInvocation.Addr.Action.Action.Type] + if !ok { + // This should have been caught earlier, but we don't want to panic + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Action %s not found in provider schema", n.ActionInvocation.Addr), + Detail: fmt.Sprintf("The action %s was not found in the provider schema for %s", n.ActionInvocation.Addr, n.ActionInvocation.ProviderAddr), + Subject: n.actionNode.Config.DeclRange.Ptr(), + }) + return diags + } + inv, err := n.ActionInvocation.Decode(&actionSchema) + if err != nil { + return diags.Append(err) + } + configVal = inv.ConfigValue + } else { + val, actionDiags := n.actionNode.EvalInstance(ctx, n.ActionInvocation.Addr, nil, caller, callerVal) + diags = diags.Append(actionDiags) + if diags.HasErrors() { + return diags + } + configVal = val } - if !configValue.IsWhollyKnown() { + if !configVal.IsWhollyKnown() { return diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Action configuration unknown during apply", @@ -104,7 +109,7 @@ func (n *actionTriggerApplyInstance) invoke(ctx EvalContext, caller addrs.Refere // We don't want to send the marks, but all marks are okay in the context // of an action invocation. We can't reuse our ephemeral free value from // above because we want the ephemeral values to be included. - unmarkedConfigValue, _ := configValue.UnmarkDeep() + unmarkedConfigValue, _ := configVal.UnmarkDeep() resp := provider.InvokeAction(providers.InvokeActionRequest{ ActionType: n.ActionInvocation.Addr.Action.Action.Type, PlannedActionData: unmarkedConfigValue, diff --git a/internal/terraform/context_apply_action_test.go b/internal/terraform/context_apply_action_test.go index e707968046..9bcc2babcf 100644 --- a/internal/terraform/context_apply_action_test.go +++ b/internal/terraform/context_apply_action_test.go @@ -175,6 +175,57 @@ resource "test_object" "a" { expectInvokeActionCalled: true, }, + "after_destroy triggered": { + module: map[string]string{ + "main.tf": ` +action "action_example" "hello" { + config { + attr = caller.test_string + } +} + +resource "test_object" "a" { + test_string = "new name" + lifecycle { + action_trigger { + events = [after_destroy] + actions = [action.action_example.hello] + } + } +} +`, + }, + prevRunState: states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("test_object.a"), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectTainted, + AttrsJSON: []byte(`{"test_string":"old name"}`), + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + }), + events: func(req providers.InvokeActionRequest) []providers.InvokeActionEvent { + attr := req.PlannedActionData.GetAttr("attr").AsString() + if attr != "old name" { + return []providers.InvokeActionEvent{ + providers.InvokeActionEvent_Completed{ + Diagnostics: tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "destroy used wrong state value", + "action invoked with "+attr, + ), + }, + }, + } + } + return []providers.InvokeActionEvent{ + providers.InvokeActionEvent_Completed{}, + } + }, + expectInvokeActionCalled: true, + }, + "before_create failing": { module: map[string]string{ "main.tf": ` diff --git a/internal/terraform/context_plan_actions_test.go b/internal/terraform/context_plan_actions_test.go index d4fdcf10a0..15e6d48d8e 100644 --- a/internal/terraform/context_plan_actions_test.go +++ b/internal/terraform/context_plan_actions_test.go @@ -1449,6 +1449,222 @@ resource "test_object" "a" { }) }, }, + "after_destroy": { + module: map[string]string{ + "main.tf": ` +action "test_action" "test" { + config { + attr = caller.name + } +} +resource "test_object" "a" { + name = "new" + lifecycle { + action_trigger { + events = [after_destroy] + actions = [action.test_action.test] + } + } +} +`, + }, + planResourceFn: func(t *testing.T, req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { + resp.PlannedState = cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("new"), + }) + resp.RequiresReplace = []cty.Path{cty.GetAttrPath("name")} + return resp + }, + buildState: func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("test_object.a"), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"name":"current"}`), + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + }, + planActionFn: func(t *testing.T, req providers.PlanActionRequest) providers.PlanActionResponse { + attr := req.ProposedActionData.GetAttr("attr").AsString() + if attr != "current" { + t.Fatalf("expected action plan to be 'current', got %s\n", attr) + } + return providers.PlanActionResponse{} + }, + expectPlanActionCalled: true, + }, + + "no ephemeral in destroy": { + module: map[string]string{ + "main.tf": ` +action "test_action_wo" "test" { + config { + attr = terraform.applying + } +} +resource "test_object" "a" { + name = "new" + lifecycle { + action_trigger { + events = [before_destroy] + actions = [action.test_action_wo.test] + } + } +} +`, + }, + planResourceFn: func(t *testing.T, req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { + resp.PlannedState = cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("new"), + }) + resp.RequiresReplace = []cty.Path{cty.GetAttrPath("name")} + return resp + }, + buildState: func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("test_object.a"), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"name":"current"}`), + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + }, + expectPlanActionCalled: false, + expectPlanDiagnostics: func(m *configs.Config) (diags tfdiags.Diagnostics) { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Action config contains ephemeral values", + Detail: "A destroy action configuration must be fully planned, and cannot contain ephemeral values.", + Subject: &hcl.Range{ + Filename: filepath.Join(m.Module.SourceDir, "main.tf"), + Start: hcl.Pos{Line: 2, Column: 1, Byte: 1}, + End: hcl.Pos{Line: 2, Column: 31, Byte: 31}, + }, + }) + }, + }, + + "destroy action must be known": { + module: map[string]string{ + "main.tf": ` +resource "test_object" "new" { +} + +action "test_action" "test" { + config { + attr = test_object.new.name + } +} +resource "test_object" "a" { + name = "new" + lifecycle { + action_trigger { + events = [after_destroy] + actions = [action.test_action.test] + } + } +} +`, + }, + planResourceFn: func(t *testing.T, req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { + if req.PriorState.IsNull() { + resp.PlannedState = cty.ObjectVal(map[string]cty.Value{ + "name": cty.UnknownVal(cty.String), + }) + return resp + } + resp.PlannedState = cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("new"), + }) + resp.RequiresReplace = []cty.Path{cty.GetAttrPath("name")} + return resp + }, + buildState: func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("test_object.a"), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"name":"current"}`), + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + }, + expectPlanActionCalled: false, + expectPlanDiagnostics: func(m *configs.Config) (diags tfdiags.Diagnostics) { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unknown action config", + Detail: "Action configuration must be known to plan a destroy action.", + Subject: &hcl.Range{ + Filename: filepath.Join(m.Module.SourceDir, "main.tf"), + Start: hcl.Pos{Line: 5, Column: 1, Byte: 35}, + End: hcl.Pos{Line: 5, Column: 28, Byte: 62}, + }, + }) + }, + }, + + "destroy condition must be known": { + module: map[string]string{ + "main.tf": ` +resource "test_object" "new" { +} + +action "test_action" "test" { + config { + attr = caller.name + } +} +resource "test_object" "a" { + name = "new" + lifecycle { + action_trigger { + condition = test_object.new.name == "ready" + events = [after_destroy] + actions = [action.test_action.test] + } + } +} +`, + }, + planResourceFn: func(t *testing.T, req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { + if req.PriorState.IsNull() { + // this is the new instance being created + resp.PlannedState = cty.ObjectVal(map[string]cty.Value{ + "name": cty.UnknownVal(cty.String), + }) + return resp + } + + // this is directing the existing "a" instance to be replaced + resp.PlannedState = cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("new"), + }) + resp.RequiresReplace = []cty.Path{cty.GetAttrPath("name")} + return resp + }, + buildState: func(s *states.SyncState) { + s.SetResourceInstanceCurrent(mustResourceInstanceAddr("test_object.a"), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"name":"current"}`), + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + }, + expectPlanActionCalled: false, + expectPlanDiagnostics: func(m *configs.Config) (diags tfdiags.Diagnostics) { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unknown action trigger condition", + Detail: "Condition expression must be known to plan a destroy action.", + Subject: &hcl.Range{ + Filename: filepath.Join(m.Module.SourceDir, "main.tf"), + Start: hcl.Pos{Line: 14, Column: 19, Byte: 202}, + End: hcl.Pos{Line: 14, Column: 50, Byte: 233}, + }, + }) + }, + }, "caller can be used for triggering resource": { module: map[string]string{ @@ -3338,28 +3554,6 @@ resource "test_object" "a" { expectPlanActionCalled: true, }, - "after_destroy": { - module: map[string]string{ - "main.tf": ` -action "test_action" "test" { - config { - attr = caller.name - } -} -resource "test_object" "a" { - name = "new" - lifecycle { - action_trigger { - events = [after_destroy] - actions = [action.test_action.test] - } - } -} -`, - }, - expectPlanActionCalled: true, - }, - "non-boolean condition": { module: map[string]string{ "main.tf": ` @@ -4074,6 +4268,7 @@ resource "test_object" "b" { "name": { Type: cty.String, Optional: true, + Computed: true, }, }, }, diff --git a/internal/terraform/node_action.go b/internal/terraform/node_action.go index 8b6333401e..fb3a258ae4 100644 --- a/internal/terraform/node_action.go +++ b/internal/terraform/node_action.go @@ -422,6 +422,7 @@ func (n *NodeActionConfig) EvalInstance(ctx EvalContext, inst addrs.AbsActionIns for _, instAddr := range instances { if instAddr.Equal(inst) { found = true + break } } if !found { diff --git a/internal/terraform/node_action_invoke.go b/internal/terraform/node_action_invoke.go index 09e6de02c6..1d271fb629 100644 --- a/internal/terraform/node_action_invoke.go +++ b/internal/terraform/node_action_invoke.go @@ -258,7 +258,7 @@ func (n *nodeActionInvokeApplyInstance) Name() string { func (n *nodeActionInvokeApplyInstance) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { // FIXME: caller! - return n.invoke(ctx, n.ActionInvocation.Caller, cty.NilVal) + return n.Invoke(ctx, n.ActionInvocation.Caller, cty.NilVal, true) } func (n *nodeActionInvokeApplyInstance) References() []*addrs.Reference { diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 9017d0ed39..5e6f01e6df 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -6,6 +6,7 @@ package terraform import ( "fmt" "log" + "slices" "strings" "github.com/hashicorp/hcl/v2" @@ -53,6 +54,8 @@ type NodeAbstractResourceInstance struct { // override is set by the graph itself, just before this node executes. override *configs.Override + + actionApplyTriggers []*actionTriggerApplyInstance } var ( @@ -3108,3 +3111,113 @@ func getRequiredReplaces(priorVal, plannedNewVal cty.Value, writeOnly []cty.Path return reqRep, diags } + +// pre-check for before actions since being able to evaluate actions requires a +// slightly different behavior for diff handling, we guard that change by seeing if +// it's needed at all. +func (n *NodeAbstractResourceInstance) hasBeforeActions() bool { + for _, trigger := range n.actionApplyTriggers { + event := trigger.ActionInvocation.ActionTrigger.TriggerEvent() + if event.IsBefore() { + return true + } + } + return false +} + +// invokeDestroyAction currently cannot reevaluate the action config due to not +// being able to order dependencies without causing cycles. The entire config +// and condition must be known at plan time, so if we have a planned action we +// simply decode and call invoke. +func (n *NodeAbstractResourceInstance) invokeDestroyActions(ctx EvalContext, forEvent configs.ActionTriggerEvent) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + for _, trigger := range n.actionApplyTriggers { + event := trigger.ActionInvocation.ActionTrigger.TriggerEvent() + if event != forEvent { + continue + } + + log.Printf("[DEBUG] NodeAbstractesourceInstance: invoking destroy action %s", trigger.ActionInvocation.Addr) + diags = diags.Append(trigger.Invoke(ctx, n.Addr.Resource, cty.DynamicVal, true)) + if diags.HasErrors() { + break + } + } + + return diags +} + +// invokeActions invokes any actions triggered for the listed events. Condition +// expressions are reevaluated here when they exist, and failing conditions are +// skipped. +func (n *NodeAbstractResourceInstance) invokeActions(ctx EvalContext, repData instances.RepetitionData, forEvents []configs.ActionTriggerEvent, callerVal cty.Value) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + for _, trigger := range n.actionApplyTriggers { + event := trigger.ActionInvocation.ActionTrigger.TriggerEvent() + if !slices.Contains(forEvents, event) { + continue + } + + condOK, condDiags := n.evalActionCondition(ctx, trigger, repData) + diags = diags.Append(condDiags) + if diags.HasErrors() { + return diags + } + + if !condOK { + log.Printf("[DEBUG] NodeAbstractesourceInstance: action condition false, skipping %s", trigger.ActionInvocation.Addr) + continue + } + + diags = diags.Append(trigger.Invoke(ctx, n.Addr.Resource, callerVal, false)) + if diags.HasErrors() { + break + } + } + + return diags +} + +// We need to lookup any condition expression from the action block before +// execution, because the condition is part of the resource config, while the +// action is planned as an ActionInvocation. +func (n *NodeAbstractResourceInstance) evalActionCondition(ctx EvalContext, trigger *actionTriggerApplyInstance, repData instances.RepetitionData) (bool, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + // this can't be an invoked trigger + rat := trigger.ActionInvocation.ActionTrigger.(*plans.ResourceActionTrigger) + triggerBlock := n.Config.Managed.ActionTriggers[rat.ActionTriggerBlockIndex] + + if triggerBlock.Condition == nil { + return true, diags + } + + scope := ctx.EvaluationScope(n.Addr.Resource, nil, repData) + cond, conditionEvalDiags := scope.EvalExpr(triggerBlock.Condition, cty.Bool) + diags = diags.Append(conditionEvalDiags) + if diags.HasErrors() { + return false, diags + } + + if !cond.IsKnown() { + // this should not happen, but give the user a good diagnostic to help + // reproduce the problem in case it does. + return false, diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unknown condition when invoking action before apply", + Detail: "The action trigger condition must be known before it can be invoked.", + Subject: triggerBlock.Condition.Range().Ptr(), + }) + } + + return cond.True(), diags +} + +func (n *NodeAbstractResourceInstance) ActionProviders() []ProviderRef { + var refs []ProviderRef + for _, trigger := range n.actionApplyTriggers { + refs = append(refs, ProviderRef{Addr: trigger.actionNode.ResolvedProvider, Resolved: true}) + } + return refs +} diff --git a/internal/terraform/node_resource_apply_instance.go b/internal/terraform/node_resource_apply_instance.go index 12e11a29c5..404a9ce7e0 100644 --- a/internal/terraform/node_resource_apply_instance.go +++ b/internal/terraform/node_resource_apply_instance.go @@ -6,7 +6,6 @@ package terraform import ( "fmt" "log" - "slices" "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" @@ -36,8 +35,6 @@ type NodeApplyableResourceInstance struct { // forceReplace indicates that this resource is being replaced for external // reasons, like a -replace flag or via replace_triggered_by. forceReplace bool - - actionTriggers []*actionTriggerApplyInstance } var ( @@ -378,93 +375,6 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) return diags } -// pre-check for before actions since being able to evaluate actions requires a -// slightly different behavior for diff handling, we guard that change by seeing if -// it's needed at all. -func (n *NodeApplyableResourceInstance) hasBeforeActions() bool { - for _, trigger := range n.actionTriggers { - event := trigger.ActionInvocation.ActionTrigger.TriggerEvent() - if slices.Contains(configs.BeforeEvents, event) { - return true - } - } - return false -} - -// invokeActions invokes any actions triggered for the listed events. Condition -// expressions are reevaluated here when they exist, and failing conditions are -// skipped. -func (n *NodeApplyableResourceInstance) invokeActions(ctx EvalContext, repData instances.RepetitionData, forEvents []configs.ActionTriggerEvent, callerVal cty.Value) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics - for _, trigger := range n.actionTriggers { - event := trigger.ActionInvocation.ActionTrigger.TriggerEvent() - if !slices.Contains(forEvents, event) { - continue - } - - condOK, condDiags := n.evalActionCondition(ctx, trigger, repData) - diags = diags.Append(condDiags) - if diags.HasErrors() { - return diags - } - - if !condOK { - log.Printf("[DEBUG] NodeApplyableResourceInstance: action condition false, skipping %s", trigger.ActionInvocation.Addr) - continue - } - - diags = diags.Append(trigger.invoke(ctx, n.Addr.Resource, callerVal)) - if diags.HasErrors() { - break - } - } - - return diags -} - -// We need to lookup any condition expression from the action block before -// execution, because the condition is part of the resource config, while the -// action is planned as an ActionInvocation. -func (n *NodeApplyableResourceInstance) evalActionCondition(ctx EvalContext, trigger *actionTriggerApplyInstance, repData instances.RepetitionData) (bool, tfdiags.Diagnostics) { - var diags tfdiags.Diagnostics - - // this can't be an invoked trigger - rat := trigger.ActionInvocation.ActionTrigger.(*plans.ResourceActionTrigger) - triggerBlock := n.Config.Managed.ActionTriggers[rat.ActionTriggerBlockIndex] - - if triggerBlock.Condition == nil { - return true, diags - } - - scope := ctx.EvaluationScope(n.Addr.Resource, nil, repData) - cond, conditionEvalDiags := scope.EvalExpr(triggerBlock.Condition, cty.Bool) - diags = diags.Append(conditionEvalDiags) - if diags.HasErrors() { - return false, diags - } - - if !cond.IsKnown() { - // this should not happen, but give the user a good diagnostic to help - // reproduce the problem in case it does. - return false, diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Unknown condition when invoking action before apply", - Detail: "The action trigger condition must be known before it can be invoked.", - Subject: triggerBlock.Condition.Range().Ptr(), - }) - } - - return cond.True(), diags -} - -func (n *NodeApplyableResourceInstance) ActionProviders() []ProviderRef { - var refs []ProviderRef - for _, trigger := range n.actionTriggers { - refs = append(refs, ProviderRef{Addr: trigger.actionNode.ResolvedProvider, Resolved: true}) - } - return refs -} - func (n *NodeApplyableResourceInstance) managedResourcePostconditions(ctx EvalContext, repeatData instances.RepetitionData) (diags tfdiags.Diagnostics) { checkDiags := evalCheckRules( diff --git a/internal/terraform/node_resource_destroy.go b/internal/terraform/node_resource_destroy.go index 82a8aa4065..b6e99b2fb6 100644 --- a/internal/terraform/node_resource_destroy.go +++ b/internal/terraform/node_resource_destroy.go @@ -183,6 +183,14 @@ func (n *NodeDestroyResourceInstance) managedResourceExecute(ctx EvalContext) (d } } + if n.hasBeforeActions() { + log.Printf("[DEBUG] NodeApplyableResourceInstance: invoking before actions for %s", n.Addr) + diags = diags.Append(n.invokeDestroyActions(ctx, configs.BeforeDestroy)) + if diags.HasErrors() { + return diags + } + } + // Managed resources need to be destroyed, while data sources // are only removed from state. // we pass a nil configuration to apply because we are destroying @@ -212,6 +220,9 @@ func (n *NodeDestroyResourceInstance) managedResourceExecute(ctx EvalContext) (d }) } + // after destroy we continue to use the before value, since there is no after + diags = diags.Append(n.invokeDestroyActions(ctx, configs.AfterDestroy)) + // create the err value for postApplyHook diags = diags.Append(n.postApplyHook(ctx, state, diags.Err())) diags = diags.Append(updateStateHook(ctx)) diff --git a/internal/terraform/node_resource_destroy_deposed.go b/internal/terraform/node_resource_destroy_deposed.go index 907f50f14b..33931ca663 100644 --- a/internal/terraform/node_resource_destroy_deposed.go +++ b/internal/terraform/node_resource_destroy_deposed.go @@ -8,6 +8,7 @@ import ( "log" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/plans" @@ -312,6 +313,14 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w return diags } + if n.hasBeforeActions() { + log.Printf("[DEBUG] NodeApplyableResourceInstance: invoking before actions for %s", n.Addr) + diags = diags.Append(n.invokeDestroyActions(ctx, configs.BeforeDestroy)) + if diags.HasErrors() { + return diags + } + } + // we pass a nil configuration to apply because we are destroying state, applyDiags := n.apply(ctx, state, change, nil, instances.RepetitionData{}, false) diags = diags.Append(applyDiags) @@ -326,6 +335,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w return diags } + // after destroy we continue to use the before value, since there is no after + diags = diags.Append(n.invokeDestroyActions(ctx, configs.AfterDestroy)) + diags = diags.Append(n.postApplyHook(ctx, state, diags.Err())) return diags.Append(updateStateHook(ctx)) diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index 2736b48eb9..9ebbe46376 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -606,8 +606,10 @@ func (n *NodePlannableResourceInstance) planActionTriggers(ctx EvalContext, resR for _, trigger := range n.actionTriggers { scope := ctx.EvaluationScope(n.Addr.Resource, nil, resRepData) + cond := cty.True if trigger.config.Condition != nil { - cond, conditionEvalDiags := scope.EvalExpr(trigger.config.Condition, cty.Bool) + var conditionEvalDiags tfdiags.Diagnostics + cond, conditionEvalDiags = scope.EvalExpr(trigger.config.Condition, cty.Bool) diags = diags.Append(conditionEvalDiags) if diags.HasErrors() { continue @@ -625,6 +627,16 @@ func (n *NodePlannableResourceInstance) planActionTriggers(ctx EvalContext, resR // though because the event is set within a nested interface inside a // pointer to the ActionInvocationInstance. for _, event := range eventsForPlannedAction(trigger.config.Events, n.change.Action) { + if event.IsDestroy() && !cond.IsKnown() { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unknown action trigger condition", + Detail: "Condition expression must be known to plan a destroy action.", + Subject: trigger.config.Condition.Range().Ptr(), + }) + return diags + } + for _, action := range trigger.actionRefs { ai, deferred, planDiags := n.planActionTrigger(ctx, resRepData, action, event) diags = diags.Append(planDiags) @@ -696,12 +708,42 @@ func (n *NodePlannableResourceInstance) planActionTrigger(ctx EvalContext, resRe return } - actionVal, actionDiags := actionRef.actionNode.EvalInstance(ctx, actionInst.Absolute(ctx.Path()), actionRef.configRef.Expr.Range().Ptr(), n.Addr.Resource, n.change.After) + callerVal := n.change.After + // If the resource is being destroyed, we want the before val. This works + // for replacement (this node doesn't handle full destroys), because the + // caller is associated with the existing resource instance rather than the + if event == configs.BeforeDestroy || event == configs.AfterDestroy { + callerVal = n.change.Before + } + + actionVal, actionDiags := actionRef.actionNode.EvalInstance(ctx, actionInst.Absolute(ctx.Path()), actionRef.configRef.Expr.Range().Ptr(), n.Addr.Resource, callerVal) diags = diags.Append(actionDiags) if diags.HasErrors() { return } + if event.IsDestroy() { + if !actionVal.IsWhollyKnown() { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unknown action config", + Detail: "Action configuration must be known to plan a destroy action.", + Subject: actionRef.actionNode.Config.DeclRange.Ptr(), + }) + return + } + + if len(ephemeral.EphemeralValuePaths(actionVal)) > 0 { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Action config contains ephemeral values", + Detail: "A destroy action configuration must be fully planned, and cannot contain ephemeral values.", + Subject: actionRef.actionNode.Config.DeclRange.Ptr(), + }) + return + } + } + provider, _, err := getProvider(ctx, actionRef.actionNode.ResolvedProvider) if err != nil { diags = diags.Append(&hcl.Diagnostic{ diff --git a/internal/terraform/transform_action_diff.go b/internal/terraform/transform_action_diff.go index 781b772712..f652442ce3 100644 --- a/internal/terraform/transform_action_diff.go +++ b/internal/terraform/transform_action_diff.go @@ -53,8 +53,7 @@ func (t *ActionDiffTransformer) Transform(g *Graph) error { for _, atn := range atns { if destroy { if n, ok := atn.(*NodeDestroyResourceInstance); ok { - // FIXME: this doesn't deal with deposed or forget instances - n.actionTriggers = append(n.actionTriggers, &actionTriggerApplyInstance{ + n.actionApplyTriggers = append(n.actionTriggers, &actionTriggerApplyInstance{ ActionInvocation: ai, actionNode: actionConfig, }) @@ -65,7 +64,7 @@ func (t *ActionDiffTransformer) Transform(g *Graph) error { } if n, ok := atn.(*NodeApplyableResourceInstance); ok { - n.actionTriggers = append(n.actionTriggers, &actionTriggerApplyInstance{ + n.actionApplyTriggers = append(n.actionApplyTriggers, &actionTriggerApplyInstance{ ActionInvocation: ai, actionNode: actionConfig, }) From c0e2998dded19091d6728ddc28f38e640e6ae8c0 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 3 Jun 2026 14:35:40 -0400 Subject: [PATCH 4/7] some provider transformer refactoring The nested maps for requested providers weren't required, and the recent refactoring made it more apparent that the nested would have been overwritten even if multiple providers were allowed. Action providers were not being connected, and because we need to avoid using them as resolved providers, we need to connect those in a separate loop. For now we can accomplish that by capturing the existing loop as a closure, and running in twice for each map. --- internal/terraform/transform_provider.go | 161 +++++++++++++---------- 1 file changed, 88 insertions(+), 73 deletions(-) diff --git a/internal/terraform/transform_provider.go b/internal/terraform/transform_provider.go index 228c6e5a18..c9196ed9fd 100644 --- a/internal/terraform/transform_provider.go +++ b/internal/terraform/transform_provider.go @@ -171,25 +171,20 @@ func (t *ProviderTransformer) Transform(g *Graph) error { var diags tfdiags.Diagnostics - // To start, we'll collect the _requested_ provider addresses for each + // To start, we'll collect the _requested_ provider address for each // node, which we'll then resolve (handling provider inheritance, etc) in // the next step. - // Our "requested" map is from graph vertices to string representations of - // provider config addresses (for deduping) to requests. - requested := map[dag.Vertex]map[string]ProviderRef{} + requested := map[dag.Vertex]ProviderRef{} needConfigured := map[string]addrs.AbsProviderConfig{} // forActions stores provider used only for actions by a resource. These are // only to connect the resource to the correct nodes, and are not for // resolution of the resource's own provider. - forActions := map[dag.Vertex]map[string]ProviderRef{} + forActions := map[dag.Vertex][]ProviderRef{} for _, v := range g.Vertices() { if pv, ok := v.(GraphNodeActionProviderConsumer); ok { - for _, ref := range pv.ActionProviders() { - forActions[v] = make(map[string]ProviderRef) - forActions[v][ref.String()] = ref - } + forActions[v] = pv.ActionProviders() } if pv, ok := v.(GraphNodeProviderConsumer); ok { @@ -199,90 +194,110 @@ func (t *ProviderTransformer) Transform(g *Graph) error { continue } - requested[v] = make(map[string]ProviderRef) - requested[v][ref.String()] = ref + requested[v] = ref // Direct references need the provider configured as well as initialized needConfigured[ref.String()] = ref.AbsProviderConfig() } - } // Now we'll go through all the requested addresses we just collected and // figure out which _actual_ config address each belongs to, after resolving // for provider inheritance and passing. m := providerVertexMap(g) - for v, reqs := range requested { - for key, req := range reqs { - absProvider := req.AbsProviderConfig() - target := m[key] - _, ok := v.(GraphNodeModulePath) - if !ok && target == nil { - // No target and no path to traverse up from - diags = diags.Append(fmt.Errorf("%s: provider %s couldn't be found", dag.VertexName(v), absProvider)) - continue - } + // We need to run this separately for both requested and forActions maps. + // TODO: this probably shouldn't be a closure, but this is the most + // straightforward refactor from the existing nested loos + resolveProvider := func(v dag.Vertex, ref ProviderRef) GraphNodeProvider { + absProvider := ref.AbsProviderConfig() + target := m[ref.String()] - if target != nil { - log.Printf("[TRACE] ProviderTransformer: exact match for %s serving %s", absProvider, dag.VertexName(v)) - } + _, ok := v.(GraphNodeModulePath) + if !ok && target == nil { + // No target and no path to traverse up from + diags = diags.Append(fmt.Errorf("%s: provider %s couldn't be found", dag.VertexName(v), absProvider)) + return nil + } - // if we don't have a provider at this level, walk up the path looking for one, - // unless we were told to be exact. - if target == nil && !req.Resolved { - for pp, ok := absProvider.Inherited(); ok; pp, ok = pp.Inherited() { - key := pp.String() - target = m[key] - if target != nil { - log.Printf("[TRACE] ProviderTransformer: %s uses inherited configuration %s", dag.VertexName(v), pp) - break - } - log.Printf("[TRACE] ProviderTransformer: looking for %s to serve %s", pp, dag.VertexName(v)) + if target != nil { + log.Printf("[TRACE] ProviderTransformer: exact match for %s serving %s", absProvider, dag.VertexName(v)) + } + + // if we don't have a provider at this level, walk up the path looking for one, + // unless we were told to be exact. + if target == nil && !ref.Resolved { + for pp, ok := absProvider.Inherited(); ok; pp, ok = pp.Inherited() { + key := pp.String() + target = m[key] + if target != nil { + log.Printf("[TRACE] ProviderTransformer: %s uses inherited configuration %s", dag.VertexName(v), pp) + break } + log.Printf("[TRACE] ProviderTransformer: looking for %s to serve %s", pp, dag.VertexName(v)) } + } - // If this provider doesn't need to be configured then we can just - // stub it out with an init-only provider node, which will just - // start up the provider and fetch its schema. - if _, exists := needConfigured[key]; target == nil && !exists { - stubAddr := addrs.AbsProviderConfig{ - Module: addrs.RootModule, - Provider: absProvider.Provider, - } - stub := &NodeEvalableProvider{ - &NodeAbstractProvider{ - Addr: stubAddr, - }, - } - m[stubAddr.String()] = stub - log.Printf("[TRACE] ProviderTransformer: creating init-only node for %s", stubAddr) - target = stub - g.Add(target) + // If this provider doesn't need to be configured then we can just + // stub it out with an init-only provider node, which will just + // start up the provider and fetch its schema. + if _, exists := needConfigured[ref.String()]; target == nil && !exists { + stubAddr := addrs.AbsProviderConfig{ + Module: addrs.RootModule, + Provider: absProvider.Provider, } + stub := &NodeEvalableProvider{ + &NodeAbstractProvider{ + Addr: stubAddr, + }, + } + m[stubAddr.String()] = stub + log.Printf("[TRACE] ProviderTransformer: creating init-only node for %s", stubAddr) + target = stub + g.Add(target) + } + if target == nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider configuration not present", + fmt.Sprintf( + "To work with %s its original provider configuration at %s is required, but it has been removed. This occurs when a provider configuration is removed while objects created by that provider still exist in the state. Re-add the provider configuration to destroy %s, after which you can remove the provider configuration again.", + dag.VertexName(v), absProvider, dag.VertexName(v), + ), + )) + return nil + } + + // see if this is a proxy provider pointing to another concrete config + if p, ok := target.(*graphNodeProxyProvider); ok { + g.Remove(p) + target = p.Target() + } + return target + } + + for v, ref := range requested { + target := resolveProvider(v, ref) + if target == nil { + // something happened, and we already have the diags + return diags.Err() + } + + log.Printf("[DEBUG] ProviderTransformer: %q (%T) needs %s", dag.VertexName(v), v, dag.VertexName(target)) + if pv, ok := v.(GraphNodeProviderConsumer); ok { + pv.SetProvider(target.ProviderAddr()) + } + g.Connect(dag.BasicEdge(v, target)) + } + + for v, refs := range forActions { + for _, ref := range refs { + target := resolveProvider(v, ref) if target == nil { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Provider configuration not present", - fmt.Sprintf( - "To work with %s its original provider configuration at %s is required, but it has been removed. This occurs when a provider configuration is removed while objects created by that provider still exist in the state. Re-add the provider configuration to destroy %s, after which you can remove the provider configuration again.", - dag.VertexName(v), absProvider, dag.VertexName(v), - ), - )) - break - } - - // see if this is a proxy provider pointing to another concrete config - if p, ok := target.(*graphNodeProxyProvider); ok { - g.Remove(p) - target = p.Target() - } - - log.Printf("[DEBUG] ProviderTransformer: %q (%T) needs %s", dag.VertexName(v), v, dag.VertexName(target)) - if pv, ok := v.(GraphNodeProviderConsumer); ok { - pv.SetProvider(target.ProviderAddr()) + return diags.Err() } + log.Printf("[DEBUG] ProviderTransformer: %q (%T) actions need %s", dag.VertexName(v), v, dag.VertexName(target)) g.Connect(dag.BasicEdge(v, target)) } } From 58d8d50d2568eb334c746242a38f019859d28b8b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 3 Jun 2026 14:47:12 -0400 Subject: [PATCH 5/7] make sure destroy nodes return the correct action providers --- internal/terraform/action_trigger_instance_apply.go | 2 +- internal/terraform/node_resource_abstract_instance.go | 2 +- internal/terraform/node_resource_destroy.go | 2 -- internal/terraform/transform_action_diff.go | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/terraform/action_trigger_instance_apply.go b/internal/terraform/action_trigger_instance_apply.go index 4da9bd9b60..6102631a34 100644 --- a/internal/terraform/action_trigger_instance_apply.go +++ b/internal/terraform/action_trigger_instance_apply.go @@ -54,7 +54,7 @@ func (n *actionTriggerApplyInstance) Invoke(ctx EvalContext, caller addrs.Refere return diags } - configVal := cty.DynamicVal + var configVal cty.Value // fromPlan indicates we can use the entire planned value for the action, // and should not attempt to reevaluate the config. diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 5e6f01e6df..8f88bc9aa1 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -3217,7 +3217,7 @@ func (n *NodeAbstractResourceInstance) evalActionCondition(ctx EvalContext, trig func (n *NodeAbstractResourceInstance) ActionProviders() []ProviderRef { var refs []ProviderRef for _, trigger := range n.actionApplyTriggers { - refs = append(refs, ProviderRef{Addr: trigger.actionNode.ResolvedProvider, Resolved: true}) + refs = append(refs, ProviderRef{Addr: trigger.ActionInvocation.ProviderAddr, Resolved: true}) } return refs } diff --git a/internal/terraform/node_resource_destroy.go b/internal/terraform/node_resource_destroy.go index b6e99b2fb6..fef816da0a 100644 --- a/internal/terraform/node_resource_destroy.go +++ b/internal/terraform/node_resource_destroy.go @@ -21,8 +21,6 @@ import ( // destroyed. type NodeDestroyResourceInstance struct { *NodeAbstractResourceInstance - - actionTriggers []*actionTriggerApplyInstance } var ( diff --git a/internal/terraform/transform_action_diff.go b/internal/terraform/transform_action_diff.go index f652442ce3..7535ede703 100644 --- a/internal/terraform/transform_action_diff.go +++ b/internal/terraform/transform_action_diff.go @@ -53,7 +53,7 @@ func (t *ActionDiffTransformer) Transform(g *Graph) error { for _, atn := range atns { if destroy { if n, ok := atn.(*NodeDestroyResourceInstance); ok { - n.actionApplyTriggers = append(n.actionTriggers, &actionTriggerApplyInstance{ + n.actionApplyTriggers = append(n.actionApplyTriggers, &actionTriggerApplyInstance{ ActionInvocation: ai, actionNode: actionConfig, }) From a6164f2de8bfd9b39fa9878f443872294395bb41 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 3 Jun 2026 15:03:25 -0400 Subject: [PATCH 6/7] missing append assignment --- internal/terraform/node_resource_destroy_deposed.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/terraform/node_resource_destroy_deposed.go b/internal/terraform/node_resource_destroy_deposed.go index 33931ca663..cb4ce2431f 100644 --- a/internal/terraform/node_resource_destroy_deposed.go +++ b/internal/terraform/node_resource_destroy_deposed.go @@ -330,7 +330,7 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w // was successfully destroyed it will be pruned. If it was not, it will // be caught on the next run. writeDiags := n.writeResourceInstanceState(ctx, state) - diags.Append(writeDiags) + diags = diags.Append(writeDiags) if diags.HasErrors() { return diags } From d31648330ca1df6287a47e75a82952ea41ac532c Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 3 Jun 2026 16:26:18 -0400 Subject: [PATCH 7/7] improve static validation for caller Pass in a correctly-typed caller value for static validation. --- internal/terraform/node_action.go | 5 ++--- internal/terraform/node_action_validate.go | 3 ++- internal/terraform/node_resource_validate.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/terraform/node_action.go b/internal/terraform/node_action.go index fb3a258ae4..d8d9a6e344 100644 --- a/internal/terraform/node_action.go +++ b/internal/terraform/node_action.go @@ -128,7 +128,7 @@ func (n *NodeActionConfig) recordActionExpansion(ctx EvalContext) tfdiags.Diagno // Validate validates the action config, with an optional caller address if the // action is invoked from a resource action trigger. -func (n *NodeActionConfig) Validate(ctx EvalContext, caller addrs.Referenceable) tfdiags.Diagnostics { +func (n *NodeActionConfig) Validate(ctx EvalContext, caller addrs.Referenceable, callerVal cty.Value) tfdiags.Diagnostics { var diags tfdiags.Diagnostics provider, _, err := getProvider(ctx, n.ResolvedProvider) @@ -170,8 +170,7 @@ func (n *NodeActionConfig) Validate(ctx EvalContext, caller addrs.Referenceable) } scope := ctx.EvaluationScope(caller, nil, keyData) - configVal, valDiags := scope.EvalActionBlock(config, n.Schema.ConfigSchema, cty.DynamicVal) - // configVal, _, valDiags := ctx.EvaluateBlock(config, n.Schema.ConfigSchema, caller, keyData) + configVal, valDiags := scope.EvalActionBlock(config, n.Schema.ConfigSchema, callerVal) if valDiags.HasErrors() { // If there was no config block at all, we'll add a Context range to the returned diagnostic if n.Config.Config == nil { diff --git a/internal/terraform/node_action_validate.go b/internal/terraform/node_action_validate.go index d34bcf35a6..a18fc45866 100644 --- a/internal/terraform/node_action_validate.go +++ b/internal/terraform/node_action_validate.go @@ -6,6 +6,7 @@ package terraform import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/zclconf/go-cty/cty" ) // NodeValidatableAction represents an action that is used for validation only. @@ -29,5 +30,5 @@ func (n *NodeValidatableAction) Path() addrs.ModuleInstance { } func (n *NodeValidatableAction) Execute(ctx EvalContext, _ walkOperation) tfdiags.Diagnostics { - return n.Validate(ctx, nil) + return n.Validate(ctx, nil, cty.DynamicVal) } diff --git a/internal/terraform/node_resource_validate.go b/internal/terraform/node_resource_validate.go index fd5c0fe887..53a251697c 100644 --- a/internal/terraform/node_resource_validate.go +++ b/internal/terraform/node_resource_validate.go @@ -140,7 +140,7 @@ func (n *NodeValidatableResource) validateActions(ctx EvalContext) tfdiags.Diagn _, self := n.stubRepetitionData() for _, ref := range actions.Iter() { - diags = diags.Append(ref.actionNode.Validate(ctx, self)) + diags = diags.Append(ref.actionNode.Validate(ctx, self, cty.UnknownVal(n.Schema.Body.ImpliedType()))) } return diags