search

hashicorp / terraform-plugin-framework

A next-generation framework for building Terraform providers.
https://developer.hashicorp.com/terraform/plugin/framework
Mozilla Public License 2.0
282 stars 91 forks source link

Consider Flipping Attribute Plan Modifier Algorithm from Top Down to Bottom Up #225

Closed bflad closed 1 year ago

bflad commented 2 years ago

Module version

v0.4.2

Description

As mentioned in #224, there can be some reasoning to prefer running attribute plan modifiers bottom up instead of top down which is the algorithm since the attribute plan modifier functionality was introduced. There is likely also good reasoning to also keep the existing algorithm. More complex algorithms like BFT/DFT could potentially be discussed, but this use case probably does not fit well with that type of value traversal nor require that complexity.

This issue is to track some of the analysis and discussion on various use cases, either which way. Some helpful breadcrumbs as a part of this effort may be leaving some additional design or Go documentation on the chosen algorithm and rationale.

References

paddycarver commented 2 years ago

My argument for "children first, then parent" is that the parent can then have an accurate view of the children when computing its value. As the children have no view of the parent, I'm unsure of when they would need the parent to be updated before their plan modifier runs.

bflad commented 2 years ago

I guess the optimization that occurs in the situation where a parent may remove children elements isn't the biggest concern. Adding elements is an interesting case because children plan modifiers may contain default values and such, I'm not sure how often that would occur in real world usage or how that would be resolved if the parent was resolved after children.

bflad commented 2 years ago

Another drive-by thought: what should the error handling behavior be? Currently, any failed plan modifier is able to prevent further attribute plan modifier and nested attribute plan modifiers from executing since the parent logic exits early. If the logic is flipped around, the framework would need to check that an error is a child attribute path, so as to not prevent execution of other (unrelated) attribute plan modifiers.

paddycarver commented 2 years ago

Would it, though? In the event of an error, no plan will be produced anyways. So we could always say that the first error returned during plan modification will terminate all plan modification. We'd also need to be careful, in that situation, of cascading errors; e.g., an error in a child (or unrelated parent) meaning that another plan modifier wouldn't know how to handle that value as input.

bflad commented 1 year ago

Closing this out for now, it's unlikely to be discussed more or changed before the first major version with compatibility promises.

github-actions[bot] commented 1 year ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.