Skip to content

actions: remove actions.Action global registry and replace with existing patterns #38208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mildwonkey wants to merge 2 commits into
base: main
Choose a base branch
from
Open

actions: remove actions.Action global registry and replace with existing patterns #38208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fefc847
actions: remove actions.Action global registry and replace with exist…
mildwonkey Feb 24, 2026
8ddcb5d
actions: check for deprecated values at each eval step
mildwonkey Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 0 additions & 101 deletions internal/actions/actions.go
View file
Open in desktop

This file was deleted.

9 changes: 9 additions & 0 deletions internal/configs/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,15 @@ func (m *Module) ResourceByAddr(addr addrs.Resource) *Resource {
}
}

// ActionByAddr returns the configuration for the action with the given
// address, or nil if there is no such action.
func (m *Module) ActionByAddr(addr addrs.Action) *Action {
if a, ok := m.Actions[addr.String()]; ok {
return a
}
return nil
}

func (m *Module) appendFile(file *File) hcl.Diagnostics {
var diags hcl.Diagnostics

Expand Down
43 changes: 42 additions & 1 deletion internal/instances/expander.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,20 @@ func (e *Expander) knowsResource(want addrs.AbsResource) bool {
return e.exps.knowsResource(want)
}

func (e *Expander) knowsActionInstance(want addrs.AbsActionInstance) bool {
e.mu.Lock()
defer e.mu.Unlock()

return e.exps.knowsActionInstance(want)
}

func (e *Expander) knowsAction(want addrs.AbsAction) bool {
e.mu.Lock()
defer e.mu.Unlock()

return e.exps.knowsAction(want)
}

type expanderModule struct {
moduleCalls map[addrs.ModuleCall]expansion
resources map[addrs.Resource]expansion
Expand Down Expand Up @@ -1055,7 +1069,7 @@ func (m *expanderModule) onlyActionInstances(actionAddr addrs.Action, parentAddr

// GetActionInstanceRepetitionData returns an object describing the values
// that should be available for each.key, each.value, and count.index within
// the definition block for the given resource instance.
// the definition block for the given action instance.
func (e *Expander) GetActionInstanceRepetitionData(addr addrs.AbsActionInstance) RepetitionData {
e.mu.RLock()
defer e.mu.RUnlock()
Expand Down Expand Up @@ -1100,3 +1114,30 @@ func (e *Expander) ActionInstanceKeys(addr addrs.AbsAction) (keyType addrs.Insta
}
return exp.instanceKeys()
}

func (m *expanderModule) knowsActionInstance(want addrs.AbsActionInstance) bool {
modInst := m.getModuleInstance(want.Module)
if modInst == nil {
return false
}
resourceExp := modInst.actions[want.Action.Action]
if resourceExp == nil {
return false
}
_, knownKeys, _ := resourceExp.instanceKeys()
for _, key := range knownKeys {
if key == want.Action.Key {
return true
}
}
return false
}

func (m *expanderModule) knowsAction(want addrs.AbsAction) bool {
modInst := m.getModuleInstance(want.Module)
if modInst == nil {
return false
}
_, ret := modInst.actions[want.Action]
return ret
}
12 changes: 12 additions & 0 deletions internal/instances/set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,15 @@ func (s Set) HasResource(want addrs.AbsResource) bool {
func (s Set) InstancesForModule(modAddr addrs.Module, includeDirectOverrides bool) []addrs.ModuleInstance {
return s.exp.expandModule(modAddr, true, includeDirectOverrides)
}

// HasActionInstance returns true if and only if the set contains the actions
// instance with the given address.
func (s Set) HasActionInstance(want addrs.AbsActionInstance) bool {
return s.exp.knowsActionInstance(want)
}

// HasAction returns true if and only if the set contains the actions with
// the given address, even if that action has no instances.
func (s Set) HasAction(want addrs.AbsAction) bool {
return s.exp.knowsAction(want)
}
12 changes: 10 additions & 2 deletions internal/terraform/context_validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2870,6 +2870,14 @@ func TestContext2Validate_deprecatedAttr(t *testing.T) {
"main.tf": `
resource "aws_instance" "test" {
}
resource "aws_instance" "other" {
lifecycle {
action_trigger {
events = [ before_create ]
actions = [ action.aws_register.example]
}
}
}
action "aws_register" "example" {
config {
host = aws_instance.test.foo
Expand All @@ -2884,8 +2892,8 @@ func TestContext2Validate_deprecatedAttr(t *testing.T) {
Detail: `Deprecated resource attribute "foo" used. Refer to the provider documentation for details.`,
Subject: &hcl.Range{
Filename: filepath.Join(c.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 6, Column: 24, Byte: 152},
End: hcl.Pos{Line: 6, Column: 45, Byte: 173},
Start: hcl.Pos{Line: 14, Column: 24, Byte: 328},
End: hcl.Pos{Line: 14, Column: 45, Byte: 349},
},
})
},
Expand Down
2 changes: 0 additions & 2 deletions internal/terraform/context_walk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"log"
"time"

"github.com/hashicorp/terraform/internal/actions"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/configs"
Expand Down Expand Up @@ -201,7 +200,6 @@ func (c *Context) graphWalker(graph *Graph, operation walkOperation, opts *graph
PlanTimestamp: opts.PlanTimeTimestamp,
functionResults: opts.FunctionResults,
Forget: opts.Forget,
Actions: actions.NewActions(),
Deprecations: deprecation.NewDeprecations(),
}
}
6 changes: 0 additions & 6 deletions internal/terraform/eval_context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/terraform/internal/actions"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/configs"
Expand Down Expand Up @@ -219,11 +218,6 @@ type EvalContext interface {
// only allowed in the context of a destroy plan.
Forget() bool

// Actions returns the actions object that tracks all of the action
// declarations and their instances that are available in this
// EvalContext.
Actions() *actions.Actions

// Deprecations returns the deprecations object that tracks meta-information
// about deprecation, e.g. which module calls suppress deprecation warnings.
Deprecations() *deprecation.Deprecations
Expand Down
6 changes: 0 additions & 6 deletions internal/terraform/eval_context_builtin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/function"

"github.com/hashicorp/terraform/internal/actions"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/configs"
Expand Down Expand Up @@ -93,7 +92,6 @@ type BuiltinEvalContext struct {
InstanceExpanderValue *instances.Expander
MoveResultsValue refactoring.MoveResults
OverrideValues *mocking.Overrides
ActionsValue *actions.Actions
DeprecationsValue *deprecation.Deprecations
}

Expand Down Expand Up @@ -663,10 +661,6 @@ func (ctx *BuiltinEvalContext) ClientCapabilities() providers.ClientCapabilities
}
}

func (ctx *BuiltinEvalContext) Actions() *actions.Actions {
return ctx.ActionsValue
}

func (ctx *BuiltinEvalContext) Deprecations() *deprecation.Deprecations {
return ctx.DeprecationsValue
}
9 changes: 0 additions & 9 deletions internal/terraform/eval_context_mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/convert"

"github.com/hashicorp/terraform/internal/actions"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/configs"
Expand Down Expand Up @@ -169,9 +168,6 @@ type MockEvalContext struct {
ForgetCalled bool
ForgetValues bool

ActionsCalled bool
ActionsState *actions.Actions

DeprecationCalled bool
DeprecationState *deprecation.Deprecations
}
Expand Down Expand Up @@ -451,11 +447,6 @@ func (ctx *MockEvalContext) ClientCapabilities() providers.ClientCapabilities {
}
}

func (c *MockEvalContext) Actions() *actions.Actions {
c.ActionsCalled = true
return c.ActionsState
}

func (c *MockEvalContext) Deprecations() *deprecation.Deprecations {
c.DeprecationCalled = true
if c.DeprecationState != nil {
Expand Down
3 changes: 0 additions & 3 deletions internal/terraform/graph_walk_context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"sync"
"time"

"github.com/hashicorp/terraform/internal/actions"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/collections"
Expand Down Expand Up @@ -57,7 +56,6 @@ type ContextGraphWalker struct {
// only allowed in the context of a destroy plan.
Forget bool

Actions *actions.Actions
Deprecations *deprecation.Deprecations

// This is an output. Do not set this, nor read it while a graph walk
Expand Down Expand Up @@ -146,7 +144,6 @@ func (w *ContextGraphWalker) EvalContext() EvalContext {
Evaluator: evaluator,
OverrideValues: w.Overrides,
forget: w.Forget,
ActionsValue: w.Actions,
DeprecationsValue: w.Deprecations,
}

Expand Down
39 changes: 1 addition & 38 deletions internal/terraform/node_action_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
package terraform

import (
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/lang/langrefs"
Expand Down Expand Up @@ -42,46 +40,11 @@ func (n *NodeAbstractActionInstance) Path() addrs.ModuleInstance {
}

func (n *NodeAbstractActionInstance) Execute(ctx EvalContext, _ walkOperation) tfdiags.Diagnostics {
var diags tfdiags.Diagnostics

deferrals := ctx.Deferrals()
if deferrals.DeferralAllowed() && deferrals.ShouldDeferAction(n.Dependencies) {
deferrals.ReportActionDeferred(n.Addr, providers.DeferredReasonDeferredPrereq)
return diags
}

// This should have been caught already
if n.Schema == nil {
panic("NodeActionDeclarationInstance.Execute called without a schema")
}

allInsts := ctx.InstanceExpander()
keyData := allInsts.GetActionInstanceRepetitionData(n.Addr)

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(), nil, keyData)

diags = diags.Append(configDiags)
if configDiags.HasErrors() {
return 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()))

if diags.HasErrors() {
return diags
}
}

ctx.Actions().AddActionInstance(n.Addr, configVal, n.ResolvedProvider)
Copy link
Member

@jbardin jbardin Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this node's only purpose was to record the config in Actions(). These diags should probably come from somewhere else now

Copy link
Contributor Author

@mildwonkey mildwonkey Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was a good prompt to copy the deprecations bits around.

This caused a slight change in the deprecation tests, specific to deprecations that are caught at plan/apply: the action has to be actually triggered for the deprecation to be found, since we only evaluate* the configuration in the node_action_trigger_instance_plan/apply (*re-evaluate after Validate, at least)

The other thing this Execute does is check for the action deferral. I'm not going to change anything else there right now, that's not surviving the refactor anyway.

Copy link
Member

@jbardin jbardin Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, deprecations are somewhat dynamic, and can't always be caught without the actual data being used. While instances are only evaluated if they are triggered, there might be a place to evaluate the config in a validate context somewhere, either within resource validation or maybe a specific node (but I'd rather not have the validation graph structured differently if we can help it)

Copy link
Contributor Author

@mildwonkey mildwonkey Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding this to the list for the refactor - I can look into attaching action configs to resource nodes during validate just for validation purposes.

Unless this was something you wanted me to fix in this PR? I can also revert this change and leave this node's Execute as-is, either is fine with me!

return diags
return nil
}

// GraphNodeReferenceable
Expand Down
Loading