Skip to content
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

Proposal: Allow removing key from Private data #910

Closed
gdavison opened this issue Jan 25, 2024 · 3 comments · Fixed by #911
Closed

Proposal: Allow removing key from Private data #910

gdavison opened this issue Jan 25, 2024 · 3 comments · Fixed by #911
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@gdavison
Copy link

Module version

github.com/hashicorp/terraform-plugin-framework v1.5.0

Use-cases

Resource Private data may sometimes no longer be needed. There is currently no way to remove it.

Attempted Solutions

Setting a key to nil or "" causes an error, such as

The value being supplied for key "aws_lb_listener.importing" is invalid.
Please verify that the value is valid JSON.

Proposal

Add a corresponding function RemoveKey to *privatestate.ProviderData.

@gdavison gdavison added the enhancement New feature or request label Jan 25, 2024
@bflad
Copy link
Member

bflad commented Jan 25, 2024

Hey @gdavison 👋 Thank you for raising this and we should certainly support this in some manner.

As a workaround for now, does SetKey(ctx, "key", []byte("null")) work for your use case? That should at least insert a literal JSON null as the value and appears to work in my quick SetKey() unit test trial -- leaving the key with a literal null value. Not exactly the most ideal, but should operate fairly similar from your end, I would think, in that reading it would require a pointer type that would unmarshal to nil.

In terms of actual key removal, I think we could support either a new RemoveKey(key string) method and/or update the existing SetKey() so it can support nil to remove the key. The latter might actually be preferable because it is likely that other developers may try exactly what you did and expect it to work. Semantically it feels like it would be performing the same change as what a RemoveKey() method would do under the hood anyways.

@gdavison
Copy link
Author

Yes, I'm using the "null" workaround. I'm using the values true or null, so I check the content for equality instead of simple presence.

SetKey supporting nil would work

bflad added a commit that referenced this issue Jan 26, 2024
Reference: #910

This change ensures that developers can fully remove private state keys by setting the value to `nil` or `[]byte{}`. Attempting to set the value to either value would previously generate an error. The closest available workaround was to set keys to the literal JSON `null` and handle that value during unmarshaling, rather than purely checking existence via `GetKey()` returning `nil`.
@bflad bflad added this to the v1.6.0 milestone Jan 26, 2024
@bflad bflad self-assigned this Jan 26, 2024
bflad added a commit that referenced this issue Jan 26, 2024
#911)

Reference: #910

This change ensures that developers can fully remove private state keys by setting the value to `nil` or `[]byte{}`. Attempting to set the value to either value would previously generate an error. The closest available workaround was to set keys to the literal JSON `null` and handle that value during unmarshaling, rather than purely checking existence via `GetKey()` returning `nil`.
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
2 participants