Skip to content

all: Adjust UseStateForUnknown plan modifiers to preserve known null state values #1161

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

austinvalle
Copy link
Member

Related Issue

Closes #1117

Description

This PR adjusts all of the UseStateForUnknown plan modifiers to also preserve null state values (which are technically known). The original logic was excluding all null state values although we believe the intended behavior was to skip when creating resources (which should be checking the resource instance state for null, not the specific state value).

Note

One of the impacts of fixing this bug would be any attributes in existing resource implementations that are relying on null values being set to unknown during update plans, which would now start to preserve the prior state (keeping null), so if the provider is not preserving those prior state values during apply, it could cause data consistency issues if that unknown marking was being relied on.

Off the top of my head, I'm not sure why you'd use UseStateForUnknown in this way, since the goal of this plan modifier has always been to denote an attribute that doesn't change during apply on updates. Fixing this bug doesn't change that goal, it just makes it more precise.

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

No

@austinvalle austinvalle added this to the v1.16.0 milestone Jun 12, 2025
@austinvalle austinvalle requested a review from a team as a code owner June 12, 2025 15:53
Copy link
Member Author

Choose a reason for hiding this comment

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

The changes to this test and internal/fwserver/block_plan_modification_test.go were just to ensure the intended behavior was still tested, as the original tests did not actually set up the req.State value properly 😄

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.

UseStateForUnknown doesn't preserve known null prior state values
1 participant