-
Notifications
You must be signed in to change notification settings - Fork 591
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
dynblock,hcldec: Decode unknown dynamic block bodies as entirely unknown values #461
Conversation
cb21ec4
to
db8d67d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been chatting directly about whether we need this separate ExpandWithUnknown
entry point or if it's better to change the (arguably incorrect) behavior of the existing entry point, so we'll continue researching that, but I had a few little bits I noticed while I was reading through before we had that discussion that I just wanted to save here so I don't forget about them!
f33cc90
to
fbbccfe
Compare
Rebased with some refactoring to allow dynamic to no longer interfere with the |
Allow unknown block bodies during decoding
Allow dynamic blocks to indicate when the entire block value is unknown
fbbccfe
to
adfdd23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me!
The old result here was confusing and inconsistent, so I think it's justified to change it to behave more in line with the design intent, rather than staying bug-compatible. The old behavior remains for applications that aren't using unknowns, or that are using the other API entry points (the main hcl
package API or gohcl
), so this should only affect applications that are intentionally working with unknown values and thus should be ready to deal with unknown values in results, per our usual assumptions.
The dynblock extension paired with
hcldec
previously used a single object with unknown attributes as a sentinel to indicate that a decodeddynamic
block was unknown. This however poses multiple problems with consumers of the data which are not aware of the dynamic construct, since they may misinterpret the number of values, and can be confused when known values are mixed with the sentinel value.Rather than using a sentinel value within the container, here we propose to ensure a decoded block container which contains any
dynamic
block with an unknownfor_each
argument will result in an entirely unknown value. This better serves the purpose of the original sentinel value, as there is no other way to assign an unknown value to a block container, and allows consumers to understand that the final container value itself is not entirely known.We accomplish this by adding an
UnknownBody
interface to thehcldec
package, which can be used to signal when a block should be decoded as unknown. Any body that satisfies this interface and returnstrue
will cause the entire block container to be decoded as an unknown value, regardless of other known bodies being decoded for that particular block.