Merge pull request #1705 from chef/lcg/fix-fqdn-for-busted-reverse-lookup

2 files changed, 22 insertions, 2 deletions

diff --git a/lib/ohai/mixin/network_helper.rb b/lib/ohai/mixin/network_helper.rb
index 71ea0bdd..8a75ad08 100644
--- a/lib/ohai/mixin/network_helper.rb
+++ b/lib/ohai/mixin/network_helper.rb
@@ -18,6 +18,8 @@
 # limitations under the License.
 #
 
+require "socket" unless defined?(Socket)
+
 module Ohai
   module Mixin
     module NetworkHelper
@@ -34,10 +36,16 @@ module Ohai
       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)
+      # 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
+      # hostname back.  The behavior on failure of the reverse lookup is both vitally important
+      # to this API, and completely untested, so changes to this method (not recommended) need
+      # to be manually validated by hand by setting up a DNS server with a broken A record to
+      # an IP without a PTR record (e.g. any RFC1918 space not served by the configured DNS
+      # server), and the method should return the hostname and not the IP address.
       #
       def canonicalize_hostname(hostname)
-        Addrinfo.getaddrinfo(hostname, nil).first.getnameinfo.first
+        Addrinfo.getaddrinfo(hostname, nil, nil, nil, nil, Socket::AI_CANONNAME).first.canonname
       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 a57af17f..4a2c4cbf 100644
--- a/spec/unit/mixin/network_helper_spec.rb
+++ b/spec/unit/mixin/network_helper_spec.rb
@@ -27,4 +27,16 @@ describe Ohai::Mixin::NetworkHelper, "Network Helper Mixin" do
       expect(mixin.hex_to_dec_netmask("ffff0000")).to eq("255.255.0.0")
     end
   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)
+    end
+  end
 end