hashicorp / terraform-plugin-framework

A next-generation framework for building Terraform providers.
https://developer.hashicorp.com/terraform/plugin/framework
Mozilla Public License 2.0
298 stars 92 forks source link

Document Mitigation for Empty String to Null Plans for SDK Migrations #510

Open bflad opened 1 year ago

bflad commented 1 year ago

Module version

v0.14.0

Description

In certain situations, terraform-plugin-sdk would save unconfigured (null) string attributes into the Terraform state as "" instead of null. When migrating to the framework, this can cause unexpected plan differences as the framework correctly now handles null values and since Terraform correctly considers these values to be different.

Separately, Terraform CLI does not currently show certain plan differences relating to "" => null and vice-versa, which can make this extra confusing for provider developers.

Provider developers have (at least) two options to handle this migration situation:

We'll also need to determine all the cases where terraform-plugin-sdk would have done this. One known case is Optional-only string attributes within list blocks.

It may be preferable to also introduce a built-in attribute plan modifier for this particular situation, but that can be treated separately as having some documentation around this issue is more important.

References

bflad commented 1 year ago

From an out-of-band conversation, please note that adding configuration to state upgrades to potentially update these "transparently" is not being considered upstream.

ewbankkit commented 1 year ago

The Terraform AWS Provider recently encountered this issue during the migration of the aws_globalaccelerator_accelerator data source from Plugin SDK v2 to Plugin Framework: https://github.com/hashicorp/terraform-provider-aws/pull/27282. In particular, an existing acceptance test case caught a nested attribute (attributes.0.flow_logs_s3_prefix) not being present vs. having an empty string value.

The relevant code is https://github.com/hashicorp/terraform-provider-aws/blob/2d2308a4872843b878a292dd2c95730e16a7a2e6/internal/service/globalaccelerator/accelerator_data_source.go#L281-L297.

From experience this will likely affect more than just strings - other types have their zero value (0, false etc.) set instead of null.

bflad commented 1 year ago

Another odd one, doing d.Set("map_attribute", nil) in the sdk would save a known empty map into the state, not null, at least for data sources.

austinvalle commented 1 year ago

A relevant write-up that might be useful when we document this: https://discuss.hashicorp.com/t/framework-migration-test-produces-non-empty-plan/54523/12