Resource value may be an object in self evaluation #23215
Conversation
ghost
added
the
sdkv1
jbardin
force-pushed
the
jbardin/for-each-self
branch
from
e7c55eb to
fbc8cd1
Compare
jbardin
force-pushed
the
jbardin/for-each-self
branch
from
fbc8cd1 to
fb7fc5e
Compare
| case addrs.StringKey: | ||
| self = val.Index(cty.StringVal(string(k))) | ||
| self, hclDiags = hcl.Index(val, cty.StringVal(string(k)), ref.SourceRange.ToHCL().Ptr()) | ||
| diags.Append(hclDiags) |
There was a problem hiding this comment.
I'm pondering what happens if hcl.Index fails here.
It looks like it shouldn't happen because val should always be of a suitable type by the time we get here (or else we would've been panicking before anyway), so maybe not a big deal, but perhaps worth adding a comment to that effect anyway to assuage the concerns of future readers.
It looks like hcl.Index is consistent with the usual HCL convention that if it returns errors then it also returns an incomplete-but-valid placeholder value (cty.DynamicVal, in this case) so later code below will hopefully be okay with self being cty.DynamicVal and just short-circuit as normal and allow the error to be returned. Might therefore be okay to just add a comment here noting that we're not expecting this to return errors, but if it does then self will be cty.DynamicVal, in case that's helpful to a future maintainer.
There was a problem hiding this comment.
That matches my thoughts here too. I'm actually not sure which is better now, since in the rare case of an internal error making it this far we panic with the exact location. I'm still leaning towards collecting the error and passing it along for user ergonomics, so I'll just add another comment for now.
The fallback type for GetResource from an EachMap is a cty.Object, because resource schemas may contain dynamically typed attributes. Check for an Object type in the evaluation of self, to use the proper GetAttr method when extracting the value.
jbardin
force-pushed
the
jbardin/for-each-self
branch
from
fb7fc5e to
3839405
Compare
|
Thank you! I can confirm that this PR fixes #23194 . |
|
Hi @isometry so I can safely upgrade now? :) |
Not yet. You should wait for the release of 0.12.13. |
|
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. |
ghost
locked and limited conversation to collaborators
The fallback type for
GetResourcefrom anEachMapis acty.Object,because resource schemas may contain dynamically typed attributes.
Check for an Object type in the evaluation of self, to use the proper
GetAttrmethod when extracting the value.Fixes #23194