Merge pull request #1699 from chef/lcg/fix-deprecation-error

Fix hostname deprecation error

9 files changed, 58 insertions, 118 deletions

diff --git a/.github/workflows/exec.yml b/.github/workflows/exec.yml
index 2d35a0e4..5bc19646 100644
--- a/.github/workflows/exec.yml
+++ b/.github/workflows/exec.yml
@@ -10,6 +10,7 @@ name: exec
 jobs:
   test:
     strategy:
+      fail-fast: false
       matrix:
         os: ['macos-latest', 'ubuntu-latest', 'windows-latest']
         ruby: ['2.7', '3.0']
diff --git a/.github/workflows/unit.yml b/.github/workflows/unit.yml
index d98f705b..14fae5bc 100644
--- a/.github/workflows/unit.yml
+++ b/.github/workflows/unit.yml
@@ -10,6 +10,7 @@ name: unit
 jobs:
   test:
     strategy:
+      fail-fast: false
       matrix:
         os: ['ubuntu-latest', 'macos-latest', 'windows-latest']
         ruby: ['2.7', '3.0']
diff --git a/lib/ohai/mixin/constant_helper.rb b/lib/ohai/mixin/constant_helper.rb
index a02aa6fa..933d4ed0 100644
--- a/lib/ohai/mixin/constant_helper.rb
+++ b/lib/ohai/mixin/constant_helper.rb
@@ -23,7 +23,7 @@ module Ohai
     module ConstantHelper
 
       def remove_constants
-        new_object_constants = Object.constants - @object_pristine.constants
+        new_object_constants = Object.constants - @object_pristine.constants - [ :SortedSet ]
         new_object_constants.each do |constant|
           Object.send(:remove_const, constant) unless Object.const_get(constant).is_a?(Module)
         end
diff --git a/lib/ohai/mixin/network_helper.rb b/lib/ohai/mixin/network_helper.rb
index bb6d1367..71ea0bdd 100644
--- a/lib/ohai/mixin/network_helper.rb
+++ b/lib/ohai/mixin/network_helper.rb
@@ -32,6 +32,24 @@ module Ohai
         [2, 4, 6].each { |n| dec = dec + "." + netmask[n..n + 1].to_i(16).to_s(10) }
         dec
       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)
+      #
+      def canonicalize_hostname(hostname)
+        Addrinfo.getaddrinfo(hostname, nil).first.getnameinfo.first
+      end
+
+      def canonicalize_hostname_with_retries(hostname)
+        retries = 3
+        begin
+          canonicalize_hostname(hostname)
+        rescue
+          retries -= 1
+          retry if retries > 0
+          nil
+        end
+      end
     end
   end
 end
diff --git a/lib/ohai/plugins/hostname.rb b/lib/ohai/plugins/hostname.rb
index abc37996..1c95d7c3 100644
--- a/lib/ohai/plugins/hostname.rb
+++ b/lib/ohai/plugins/hostname.rb
@@ -26,7 +26,11 @@
 # limitations under the License.
 #
 
+require_relative "../mixin/network_helper"
+
 Ohai.plugin(:Hostname) do
+  include Ohai::Mixin::NetworkHelper
+
   provides "domain", "hostname", "fqdn", "machinename"
 
   # hostname : short hostname
@@ -42,38 +46,8 @@ Ohai.plugin(:Hostname) do
   end
 
   # forward and reverse lookup to canonicalize FQDN (hostname -f equivalent)
-  # this is ipv6-safe, works on ruby 1.8.7+
   def resolve_fqdn
-    require "socket" unless defined?(Socket)
-    require "ipaddr" unless defined?(IPAddr)
-
-    hostname = from_cmd("hostname")
-    begin
-      addrinfo = Socket.getaddrinfo(hostname, nil).first
-    rescue SocketError
-      # In the event that we got an exception from Socket, it's possible
-      # that it will work if we restrict it to IPv4 only either because of
-      # IPv6 misconfiguration or other bugs.
-      #
-      # Specifically it's worth noting that on macOS, getaddrinfo() will choke
-      # if it gets back a link-local address (say if you have 'fe80::1 myhost'
-      # in /etc/hosts). This will raise:
-      #    SocketError (getnameinfo: Non-recoverable failure in name resolution)
-      #
-      # But general misconfiguration could cause similar issues, so attempt to
-      # fall back to v4-only
-      begin
-        addrinfo = Socket.getaddrinfo(hostname, nil, :INET).first
-      rescue
-        # and if *that* fails, then try v6-only, in case we're in a v6-only
-        # environment with v4 config issues
-        addrinfo = Socket.getaddrinfo(hostname, nil, :INET6).first
-      end
-    end
-    iaddr = IPAddr.new(addrinfo[3])
-    Socket.gethostbyaddr(iaddr.hton)[0]
-  rescue
-    nil
+    canonicalize_hostname_with_retries(from_cmd("hostname"))
   end
 
   def collect_domain
@@ -119,58 +93,21 @@ Ohai.plugin(:Hostname) do
   collect_data(:darwin) do
     hostname from_cmd("hostname -s")
     machinename from_cmd("hostname")
-    begin
-      our_fqdn = resolve_fqdn
-      # Sometimes... very rarely, but sometimes, 'hostname --fqdn' falsely
-      # returns a blank string. WTF.
-      if our_fqdn.nil? || our_fqdn.empty?
-        logger.trace("Plugin Hostname: hostname returned an empty string, retrying once.")
-        our_fqdn = resolve_fqdn
-      end
-
-      if our_fqdn.nil? || our_fqdn.empty?
-        logger.trace("Plugin Hostname: hostname returned an empty string twice and will" +
-                        "not be set.")
-      else
-        fqdn our_fqdn
-      end
-    rescue
-      logger.trace(
-        "Plugin Hostname: hostname returned an error, probably no domain set"
-      )
-    end
+    fqdn resolve_fqdn
     domain collect_domain
   end
 
   collect_data(:freebsd) do
     hostname from_cmd("hostname -s")
     machinename from_cmd("hostname")
-    fqdn from_cmd("hostname -f")
+    fqdn resolve_fqdn
     collect_domain
   end
 
   collect_data(:linux) do
     hostname from_cmd("hostname -s")
     machinename from_cmd("hostname")
-    begin
-      our_fqdn = from_cmd("hostname --fqdn")
-      # Sometimes... very rarely, but sometimes, 'hostname --fqdn' falsely
-      # returns a blank string. WTF.
-      if our_fqdn.nil? || our_fqdn.empty?
-        logger.trace("Plugin Hostname: hostname --fqdn returned an empty string, retrying " +
-                        "once.")
-        our_fqdn = from_cmd("hostname --fqdn")
-      end
-
-      if our_fqdn.nil? || our_fqdn.empty?
-        logger.trace("Plugin Hostname: hostname --fqdn returned an empty string twice and " +
-                        "will not be set.")
-      else
-        fqdn our_fqdn
-      end
-    rescue
-      logger.trace("Plugin Hostname: hostname --fqdn returned an error, probably no domain set")
-    end
+    fqdn resolve_fqdn
     domain collect_domain
   end
 
@@ -190,22 +127,7 @@ Ohai.plugin(:Hostname) do
 
     hostname host["dnshostname"].to_s
     machinename host["name"].to_s
-
-    info = Socket.gethostbyname(Socket.gethostname)
-    if /.+?\.(.*)/.match?(info.first)
-      fqdn info.first
-    else
-      # host is not in dns. optionally use:
-      # C:\WINDOWS\system32\drivers\etc\hosts
-      info[3..info.length].reverse_each do |addr|
-        hostent = Socket.gethostbyaddr(addr)
-        if /.+?\.(.*)/.match?(hostent.first)
-          fqdn hostent.first
-          break
-        end
-      end
-      fqdn info.first unless fqdn
-    end
+    fqdn canonicalize_hostname_with_retries(Socket.gethostname)
     domain collect_domain
   end
 end
diff --git a/lib/ohai/plugins/packages.rb b/lib/ohai/plugins/packages.rb
index e9e74909..4228fb09 100644
--- a/lib/ohai/plugins/packages.rb
+++ b/lib/ohai/plugins/packages.rb
@@ -136,7 +136,7 @@ Ohai.plugin(:Packages) do
     collect_programs_from_registry_key(Win32::Registry::HKEY_LOCAL_MACHINE, 'Software\Microsoft\Windows\CurrentVersion\Uninstall')
     # on 64 bit systems, 32 bit programs are stored here moved before HKEY_CURRENT_USER otherwise it is not collected (impacts both ohai 16 & 17)
     collect_programs_from_registry_key(Win32::Registry::HKEY_LOCAL_MACHINE, 'Software\Wow6432Node\Microsoft\Windows\CurrentVersion\Uninstall')
-    collect_programs_from_registry_key(Win32::Registry::HKEY_CURRENT_USER, 'Software\Microsoft\Windows\CurrentVersion\Uninstall')
+    collect_programs_from_registry_key(Win32::Registry::HKEY_CURRENT_USER, 'Software\Microsoft\Windows\CurrentVersion\Uninstall') rescue nil
   end
 
   collect_data(:aix) do
diff --git a/spec/unit/plugins/freebsd/hostname_spec.rb b/spec/unit/plugins/freebsd/hostname_spec.rb
index 025ccbf9..dc7fb578 100644
--- a/spec/unit/plugins/freebsd/hostname_spec.rb
+++ b/spec/unit/plugins/freebsd/hostname_spec.rb
@@ -22,8 +22,8 @@ describe Ohai::System, "FreeBSD hostname plugin" do
   before do
     @plugin = get_plugin("hostname")
     allow(@plugin).to receive(:collect_os).and_return(:freebsd)
+    allow(@plugin).to receive(:canonicalize_hostname).with("katie.local").and_return("katie.bethell")
     allow(@plugin).to receive(:shell_out).with("hostname -s").and_return(mock_shell_out(0, "katie", ""))
-    allow(@plugin).to receive(:shell_out).with("hostname -f").and_return(mock_shell_out(0, "katie.bethell", ""))
     allow(@plugin).to receive(:shell_out).with("hostname").and_return(mock_shell_out(0, "katie.local", ""))
   end
 
diff --git a/spec/unit/plugins/hostname_spec.rb b/spec/unit/plugins/hostname_spec.rb
index 9819c42f..9c975fe7 100644
--- a/spec/unit/plugins/hostname_spec.rb
+++ b/spec/unit/plugins/hostname_spec.rb
@@ -25,35 +25,37 @@ describe Ohai::System, "hostname plugin" do
     allow(@plugin).to receive(:shell_out).with("hostname").and_return(mock_shell_out(0, "katie.local", ""))
   end
 
-  context "default behavior"
-  before do
-    allow(@plugin).to receive(:resolve_fqdn).and_return("katie.bethell")
-  end
+  context "default behavior" do
+    before do
+      allow(@plugin).to receive(:canonicalize_hostname).with("katie.local").and_return("katie.bethell")
+    end
 
-  it_should_check_from("linux::hostname", "machinename", "hostname", "katie.local")
+    it_should_check_from("linux::hostname", "machinename", "hostname", "katie.local")
 
-  it "uses #resolve_fqdn to find the fqdn" do
-    @plugin.run
-    expect(@plugin[:fqdn]).to eq("katie.bethell")
-  end
+    it "uses #resolve_fqdn to find the fqdn" do
+      @plugin.run
+      expect(@plugin[:fqdn]).to eq("katie.bethell")
+    end
 
-  it "sets the domain to everything after the first dot of the fqdn" do
-    @plugin.run
-    expect(@plugin[:domain]).to eq("bethell")
-  end
+    it "sets the domain to everything after the first dot of the fqdn" do
+      @plugin.run
+      expect(@plugin[:domain]).to eq("bethell")
+    end
 
-  it "sets the [short] hostname to everything before the first dot of the fqdn" do
-    @plugin.run
-    expect(@plugin[:hostname]).to eq("katie")
+    it "sets the [short] hostname to everything before the first dot of the fqdn" do
+      @plugin.run
+      expect(@plugin[:hostname]).to eq("katie")
+    end
   end
 
   context "when a system has a bare hostname without a FQDN" do
     before do
       allow(@plugin).to receive(:collect_os).and_return(:default)
       allow(@plugin).to receive(:shell_out).with("hostname").and_return(mock_shell_out(0, "katie", ""))
+      allow(@plugin).to receive(:canonicalize_hostname).with("katie").and_return("katie.bethell")
     end
 
-    it "correctlies set the [short] hostname" do
+    it "correctly sets the [short] hostname" do
       @plugin.run
       expect(@plugin[:hostname]).to eq("katie")
     end
@@ -65,14 +67,12 @@ describe Ohai::System, "hostname plugin" do
       allow(@plugin).to receive(:shell_out).with("hostname -s").and_return(
         mock_shell_out(0, "katie", "")
       )
-      allow(@plugin).to receive(:shell_out).with("hostname --fqdn").and_return(
-        mock_shell_out(0, "", ""), mock_shell_out(0, "katie.local", "")
-      )
+      expect(@plugin).to receive(:canonicalize_hostname).with("katie.local").at_least(:once).and_raise(RuntimeError)
     end
 
     it "is called twice" do
       @plugin.run
-      expect(@plugin[:fqdn]).to eq("katie.local")
+      expect(@plugin[:fqdn]).to eq(nil)
     end
   end
 
@@ -82,9 +82,7 @@ describe Ohai::System, "hostname plugin" do
       allow(@plugin).to receive(:shell_out).with("hostname -s").and_return(
         mock_shell_out(0, "katie", "")
       )
-      allow(@plugin).to receive(:shell_out).with("hostname --fqdn").and_return(
-        mock_shell_out(0, "katie.local", "")
-      )
+      expect(@plugin).to receive(:canonicalize_hostname).with("katie.local").and_return("katie.local")
     end
 
     it "is not be called twice" do
@@ -144,7 +142,7 @@ describe Ohai::System, "hostname plugin for windows", :windows_only do
 
   context "when hostname is not set for the machine" do
     it "returns short machine name" do
-      allow(Socket).to receive(:gethostbyaddr).with(anything).and_return(local_hostent)
+      expect(@plugin).to receive(:canonicalize_hostname).with(anything).and_return("local")
       @plugin.run
       expect(@plugin[:fqdn]).to eq("local")
     end
@@ -152,7 +150,7 @@ describe Ohai::System, "hostname plugin for windows", :windows_only do
 
   context "when hostname is set for the machine" do
     it "returns the fqdn of the machine" do
-      allow(Socket).to receive(:gethostbyaddr).with(anything).and_return(fqdn_hostent)
+      expect(@plugin).to receive(:canonicalize_hostname).with(anything).and_return("local.dx.internal.cloudapp.net")
       @plugin.run
       expect(@plugin[:fqdn]).to eq("local.dx.internal.cloudapp.net")
     end
diff --git a/spec/unit/plugins/linux/hostname_spec.rb b/spec/unit/plugins/linux/hostname_spec.rb
index 9bc6e86d..72ec6139 100644
--- a/spec/unit/plugins/linux/hostname_spec.rb
+++ b/spec/unit/plugins/linux/hostname_spec.rb
@@ -23,7 +23,7 @@ describe Ohai::System, "Linux hostname plugin" do
     @plugin = get_plugin("hostname")
     allow(@plugin).to receive(:collect_os).and_return(:linux)
     allow(@plugin).to receive(:shell_out).with("hostname -s").and_return(mock_shell_out(0, "katie", ""))
-    allow(@plugin).to receive(:shell_out).with("hostname --fqdn").and_return(mock_shell_out(0, "katie.bethell", ""))
+    allow(@plugin).to receive(:canonicalize_hostname).with("katie.local").and_return("katie.bethell")
     allow(@plugin).to receive(:shell_out).with("hostname").and_return(mock_shell_out(0, "katie.local", ""))
   end
 
@@ -35,7 +35,7 @@ describe Ohai::System, "Linux hostname plugin" do
 
   describe "when domain name is unset" do
     before do
-      expect(@plugin).to receive(:shell_out).with("hostname --fqdn").and_raise("Ohai::Exception::Exec")
+      allow(@plugin).to receive(:canonicalize_hostname).with("katie.local").and_raise(RuntimeError)
     end
 
     it "does not raise an error" do