breaking: define CustomData as interface{} in Device and IPAddressCommon

[displague]

The API will accept and return CustomData with any JSON value or
structure. The previous type (map[string]interface{}) did not allow for
string only values, which are acceptable, as are ints, bools, and null.

Signed-off-by: Marques Johansson [email protected]

[t0mk]

I've jsut realized that it's not easy to unset the custom_data if we keep it interface{} and omitempty. If you set nil in the the DeviceUpdateRequests, the field will not be present in the HTTP PUT.

If you put there

	updateCustomData = nil
	_, _, err = c.Devices.Update(dID, &DeviceUpdateRequest{
	CustomData: &updateCustomData,
})

then on the wires it will look as {"customdata":null}, and consquent HTTP GET on the device resource will contain "customdata":{}, which is not the same. Maybe this is an API bug?

Funnily enough, when I try to update the device with

	updateCustomData = map[string]interface{}{}
	_, _, err = c.Devices.Update(dID, &DeviceUpdateRequest{
	CustomData: &updateCustomData,
})

.. which serializes to {"customdata":{}} in the updating HTTP PUT, it seems that the customdata in the API are not updated. The API consideres the empty JSON map as a nil value (for omitempty). Consequent GET to the device resource will still contain previous customdata value.

[displague]

Outside of this patch, on the Device customdata, I observed in the PACKNGO_DEBUG output that a POST {"customdata": "{\"foo\":\"bar\"}"} will result in a GET of {"customdata": {"foo": "bar"}}. It would seem that the API service is unmarhalling strings that look like JSON structures.

I was using 0.5.1 of packngo. I want to confirm this behavior (for this data type and others) because it is unexpected and changes how I would want to interact with this field.

resource	customdata in	customdata out	metadata customdata	notes
device	"{\"foo\":\"bar\"}"	{"foo": "bar"}	{"foo": "bar"}	expected string
device	{"foo": "bar"}
device	"{\"invalid\"}"
device	"\"null\""
device	"null"
device	null
device	"[\"1\"]"
device	"[1]"
device	[1]
device	"\"1\""
device	"1"
device	1
device	"\"0\""
device	"0"
device	0
device	"\"3.14\""
device	"3.14"
device	3.14
device	"\"1e10\""
device	"1e10"
device	1e10
device	"\"true\""
device	"true"
device	true
device	"\"test\""
device	"test"
device	""
device	"\"\""

It may also be helpful to verify that POST and PUT behave the same way.

Metadata customdata can be fetched with curl https://metadata.platformequinix.com/customdata | jq .customdata.

This PR should include a table of tests for the values described here.

[displague]

@t0mk definitely a concern. We would need for PUT requests to be able to omit customdata when no change is requested, and still be able to overwrite existing values with any of the supported empty values.

[t0mk]

I compiled following table where the columns are

1. Type of CustomData value to set
2. golang value used as var v interface{}; v = <the_value>; updateRequest.CustomData = &v;
3. value from "customdata" field of the JSON in PUT HTTP request to Device resource
4. value from "customdata" field of the JSON in HTTP Reponse to a GET to the to Device resource updated from the same row
5. Golang value populated in result from Device.Get() in packngo
6. deserialized Golang type of CustomData in Device struct

TypeOf CustomData In	CustomData in	serialized in Request	serialized in Response	CustomData out	TypeOf CustomData out
string	"{\"foo\":\"bar\"}"	"{\"foo\":\"bar\"}"	{"foo":"bar"}	map[string]interface {}{"foo":"bar"}	map[string]interface {}
map[string]interface {}	map[string]interface {}{"foo":"bar"}	{"foo":"bar"}	{"foo":"bar"}	map[string]interface {}{"foo":"bar"}	map[string]interface {}
string	{"invalid"}	N/A (error)	N/A (error)	N/A (error)	N/A (error)
string	"\"null\""	"\"null\""	"null"	"null"	string
string	"null"	"null"	{}	map[string]interface {}{}	map[string]interface {}
%!s()	<nil>	null	{}	map[string]interface {}{}	map[string]interface {}
string	"[\"1\"]"	"[\"1\"]"	["1"]	[]interface {}{"1"}	[]interface {}
string	"[1]"	"[1]"	[1]	[]interface {}{1}	[]interface {}
[]int32	[]int32{1}	[1]	[1]	[]interface {}{1}	[]interface {}
string	"\"1\""	"\"1\""	"1"	"1"	string
string	"1"	"1"	1	1	float64
int	1	1	1	1	float64
string	"\"0\""	"\"0\""	"0"	"0"	string
string	"0"	"0"	0	0	float64
int	0	0	0	0	float64
string	"\"3.14\""	"\"3.14\""	"3.14"	"3.14"	string
string	"3.14"	"3.14"	3.14	3.14	float64
float64	3.14	3.14	3.14	3.14	float64
string	"\"1e10\""	"\"1e10\""	"1e10"	"1e10"	string
string	"1e10"	"1e10"	10000000000.0	1e+10	float64
float64	1e+10	10000000000	10000000000	1e+10	float64
string	"\"true\""	"\"true\""	"true"	"true"	string
string	"true"	"true"	true	true	bool
bool	true	true	true	true	bool
string	"\"test\""	"\"test\""	"test"	"test"	string
string	test	N/A (error)	N/A (error)	N/A (error)	N/A (error)
string	""	""	{}	map[string]interface {}{}	map[string]interface {}
string	"\"\""	"\"\""	""	""	string

.. I only tried with Device and only did the update (HTTP PUT).

[displague]

Thanks, @t0mk. I think we'll be able to refer to this table as we continue to build CustomData support and we can certainly refer users here for specific needs.

I believe @grahamc has been using customdata, I'm curious if he's run into any of these edge cases.

[grahamc]

Cool! I’ve tried to submit more raw like data but didn’t mind needing to make a full object. None of these comments seem concerning to me 🙂

…

On Mon, Dec 14, 2020, at 10:42 PM, Marques Johansson wrote: Thanks, @t0mk <https://github.com/t0mk>. I think we'll be able to refer to this table as we continue to build CustomData support and we can certainly refer users here for specific needs. I believe @grahamc <https://github.com/grahamc> has been using customdata, I'm curious if he's run into any of these edge cases. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#225 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAASXLDTLREKAAPVPZACP6TSU3LKPANCNFSM4UY3MPYA>.

[t0mk]

@grahamc as @displague probably foresees as well, the fact that EM API will return different value than you store for the customdata, is quite a problem for the TF provider, where we can't have this as ForceNew. TF might detect a diff even when there was no user-made change in the customdata, and then attempt to re-create the device resource.

It might still be possible to make it a parameter with Optional: true, and ForceNew: false; but it would be more clean to try to fix it from the API side. It's not too likely though IMO.

[displague]

Noting that this is related to #221 which added CustomData for IPReservation resources.

[displague]

I stumbled upon https://golang.org/pkg/encoding/json/#RawMessage and wondered if this might be a type to use in our CustomData fields.

[displague]

Project.CustomData should also be exposed.

Upon revisiting this issue,json.RawMessage seems to be the best approach, allowing for all JSON types without the ambiguity of interface{}.