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

No diff shown when a computed + optional property is removed #2152

Open
VenelinMartinov opened this issue Jul 1, 2024 · 5 comments
Open
Labels
kind/bug Some behavior is incorrect or out of spec

Comments

@VenelinMartinov
Copy link
Contributor

What happened?

Due to https://developer.hashicorp.com/terraform/plugin/sdkv2/resources/data-consistency-errors#planned-value-for-a-non-computed-attribute the plugin SDK always keeps the state value for a Computed + Optional property which was removed.

We might want to consider a bridge fix. This will diverge from TF behaviour but this behaviour is pretty bad.

Example

pulumi/pulumi-azure#1881

Output of pulumi about

.

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@VenelinMartinov VenelinMartinov added the kind/bug Some behavior is incorrect or out of spec label Jul 1, 2024
@VenelinMartinov
Copy link
Contributor Author

@t0yv0
Copy link
Member

t0yv0 commented Jul 17, 2024

We need a very high threshold for willingly diverge from TF behavior because community providers are designed with TF behavior in mind and gravitate to that. Why diverge?

@VenelinMartinov
Copy link
Contributor Author

This is a major shortcoming of the SDKv2 - it is the reason why the tf-azure provider has a policy AGAINST using Computed even for properties set by the cloud.

It is also very unintuitive behaviour. It'd be useful to at least consider how this interacts with the provider code.

@t0yv0
Copy link
Member

t0yv0 commented Jul 17, 2024

Perhaps an option that a provider author can opt into? But even then, what are the concrete use cases this would target fixing?

why the tf-azure provider has a policy AGAINST using Computed even for properties set by the cloud.

So it sounds like tf-azure is not affected, what is affected?

This comment you linked is a good reference but doesn't quite explain the rationale.

Otherwise, this may not be resolvable when the resource is implemented with terraform-plugin-sdk. Having Optional: true while also setting the attribute's Computed: true flag in the schema will also enable this SDK's behavior of keeping the prior state value if the configuration value is removed (set to null) during an update. That SDK behavior is unavoidable. This SDK will also raise an implementation error if both Computed: true and Default are set, since the value will never reset to the default value because of that behavior. If that behavior is not acceptable, this error is unavoidable until the resource is migrated to terraform-plugin-framework, which does not have implicit behaviors when enabling the Computed: true flag and instead provider developers are expected to decide whether the prior state preservation behavior should occur or not by using the UseStateForUnknown schema plan modifier.

Looks like hashicorp/terraform-plugin-sdk#1101 can be another reference. Looks like upstream stance is to migrate to Plugin Framework.

@t0yv0
Copy link
Member

t0yv0 commented Jul 17, 2024

Tangentially, the rationale for this may have something to do with __defaults mechanism in the V3 Bridge. There is uncertainty for some values whether they were introduced by the user or the provider, what's the origin of the value. Looks like this SDKv2 behavior may be assuming that an Optional+Computed value was written to the state by the provider (as Computed), and therefore should not be removed even if it is omitted from the program. A side channel like __defaults was used to try to track who wrote the value. We opted not to do it in PF leading to similar issues with unexpected non-removals. I remain slightly skeptical we can force a good design here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

2 participants