terraform-plugin-sdk
terraform-plugin-sdk copied to clipboard
Fix read data failure for TypeList defined in TypeSet
Fix https://github.com/hashicorp/terraform-plugin-sdk/issues/1197
Scenario
If a schema has a set property which contains a list property as follows, GetOkExists may failed to read values of prop_list
.
"prop_set": {
Type: pluginsdk.TypeSet,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
...
"prop_list": {
Type: pluginsdk.TypeList,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
},
},
},
},
},
Reason
This line of readListField
will finally run this code to change the address
from prop_set.xxxxx.prop_list.0
to prop_set.0.prop_list.0
. Since the address
is changed, readListField
cannot read values in array in subsequent code.
Solution
Add a deep copy in func (r *ConfigFieldReader) readField
when changing the address.
Hi @ms-zhenhua 👋 Thank you for submitting this change. The maintainers here cannot commit to reviewing the full consequences of the change at this time, however it would be helpful to understand the provider code leading up to the reason for this change. Generally opening a bug report issue would capture these details, such as how to reproduce the problem, before getting into code changes.
In general though, the schema.ResourceData
getter and setter methods should only be used directly with top-level attributes. They may work today in certain scenarios with deeper nesting of attributes/blocks, but a lot of this SDK's code is unfortunately not well suited for the conversion between the "real" data path and the "stringified" data path, especially in the cases of sets where the set index sometimes gets converted to an underlying Go slice index. It is difficult to gauge whether there may be regression concerns for developers who may have somehow been dependent on the old behavior and whether fixing the potential bug is worth the provider developer burden for unexpected or unintentional usage. Adding some covering unit testing may alleviate some of those concerns, however this SDK is not exhaustive with edge case coverage.
If we can better determine what situations this issue may occur and how it will affect existing dependent provider code, then we can potentially prioritize this better. 👍 Thanks again.
Thanks @bflad. I have created an issue to give more details for this problem.