actions: add invoke nodes to the graph #37521
Conversation
|
|
||
| func NewActionStart(id terraform.HookActionIdentity) Hook { | ||
| at, ok := id.ActionTrigger.(plans.LifecycleActionTrigger) | ||
| at, ok := id.ActionTrigger.(*plans.LifecycleActionTrigger) |
There was a problem hiding this comment.
Please ignore if this is in another PR, but wouldn't you need to handle InvokeActionTrigger here as well?
There was a problem hiding this comment.
I didn't notice this - I'll do the JSON outputs in a follow up PR.
| } | ||
|
|
||
| if at, ok := aiSrc.ActionTrigger.(plans.LifecycleActionTrigger); ok { | ||
| if at, ok := aiSrc.ActionTrigger.(*plans.LifecycleActionTrigger); ok { |
There was a problem hiding this comment.
I would have thought we would want to somehow serialize that it's an invoke triggered action here as well, just the absence of a lifecycle seems a bit ambiguous if we might add new ways of triggering actions in the future.
There was a problem hiding this comment.
I didn't notice this - I'll do the JSON outputs in a follow up PR.
|
|
||
| expectDiagnostics func(m *configs.Config) tfdiags.Diagnostics | ||
| }{ | ||
| "unreferenced": { |
There was a problem hiding this comment.
I think this test doesn't make sense in the context of an apply. The plan returned by this configuration and options is a no-op, and has Applyable = false. I added a check below for all the other tests that applyable is set to true, we're expecting valid plans for all the other tests, but this one just isn't valid so I took it out.
There's still the plan test equivalent that makes sure this plans successfully, but then produces a non-applyable plan.
| @@ -0,0 +1,186 @@ | |||
| // Copyright (c) HashiCorp, Inc. | |||
There was a problem hiding this comment.
I'm wondering if we can't re-use node_action_trigger_plan / apply here. From what I understand broadly speaking we are doing the same at the point where we have action addrs. It's just that there is a bit of lifecycle specific work done before in the trigger node.
There was a problem hiding this comment.
I'll dig into this a bit more in a follow up PR. I did try to merge the code paths, but essentially the logic in this node is so simple that trying to share made the whole thing more complicated overall so I felt just keeping them separate was better.
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
This PR adds new "invoke" nodes into the plan graph that are added when the
-invokeflag is used in the CLI.This PR also makes the
-invokeflag experimental, as the action config blocks are also experimental.The approach is to modify the action plan transformer so when it has actions to invoke instead of adding triggers from the configuration, it instead adds nodes based on the targeted actions. The apply operation doesn't actually need any changes, as they already trigger based on the invocations in the plan and both types of triggers (lifecycle and invoked) are put into the plan the same way.
Target Release
1.14.x
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry