Refactoring the IP support, and fixing #1846.

I've made the IPMess stuff a lot less messy, and refactored
a lot of the util/ip module, including naming it more sensibly.

The biggest changes are that I moved the big case statement into
a constant and then used a bit of dispatch-style logic to use it,
and I eliminated a bunch of duplicate code in the ipmess.rb file.

Added some test data for FreeBSD and fixed a bug in my map
logic pointed out by Paul Nasrat.

I've also fixed #1846, in that the interface list now s/:/_/g.

Signed-off-by: Luke Kanies <luke@madstop.com>

2 files changed, 62 insertions, 55 deletions

diff --git a/lib/facter/ipmess.rb b/lib/facter/ipmess.rb
index c61b1ec..2887b64 100644
--- a/lib/facter/ipmess.rb
+++ b/lib/facter/ipmess.rb
@@ -7,39 +7,26 @@
 
 require 'facter/util/ip'
 
+# Note that most of this only works on a fixed list of platforms; notably, Darwin
+# is missing.
+
 Facter.add(:interfaces) do
-    confine :kernel => [ :sunos, :freebsd, :openbsd, :netbsd, :linux ]
+    confine :kernel => Facter::Util::IP.supported_platforms
     setcode do
-        Facter::IPAddress.get_interfaces.join(",")
+        Facter::Util::IP.get_interfaces.collect { |iface| Facter::Util::IP.alphafy(iface) }.join(",")
     end
 end
 
-case Facter.value(:kernel)
-when 'SunOS', 'Linux', 'OpenBSD', 'NetBSD', 'FreeBSD'
-    Facter::IPAddress.get_interfaces.each do |interface|
-        mi = interface.gsub(/[:.]/, '_')
-
-        Facter.add("ipaddress_" + mi) do
-            confine :kernel => [ :sunos, :freebsd, :openbsd, :netbsd, :linux ]
-            setcode do
-                label = 'ipaddress'
-                Facter::IPAddress.get_interface_value(interface, label)
-            end
-        end
-
-        Facter.add("macaddress_" + mi) do
-            confine :kernel => [ :sunos, :freebsd, :openbsd, :netbsd, :linux ]
-            setcode do
-                label = 'macaddress'
-                Facter::IPAddress.get_interface_value(interface, label)
-            end
-        end
+Facter::Util::IP.get_interfaces.each do |interface|
+    mi = Facter::Util::IP.alphafy(interface)
 
-        Facter.add("netmask_" + mi) do
-            confine :kernel => [ :sunos, :freebsd, :openbsd, :netbsd, :linux ]
+    # Make a fact for each detail of each interface.  Yay.
+    #   There's no point in confining these facts, since we wouldn't be able to create
+    # them if we weren't running on a supported platform.
+    %w{ipaddress macaddress netmask}.each do |label|
+        Facter.add(label + "_" + mi) do
             setcode do
-                label = 'netmask'
-                Facter::IPAddress.get_interface_value(interface, label)
+                Facter::Util::IP.get_interface_value(interface, label)
             end
         end
     end
diff --git a/lib/facter/util/ip.rb b/lib/facter/util/ip.rb
index 3dbfcda..598b084 100644
--- a/lib/facter/util/ip.rb
+++ b/lib/facter/util/ip.rb
@@ -1,11 +1,48 @@
 # A base module for collecting IP-related
 # information from all kinds of platforms.
-module Facter::IPAddress
-    def self.get_interfaces
+module Facter::Util::IP
+    # A map of all the different regexes that work for
+    # a given platform or set of platforms.
+    REGEX_MAP = {
+        :linux => {
+            :ipaddress => /inet addr:([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/,
+            :macaddress  => /(?:ether|HWaddr)\s+(\w{1,2}:\w{1,2}:\w{1,2}:\w{1,2}:\w{1,2}:\w{1,2})/,
+            :netmask => /Mask:([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/
+        },
+        :bsd => {
+            :aliases => [:openbsd, :netbsd, :freebsd],
+            :ipaddress => /inet\s+([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/,
+            :macaddress  => /(?:ether|lladdr)\s+(\w\w:\w\w:\w\w:\w\w:\w\w:\w\w)/,
+            :netmask => /netmask\s+(\w{10})/
+        },
+        :sunos => {
+            :addr => /inet\s+([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/,
+            :macaddress  => /(?:ether|lladdr)\s+(\w?\w:\w?\w:\w?\w:\w?\w:\w?\w:\w?\w)/,
+            :netmask => /netmask\s+(\w{8})/
+        }
+    }
+
+    # Convert an interface name into purely alpha characters.
+    def self.alphafy(interface)
+        interface.gsub(/[:.]/, '_')
+    end
+
+    def self.supported_platforms
+        REGEX_MAP.inject([]) do |result, tmp|
+            key, map = tmp
+            if map[:aliases]
+                result += map[:aliases]
+            else
+                result << key
+            end
+            result
+        end
+    end
 
+    def self.get_interfaces
         int = nil
 
-        output =  Facter::IPAddress.get_all_interface_output()
+        output =  Facter::Util::IP.get_all_interface_output()
 
         # We get lots of warnings on platforms that don't get an output
         # made.
@@ -60,33 +97,16 @@ module Facter::IPAddress
     def self.get_interface_value(interface, label)
         tmp1 = []
 
-        # LAK:NOTE This is pretty ugly - two case statements being used for most of the
-        # logic.  These should be pulled into a small dispatch table.  We don't have tests for this code,
-        # though, so it's not exactly trivial to do so.
-        case Facter.value(:kernel)
-        when 'Linux'
-            addr = /inet addr:([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/
-            mac  = /(?:ether|HWaddr)\s+(\w{1,2}:\w{1,2}:\w{1,2}:\w{1,2}:\w{1,2}:\w{1,2})/
-            mask = /Mask:([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/
-        when 'OpenBSD', 'NetBSD', 'FreeBSD'
-            addr = /inet\s+([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/
-            mac  = /(?:ether|lladdr)\s+(\w\w:\w\w:\w\w:\w\w:\w\w:\w\w)/
-            mask = /netmask\s+(\w{10})/
-        when 'SunOS'
-            addr = /inet\s+([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/
-            mac  = /(?:ether|lladdr)\s+(\w?\w:\w?\w:\w?\w:\w?\w:\w?\w:\w?\w)/
-            mask = /netmask\s+(\w{8})/
-        end
+        kernel = Facter.value(:kernel).downcase.to_sym
 
-        case label
-        when 'ipaddress'
-            regex = addr
-        when 'macaddress'
-            regex = mac
-        when 'netmask'
-            regex = mask
+        # If it's not directly in the map or aliased in the map, then we don't know how to deal with it.
+        unless map = REGEX_MAP[kernel] || REGEX_MAP.values.find { |tmp| tmp[:aliases] and tmp[:aliases].include?(kernel) }
+            return []
         end
 
+        # Pull the correct regex out of the map.
+        regex = map[label.to_sym]
+
         # Linux changes the MAC address reported via ifconfig when an ethernet interface
         # becomes a slave of a bonding device to the master MAC address.
         # We have to dig a bit to get the original/real MAC address of the interface.
@@ -99,7 +119,7 @@ module Facter::IPAddress
             output_int = get_single_interface_output(interface)
 
             if interface != "lo" && interface != "lo0"
-                output_int.each { |s|
+                output_int.each do |s|
                     if s =~ regex
                         value = $1
                         if label == 'netmask' && Facter.value(:kernel) == "SunOS"
@@ -107,7 +127,7 @@ module Facter::IPAddress
                         end
                         tmp1.push(value)
                     end
-                }
+                end
             end
 
             if tmp1