actions: when targeting a resource the associated actions should be run

[DanielMSchmidt]

When one uses the -target flag on a resource we want all of the resource to execute, including all associated actions. This also includes actions associated with dependent resources since those resources also are applied / planned.

Fixes #

Target Release

1.15.x

Rollback Plan

• If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

• This change is user-facing and I added a changelog entry.
• This change is not user-facing.

[liamcervante]

Looks good to me, will let someone else approve it though as I helped write it!

[mildwonkey]

This seems odd to me - we already have dependency transformers on resources, so this method should only add action dependencies, correct? If I'm reading it correctly, let's name this something clearer, like addTriggeredActionDependencies (addMaybeTriggeredActionDependencies?)

[DanielMSchmidt]

Yeah at this point in the graph we have already added all dependencies and such. What this does is adding the dependencies we identify for the vertex (so the ancestors being what we added so far and the children that are action trigger related is what we are adding now) to the list of targeted nodes. I'll add a more descirptive name mentioning that the targeted nodes is being changed, I think that should help

[mildwonkey]

Why is this necessary, is this another quirk of actions runtime behaviors? The targets transformer should drop all unnecessary nodes from the graph, so I would hope we wouldn't need to manage that behavior here as well as in the transformer?

[DanielMSchmidt]

We have a similar thing in the resource nodes. It is necessary because the expansion happens as part of the graph execution, therefore if you target a resource instance we need to filter out all other resource instances ad-hoc. The same applies for action triggers of not-targeted resource instances. Let me add a comment there to explain this.

[mildwonkey]

Quick comment to capture internal conversation - resources which are dependences of actions that haven't been triggered (but are in the lifecycle of the targeted resource) are getting pulled in during targeting, so we need to remove (or not add) non-triggered action dependencies from the graph.

Update: wiser terraformers than I pointed out that this behavior is actually quite correct, and I agree now that I understand how targeting works. Targeting only takes static configuration into account, and therefore will pick up all actions included in a resource's action_triggers (and therefore all of those actions' dependencies) regardless of if they are triggered by the current operation. That is for the best - if a user is targeting a resource this means that terraform will ensure all actions are "ready to go".

(update in the original comment - this behavior is correct)

[jbardin]

I think this looks correct for the current implementation as I understand it. The special cases for specific node types can be kind of fragile, but given the state of flux I don't have a better idea at the moment.

[mildwonkey]

I played with this locally a bit, and I noticed that we're no longer pulling in dependencies from non-triggered actions. Last I recall, we'd decided that was correct behavior - even if the action isn't triggered by the current (-target) operation, we do still want terraform to ensure that all possible dependencies of the targeted resource are included in the plan. I could be wrong - there's been a lot of back and forth on features - but I didn't think we wanted to lose that functionality. WDYT?