search

fanatic / go-infoblox

Golang Infoblox WAPI Library (Deprecated)
16 stars 26 forks source link

removing `omitempty` on `configure_for_dns` forces it always to be `false` #38

Closed sjwl closed 5 years ago

sjwl commented 5 years ago

for some reason configure_for_dns is not returned when getting Host Record by reference

GET https://grid:443/wapi/v2.9/record:host/ZG5zLmhvc3QkLl9kZWZhdWx0LmNvbS5zdHRndHMuY2xvdWQubWxjMS5wMDEuc2Ft:REDACTED/default
{
    "_ref": "record:host/ZG5zLmhvc3QkLl9kZWZhdWx0LmNvbS5zdHRndHMuY2xvdWQubWxjMS5wMDEuc2Ft:REDACTED/default",
    "ipv4addrs": [
        {
            "_ref": "record:host_ipv4addr/ZG5zLmhvc3RfYWRkcmVzcyQuX2RlZmF1bHQuY29tLnN0dGd0cy5jbG91ZC5tbGMxLnAwMS5zYW0uMTAuOTcuOC4xLg:10.97.8.1/REDACTED/default",
            "configure_for_dhcp": false,
            "host": "REDACTED",
            "ipv4addr": "10.97.8.1"
        }
    ],
    "name": "REDACTED",
    "view": "default"
}

And therefore, since configure_for_dns is omitted, ConfigureForDNS member of RecordHostObject will always be interpreted to false, even if it's actually true in InfoBlox. A simple fix could be to fix this line

// RecordHostObject defines the HOST record object's fields
type RecordHostObject struct {
...
    ConfigureForDNS bool           `json:"configure_for_dns"`
...
}

to

    ConfigureForDNS bool           `json:"configure_for_dns,omitempty"`

The use case exposing this issue: https://github.com/prudhvitella/terraform-provider-infoblox/issues/46

The following commit caused this issue: 7aa0907ee451547e97867213c63661a567f4f6f2

fanatic commented 5 years ago

Give that a shot - I'm not sure it'll fix your issue, but it won't hurt either.

JackBracken commented 5 years ago

Hi, so this is not actually having the effect that you think it is and is causing some buggy behaviour downstream.

When you serialise a Go struct into a JSON object with omitempty it will omit the field entirely if it is a zero value. In Go a boolean's zero value is false, and as the default for configure_for_dns in Infoblox is true, this makes it impossible to set that value to false.

The actual root cause you're seeing is that "configure_for_dns" needs to be added as a return field in the Infoblox options when instantiating with infobloxClient.GetRecordHost(ref, opts). Whether this should be done in go-infoblox or the terraform provider I'm not sure, but I'm going to add to the provider anyway.

I'll also open a PR reversing this shortly.

Cheers!

fanatic commented 5 years ago

Thanks for following up. I see that Infoblox has created an official Go library, so I'll recommend any new changes go on their repository.

https://github.com/infobloxopen/infoblox-go-client