Two computed values on a resource with one planmodifier results in unexpected new value

[mvantellingen]

Module version

github.com/hashicorp/go-uuid v1.0.3
github.com/hashicorp/terraform-plugin-framework v1.0.1
github.com/hashicorp/terraform-plugin-go v0.14.2
github.com/hashicorp/terraform-plugin-log v0.7.0
github.com/hashicorp/terraform-plugin-sdk/v2 v2.23.0

Relevant provider source code

See the repository at https://github.com/mvantellingen/terraform-pf-testcase/tree/computed-atts for reproducing the issue. Resource schema defined at https://github.com/mvantellingen/terraform-pf-testcase/blob/computed-atts/hashicups/order_resource.go#L53

Expected Behavior

The end-goal here is to implement default values. In this case the my_default_boolean should always default to false since that is the default for the endpoint. This might also be a different primitive value.

Actual Behavior

When running the test I receive the following error:

Error: Provider produced inconsistent result after apply

When applying changes to hashicups_order.test, provider
"provider[\"registry.terraform.io/hashicorp/hashicups\"]" produced an
unexpected new value: .computed_value: was cty.NumberIntVal(1), but now
cty.NumberIntVal(2).

This is a bug in the provider, which should be reported in the provider's own
issue tracker.

ComputedValue here is a dummy value. In the real scenario it is a version identifier that gets increment with each change in the remote resource.

Steps to Reproduce

1. git clone https://github.com/mvantellingen/terraform-pf-testcase
2. git checkout computed-atts
3. make testacc

[bflad]

Hi again @mvantellingen 👋 The direct reason for the Terraform error is because the generated plan is:

2023-01-10T12:31:37.960-0500 [TRACE] sdk.helper_resource: Created plan with changes:
  test_terraform_plan=
  | 
  | Terraform used the selected providers to generate the following execution
  | plan. Resource actions are indicated with the following symbols:
  |   ~ update in-place
  | 
  | Terraform will perform the following actions:
  | 
  |   # hashicups_order.test will be updated in-place
  |   ~ resource "hashicups_order" "test" {
  |         id                 = "f555e6a0-fc47-9ca3-97a2-66b3e1018992"
  |       ~ my_default_boolean = true -> false
  |         # (2 unchanged attributes hidden)
  |     }
  | 
  | Plan: 0 to add, 1 to change, 0 to destroy.
   test_name=TestAccOrderResourceCrash test_step_number=2 test_terraform_path=/opt/homebrew/bin/terraform test_working_directory=/var/folders/f3/2mhr8hkx72z9dllv0ry81zm40000gq/T/plugintest614209255

The computed_value attribute is not showing in the plan changing to unknown (known after apply) or known as changing to 2. Normally when there are changes between the prior state and plan, the framework should be automatically marking unconfigured Computed attributes without the UseStateForUnknown() plan modifier or similar as unknown, which should prevent this error. This will need additional investigation in this case.

If it is known that this particular attribute value will always be incremented by one during in-place updates, a plan modifier can be added that signals to the resource plan (and any potential downstream references to that attribute) about the expected value change. This would prevent downstream references from also showing unknown (known after apply) in plans. Having something like this in place would hopefully override whatever the framework is missing in this case to prevent the original error.

[bflad]

What's strange in this case is that the PlanResourceChange RPC request data for the computed_value attribute is:

• Config: null
• PriorState: 1
• ProposedNewState: 1

Which does indeed match the plan UI output. This may be due to the fact that the only proposed change is to an Optional+Computed attribute, which imposes some special handling in Terraform. I will need to followup with the Terraform maintainers to get some clarifications on the expected behavior here.

Updating the Required attribute in the second test step, e.g.

			{
				Config: providerConfig + `
					resource "hashicups_order" "test" {
						my_boolean = true
						my_default_boolean = true
					}
				`,
				Check: resource.ComposeAggregateTestCheckFunc(
					resource.TestCheckResourceAttr("hashicups_order.test", "computed_value", "2"),
					resource.TestCheckResourceAttr("hashicups_order.test", "my_default_boolean", "true"),
				),
			},

Does do the correct planning behavior according to the resource definition:

2023-01-10T13:25:31.960-0500 [TRACE] sdk.helper_resource: Created plan with changes: test_terraform_path=/opt/homebrew/bin/terraform test_working_directory=/var/folders/f3/2mhr8hkx72z9dllv0ry81zm40000gq/T/plugintest133313823
  test_terraform_plan=
  | 
  | Terraform used the selected providers to generate the following execution
  | plan. Resource actions are indicated with the following symbols:
  |   ~ update in-place
  | 
  | Terraform will perform the following actions:
  | 
  |   # hashicups_order.test will be updated in-place
  |   ~ resource "hashicups_order" "test" {
  |       ~ computed_value     = 1 -> (known after apply)
  |       ~ id                 = "9f96e3ed-83c8-7ed9-e54f-945f8980d334" -> (known after apply)
  |       ~ my_boolean         = false -> true
  |         # (1 unchanged attribute hidden)
  |     }
  | 
  | Plan: 0 to add, 1 to change, 0 to destroy.
   test_name=TestAccOrderResourceCrash test_step_number=2

[bflad]

I think I narrowed down the internal Terraform plan data logic to the following:

• If an attribute is Optional+Computed and the configuration value is null, the proposed new state (plan) value will be the prior state value
• If an attribute is Optional+Computed and the configuration value is not null, the proposed new state (plan) value will be the configuration value
• If an attribute is Computed, the proposed new state (plan) value will be the prior state value

This means that between your two test step configurations, that the entire prior state will exactly match the entire proposed new state (plan). This defeats the framework logic for Computed unknown marking that compares the entire prior state to the entire proposed new state (plan), which occurs before attribute plan modifiers and resource-level plan modification.

😖 This leaves us in a particular bind from a framework perspective. We've held off running attribute/resource plan modification prior to the unknown marking because it could heavily impact how attribute plan modifiers need to be coded (#183). One potential path forward might be to introduce separate, native "default value" handling for attributes, which does run before the unknown value marking. In this situation then, the prior state would mismatch the proposed new state due to the value being updated and therefore the unknown marking would occur. We'll want to followup with the Terraform to ensure our understanding of this situation is correct though first before embarking on that endeavor.

---

All this being said, if you are not able to implement a custom attribute plan modifier that returns a known value while planning in-place updates, then that logic can at least mark the value explicitly as unknown to prevent the Terraform error, similar to what the framework should've already done.

[mvantellingen]

Hi @bflad, thanks again for the detailed analysis. Really interesting to read.

My take is that it indeed errors incorrectly when the only change in the plan is that of a computed/optional value.

If it is known that this particular attribute value will always be incremented by one during in-place updates, a plan modifier can be added that signals to the resource plan (and any potential downstream references to that attribute) about the expected value change.

It is unfortunately not a known computed value, and I don't want to take assumptions there since the remote API should be the source of truth for this value.

All this being said, if you are not able to implement a custom attribute plan modifier that returns a known value while planning in-place updates, then that logic can at least mark the value explicitly as unknown to prevent the Terraform error, similar to what the framework should've already done.

I created a modifier for the computed_value to force it to always be unknown as a test. However then I end up with an error during test that the plan is not empty (After applying this test step, the plan was not empty.)

I'll see if I can stop using the modifiers for default values for now and if there are other workarounds there.

[bflad]

This should be covered by #668, which introduces new resource default value handling before the unknown marking, which will release with terraform-plugin-framework version 1.2.0 🔜 . If there are further bug reports, feature requests, or documentation suggestions with resource default value handling, please raise a new issue. 👍 Otherwise if the use case is not fully covered, #605 may be helpful here.

[github-actions]

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.