Fix FQDN resolution, scan all IPs associated with host for FQDN

[jseely]

Description

Currently on Windows Server 2016, the FQDN is being set as the machine name instead of the FQDN. This boils down to the underlying winsock library calls which, when given an IPv6 address, return only the machine name instead of the FQDN.

The work around involves calling Socket.gethostbyaddr with the IPv4 address returned in the h_addr_list instead of the IPv6 address.

Check List

• [ x] New functionality includes tests (no new functionality)
• [x ] All tests pass
• [ x] RELEASE_NOTES.md, has been updated if required (not required for bugfixes, required for API changes) (bugfix)
• [ x] All commits have been signed-off for the Developer Certificate of Origin. See https://github.com/chef/chef/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco

[tas50]

@jseely This looks like a great change. Can you add a spec for the decision that mocks out what the response would like like under the ipv4 vs. ipv6 conditions?

[jseely]

@tas50 So I thought of a new approach to take on this that is more resilient, instead of choosing just one IP address to attempt a host lookup with, we now will try them all until we find one that contains a period (ie. is an FQDN). If none of them do it falls back to the machine name, as it currently does.

As for IPv6 vs IPv4 responses I don't think I've tested this exhaustively enough to make statements about its function across all possible configurations of all versions of Windows in all environments. But from my experimentation with Server 2012R2 and Server 2016 VMs in Azure with IPv4 and IPv6 addresses associated to the host record, when IPv4 addresses are used to perform the gethostbyaddr lookup it correctly returns the FQDN, however, when IPv6 addresses are used it returns back the machine name only.

This could be a network configuration issue on Azure, or a bug in the win sock library. Either way I think this new approach guarantees better success in looking up an FQDN then the current model.

The current lookup process assumes that if the h_name field (first field in the array) of gethostbyname is not an FQDN; then the h_name field of gethostbyaddr using the last address in the h_addr_list from the first call will be an FQDN. In many cases (all the ones where the last address in h_addr_list was IPv4 for me) this is a correct assumption.

The new lookup process does not make this assumption, if the h_name field of gethostbyname is not an FQDN; then we will loop over all the addresses in h_addr_list until we find one where h_name is an FQDN; if no h_addr_list address have an FQDN associated to them we fall back to machine name (which is the current failure case in the current process).

** Let me know if this covers what you wanted out of a spec, or if I need to provide something more detailed in a specific format.

[tas50]

@jseely It sounds like you've tested this in detail. I just want to make sure we get some unit tests around the logic within your change. A few years down the line someone might make a slight change there without the background of what you've gone through. If we get some specs added here we can ensure we don't introduce a regression later on.

[lamont-granquist]

can this just be fqdn info.first unless fqdn and eliminate the found_fqdn variable entirely? (i don't know if bare fqdn is a getter or not in this context...)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.