IPReservationRequest: change type of attribute CustomData to interface{}

[t0mk]

This PR changes type of CustomData from map[string]interface{} to string in struct for creating IP reservation. There is no presrcibed form for custom_data, so there's no reason to assume that it should be a map.

Also, custom_data of device resource is already a string.

Signed-off-by: Tomas Karasek [email protected]

[t0mk]

@displague this is to bring custom_data for IP reservation to Terraform in the same way as in the device resource.

[displague]

@t0mk under the hood, these custom_data fields are JSON native values. They are fully user-definable beyond that, meaning they may be scalar (int,string,boolean), maps (with any json shape), or arrays (with any json members).

The API will return the value in the format that it was supplied in.

Whether a JSON structure is supplied structured or as a string, shouldn't make much of a difference as long as the tool handling the input and output is consistent in how it serializes and deserializes the data. But, we have to be mindful of the approach taken by other API clients and should try to remain consistent with those.

I don't know what a usage survey would turn up, but the client that I think we can guide our decision on is console.equinix.com.
If the CustomData field there treats a UI entered a JSON structure as a structure, then we should do that too. If it converts it to a string, then perhaps we should do that too.

Between what we do with Packngo and Terraform, we should be prepared to receive and disseminate structures that the user entered into the console when a user decides to terraform import that value later.

I worry that map[string]interface{} or *string unmarshalling could fail based on what the API returns.
Perhaps interface{} is the only safe value here? I tried []byte but that was not handled.

https://play.golang.org/p/fjSBy-W00uA (try *string too)

Terraform offers https://www.terraform.io/docs/configuration/functions/jsondecode.html, so perhaps in Terraform we could marshall the packngointerface{} back to a JSON string and the user could use jsondecode to interact with the value.

[t0mk]

@displague Ok, I changed it to an interface{}. The relevant tests succeed.

I will change the CustomData of the Device struct to the same, and once that's in too, we can do a release and update the customdata(s) in device and IP reservation in the TF provider.

Sounds good?

[displague]

The map[string]interface{} tests should still work, do they not? These would save JSON structures in the EM API, and would result in JSON structures in return.

The updated tests will be storing strings and I assume receiving strings.

[t0mk]

I thought we established that there can be non-map types coming from the EM API (strings, lists, numbers). Then, map[string]interface{} woulndn't work I think.

panic: interface conversion: interface {} is string, not map[string]interface {}

[displague]

@t0mk you can ignore that comment, I believe it is from before the investigation and discovery of how customdata is stored and delivered.

[displague]

Oh, I misunderstood myself :-)

I mean, we should still be able to test with: customData := map[string]interface{}{"custom1": "data", "custom2": map[string]interface{}{"nested": "data"}}. This would verify that arbitrary json structures are stored in their native format and returned the same.

By testing with:

customData := `{"custom1":"data","custom2":{"nested":"data"}}`

we are verifying that scalars (string) work, and we could test with a simpler string foo to prove that.

[displague]

Both tests should be valid.

[t0mk]

Ah, you mean the concrete testing value in the Public IP reservation test. You're right that nested map[string]interface{} is more fitting here, thanks. I overlooked that there was still a string. I fixed it.

I also changed the name of this PR.

[displague]

Ran this locally - tests passed. Sorry for the delay, I first ran this during a scheduled maintenance window.