list: add conversion functions for cty values to tftypes values #1508
Conversation
stephybun
force-pushed
the
f/convert-cty-to-tftype
branch
3 times, most recently
from
cb7d9ba to
fc84332
Compare
austinvalle
left a comment
austinvalle
left a comment
There was a problem hiding this comment.
Nice work @stephybun ! I have a couple questions and comments but overall all the code is looking good 👍🏻
stephybun
force-pushed
the
f/convert-cty-to-tftype
branch
from
7eabc12 to
9419032
Compare
austinvalle
left a comment
austinvalle
left a comment
There was a problem hiding this comment.
Great work!
Left 2 comments, but neither are deal breakers for what you're looking to merge here. I do think the unknown one should be addressed at some point 👍🏻
internal/plugin/convert/value.go
Outdated
| if in.IsNull() || !in.IsKnown() { | ||
| return nullTfValue(primitiveType), nil | ||
| } |
There was a problem hiding this comment.
Same comment for all of the other functions
If it's unknown, we should send back an unknown value instead of null. State should never have an unknown value in it, so this might be a moot point, but if we have any bugs in how the resource data is setup, we want to make sure we don't obfuscate the data 🙂
You can create an unknown value in tftypes like so:
tftypes.NewValue(ty, tftypes.UnknownValue)| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
|
|
||
| package convert |
There was a problem hiding this comment.
Just for completeness, I'd add some tests here for null/unknown values.
Maybe a test for dynamic pseudo type in primitives (which would be an unknown dynamic and a null dynamic)
stephybun
force-pushed
the
f/convert-cty-to-tftype
branch
from
65b3247 to
6590c69
Compare
| "breed": tftypes.NewValue(tftypes.String, "Golden"), | ||
| "weight": tftypes.NewValue(tftypes.Number, 30), | ||
| "toys": tftypes.NewValue(tftypes.List{ElementType: tftypes.String}, []tftypes.Value{ | ||
| tftypes.NewValue(tftypes.String, "dummy"), |
There was a problem hiding this comment.
Bobby over here playing with dummies 😮
| "ananas": tftypes.NewValue(tftypes.String, "pineapple"), | ||
| "erdbeere": tftypes.NewValue(tftypes.String, "strawberry"), |
rainkwan
left a comment
rainkwan
left a comment
There was a problem hiding this comment.
Seems like the changes should address previous comments so LGTM 👍🏽
3 participants
Description
ResourceDatathat convert resource identity or resource data in cty to tftypesRollback Plan
Changes to Security Controls
None