Copilot commented on code in PR #295:
URL: 
https://github.com/apache/cloudstack-terraform-provider/pull/295#discussion_r3557478767


##########
cloudstack/resource_cloudstack_instance.go:
##########
@@ -462,13 +462,11 @@ func resourceCloudStackInstanceCreate(d 
*schema.ResourceData, meta interface{})
        }
 
        if userdataDetails, ok := d.GetOk("userdata_details"); ok {
-               udDetails := make(map[string]string)
-               index := 0
+               ud := make(map[string]string)
                for k, v := range userdataDetails.(map[string]interface{}) {
-                       udDetails[fmt.Sprintf("userdatadetails[%d].%s", index, 
k)] = v.(string)
-                       index++
+                       ud[k] = fmt.Sprint(v)
                }

Review Comment:
   `userdata_details` is declared as `TypeMap` with `Elem: TypeString`, so 
using `fmt.Sprint(v)` here can silently turn unexpected values (including `nil` 
-> "<nil>") into strings. For consistency with other map-to-string conversions 
in this file (e.g., `details`) and to avoid masking type issues, prefer a 
string type assertion.



##########
cloudstack/resource_cloudstack_instance.go:
##########
@@ -825,13 +823,11 @@ func resourceCloudStackInstanceUpdate(d 
*schema.ResourceData, meta interface{})
 
                        p := 
cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id())
                        if userdataDetails, ok := d.GetOk("userdata_details"); 
ok {
-                               udDetails := make(map[string]string)
-                               index := 0
+                               ud := make(map[string]string)
                                for k, v := range 
userdataDetails.(map[string]interface{}) {
-                                       
udDetails[fmt.Sprintf("userdatadetails[%d].%s", index, k)] = v.(string)
-                                       index++
+                                       ud[k] = fmt.Sprint(v)
                                }

Review Comment:
   Same as in create(): `userdata_details` values should already be strings due 
to the schema, so `fmt.Sprint(v)` can mask unexpected types (or serialize `nil` 
as "<nil>"). Prefer a string type assertion for consistency with the rest of 
the resource.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to