Can't update Computed fields

[paddycarver]

Module version

main@84afb9afc9ce3e8e4777ca9024bd641ff646c363

Relevant provider source code

tfsdk.Schema{
                 Attributes: map[string]tfsdk.Attribute{
                         "last_updated": {
                                 Type:     types.StringType,
                                 Computed: true,
                         },
                  },
}

func (r resource) Update(_ context.Context, _ tfsdk.UpdateResourceRequest, resp tfsdk.UpdateResourceResponse) {
  err := resp.State.SetAttribute(tftypes.NewAttributePath().WithAttributeName("last_updated"), time.Now().String())
  if err != nil {
    panic(err)
  }
}

Expected Behavior

On update, last_updated should reflect the current time.

Actual Behavior

Teraform threw an error about inconsistent results after apply:

│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to hashicups_order.edu, provider "provider[\"registry.terraform.io/hashicorp/hashicups-pf\"]" produced an
│ unexpected new value: .last_updated: was cty.StringVal("Tuesday, 21-Sep-21 12:34:00 PDT"), but now cty.StringVal("Tuesday, 21-Sep-21
│ 17:11:04 PDT").
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

Steps to Reproduce

1. Apply a resource
2. Change any value, making it update
3. Apply the resource again

Explanation

The root cause of this is that our markComputedNilsAsUnknown transform function is just... wrong.

The way it behaves today is that any computed attribute in the proposed new state that is null gets set to unknown. Which lets the provider choose a value for it at apply time, as Computed promises. But! A wrinkle. The proposed new state will use the state value, not null, for any computed attribute with a non-null value in state:

Proposed New State: Terraform Core merges the non-null values from the configuration with any computed attribute results in the prior state to produce a combined object that includes both, to avoid each provider having to re-implement that merging logic. Will be null when planning a delete operation.

This means once these computed attributes get a value in state, we no longer mark them as unknown. We even have a test that confirms that behavior.

However, this is non-obvious behavior and we should instead mark the field as unknown automatically if it's null in the config and computed, so that the provider always has the option of changing it whenever Terraform will allow it. We should do this before plan modifiers are run, so providers have the opportunity to override this behavior.

[bflad]

For additional context, this also applied elsewhere in the same resource and unrelated to purely timestamp updates. For example, this schema:

"items": {
				// If Required is true, Terraform will throw error if user
				// doesn't specify value
				// If Optional is true, user can choose to supply a value
				Required: true,
				Attributes: tfsdk.ListNestedAttributes(map[string]tfsdk.Attribute{
					"quantity": {
						Type:     types.NumberType,
						Required: true,
					},
					"coffee": {
						Required: true,
						Attributes: tfsdk.SingleNestedAttributes(map[string]tfsdk.Attribute{
							"id": {
								Type:     types.NumberType,
								Required: true,
							},
							"name": {
								Type:     types.StringType,
								Computed: true,
							},
							"teaser": {
								Type:     types.StringType,
								Computed: true,
							},
							"description": {
								Type:     types.StringType,
								Computed: true,
							},
							"price": {
								Type:     types.NumberType,
								Computed: true,
							},
							"image": {
								Type:     types.StringType,
								Computed: true,
							},
						}),
					},
				}, tfsdk.ListNestedAttributesOptions{}),
			},

Was also generating additional errors in the apply output during update:

$ terraform apply
...
hashicups_order.edu: Refreshing state... [id=1]

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.edu will be updated in-place
  ~ resource "hashicups_order" "edu" {
        id           = "1"
      ~ items        = [
          ~ {          },
          # (1 unchanged element hidden)
        ]
        # (1 unchanged attribute hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Changes to Outputs:
  ~ edu_order = {
      ~ items        = [
            {
                coffee   = {
                    description = ""
                    id          = 3
                    image       = "/nomad.png"
                    name        = "Nomadicano"
                    price       = 150
                    teaser      = "Drink one today and you will want to schedule another"
                }
                quantity = 2
            },
          ~ {
              ~ coffee   = {
                  ~ id          = 1 -> 2
                    # (5 unchanged elements hidden)
                }
                # (1 unchanged element hidden)
            },
        ]
        # (2 unchanged elements hidden)
    }

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

hashicups_order.edu: Modifying... [id=1]
╷
│ Error: Provider produced inconsistent result after apply
│ 
│ When applying changes to hashicups_order.edu, provider "provider[\"registry.terraform.io/hashicorp/hashicups\"]" produced an unexpected new value: .items: was
│ cty.ListVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{"coffee":cty.ObjectVal(map[string]cty.Value{"description":cty.StringVal(""), "id":cty.NumberIntVal(3),
│ "image":cty.StringVal("/nomad.png"), "name":cty.StringVal("Nomadicano"), "price":cty.NumberIntVal(150), "teaser":cty.StringVal("Drink one today and you will want to
│ schedule another")}), "quantity":cty.NumberIntVal(2)}),
│ cty.ObjectVal(map[string]cty.Value{"coffee":cty.ObjectVal(map[string]cty.Value{"description":cty.StringVal(""), "id":cty.NumberIntVal(2),
│ "image":cty.StringVal("/packer.png"), "name":cty.StringVal("Packer Spiced Latte"), "price":cty.NumberIntVal(350), "teaser":cty.StringVal("Packed with goodness to
│ spice up your images")}), "quantity":cty.NumberIntVal(2)})}), but now null.
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵
╷
│ Error: Provider produced inconsistent result after apply
│ 
│ When applying changes to hashicups_order.edu, provider "provider[\"registry.terraform.io/hashicorp/hashicups\"]" produced an unexpected new value: .last_updated: was
│ cty.StringVal("Friday, 24-Sep-21 10:23:33 EDT"), but now cty.StringVal("Friday, 24-Sep-21 10:24:41 EDT").
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵

We spent some time investigating the behavior with the same provider using Terraform Plugin SDK version 2. That version of the framework also has the same errant behavior, but you will notice that no immediate error is thrown:

$ terraform apply
...
hashicups_order.edu: Refreshing state... [id=2]

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.edu will be updated in-place
  ~ resource "hashicups_order" "edu" {
        id = "2"

      ~ items {
            # (1 unchanged attribute hidden)

          ~ coffee {
              ~ id     = 2 -> 1
                name   = "Vaulatte"
                # (3 unchanged attributes hidden)
            }
        }
        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Changes to Outputs:
  ~ edu_order = {
      ~ items        = [
            {
                coffee   = [
                    {
                        description = ""
                        id          = 3
                        image       = "/nomad.png"
                        name        = "Nomadicano"
                        price       = 150
                        teaser      = "Drink one today and you will want to schedule another"
                    },
                ]
                quantity = 2
            },
          ~ {
              ~ coffee   = [
                  ~ {
                      ~ id          = 2 -> 1
                        # (5 unchanged elements hidden)
                    },
                ]
                # (1 unchanged element hidden)
            },
        ]
        # (2 unchanged elements hidden)
    }

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

hashicups_order.edu: Modifying... [id=2]
hashicups_order.edu: Modifications complete after 0s [id=2]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

Outputs:

edu_order = {
  "id" = "2"
  "items" = tolist([
    {
      "coffee" = tolist([
        {
          "description" = ""
          "id" = 3
          "image" = "/nomad.png"
          "name" = "Nomadicano"
          "price" = 150
          "teaser" = "Drink one today and you will want to schedule another"
        },
      ])
      "quantity" = 2
    },
    {
      "coffee" = tolist([
        {
          "description" = ""
          "id" = 1
          "image" = "/vault.png"
          "name" = "Vaulatte"
          "price" = 200
          "teaser" = "Nothing gives you a safe and secure feeling like a Vaulatte"
        },
      ])
      "quantity" = 3
    },
  ])
  "last_updated" = tostring(null)
}

Terraform CLI has special rules for the old framework that allow this misbehavior, demoting the issue to a warning log (unless these computed values are referenced elsewhere, then the error is thrown).

2021-09-24T10:33:59.151-0400 [WARN]  Provider "provider[\"registry.terraform.io/hashicorp/hashicups\"]" produced an unexpected new value for hashicups_order.edu, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .last_updated: was null, but now cty.StringVal("Friday, 24-Sep-21 10:33:59 EDT")
      - .items[1].coffee[0].image: was cty.StringVal("/vault.png"), but now cty.StringVal("/packer.png")
      - .items[1].coffee[0].name: was cty.StringVal("Vaulatte"), but now cty.StringVal("Packer Spiced Latte")
      - .items[1].coffee[0].price: was cty.NumberIntVal(200), but now cty.NumberIntVal(350)
      - .items[1].coffee[0].teaser: was cty.StringVal("Nothing gives you a safe and secure feeling like a Vaulatte"), but now cty.StringVal("Packed with goodness to spice up your images")

This was done because the change would have been too disruptive to the existing Terraform provider ecosystem and since the old framework could not necessarily distinguish between empty (e.g. "") and missing (nil/null) values in certain scenarios.

The error behavior is expected with the new framework, which is no longer treated special. Also worth noting that the old framework did not have any implicit marking of attributes as unknown either, potentially because of this special Terraform CLI handling.

---

The new framework has two default modes to choose between in this case:

The first is "do nothing" or keeping the existing behavior, which means these errors are surfaced to provider developers and practitioners unless the provider is meticulously correct in implementation by introducing plan modifiers that set planned values for Computed only attributes to unknown or a known value as appropriate in every case.

The second is to always default Computed only attributes to unknown in plans, which will show in the plan output as OLD => (known after apply) for all of them but likely removes this unexpected error. However, the downside of this plan output is that it might be noisy for practitioners unless providers add plan modifiers that set the value of these attributes appropriately (such as copying the current state to the plan, if it is not expected to change).

---

The better experience for provider developers and practitioners in this new framework appears that it should default to being conservative and choose the second option of marking all Computed only attributes as unknown in plans by default. This is the example implementation in #176.

[bflad]

Created #180 to consider adding AttributePlanModifier helpers to ease state to plan copying.

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