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

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

Open
bflad opened this issue Oct 12, 2022 · 4 comments
Open

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

bflad opened this issue Oct 12, 2022 · 4 comments
Labels
documentation Improvements or additions to documentation

Comments

@bflad
Copy link
Member

bflad commented Oct 12, 2022

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:

  • Switch the attribute to Optional, Computed, and include an attribute plan modifier that handles the "" behavior similarly to terraform-plugin-sdk. This would prevent the plan differences and prevent potential issues with modules where differences between "" and null may matter.
  • Not do anything, but consider the change to be a breaking change as part of the migration, and bump the provider major version.

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 bflad added the documentation Improvements or additions to documentation label Oct 12, 2022
@bflad
Copy link
Member Author

bflad commented Oct 12, 2022

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
Copy link
Contributor

ewbankkit commented Oct 20, 2022

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: hashicorp/terraform-provider-aws#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
Copy link
Member Author

bflad commented Oct 27, 2022

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants