(FACT-2872_2) Reimplement linux networking resolver

Filipovici-Andrei · Filipovici-Andrei · commit 59de5f7a24c6 · 2020-12-10T15:23:38.000+02:00
diff --git a/agent/facter-ng.gemspec b/agent/facter-ng.gemspec
@@ -5,7 +5,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 Gem::Specification.new do |spec|
   spec.name          = 'facter-ng'
-  spec.version       = '4.0.46'
+  spec.version       = '4.0.47'
   spec.authors       = ['Puppet']
   spec.email         = ['team-nw@puppet.com']
 
diff --git a/facter.gemspec b/facter.gemspec
@@ -5,7 +5,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 Gem::Specification.new do |spec|
   spec.name          = 'facter'
-  spec.version       = '4.0.46'
+  spec.version       = '4.0.47'
   spec.authors       = ['Puppet']
   spec.email         = ['team-nw@puppet.com']
 
diff --git a/lib/facter/custom_facts/util/collection.rb b/lib/facter/custom_facts/util/collection.rb
@@ -115,8 +115,7 @@ def custom_facts
         internal_loader.load_all unless @loaded
         @loaded = true
 
-        custom_facts = @facts.select { |_k, v| v.options[:fact_type] == :custom }
-        @custom_facts = Facter::Utils.deep_copy(custom_facts.keys)
+        @custom_facts = @facts.select { |_k, v| v.options[:fact_type] == :custom }
       end
 
       def load(name)
diff --git a/lib/facter/custom_facts/util/loader.rb b/lib/facter/custom_facts/util/loader.rb
@@ -143,7 +143,7 @@ def load_env(fact = nil)
           # match it.
           next if fact && (env_name != fact)
 
-          LegacyFacter.add(Regexp.last_match(1), fact_type: :external) do
+          LegacyFacter.add(Regexp.last_match(1), fact_type: :external, is_env: true) do
             has_weight 1_000_000
             setcode { value }
           end
diff --git a/lib/facter/custom_facts/util/resolution.rb b/lib/facter/custom_facts/util/resolution.rb
@@ -110,7 +110,7 @@ def evaluate(&block)
       #
       # @api private
       def options(options)
-        accepted_options = %i[name value timeout weight fact_type file]
+        accepted_options = %i[name value timeout weight fact_type file is_env]
 
         accepted_options.each do |option_name|
           instance_variable_set("@#{option_name}", options.delete(option_name)) if options.key?(option_name)
diff --git a/lib/facter/framework/core/fact_loaders/external_fact_loader.rb b/lib/facter/framework/core/fact_loaders/external_fact_loader.rb
@@ -21,8 +21,9 @@ def load_custom_facts
 
       custom_facts_to_load = LegacyFacter.collection.custom_facts
 
-      custom_facts_to_load&.each do |custom_fact_name|
-        loaded_fact = LoadedFact.new(custom_fact_name.to_s, nil, :custom)
+      custom_facts_to_load&.each do |k, v|
+        loaded_fact = LoadedFact.new(k.to_s, nil, :custom)
+        loaded_fact.is_env = v.options[:is_env] if v.options[:is_env]
         custom_facts << loaded_fact
       end
 
@@ -37,6 +38,7 @@ def load_external_facts
       external_facts_to_load&.each do |k, v|
         loaded_fact = LoadedFact.new(k.to_s, nil, :external)
         loaded_fact.file = v.options[:file]
+        loaded_fact.is_env = v.options[:is_env]
         external_facts << loaded_fact
       end
 
diff --git a/lib/facter/framework/core/fact_loaders/fact_loader.rb b/lib/facter/framework/core/fact_loaders/fact_loader.rb
@@ -24,11 +24,27 @@ def load(options)
       @internal_facts = load_internal_facts(options)
       @external_facts = load_external_facts(options)
 
+      filter_env_facts
+
       @facts = @internal_facts + @external_facts
     end
 
     private
 
+    def filter_env_facts
+      env_fact_names = @external_facts.select { |fact| fact.is_env == true }.map(&:name)
+      return unless env_fact_names.any?
+
+      @internal_facts.delete_if do |fact|
+        if env_fact_names.include?(fact.name)
+          @log.debug("Reading #{fact.name} fact from environment variable")
+          true
+        else
+          false
+        end
+      end
+    end
+
     def load_internal_facts(options)
       @log.debug('Loading internal facts')
       internal_facts = []
diff --git a/lib/facter/framework/logging/logger.rb b/lib/facter/framework/logging/logger.rb
@@ -100,10 +100,10 @@ def error(msg, colorize = false)
     end
 
     def log_exception(exception)
-      msg = colorize(exception.message, RED) + "\n"
-      msg += exception.backtrace.join("\n") if Options[:trace]
+      msg = exception.message
+      msg += "\n" + exception.backtrace.join("\n") if Options[:trace]
 
-      @@logger.error(@class_name + ' - ' + msg)
+      error(msg, true)
     end
 
     private
diff --git a/lib/facter/models/loaded_fact.rb b/lib/facter/models/loaded_fact.rb
@@ -3,13 +3,14 @@
 module Facter
   class LoadedFact
     attr_reader :name, :klass, :type
-    attr_accessor :file
+    attr_accessor :file, :is_env
 
     def initialize(name, klass, type = nil, file = nil)
       @name = name
       @klass = klass
       @type = type.nil? ? :core : type
       @file = file
+      @is_env = false
     end
   end
 end
diff --git a/lib/facter/resolvers/base_resolver.rb b/lib/facter/resolvers/base_resolver.rb
@@ -27,6 +27,9 @@ def self.resolve(fact_name)
 
           cache_nil_for_unresolved_facts(fact_name)
         end
+      rescue NoMethodError => e
+        log.debug("Could not resolve #{fact_name}, got #{e}")
+        @fact_list[fact_name] = nil
       rescue LoadError, NameError => e
         log.debug("resolving fact #{fact_name}, but #{e}")
         @fact_list[fact_name] = nil
diff --git a/lib/facter/resolvers/hostname_resolver.rb b/lib/facter/resolvers/hostname_resolver.rb
@@ -42,7 +42,12 @@ def retrieve_from_fqdn(output)
         end
 
         def retrieve_with_addrinfo(host)
-          name = Socket.getaddrinfo(host, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME)[0]
+          begin
+            name = Socket.getaddrinfo(host, 0, Socket::AF_UNSPEC, Socket::SOCK_STREAM, nil, Socket::AI_CANONNAME)[0]
+          rescue StandardError => e
+            @log.debug("Socket.getaddrinfo failed to retrieve fqdn for hostname #{host} with: #{e}")
+            return
+          end
           return if name.nil? || name.empty? || host == name[2]
 
           name[2]
diff --git a/lib/facter/resolvers/networking_linux_resolver.rb b/lib/facter/resolvers/networking_linux_resolver.rb
@@ -29,6 +29,7 @@ def retrieve_network_info(fact_name)
           mtu_and_indexes = interfaces_mtu_and_index
           retrieve_interface_info(mtu_and_indexes)
           add_info_from_routing_table
+          retrieve_primary_interface
           Facter::Util::Resolvers::Networking.expand_main_bindings(@fact_list)
           @fact_list[fact_name]
         end
@@ -39,7 +40,7 @@ def interfaces_mtu_and_index
           output.each_line do |line|
             next unless line.include?('mtu')
 
-            mtu = line.match(/mtu (\d+)/)&.captures&.first.to_i
+            mtu = line.match(/mtu (\d+)/)&.captures&.first&.to_i
             index_tokens = line.split(':')
             index = index_tokens[0].strip
             # vlans are displayed as <vlan_name>@<physical_interface>
@@ -64,17 +65,16 @@ def retrieve_interface_info(mtu_and_indexes)
               mtu = mtu_and_indexes.dig(interface.name, 1)
               interfaces[interface.name][:mtu] = mtu unless mtu.nil?
             end
-            next if interface.addr.nil? || interface.netmask.nil?
-
-            begin
-              # ipv6 ips are retrieved as <ip>%<interface_name>
-              ip = interface.addr.ip_address.split('%').first if interface.addr.ip?
-              netmask = interface.netmask.ip_address
-              add_binding(interfaces, interface.name, ip, netmask, interface.addr.ipv4?)
-            rescue SocketError => e
-              next
-            end
 
+            if !interface.addr.nil? && !interface.netmask.nil?
+              begin
+                # ipv6 ips are retrieved as <ip>%<interface_name>
+                ip = interface.addr.ip_address.split('%').first if interface.addr.ip?
+                netmask = interface.netmask.ip_address
+                add_binding(interfaces, interface.name, ip, netmask, interface.addr.ipv4?)
+              rescue SocketError => e
+              end
+            end
             find_dhcp!(interface.name, mtu_and_indexes, interfaces)
           end
 
@@ -85,8 +85,10 @@ def extract_mac_address(interface)
           mac = get_bonded_interface_mac(interface.name)
           if mac.nil?
             if Socket.const_defined? :PF_LINK
-              mac = interface.addr&.getnameinfo.first #sometimes it returns localhost, ip but mac also
+              mac = interface.addr&.getnameinfo&.first #sometimes it returns localhost, ip but mac also
             elsif Socket.const_defined? :PF_PACKET
+              return if interface.addr.nil? || interface.addr.inspect_sockaddr.nil?
+
               mac = interface.addr&.inspect_sockaddr[/hwaddr=([\h:]+)/, 1]
             end
           end
@@ -184,6 +186,8 @@ def compare_ips(routes, binding_key)
 
         def find_dhcp!(interface_name, interface_indexes, interfaces)
           return if !interfaces[interface_name] || interfaces[interface_name][:dhcp]
+          require 'pry-byebug'
+          binding.pry
 
           log.debug("Get DHCP for interface #{interface_name}")
           unless interface_indexes[interface_name].nil?
@@ -236,6 +240,16 @@ def search_internal_leases(interface_name)
           dhcp = dhcp.match(/SERVER_ADDRESS=(.*)/)
           dhcp[1] if dhcp
         end
+
+        def retrieve_primary_interface
+          primary_helper = Facter::Util::Resolvers::Networking::PrimaryInterface
+          primary_interface = primary_helper.read_from_proc_route
+          primary_interface ||= primary_helper.read_from_ip_route
+          primary_interface ||= primary_helper.find_in_interfaces(@fact_list[:interfaces])
+
+          @fact_list[:primary_interface] = primary_interface
+        end
+
       end
     end
   end
diff --git a/lib/facter/resolvers/open_vz.rb b/lib/facter/resolvers/open_vz.rb
@@ -15,9 +15,7 @@ def post_resolve(fact_name)
         end
 
         def check_proc_vz(fact_name)
-          if !Dir.exist?('/proc/vz') || !File.executable?('/proc/lve/list') || Dir.entries('/proc/vz').count.equal?(2)
-            return
-          end
+          return if !Dir.exist?('/proc/vz') || File.file?('/proc/lve/list') || Dir.entries('/proc/vz').count.equal?(2)
 
           @fact_list[:vm] = read_proc_status
           @fact_list[fact_name]
diff --git a/lib/facter/resolvers/solaris/networking.rb b/lib/facter/resolvers/solaris/networking.rb
@@ -26,7 +26,7 @@ def read_facts(fact_name)
               end
 
               @fact_list = { interfaces: @interfaces } unless @interfaces.empty?
-              @fact_list[:primary_interface] = Facter::Util::Resolvers::Networking::PrimaryInterface.get(@log)
+              @fact_list[:primary_interface] = Facter::Util::Resolvers::Networking::PrimaryInterface.read_from_route
 
               Facter::Util::Resolvers::Networking.expand_main_bindings(@fact_list)
             rescue StandardError => e
diff --git a/lib/facter/util/resolvers/networking/primary_interface.rb b/lib/facter/util/resolvers/networking/primary_interface.rb
@@ -5,12 +5,72 @@ module Util
     module Resolvers
       module Networking
         module PrimaryInterface
+          @log ||= Log.new(self)
+
           class << self
-            def get(log = nil)
-              result = Facter::Core::Execution.execute('route -n get default', logger: log)
+            ROUTE_TABLE_MAPPING = {
+              'Iface' => 0,
+              'Destination' => 1,
+              'Gateway' => 2,
+              'Flags' => 3,
+              'RefCnt' => 4,
+              'Use' => 5,
+              'Metric' => 6,
+              'Mask' => 7,
+              'MTU' => 8,
+              'Window' => 9,
+              'IRTT' => 10
+            }.freeze
+
+            def read_from_route
+              return if Facter::Core::Execution.which('route').nil?
+
+              result = Facter::Core::Execution.execute('route -n get default', logger: @log)
 
               result.match(/interface: (.+)/)&.captures&.first
             end
+
+            def read_from_proc_route
+              content = Facter::Util::FileHelper.safe_read('/proc/net/route', '')
+
+              content.each_line.with_index do |line, index|
+                next if index.zero?
+
+                route = line.strip.split("\t")
+                if route.count > 7 &&
+                   route[ROUTE_TABLE_MAPPING['Destination']] == '00000000' &&
+                   route[ROUTE_TABLE_MAPPING['Mask']] == '00000000'
+                  return route[ROUTE_TABLE_MAPPING['Iface']]
+                end
+              end
+              nil
+            end
+
+            def read_from_ip_route
+              return if Facter::Core::Execution.which('ip').nil?
+
+              output = Facter::Core::Execution.execute('ip route show default', logger: @log)
+              primary_interface = nil
+              output.each_line do |line|
+                primary_interface = line.strip.split(' ')[4] if line.start_with?('default')
+              end
+
+              primary_interface
+            end
+
+            def find_in_interfaces(interfaces)
+              interfaces.each do |iface_name, interface|
+                interface[:bindings].each do |binding|
+                  return iface_name unless Facter::Util::Resolvers::Networking.ignored_ip_address(binding[:address])
+                end
+
+                interface[:bindings6].each do |binding|
+                  return iface_name unless Facter::Util::Resolvers::Networking.ignored_ip_address(binding[:address])
+                end
+              end
+
+              nil
+            end
           end
         end
       end
diff --git a/lib/facter/version.rb b/lib/facter/version.rb
@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Facter
-  VERSION = '4.0.46' unless defined?(VERSION)
+  VERSION = '4.0.47' unless defined?(VERSION)
 end
diff --git a/spec/custom_facts/util/collection_spec.rb b/spec/custom_facts/util/collection_spec.rb
@@ -352,7 +352,7 @@
       end
 
       it 'returns my_fact custom fact' do
-        expect(collection.custom_facts.first).to eq(:my_fact)
+        expect(collection.custom_facts.first[0]).to eq(:my_fact)
       end
     end
 
@@ -375,7 +375,7 @@
       end
 
       it 'loads the new fact' do
-        expect(collection.custom_facts.first). to eq(:new_fact)
+        expect(collection.custom_facts.first[0]). to eq(:new_fact)
       end
     end
 
@@ -398,7 +398,7 @@
       end
 
       it 'loads the new fact' do
-        expect(collection.custom_facts.first). to eq(:new_fact)
+        expect(collection.custom_facts.first[0]). to eq(:new_fact)
       end
     end
   end
diff --git a/spec/custom_facts/util/loader_spec.rb b/spec/custom_facts/util/loader_spec.rb
@@ -160,7 +160,7 @@ def loader_from(places)
 
       loader.load(:testing)
 
-      expect(LegacyFacter).to have_received(:add).with('testing', { fact_type: :external })
+      expect(LegacyFacter).to have_received(:add).with('testing', { fact_type: :external, is_env: true })
     end
 
     it 'loads any files in the search path with names matching the fact name' do
diff --git a/spec/facter/resolvers/aix/serialnumber_spec.rb b/spec/facter/resolvers/aix/serialnumber_spec.rb
@@ -32,7 +32,7 @@
     end
 
     context 'when fails to retrieve fact' do
-      let(:result) { nil }
+      let(:result) { '' }
 
       it 'detects serialnumber as nil' do
         expect(Facter::Resolvers::Aix::Serialnumber.resolve(:serialnumber)).to be(nil)
diff --git a/spec/facter/resolvers/hostname_resolver_spec.rb b/spec/facter/resolvers/hostname_resolver_spec.rb
@@ -81,5 +81,17 @@
         expect(hostname_resolver.resolve(:domain)).to be(nil)
       end
     end
+
+    context 'when getaddrinfo throws exception' do
+      let(:host) { 'foo' }
+
+      before do
+        allow(Socket).to receive(:getaddrinfo).and_raise('socket exception')
+      end
+
+      it 'detects domain from resolv.conf' do
+        expect(hostname_resolver.resolve(:domain)).to eq('baz')
+      end
+    end
   end
 end
diff --git a/spec/facter/resolvers/networking_linux_resolver_spec.rb b/spec/facter/resolvers/networking_linux_resolver_spec.rb
diff --git a/spec/facter/resolvers/open_vz_spec.rb b/spec/facter/resolvers/open_vz_spec.rb
diff --git a/spec/facter/util/resolvers/networking/primary_interface_spec.rb b/spec/facter/util/resolvers/networking/primary_interface_spec.rb
diff --git a/spec/fixtures/ip_route_show_default b/spec/fixtures/ip_route_show_default
diff --git a/spec/fixtures/proc_net_route b/spec/fixtures/proc_net_route
diff --git a/spec/fixtures/route_n_get_default b/spec/fixtures/route_n_get_default
diff --git a/spec/framework/core/fact/internal/internal_fact_manager_spec.rb b/spec/framework/core/fact/internal/internal_fact_manager_spec.rb
diff --git a/spec/framework/core/fact_loaders/external_fact_loader_spec.rb b/spec/framework/core/fact_loaders/external_fact_loader_spec.rb
diff --git a/spec/framework/core/fact_loaders/fact_loader_spec.rb b/spec/framework/core/fact_loaders/fact_loader_spec.rb
diff --git a/spec/framework/logging/logger_spec.rb b/spec/framework/logging/logger_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -110,7 +110,7 @@ def evaluate(&block)`
`110`	`110`	`#`
`111`	`111`	`# @api private`
`112`	`112`	`def options(options)`
`113`		`- accepted_options = %i[name value timeout weight fact_type file]`
	`113`	`+ accepted_options = %i[name value timeout weight fact_type file is_env]`
`114`	`114`
`115`	`115`	`accepted_options.each do \|option_name\|`
`116`	`116`	`instance_variable_set("@#{option_name}", options.delete(option_name)) if options.key?(option_name)`