Action Validate Graph

[mildwonkey]

This is a decently large change, so it's split into two commits; I do recommend that you look at this commit by commit.

• 586c069 adds a NodeActionAbstract interface for use in the next commit. This is separated out so you can see that it does not impact the existing implementation
• 6314579 adds a ConcreteActionFunc so that we can tell the ConfigTransformer to create ValidateNodes vs ExpandNodes depending on the graph walk. This isn't 100% following the node resource abstract pattern; I added a DefaultConcreteAction so I didn't need to figure out why I was only getting the NodeActionAbstract in certain tests.

Fixes #

Target Release

1.14.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.

[DanielMSchmidt]

LGTM 👍

[DanielMSchmidt]

Asking from a lack of knowledge: Since we embedded the abstract node, can't we remove all the methods that the abstract node implements from the expand node?

[mildwonkey]

Yeah, thanks for the push! I was cautious while working on this (there's so much repetition in the resource code it's really hard to tell what's important and what just hasn't been cleaned up) but I removed everything but the GraphNodeDynamicExpandable methods and it's working fine!

[mildwonkey]

👋🏻 Please take note of this, reviewers, 'cuz it's weird and I'm a bit concerned (basically I think it's fine, and also I don't trust myself on that). In Action Nodes we can have a nil config, and that causes panics in a few places (evalblock, msgpack), so I'm explicitly passing in a null value of the correct type during validate. Would it make more sense to do this during the config transformer step, or earlier in config parsing (not then; I don't have the schema yet for the implied type)?

[DanielMSchmidt]

I think it's fine as is. We do the same during plan where we send the null val with the schema type as action data as well, so it seems consistent to me :)

[github-actions]

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.