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

fix nil-relaxing behavior #254

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix nil-relaxing behavior #254

wants to merge 1 commit into from

Conversation

tosuke
Copy link
Contributor

@tosuke tosuke commented Jan 31, 2025

This PR addresses the behavior of the NilRelaxed plan modifier.

The Terraform Plugin SDK silently updates the state for optional attributes, while the Plugin Framework does not. To bridge this gap, we introduced the NilRelaxed plan modifier in #248. However, as discussed in hashicorp/terraform-plugin-framework#510, we discovered that it cannot completely replicate the SDK's behavior.

The SDK handles attributes with null or zero values in the following way:

  1. Sets them to null during the first apply
  2. Converts null values to type-appropriate zero values during refresh operations

When examining the relationship between configuration values and state values, the SDK considers these scenarios valid:

  1. Configuration is null, state is null
  2. Configuration is zero value, state is null
  3. Configuration is null, state is zero value
  4. Configuration is zero value, state is zero value

Of these scenarios, cases 2 and 3 generate non-empty plans in the Framework due to mismatches between configuration and state. The NilRelaxed plan modifier aimed to prevent unnecessary plan generation in these cases by adopting the previous state's value.

However, due to constraints in Terraform's type system (go-cty), we discovered that case 2 cannot be handled - specifically, we cannot revert a plan that converts a zero value back to null without raising an error. Since case 2 only occurs during the first apply, we've decided to accept these plan generations as expected behavior.

Output from acceptance testing:

$ make testacc TESTS=TestAccXXX

...

@tosuke tosuke changed the title fix nil-relaxing behaviour fix nil-relaxing behavior Jan 31, 2025
@@ -28,18 +29,16 @@ func (_ nilRelaxedModifier) MarkdownDescription(context.Context) string {

func (_ nilRelaxedModifier) PlanModifyMap(ctx context.Context, req planmodifier.MapRequest, resp *planmodifier.MapResponse) {
if req.PlanValue.IsUnknown() {
return
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handling unknown values to emulate Optional behavior

@tosuke tosuke marked this pull request as ready for review January 31, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant