Forcenew sensitive

[uturunku1]

Description

This PR resolves this GitHub issue: #317

Replicate the Issue

1. To replicate this issue, and prior to pulling changes from this branch, create a variable in your terraform config that has sensitive param set to true. For example:

resource "tfe_variable" "foobar" {
  key          = "some-key-name"
  value        = "some-value"
  description  = "some-description"
  category     = "terraform"
  workspace_id = "some-workspace-id"
  sensitive = true
}

2. Run terraform apply to create the variable
3. Then make a change to this variable configuration by changing sensitive = true to false
4. Run terraform apply and terraform will request an update in place:

5. When the prompt asks "Do you want to perform these actions?", enter the value "yes"
6. You should see this error pop up:

Solution

We have previously used CustomizeDiff func to check if the value of the key parameter has changed:

terraform-provider-tfe/tfe/resource_tfe_variable.go

Line 88 in 0259f1b

CustomizeDiff: customdiff.ForceNewIf("key", func(_ context.Context, d *schema.ResourceDiff, m interface{}) bool {

If it has changed and the sensitive param was set to true, terraform forces a replacement, which means it will destroy the variable that owns that key first and then it will create a new resource using the diff info. This is accomplished with the forceNew method from the plugin SDK:

https://github.com/hashicorp/terraform-plugin-sdk/blob/56627b161e2cf04e9350e77fc9c303ecbae9c5bc/helper/schema/resource_diff.go#L340

Rather than terraform attempting to "update in place" when a sensitive is changed to false, seems more convenient to reuse this customizeDiff func to recreate this resource. Right now users who are changing sensitive to false are forced to manually delete their resource and add a new one anyways, so we might as well force the recreation for them.

Output from acceptance tests

To smoke test, pull the changes from this branch locally, and follow the instructions to build the provider in your dev environment: https://github.com/hashicorp/terraform-provider-tfe#manually-building-the-provider.

Cautious note: do not configure a remote execution mode when testing the changes because the TFC workers may attempt to use their own version of the provider instead of the one you built locally.

Now you can re-do the steps from the above section "Replicate the Issue" up to step number 4. Except that this time, after running terraform apply, you'll see a "force replacement" message instead of "update in-place":

After you enter "yes" for the prompt, the recreation of the resource should succeed:

Acceptance tests can be run locally:

$ TESTARGS="-run TestAccTFEWorkspace" make testacc 

...

Or just look at the output of this test in CircleCi :)

[sebasslash]

should the check here be d.HasChange("key") && d.Get("sensitive").(bool) ? I'm basing this off the comment you left... feel free to clarify here.

[uturunku1]

@sebasslash taking a look to https://github.com/hashicorp/terraform-plugin-sdk/blob/56627b161e2cf04e9350e77fc9c303ecbae9c5bc/helper/schema/resource_diff.go#L341, I see that ForceNew is also checking for d.HasChange("key"), which, at the beginning, I thought it would take care of that d.HasChange("key") check for me, but! Now I realize this would return an error for the poor customer, which is something I want to avoid. So, you are right, I should do that check myself too, before running d.ForceNew("key")

[sebasslash]

When ForceNew() is called and the key specified has not changed it will throw an error which is not something we want. However if this is intended behavior (which I doubt), we could add a comment then for readability, something like: // We don't explicitly check "key" param has changed because ForceNew() takes care of this check.

My money is on preventing this error from being thrown in the first place so I would go with the preemptive check.

[sebasslash]

You should return d.ForceNew("sensitive") as that function returns an error and you want to capture that if it happens.

[uturunku1]

Indeed!