From f14baf178dfed477e0e1a43265f790da4adbda1a Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Tue, 26 Aug 2025 16:36:05 +0200 Subject: [PATCH] [TF-28590] Actions: Add support for write-only attributes (#37491) --- .../terraform/context_apply_action_test.go | 69 ++++++++++++++++++- .../terraform/context_plan_actions_test.go | 63 +++++++++++++++++ .../terraform/node_action_trigger_apply.go | 16 +++-- .../node_action_trigger_instance_plan.go | 22 +++--- 4 files changed, 148 insertions(+), 22 deletions(-) diff --git a/internal/terraform/context_apply_action_test.go b/internal/terraform/context_apply_action_test.go index 73168ea615..d0b000d1ce 100644 --- a/internal/terraform/context_apply_action_test.go +++ b/internal/terraform/context_apply_action_test.go @@ -8,6 +8,8 @@ import ( "testing" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" @@ -17,7 +19,6 @@ import ( testing_provider "github.com/hashicorp/terraform/internal/providers/testing" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" ) func TestContext2Apply_actions(t *testing.T) { @@ -29,7 +30,8 @@ func TestContext2Apply_actions(t *testing.T) { events func(req providers.InvokeActionRequest) []providers.InvokeActionEvent callingInvokeReturnsDiagnostics func(providers.InvokeActionRequest) tfdiags.Diagnostics - planOpts *PlanOpts + planOpts *PlanOpts + applyOpts *ApplyOpts expectInvokeActionCalled bool expectInvokeActionCalls []providers.InvokeActionRequest @@ -1291,6 +1293,54 @@ resource "test_object" "resource" { }), }}, }, + + "write-only attributes": { + module: map[string]string{ + "main.tf": ` +variable "attr" { + type = string + ephemeral = true +} + +resource "test_object" "resource" { + name = "hello" + lifecycle { + action_trigger { + events = [before_create] + actions = [action.act_unlinked_wo.hello] + } + } +} + +action "act_unlinked_wo" "hello" { + config { + attr = var.attr + } +} +`, + }, + expectInvokeActionCalled: true, + expectInvokeActionCalls: []providers.InvokeActionRequest{ + { + ActionType: "act_unlinked_wo", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("wo-apply"), + }), + }, + }, + planOpts: SimplePlanOpts(plans.NormalMode, InputValues{ + "attr": { + Value: cty.StringVal("wo-plan"), + }, + }), + applyOpts: &ApplyOpts{ + SetVariables: InputValues{ + "attr": { + Value: cty.StringVal("wo-apply"), + }, + }, + }, + }, } { t.Run(name, func(t *testing.T) { if tc.toBeImplemented { @@ -1360,6 +1410,19 @@ resource "test_object" "resource" { }, }, + Unlinked: &providers.UnlinkedAction{}, + }, + "act_unlinked_wo": { + ConfigSchema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "attr": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + Unlinked: &providers.UnlinkedAction{}, }, }, @@ -1412,7 +1475,7 @@ resource "test_object" "resource" { plan, diags := ctx.Plan(m, tc.prevRunState, planOpts) tfdiags.AssertNoDiagnostics(t, diags) - _, diags = ctx.Apply(plan, m, nil) + _, diags = ctx.Apply(plan, m, tc.applyOpts) if tc.expectDiagnostics != nil { tfdiags.AssertDiagnosticsMatch(t, diags, tc.expectDiagnostics(m)) } else { diff --git a/internal/terraform/context_plan_actions_test.go b/internal/terraform/context_plan_actions_test.go index 8dfd62e8e4..17494d1e0f 100644 --- a/internal/terraform/context_plan_actions_test.go +++ b/internal/terraform/context_plan_actions_test.go @@ -35,6 +35,19 @@ func TestContextPlan_actions(t *testing.T) { Unlinked: &providers.UnlinkedAction{}, } + writeOnlyUnlinkedActionSchema := providers.ActionSchema{ + ConfigSchema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "attr": { + Type: cty.String, + Optional: true, + WriteOnly: true, + }, + }, + }, + + Unlinked: &providers.UnlinkedAction{}, + } for name, tc := range map[string]struct { toBeImplemented bool @@ -1709,6 +1722,54 @@ resource "test_object" "a" { } }, }, + + "write-only attributes": { + module: map[string]string{ + "main.tf": ` +variable "attr" { + type = string + ephemeral = true +} + +resource "test_object" "resource" { + name = "hello" + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_unlinked_wo.hello] + } + } +} + +action "test_unlinked_wo" "hello" { + config { + attr = var.attr + } +} +`, + }, + planOpts: SimplePlanOpts(plans.NormalMode, InputValues{ + "attr": { + Value: cty.StringVal("wo-plan"), + }, + }), + expectPlanActionCalled: true, + assertPlan: func(t *testing.T, plan *plans.Plan) { + if len(plan.Changes.ActionInvocations) != 1 { + t.Fatalf("expected exactly one invocation, and found %d", len(plan.Changes.ActionInvocations)) + } + + ais := plan.Changes.ActionInvocations[0] + ai, err := ais.Decode(&writeOnlyUnlinkedActionSchema) + if err != nil { + t.Fatal(err) + } + + if !ai.ConfigValue.GetAttr("attr").IsNull() { + t.Fatal("should have converted ephemeral value to null in the plan") + } + }, + }, } { t.Run(name, func(t *testing.T) { if tc.toBeImplemented { @@ -1722,6 +1783,8 @@ resource "test_object" "a" { Actions: map[string]providers.ActionSchema{ "test_unlinked": unlinkedActionSchema, + "test_unlinked_wo": writeOnlyUnlinkedActionSchema, + "test_lifecycle": { ConfigSchema: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ diff --git a/internal/terraform/node_action_trigger_apply.go b/internal/terraform/node_action_trigger_apply.go index f3fb8257a4..c91b686227 100644 --- a/internal/terraform/node_action_trigger_apply.go +++ b/internal/terraform/node_action_trigger_apply.go @@ -7,7 +7,9 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/plans/objchange" "github.com/hashicorp/terraform/internal/providers" @@ -77,17 +79,16 @@ func (n *nodeActionTriggerApply) Execute(ctx EvalContext, wo walkOperation) tfdi return diags } - // We don't want to send the marks, but all marks are okay in the context of an action invocation. - unmarkedConfigValue, _ := actionData.ConfigValue.UnmarkDeep() + configValue := actionData.ConfigValue // Validate that what we planned matches the action data we have. - errs := objchange.AssertObjectCompatible(actionSchema.ConfigSchema, ai.ConfigValue, unmarkedConfigValue) + errs := objchange.AssertObjectCompatible(actionSchema.ConfigSchema, ai.ConfigValue, ephemeral.RemoveEphemeralValues(configValue)) for _, err := range errs { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Provider produced inconsistent final plan", - Detail: fmt.Sprintf("When expanding the plan for %s to include new values learned so far during apply, provider %q produced an invalid new value for %s.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", - ai.Addr, actionData.ProviderAddr.Provider.String(), tfdiags.FormatError(err)), + Detail: fmt.Sprintf("When expanding the plan for %s to include new values learned so far during apply, Terraform produced an invalid new value for %s.\n\nThis is a bug in Terraform, which should be reported.", + ai.Addr, tfdiags.FormatError(err)), Subject: n.ActionTriggerRange, }) } @@ -100,6 +101,11 @@ func (n *nodeActionTriggerApply) Execute(ctx EvalContext, wo walkOperation) tfdi ctx.Hook(func(h Hook) (HookAction, error) { return h.StartAction(hookIdentity) }) + + // 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() resp := provider.InvokeAction(providers.InvokeActionRequest{ ActionType: ai.Addr.Action.Action.Type, PlannedActionData: unmarkedConfigValue, diff --git a/internal/terraform/node_action_trigger_instance_plan.go b/internal/terraform/node_action_trigger_instance_plan.go index 5ca4f468d3..8803559ac0 100644 --- a/internal/terraform/node_action_trigger_instance_plan.go +++ b/internal/terraform/node_action_trigger_instance_plan.go @@ -7,9 +7,10 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/lang/marks" + "github.com/hashicorp/terraform/internal/lang/ephemeral" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/plans/deferring" "github.com/hashicorp/terraform/internal/providers" @@ -86,7 +87,11 @@ func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkO Addr: n.actionAddress, ProviderAddr: actionInstance.ProviderAddr, ActionTrigger: n.lifecycleActionTrigger.ActionTrigger(configs.Unknown), - ConfigValue: actionInstance.ConfigValue, + + // with resources, the provider would be expected to strip the ephemeral + // values out. with actions, we don't get the value back from the + // provider so we'll do that ourselves now. + ConfigValue: ephemeral.RemoveEphemeralValues(actionInstance.ConfigValue), } // If we already deferred an action invocation on the same resource with an earlier trigger we can defer this one as well @@ -122,18 +127,7 @@ func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkO } // We remove the marks for planning, we will record the sensitive values in the plans.ActionInvocationInstance - unmarkedConfig, pvms := actionInstance.ConfigValue.UnmarkDeepWithPaths() - // We only support sensitive marks, all other marks cause an error - _, otherMarks := marks.PathsWithMark(pvms, marks.Sensitive) - if len(otherMarks) > 0 { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Unsupported marks", - Detail: "Only sensitive marks are supported in action configuration", - Subject: &n.actionConfig.DeclRange, - }) - return diags - } + unmarkedConfig, _ := actionInstance.ConfigValue.UnmarkDeepWithPaths() resp := provider.PlanAction(providers.PlanActionRequest{ ActionType: n.actionAddress.Action.Action.Type,