convert: prevent invalid types in dynamicReplace #175
Conversation
When converting an unknown map into an object with optional attributes, that object may have attributes which don't match the map element type. This conversion will be valid, but when building Null or Unknown values of that type we need to ensure all attributes have a valid type. The fallthrough path for dynamicReplace when non-primitive types didn't match contained a cty.NilVal element type, which would result in an overall invalid type. We can early returns to ensure there are no uninitialized values, and let the fallback use the original out type. Since at this point the types appear to not be convertible, we can assume they are from attributes which were skipped during conversion due to being optional, otherwise the conversion would not have been valid.
jbardin
force-pushed
the
conv-uknown-optional-attrs
branch
from
b5b1169
to
b6e7fe9
Compare
|
This one was a bit of a chin-scratcher 🤔 for me, and so here's how I understood this problem in my own words just to make sure we're agreeing about what's going on here and why: An unknown value of map type has an unknown set of map keys. When converting a map type to an object type, it's okay for the map to be missing keys that correspond with attributes of the object type as long as they are optional. If they are optional then the corresponding attributes will be automatically populated with null values of the attribute type during conversion. Previously we were effectively assuming that an unknown value of map type would always contain at least a set of keys corresponding to all of the attributes of the object type, regardless of whether they are optional attributes. That's naive, because if the map value were known then we'd tolerate the situation I described in the previous paragraph. Therefore we should -- for consistency with the usual rules for error checking during type conversion -- be optimistic that the final known map value will be valid for the target object type, raising an error only once the map value is known and we can see that it definitely isn't valid. That all makes sense to me, and so I'm going to merge this. Thanks! |
|
This change is included in v1.14.2. 🎉 |
When converting an unknown map into an object with optional attributes, that object may have optional attributes which don't match the map element type. This conversion will be valid, but when building Null or Unknown values of that type we need to ensure all attributes have a valid type.
The fallthrough path for dynamicReplace when non-primitive types didn't match contained a cty.NilVal element type, which would result in an overall invalid type. We can use early returns to ensure there are no uninitialized values, and let the fallback use the original out type. Since at this point the types appear to not be convertible, we can assume they are from attributes which were skipped during conversion due to being optional, otherwise the conversion would not have been valid.