Consider introducing helper methods for marking unconfigured `Computed` attributes as `<unknown>`

[austinvalle]

Module version

v1.14.1

Background

Briefly, terraform-plugin-framework (Framework) differs from the previous terraform-plugin-sdk/v2 (SDKv2) in how it handles plan modification during PlanResourceChange.

• SDKv2 has a CustomizeDiff function that providers can define to influence the plan, and relies soley on Terraform's proposed_new_state for it's plan, which often leads to data consistency problems as Terraform always suggests keeping prior state values for computed attributes, rather than marking them as <unknown> so they can be returned during apply. Since SDKv2 does not need to follow data consistency rules, Terraform allows this behavior in isolation, but often causes downstream problems with other providers when an inconsistent data value is returned during apply.
• Framework is required by Terraform to follow these data consistency rules exactly, so the planning behavior is different from SDKv2. Framework always marks computed attributes that are unconfigured as <unknown> if there is a "planned update" proposed by Terraform (determined with prior_state and the same proposed_new_state mentioned earlier).
  • Typically, providers can use the UseStateForUnknown plan modifier to "revert" this marking of <unknown> and get a similar behavior to SDKv2's plan.

Problem

One of the issues with this plan approach in Framework is that we detect a planned update before a provider has a chance to run plan modification. The rough order of operations is:

1. PlanResourceChange is called by Terraform
2. Framework applies Default values for computed attributes to proposed_new_state from Terraform
3. Framework checks if there is a planned updated (i.e. a diff between prior state and proposed_new_state w/ defaults
4. Framework runs provider-defined attribute plan modifiers
5. Framework runs provider-defined ModifyPlan
6. Framework returns the final plan result to Terraform

If any of the provider-defined logic in step #4 revert the original plan to being a "no-op", all of the Computed attributes will still be marked as unknown. So there will still be a plan, but just of computed values that were marked as unknown because Framework assumed there would be an update. (For most use-cases, this unknown marking is essential, but here it is causing us problems)

If you have a use-case like this, where the plan modification during step #4 might end in a no-op, you can use the UseStateForUnknown plan modifier to achieve that no-op behavior, but there are still cases where you might want a computed attribute to being marked as unknown. In that case you could write a ModifyPlan function like this example: https://github.com/austinvalle/terraform-provider-sandbox/blob/9f43a630d4c3d43b72ea67414ff711548bdc19b9/internal/provider/thing_resource.go#L64-L79

This example has a subtle bug itself and is thus error-prone, since you should check if the value is configured before marking as unknown. You can write a generic function to implement this, but since all underlying Terraform data is written with terraform-plugin-go, it's not easy or familiar for provider developers to use, see: https://github.com/austinvalle/terraform-provider-sandbox/blob/9f43a630d4c3d43b72ea67414ff711548bdc19b9/internal/provider/thing_resource.go#L83-L107

Proposal

Write a helper function that can safely mark a computed attribute as unknown given the correct config/plan data. This function could be placed on the tfsdk.Plan object, or perhaps just a function in a package. The function should ensure that it doesn't mark attributes that cannot be unknown, either due to the config having a value, or the schema not marking the attribute as Computed.

It might also be helpful to accept multiple attribute paths? Or having a "mark all computed" as unknown. The implementation should be similar to internal/fwserver/MarkComputedNilsAsUnknown, with the added flexibility of targeting a single attribute. It's unclear if it'd be useful to recognize default values here, since the usage should likely always be in the ModifyPlan function.