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. I've also fixed #1846, in that the interface list now s/:/_/g. Signed-off-by: Luke Kanies <[email protected]> --- lib/facter/ipmess.rb | 39 ++++++++---------------- lib/facter/util/ip.rb | 78 ++++++++++++++++++++++++++++++------------------ spec/unit/ipmess.rb | 19 ++++++++++++ spec/unit/util/ip.rb | 22 +++++++++----- 4 files changed, 95 insertions(+), 63 deletions(-) create mode 100644 spec/unit/ipmess.rb 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..60c5a50 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] or 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 diff --git a/spec/unit/ipmess.rb b/spec/unit/ipmess.rb new file mode 100644 index 0000000..a67216c --- /dev/null +++ b/spec/unit/ipmess.rb @@ -0,0 +1,19 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../spec_helper' + +require 'facter' + +describe "Messy IP facts" do + before do + Facter.loadfacts + end + + it "should replace the ':' in an interface list with '_'" do + # So we look supported + Facter.fact(:kernel).stubs(:value).returns("SunOS") + + Facter::Util::IP.expects(:get_interfaces).returns %w{eth0:1 eth1:2} + Facter.fact(:interfaces).value.should == %{eth0_1,eth1_2} + end +end diff --git a/spec/unit/util/ip.rb b/spec/unit/util/ip.rb index 059bdd1..88c136b 100644 --- a/spec/unit/util/ip.rb +++ b/spec/unit/util/ip.rb @@ -4,34 +4,40 @@ require File.dirname(__FILE__) + '/../../spec_helper' require 'facter/util/ip' -describe Facter::IPAddress do +describe Facter::Util::IP do + [:freebsd, :linux, :netbsd, :openbsd, :sunos].each do |platform| + it "should be supported on #{platform}" do + Facter::Util::IP.supported_platforms.should be_include(platform) + end + end + it "should return a list of interfaces" do - Facter::IPAddress.should respond_to(:get_interfaces) + Facter::Util::IP.should respond_to(:get_interfaces) end it "should return an empty list of interfaces on an unknown kernel" do Facter.stubs(:value).returns("UnknownKernel") - Facter::IPAddress.get_interfaces().should == [] + Facter::Util::IP.get_interfaces().should == [] end it "should return a list with a single interface on Linux with a single interface" do sample_output_file = File.dirname(__FILE__) + '/../data/linux_ifconfig_all_with_single_interface' linux_ifconfig = File.new(sample_output_file).read() - Facter::IPAddress.stubs(:get_all_interface_output).returns(linux_ifconfig) - Facter::IPAddress.get_interfaces().should == ["eth0"] + Facter::Util::IP.stubs(:get_all_interface_output).returns(linux_ifconfig) + Facter::Util::IP.get_interfaces().should == ["eth0"] end it "should return a value for a specific interface" do - Facter::IPAddress.should respond_to(:get_interface_value) + Facter::Util::IP.should respond_to(:get_interface_value) end it "should return a human readable netmask on Solaris" do sample_output_file = File.dirname(__FILE__) + "/../data/solaris_ifconfig_single_interface" solaris_ifconfig_interface = File.new(sample_output_file).read() - Facter::IPAddress.expects(:get_single_interface_output).with("e1000g0").returns(solaris_ifconfig_interface) + Facter::Util::IP.expects(:get_single_interface_output).with("e1000g0").returns(solaris_ifconfig_interface) Facter.stubs(:value).with(:kernel).returns("SunOS") - Facter::IPAddress.get_interface_value("e1000g0", "netmask").should == "255.255.255.0" + Facter::Util::IP.get_interface_value("e1000g0", "netmask").should == "255.255.255.0" end end -- 1.5.6.5 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
