allow plan data state comparison with legacy SDK

[jbardin]

In order to determine if we need to re-read a data source during plan,
we need to compare the newly evaluated configuration with the stored
state. To do that we create a ProposedNewVal, which if there
are no changes, should match the existing state.

A problem arises if the remote data source contains any blocks, and they
are not set in the configuration. Terraform always decodes configuration
blocks as empty containers, however the legacy SDK cannot correctly
handle empty blocks and may return a null value which is saved to the
state. In order to correctly make the comparison for planning, we need
to reify those null blocks as empty containers in the cty value.

The createEmptyBlocks helper converts any null NestingList or NestingSet
blocks to empty list or set cty values. We only need to be concerned
with List and Set, because those are the only types that can be defined
with the legacy SDK. In hindsight these could have been normalized in
the legacy SDK shims had this problem been uncovered earlier, but for the
sake of compatibility we will now normalize these in core.

Fixes #25812

[codecov]

Codecov Report

Merging #25857 into master will increase coverage by 0.01%.
The diff coverage is 84.31%.

Impacted Files	Coverage Δ
terraform/eval_read_data_plan.go	83.33% <84.31%> (-0.88%)	⬇️
terraform/node_resource_plan.go	91.80% <0.00%> (-1.64%)	⬇️
states/statefile/version4.go	57.83% <0.00%> (+0.28%)	⬆️
backend/remote/backend_common.go	55.25% <0.00%> (+0.67%)	⬆️

[mildwonkey]

I'm curious (as someone who doesn't know much of anything useful here): are there any concerns about how this might interact with providers not using the legacy SDK, or is that not a concern because those wouldn't (couldn't) get into this situation in the first place?

Edit: I saw this in the linked issue, thank you it's helpful info!

Unfortunately we don't have an indication at that point when the data source was using the legacy SDK, so any changes here need to take into account that they will apply to any new SDK versions as well.

[jbardin]

The idea with this change is that a correctly functioning provider should not be able to get into this situation.

Since a block should never be decoded from the config as null in the first place, we should not have to compare between a null block and an empty block. So a theoretical non-legacy provider specifically returning null for a List or Set block in a data source would be an error, but that would need to be caught at ReadDataSource rather than here. Unfortunately we don't have a legacy flag to check for, so we can't catch it and be as strict as we would like, but because the comparison wold otherwise not make sense, we can safely normalize these to the expected empty value.

[mildwonkey]

LGTM!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.