netbox-community/go-netbox

Setting optional attributes to their "empty" value

fbreckle opened this issue · 6 comments

Hi,

this is a generalization of tickets like #105 and #106 because it applies to way more attributes than mentioned in these issues.

The problem
Setting an optional attribute to its "empty" value (false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string) will remove the attribute from the resulting JSON that is sent to netbox. This means that these values can not be set to their empty values via go-netbox.

The cause
This happens because in the swagger.json for optional attributes looks like this (e.g. WritableVirtualMachineWithConfigContext):

"platform": {
          "title": "Platform",
          "type": "integer",
          "x-nullable": true
        },

which translates to

	// Platform
	Platform *int64 `json:"platform,omitempty"`

which in turn will cause Go's json marshaller to omit empty values, which is intended behavior.

This is explained further in https://goswagger.io/faq/faq_model.html#non-required-or-nullable-property

A solution
One possible solution which I employ in a personal fork of go-netbox is to add

        "platform": {
          "title": "Platform",
          "type": "integer",
          "x-nullable": true,
          "x-omitempty": false
        },

which will remove the omitempty annotation from the Go struct.

The problem with this solution is that it requires post-procession of the netbox-generated swaggerfile for all optional attributes.
I do not know the stance of this repo's maintainers about post-processing the swagger.json file. It definitely feels less "pure" when compared to just fetching the swagger.json from netbox and regenerating the client. On the other hand, a client that cannot update certain attributes to their empty value is really not valuable.

Ideally, this would be fixed upstream by making netbox return a correct swagger file, but their swagger support is on a best-effort basis only.

This issue is related to the way the code is implemented from the OpenAPI specification by go-swagger.

We have to migrate this project from go-swagger to openapi-generator for Netbox 3.5. Maybe this will fix the issue.

I leave the issue open until further notice.

@fbreckle we should try this. Would be nice to get rid of the custom go netbox client

@fbreckle, @FlxPeters it seems the problem comes from the use of omitempty with PATCH HTTP method. omitempty says "don't send the parameter if it is empty" the seconf says "Only uptade the parameters I send to you". Hence, empty parameters are not updated.

I saw in go-netbox code that each "update" fonction exists in 2 "flavors":

"full" update uses "PUT" method
"partial" update uses "PATCH" method

I don't really understand why those 2 falvors and when should we use one or the other. But I wonder if this wouldn't be the solution. When you have omitempty parameters you should probably use "full" update function. Isn't it?

Thank you
Vince

I don't really understand why those 2 flavors and when should we use one or the other.

A "full" update SETS the object to the values you send in the request. Anything that isn't included is set to null. A "partial" update only updates the fields you send, and won't update anything that's outside the request.

If you send a request with (in JSON cause lazy):

{
"id":1,
"name":"Switch01",
"asset_tag":"123456"
}

A PUT request will set the name and asset_tag as per the request, and also set all the other values to null. It is PUTting the update over the top of the previous object.

A PATCH request will set the name and asset_tag as per the request, and not touch another value. It is PATCHing the original object.

Essentially, use PATCH when you want to adjust something, and PUT when you want to replace something.

Hi @zeddD1abl0 and thank you for your reply,
I know the difference between a PUT and PUSH, my question was about when or what for using "full" or "partial" update flavors of the library?
From what I understand, if you know exactly which filed you will change, for instance using a form, and you know there are no "omitempty" fields then you should use the "partial" update flavor. If some of the field you can modify are "omitempty" fields then you must use "full" update flavor.

Hence, if my understanding is right, probably Netbox Terraform provider (which uses go-netbox library) should use "full" update.

v0ctor commented

A new alpha version has been released with a different software to generate the library, so hopefully this bug has been resolved.

Please feel free to test it and to provide feedback.