-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
variable: use ForceNew for key changes on sensitive variables #175
Conversation
Stumbled across https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/helper/schema?tab=doc#ResourceDiff.ForceNew, will update this to only |
Ok, this is now ready for review. I've added a test that updates a Full acceptance test output:
|
Any chance I can get a review here? I have another variable I'm hoping to update. This time it's the Datadog provider, which changed its recommended env var: https://www.terraform.io/docs/providers/datadog/index.html Not urgent, so it'd be nice to handle this here rather than having to |
Hi @bendrucker, thanks for taking the time to submit this PR. It looks good to me! I'll make sure the tests still pass and then merge it. |
Thank you, much appreciated! |
Description
Scenario: A user creates a
tfe_variable
withsensitive = true
but sets the wrong key. They go back later to set the correct key, changing it in place in the Terraform configuration.Expected: The variable is updated
Result: An API error that causes the resource update to fail
I can imagine why this limitation exists for sensitive (i.e. transit-encrypted) values.
While it's possible to update non-sensitive values in place, split behavior seems confusing and the code involved would be a lot more complex (i.e. conditionally callingDelete
andCreate
). There's no way to tell a user that a delete + create is happening other thanForceNew
.Edit: This should be using the
ForceNew
methodCloses #170
Testing plan
Output from acceptance tests