diff --git a/lib/ohai/mixin/network_helper.rb b/lib/ohai/mixin/network_helper.rb index 8547c11e5..c650674b4 100644 --- a/lib/ohai/mixin/network_helper.rb +++ b/lib/ohai/mixin/network_helper.rb @@ -19,6 +19,7 @@ # require "socket" unless defined?(Socket) +require "resolv" unless defined?(Resolv) module Ohai module Mixin @@ -37,6 +38,12 @@ def hex_to_dec_netmask(netmask) dec end + # Addrinfo#ip*? methods return true on AI_CANONNAME Addrinfo records that match + # the ipv* scheme and #ip? always returns true unless a non tcp Addrinfo + def ip?(hostname) + Resolv::IPv4::Regex.match?(hostname) || Resolv::IPv6::Regex.match?(hostname) + end + # This does a forward and reverse lookup on the hostname to return what should be # the FQDN for the host determined by name lookup (generally DNS). If the forward # lookup fails this will throw. If the reverse lookup fails this will return the @@ -47,7 +54,27 @@ def hex_to_dec_netmask(netmask) # server), and the method should return the hostname and not the IP address. # def canonicalize_hostname(hostname) - Addrinfo.getaddrinfo(hostname, nil, nil, nil, nil, Socket::AI_CANONNAME).first.canonname + ai = Addrinfo + .getaddrinfo(hostname, nil, nil, nil, nil, Socket::AI_CANONNAME) + .first + + canonname = ai&.canonname + # use canonname if it's an FQDN + # This API is preferred as it never gives us an IP address for broken DNS + # (see https://github.com/chef/ohai/pull/1705) + # However, we have found that Windows hosts that are not joined to a domain + # can return a non-qualified hostname). + # Use a '.' in the canonname as indicator of FQDN + return canonname if canonname.include?(".") + + # If we got a non-qualified name, then we do a standard reverse resolve + # which, assuming DNS is working, will work around that windows bug + # (and maybe others) + canonname = ai&.getnameinfo&.first + return canonname unless ip?(canonname) + + # if all else fails, return the name we were given as a safety + hostname end def canonicalize_hostname_with_retries(hostname) diff --git a/spec/unit/mixin/network_helper_spec.rb b/spec/unit/mixin/network_helper_spec.rb index 4a2c4cbfc..0a0bd4cb2 100644 --- a/spec/unit/mixin/network_helper_spec.rb +++ b/spec/unit/mixin/network_helper_spec.rb @@ -29,14 +29,53 @@ end describe "canonicalize hostname" do - # this is a brittle test deliberately intended to discourage modifying this API - # (see the node in the code on the necessity of manual testing) - it "uses the correct ruby API" do - hostname = "broken.example.org" - addrinfo = instance_double(Addrinfo) - expect(Addrinfo).to receive(:getaddrinfo).with(hostname, nil, nil, nil, nil, Socket::AI_CANONNAME).and_return([addrinfo]) - expect(addrinfo).to receive(:canonname).and_return(hostname) - expect(mixin.canonicalize_hostname(hostname)).to eql(hostname) + [ + { + desc: "canonname == initial hostname returns those", + initial_hostname: "fullhostname.example.com", + canonname: "fullhostname.example.com", + final_hostname: "fullhostname.example.com", + }, + { + desc: "canonname(hostname) => fqdn returns fqdn", + initial_hostname: "hostnamepart", + canonname: "hostnamepart.example.com", + final_hostname: "hostnamepart.example.com", + }, + { + desc: "hostname only canonname, getnameinfo is tried and succeeds", + initial_hostname: "hostnamepart2", + canonname: "hostnamepart2", + nameinfo: "hostnamepart2.example.com", + final_hostname: "hostnamepart2.example.com", + }, + { + desc: "hostname only canonname, getnameinfo returns ip => original hostname", + initial_hostname: "hostnameip.example.com", + canonname: "hostnameip", # totally contrived + nameinfo: "192.168.1.1", + final_hostname: "hostnameip.example.com", + }, + ].each do |example_hash| + # this is a brittle set of tests deliberately intended to discourage modifying + # this API (see the note in the code on the necessity of manual testing) + example example_hash[:desc] do + addrinfo = instance_double(Addrinfo) + expect(Addrinfo).to receive(:getaddrinfo) + .with(example_hash[:initial_hostname], nil, nil, nil, nil, Socket::AI_CANONNAME) + .and_return([addrinfo]) + expect(addrinfo).to receive(:canonname).and_return(example_hash[:canonname]) + + # only expect this call if :nameinfo key is set, otherwise code should not + # fall through to getnameinfo + if example_hash[:nameinfo] + expect(addrinfo).to receive(:getnameinfo).and_return([example_hash[:nameinfo], "0"]) + end + + # the actual input and output for #canonicalize_hostname method + expect(mixin.canonicalize_hostname(example_hash[:initial_hostname])) + .to eql(example_hash[:final_hostname]) + end end end end