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

Better planing of unknown dynamic blocks #28424

Merged
merged 5 commits into from
Apr 23, 2021
Merged

Better planing of unknown dynamic blocks #28424

merged 5 commits into from
Apr 23, 2021

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Apr 16, 2021

Make use of the new hcldec.UnknownBody feature in HCL to allow unknown dynamic blocks to be decoded as entirely unknown values. A more complete description of this new feature is in hashicorp/hcl#461

Now that the dynamic sentinel values are no longer a factor, the logic around couldHaveUnknownBlockPlaceholder is no longer needed, as AssertObjectCompatible can directly check of the entire block container is unknown.

The hcldec package will also now no longer need terraform to change MinItems and MaxItems to account for dynamic blocks, because they will no longer be compared at all when dealing with unknown dynamic blocks.

The end result here is that providers will now be able to correctly customize (within the constraints of the Resource Instance Change Lifecycle) any proposed block values during PlanResourceChange.

Fixes #28340

This will allow any dynamic blocks that are fixed up as a blocktoattr
still decode into an unknown value.
Dynamic blocks with unknown for_each expressions are now decoded into an
unknown value rather than using a sentinel object with unknown
and null attributes. This will allow providers to precisely plan the
block values, rather than trying to heuristically paper over the
incorrect plans when dynamic is in use.
The new hcldec dynamic block behavior no longer tried to validate
MinItems and MaxItems when the number of values is unknown.
@jbardin jbardin requested a review from a team April 16, 2021 21:28
@codecov
Copy link

codecov bot commented Apr 16, 2021

Codecov Report

Merging #28424 (f8493bf) into main (b7fb533) will decrease coverage by 0.01%.
The diff coverage is 56.25%.

Impacted Files Coverage Δ
plans/objchange/compatible.go 70.17% <25.00%> (-1.47%) ⬇️
lang/blocktoattr/fixup.go 72.58% <33.33%> (-2.00%) ⬇️
plans/objchange/objchange.go 79.93% <33.33%> (-0.55%) ⬇️
configs/configschema/decoder_spec.go 81.60% <100.00%> (ø)

@jbardin jbardin added the 0.15-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Apr 20, 2021
update to v2.10.0
@jbardin jbardin marked this pull request as ready for review April 20, 2021 21:05
Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

very nice, I like the change!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
0.15-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Providers cannot always correctly plan values within dynamic blocks
2 participants