From 61f3ab20f464b3adfd2e00839fb94331bff1d3f0 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Thu, 31 Jul 2025 12:59:55 +0200 Subject: [PATCH] actions: use unexpanded action nodes to deduct provider configuration --- .../terraform/context_apply_action_test.go | 124 ++++++++++++++---- .../terraform/context_plan_actions_test.go | 108 ++++++++++++++- internal/terraform/node_action.go | 11 +- internal/terraform/node_action_apply.go | 6 +- .../terraform/node_resource_apply_instance.go | 6 +- internal/terraform/node_resource_plan.go | 19 ++- internal/terraform/transform_provider.go | 20 ++- 7 files changed, 240 insertions(+), 54 deletions(-) diff --git a/internal/terraform/context_apply_action_test.go b/internal/terraform/context_apply_action_test.go index 201fdce158..0ef5d9000c 100644 --- a/internal/terraform/context_apply_action_test.go +++ b/internal/terraform/context_apply_action_test.go @@ -436,6 +436,52 @@ resource "test_object" "b" { }), }}, }, + + "aliased provider": { + module: map[string]string{ + "main.tf": ` +provider "act" { + alias = "aliased" +} +action "act_unlinked" "hello" { + provider = act.aliased +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.act_unlinked.hello] + } + } +} +`, + }, + expectInvokeActionCalled: true, + }, + + "non-default namespace provider": { + module: map[string]string{ + "main.tf": ` +terraform { + required_providers { + ecosystem = { + source = "danielmschmidt/ecosystem" + } + } +} +action "ecosystem_unlinked" "hello" {} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.ecosystem_unlinked.hello] + } + } +} +`, + }, + expectInvokeActionCalled: true, + }, } { t.Run(name, func(t *testing.T) { if tc.toBeImplemented { @@ -463,6 +509,35 @@ resource "test_object" "b" { }, } + invokeActionFn := func(req providers.InvokeActionRequest) providers.InvokeActionResponse { + invokeActionCalls = append(invokeActionCalls, req) + if tc.callingInvokeReturnsDiagnostics != nil && len(tc.callingInvokeReturnsDiagnostics(req)) > 0 { + return providers.InvokeActionResponse{ + Diagnostics: tc.callingInvokeReturnsDiagnostics(req), + } + } + + defaultEvents := []providers.InvokeActionEvent{} + defaultEvents = append(defaultEvents, providers.InvokeActionEvent_Progress{ + Message: "Hello world!", + }) + defaultEvents = append(defaultEvents, providers.InvokeActionEvent_Completed{}) + + events := defaultEvents + if len(tc.events) > 0 { + events = tc.events + } + + return providers.InvokeActionResponse{ + Events: func(yield func(providers.InvokeActionEvent) bool) { + for _, event := range events { + if !yield(event) { + return + } + } + }, + } + } actionProvider := &testing_provider.MockProvider{ GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ Actions: map[string]providers.ActionSchema{ @@ -481,42 +556,39 @@ resource "test_object" "b" { }, ResourceTypes: map[string]providers.Schema{}, }, - InvokeActionFn: func(req providers.InvokeActionRequest) providers.InvokeActionResponse { - invokeActionCalls = append(invokeActionCalls, req) - - if tc.callingInvokeReturnsDiagnostics != nil && len(tc.callingInvokeReturnsDiagnostics(req)) > 0 { - return providers.InvokeActionResponse{ - Diagnostics: tc.callingInvokeReturnsDiagnostics(req), - } - } - - defaultEvents := []providers.InvokeActionEvent{} - defaultEvents = append(defaultEvents, providers.InvokeActionEvent_Progress{ - Message: "Hello world!", - }) - defaultEvents = append(defaultEvents, providers.InvokeActionEvent_Completed{}) + InvokeActionFn: invokeActionFn, + } - events := defaultEvents - if len(tc.events) > 0 { - events = tc.events - } + ecosystem := &testing_provider.MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + Actions: map[string]providers.ActionSchema{ + "ecosystem_unlinked": { + ConfigSchema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "attr": { + Type: cty.String, + Optional: true, + }, + }, + }, - return providers.InvokeActionResponse{ - Events: func(yield func(providers.InvokeActionEvent) bool) { - for _, event := range events { - if !yield(event) { - return - } - } + Unlinked: &providers.UnlinkedAction{}, }, - } + }, + ResourceTypes: map[string]providers.Schema{}, }, + InvokeActionFn: invokeActionFn, } ctx := testContext2(t, &ContextOpts{ Providers: map[addrs.Provider]providers.Factory{ addrs.NewDefaultProvider("test"): testProviderFuncFixed(testProvider), addrs.NewDefaultProvider("act"): testProviderFuncFixed(actionProvider), + { + Type: "ecosystem", + Namespace: "danielmschmidt", + Hostname: addrs.DefaultProviderRegistryHost, + }: testProviderFuncFixed(ecosystem), }, }) diff --git a/internal/terraform/context_plan_actions_test.go b/internal/terraform/context_plan_actions_test.go index 98169e8143..ab4888428d 100644 --- a/internal/terraform/context_plan_actions_test.go +++ b/internal/terraform/context_plan_actions_test.go @@ -986,7 +986,6 @@ resource "other_object" "a" { }, "provider is within module": { - toBeImplemented: true, module: map[string]string{ "main.tf": ` module "mod" { @@ -1034,6 +1033,89 @@ resource "other_object" "a" { } }, }, + + "non-default provider namespace": { + module: map[string]string{ + "main.tf": ` +terraform { + required_providers { + ecosystem = { + source = "danielmschmidt/ecosystem" + } + } +} +action "ecosystem_unlinked" "hello" {} +resource "other_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.ecosystem_unlinked.hello] + } + } +} +`, + }, + + assertPlan: func(t *testing.T, p *plans.Plan) { + if len(p.Changes.ActionInvocations) != 1 { + t.Fatalf("expected 1 action in plan, got %d", len(p.Changes.ActionInvocations)) + } + + action := p.Changes.ActionInvocations[0] + if action.Addr.String() != "action.ecosystem_unlinked.hello" { + t.Fatalf("expected action address to be 'action.ecosystem_unlinked.hello', got '%s'", action.Addr) + } + + if !action.TriggeringResourceAddr.Equal(mustResourceInstanceAddr("other_object.a")) { + t.Fatalf("expected action to have triggering resource address 'other_object.a', but it is %s", action.TriggeringResourceAddr) + } + + if action.ProviderAddr.Provider.Namespace != "danielmschmidt" { + t.Fatalf("expected action to have the namespace 'danielmschmidt', got '%s'", action.ProviderAddr.Provider.Namespace) + } + }, + }, + + "aliased provider": { + module: map[string]string{ + "main.tf": ` +provider "test" { + alias = "aliased" +} +action "test_unlinked" "hello" { + provider = test.aliased +} +resource "other_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_unlinked.hello] + } + } +} +`, + }, + expectPlanActionCalled: true, + + assertPlan: func(t *testing.T, p *plans.Plan) { + if len(p.Changes.ActionInvocations) != 1 { + t.Fatalf("expected 1 action in plan, got %d", len(p.Changes.ActionInvocations)) + } + + action := p.Changes.ActionInvocations[0] + if action.Addr.String() != "action.test_unlinked.hello" { + t.Fatalf("expected action address to be 'action.test_unlinked.hello', got '%s'", action.Addr) + } + + if !action.TriggeringResourceAddr.Equal(mustResourceInstanceAddr("other_object.a")) { + t.Fatalf("expected action to have triggering resource address 'other_object.a', but it is %s", action.TriggeringResourceAddr) + } + + if action.ProviderAddr.Alias != "aliased" { + t.Fatalf("expected action to have a provider alias of 'aliased', got '%s'", action.ProviderAddr.Alias) + } + }, + }, } { t.Run(name, func(t *testing.T) { if tc.toBeImplemented { @@ -1126,6 +1208,25 @@ resource "other_object" "a" { }, } + ecosystem := &testing_provider.MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + Actions: map[string]providers.ActionSchema{ + "ecosystem_unlinked": { + ConfigSchema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "attr": { + Type: cty.String, + Optional: true, + }, + }, + }, + + Unlinked: &providers.UnlinkedAction{}, + }, + }, + }, + } + if tc.planActionResponse != nil { p.PlanActionResponse = *tc.planActionResponse } @@ -1136,6 +1237,11 @@ resource "other_object" "a" { // catch the error long before anything happens. addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), addrs.NewDefaultProvider("other"): testProviderFuncFixed(other), + { + Type: "ecosystem", + Namespace: "danielmschmidt", + Hostname: addrs.DefaultProviderRegistryHost, + }: testProviderFuncFixed(ecosystem), }, }) diff --git a/internal/terraform/node_action.go b/internal/terraform/node_action.go index 7e891c11cc..69f34a5616 100644 --- a/internal/terraform/node_action.go +++ b/internal/terraform/node_action.go @@ -183,16 +183,17 @@ func (n *nodeExpandActionDeclaration) ProvidedBy() (addrs.ProviderConfig, bool) return n.ResolvedProvider, true } - return addrs.AbsProviderConfig{ - Provider: n.Provider(), - Module: n.ModulePath(), + // Since we always have a config, we can use it + relAddr := n.Config.ProviderConfigAddr() + return addrs.LocalProviderConfig{ + LocalName: relAddr.LocalName, + Alias: relAddr.Alias, }, false } // GraphNodeProviderConsumer func (n *nodeExpandActionDeclaration) Provider() addrs.Provider { - // TODO: Handle provider field - return addrs.ImpliedProviderForUnqualifiedType(n.Addr.Action.ImpliedProvider()) + return n.Config.Provider } // GraphNodeProviderConsumer diff --git a/internal/terraform/node_action_apply.go b/internal/terraform/node_action_apply.go index 7cdd23b78c..4e9ab0bc5b 100644 --- a/internal/terraform/node_action_apply.go +++ b/internal/terraform/node_action_apply.go @@ -193,10 +193,10 @@ func (n *nodeActionApply) References() []*addrs.Reference { return refs } -func (n *nodeActionApply) ActionProviders() []addrs.ProviderConfig { - ret := []addrs.ProviderConfig{} +func (n *nodeActionApply) Actions() []addrs.ConfigAction { + ret := []addrs.ConfigAction{} for _, invocation := range n.ActionInvocations { - ret = append(ret, invocation.ProviderAddr) + ret = append(ret, invocation.Addr.ConfigAction()) } return ret } diff --git a/internal/terraform/node_resource_apply_instance.go b/internal/terraform/node_resource_apply_instance.go index 27c9101231..417c67ccf4 100644 --- a/internal/terraform/node_resource_apply_instance.go +++ b/internal/terraform/node_resource_apply_instance.go @@ -475,10 +475,10 @@ func (n *NodeApplyableResourceInstance) checkPlannedChange(ctx EvalContext, plan return diags } -func (n *NodeApplyableResourceInstance) ActionProviders() []addrs.ProviderConfig { - ret := []addrs.ProviderConfig{} +func (n *NodeApplyableResourceInstance) Actions() []addrs.ConfigAction { + ret := []addrs.ConfigAction{} for _, ai := range n.beforeActionInvocations { - ret = append(ret, ai.ProviderAddr) + ret = append(ret, ai.Addr.ConfigAction()) } return ret } diff --git a/internal/terraform/node_resource_plan.go b/internal/terraform/node_resource_plan.go index cd53c3df5d..27cff18c0b 100644 --- a/internal/terraform/node_resource_plan.go +++ b/internal/terraform/node_resource_plan.go @@ -684,10 +684,10 @@ func (n *nodeExpandPlannableResource) validForceReplaceTargets(instanceAddrs []a } // GraphNodeActionProviders -func (n *nodeExpandPlannableResource) ActionProviders() []addrs.ProviderConfig { - providers := []addrs.ProviderConfig{} +func (n *nodeExpandPlannableResource) Actions() []addrs.ConfigAction { + configActions := []addrs.ConfigAction{} if n.Config == nil || n.Config.Managed == nil || n.Config.Managed.ActionTriggers == nil { - return providers + return configActions } for _, at := range n.Config.Managed.ActionTriggers { @@ -697,23 +697,20 @@ func (n *nodeExpandPlannableResource) ActionProviders() []addrs.ProviderConfig { // This should have been validated before, so we panic here panic(fmt.Sprintf("failed to parse action trigger reference %s: %s", actionRef.Traversal, diags.Err())) } - var provider addrs.Provider + var action addrs.Action // TODO: This needs to take the provider field into account once we support it if a, ok := ref.Subject.(addrs.ActionInstance); ok { - provider = addrs.ImpliedProviderForUnqualifiedType(a.Action.ImpliedProvider()) + action = a.Action } else if a, ok := ref.Subject.(addrs.Action); ok { - provider = addrs.ImpliedProviderForUnqualifiedType(a.ImpliedProvider()) + action = a } else { // This should have been validated before, so we panic here panic(fmt.Sprintf("action trigger %s refers to an invalid address", actionRef.Traversal)) } - providers = append(providers, addrs.AbsProviderConfig{ - Provider: provider, - Module: addrs.RootModule, - }) + configActions = append(configActions, action.InModule(n.ModulePath())) } } - return providers + return configActions } diff --git a/internal/terraform/transform_provider.go b/internal/terraform/transform_provider.go index 32b1b9def7..96c85b1e66 100644 --- a/internal/terraform/transform_provider.go +++ b/internal/terraform/transform_provider.go @@ -88,7 +88,7 @@ type GraphNodeProviderConsumer interface { } type GraphNodeActionProviders interface { - ActionProviders() []addrs.ProviderConfig + Actions() []addrs.ConfigAction } // ProviderTransformer is a GraphTransformer that maps resources to providers @@ -317,12 +317,22 @@ func (t *CloseProviderTransformer) Transform(g *Graph) error { continue } - for _, p := range apc.ActionProviders() { - provider, ok := p.(addrs.AbsProviderConfig) - if !ok { - return fmt.Errorf("%s failed to return a provider reference", dag.VertexName(apc)) + // For each action, find the node and ask the node for its provider configuration. + // It should have been resolved already by the ProviderTransformer. + actions := apc.Actions() + actionProviders := []addrs.AbsProviderConfig{} + for _, v2 := range g.Vertices() { + if actionNode, ok := v2.(*nodeExpandActionDeclaration); ok { + // This is an action node, so check if it matches an action + for _, action := range actions { + if actionNode.ActionAddr().Equal(action) { + actionProviders = append(actionProviders, actionNode.ResolvedProvider) + } + } } + } + for _, provider := range actionProviders { closer, ok := cpm[provider.String()] if !ok { return fmt.Errorf("no graphNodeCloseProvider for %s", provider)