(FACT-2517) Open3 wrapper for executing system calls

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config --exclude-limit 1000`
-# on 2020-04-24 12:45:39 +0300 using RuboCop version 0.74.0.
+# on 2020-05-06 13:07:31 +0300 using RuboCop version 0.74.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -28,7 +28,6 @@ RSpec/FilePath:
     - 'spec/custom_facts/util/normalization_spec.rb'
     - 'spec/custom_facts/util/parser_spec.rb'
     - 'spec/custom_facts/util/resolution_spec.rb'
-    - 'spec/facter/facts_utils/bytes_to_human_readable_spec.rb'
     - 'spec/facter/facts_utils/uptime_parser_spec.rb'
     - 'spec/facter/facts_utils/windows_release_finder_spec.rb'
     - 'spec/facter/model/fact_collection_spec.rb'
@@ -50,8 +49,8 @@ RSpec/FilePath:
     - 'spec/facter/resolvers/processors_resolver_spec.rb'
     - 'spec/facter/resolvers/redhat_release_resolver_spec.rb'
     - 'spec/facter/resolvers/selinux_resolver_spec.rb'
-    - 'spec/facter/resolvers/solaris/current_zone_resolver_spec.rb'
     - 'spec/facter/resolvers/solaris/solaris_release_resolver_spec.rb'
+    - 'spec/facter/resolvers/solaris/solaris_zone_name_spec.rb'
     - 'spec/facter/resolvers/solaris/zone_resolver_spec.rb'
     - 'spec/facter/resolvers/solaris/zpool_resolver_spec.rb'
     - 'spec/facter/resolvers/suse_relese_resolver_spec.rb'
@@ -85,13 +84,13 @@ RSpec/FilePath:
     - 'spec/framework/core/options/options_validator_spec.rb'
     - 'spec/framework/core/options_spec.rb'
     - 'spec/framework/core/session_cache_spec.rb'
+    - 'spec/framework/detector/os_hierarchy_spec.rb'
     - 'spec/framework/formatters/fact_formatter_spec.rb'
     - 'spec/framework/formatters/hocon_fact_formatter_spec.rb'
     - 'spec/framework/formatters/json_fact_formatter_spec.rb'
     - 'spec/framework/formatters/legacy_fact_formatter_spec.rb'
     - 'spec/framework/formatters/yaml_fact_formatter_spec.rb'
     - 'spec/framework/utils/utils_spec.rb'
-    - 'spec/framework/detector/os_hierarchy_spec.rb'
 
 # Offense count: 15
 # Configuration parameters: AssignmentOnly.
@@ -111,7 +110,7 @@ RSpec/LeakyConstantDeclaration:
     - 'spec/facter/resolvers/macosx/mountpoints_resolver_spec.rb'
     - 'spec/facter/resolvers/utils/windows/network_utils_spec.rb'
 
-# Offense count: 118
+# Offense count: 114
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: have_received, receive
 RSpec/MessageSpies:
@@ -138,8 +137,6 @@ RSpec/MessageSpies:
     - 'spec/facter/facts/macosx/os/architecture_spec.rb'
     - 'spec/facter/facts/macosx/os/family_spec.rb'
     - 'spec/facter/facts/macosx/os/name_spec.rb'
-    - 'spec/facter/facts/sles/os/architecture_spec.rb'
-    - 'spec/facter/facts/sles/os/name_spec.rb'
     - 'spec/facter/facts/sles/os/release_spec.rb'
     - 'spec/facter/facts/solaris/os/architecture_spec.rb'
     - 'spec/facter/facts/solaris/os/name_spec.rb'
@@ -192,7 +189,7 @@ RSpec/SubjectStub:
     - 'spec/custom_facts/util/fact_spec.rb'
     - 'spec/custom_facts/util/resolution_spec.rb'
 
-# Offense count: 180
+# Offense count: 162
 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
 RSpec/VerifiedDoubles:
   Exclude:
@@ -231,11 +228,6 @@ RSpec/VerifiedDoubles:
     - 'spec/facter/resolvers/bsd/ffi_helper_spec.rb'
     - 'spec/facter/resolvers/macosx/mountpoints_resolver_spec.rb'
     - 'spec/facter/resolvers/mountpoints_resolver_spec.rb'
-    - 'spec/facter/resolvers/solaris/current_zone_resolver_spec.rb'
-    - 'spec/facter/resolvers/solaris/solaris_release_resolver_spec.rb'
-    - 'spec/facter/resolvers/solaris/zfs_resolver_spec.rb'
-    - 'spec/facter/resolvers/solaris/zone_resolver_spec.rb'
-    - 'spec/facter/resolvers/solaris/zpool_resolver_spec.rb'
     - 'spec/facter/resolvers/utils/windows/network_utils_spec.rb'
     - 'spec/facter/resolvers/utils/windows/win32ole_spec.rb'
     - 'spec/facter/resolvers/windows/dmi_bios_resolver_spec.rb'

lib/custom_facts/core/execution/base.rb

@@ -4,6 +4,8 @@ module Facter
   module Core
     module Execution
       class Base
+        STDERR_MESSAGE = 'Command %s resulted with the following stderr message: %s'
+
         def with_env(values)
           old = {}
           values.each do |var, value|
@@ -34,12 +36,13 @@ def with_env(values)
         def execute(command, options = {})
           on_fail = options.fetch(:on_fail, :raise)
           expand = options.fetch(:expand, true)
+          logger = options[:logger]
 
           # Set LC_ALL and LANG to force i18n to C for the duration of this exec;
           # this ensures that any code that parses the
           # output of the command can expect it to be in a consistent / predictable format / locale
           with_env 'LC_ALL' => 'C', 'LANG' => 'C' do
-            expanded_command = if !expand && builtin_command?(command)
+            expanded_command = if !expand && builtin_command?(command) || logger
                                  command
                                else
                                  expand_command(command)
@@ -54,30 +57,35 @@ def execute(command, options = {})
               return on_fail
             end
 
-            execute_command(expanded_command, on_fail)
+            execute_command(expanded_command, on_fail, logger)
           end
         end
 
         private
 
-        def log_stderr_from_file(msg, command)
+        def log_stderr(msg, command, logger)
           return if !msg || msg.empty?
 
-          file_name = command.split('/').last
-          logger = Facter::Log.new(file_name)
-          logger.warn(msg.strip)
+          if logger
+            logger.debug(format(STDERR_MESSAGE, command, msg.strip))
+          else
+            file_name = command.split('/').last
+            logger = Facter::Log.new(file_name)
+            logger.warn(format(STDERR_MESSAGE, command, msg.strip))
+          end
         end
 
         def builtin_command?(command)
           output, _status = Open3.capture2("type #{command}")
           output.chomp =~ /builtin/ ? true : false
         end
 
-        def execute_command(command, on_fail)
+        def execute_command(command, on_fail, logger = nil)
           begin
             out, stderr, _status_ = Open3.capture3(command.to_s)
-            log_stderr_from_file(stderr, command)
+            log_stderr(stderr, command, logger)
           rescue StandardError => e
+            return '' if logger
             return on_fail unless on_fail == :raise
 
             raise Facter::Core::Execution::ExecutionFailure.new,

lib/facts_utils/uptime_parser.rb

@@ -8,6 +8,8 @@ class UptimeParser
     SECS_IN_AN_HOUR = 3_600
     SECS_IN_A_MINUTE = 60
 
+    @log = Facter::Log.new(self)
+
     class << self
       def uptime_seconds_unix
         uptime_proc_uptime || uptime_sysctl || uptime_executable
@@ -16,19 +18,20 @@ def uptime_seconds_unix
       private
 
       def uptime_proc_uptime
-        output, _stderr, _status = Open3.capture3("/bin/cat #{uptime_file} 2>/dev/null")
+        output = Facter::Core::Execution.execute("/bin/cat #{uptime_file}", logger: @log)
 
         output.chomp.split(' ').first.to_i unless output.empty?
       end
 
       def uptime_sysctl
-        output, _stderr, _status = Open3.capture3("sysctl -n #{uptime_sysctl_variable} 2>/dev/null")
+        output = Facter::Core::Execution.execute("sysctl -n #{uptime_sysctl_variable}", logger: @log)
 
         compute_uptime(Time.at(output.match(/\d+/)[0].to_i)) unless output.empty?
       end
 
       def uptime_executable
-        output, _stderr, _status = Open3.capture3(uptime_executable_cmd + ' 2>/dev/null')
+        output = Facter::Core::Execution.execute(uptime_executable_cmd, logger: @log)
+
         return unless output
 
         up = 0

lib/resolvers/aix/mountpoints.rb

@@ -17,7 +17,7 @@ def post_resolve(fact_name)
 
           def read_mount(fact_name)
             @fact_list[:mountpoints] = {}
-            output, _status = Open3.capture2('mount 2>/dev/null')
+            output = Facter::Core::Execution.execute('mount', logger: log)
             output.split("\n").map do |line|
               next if line =~ /\snode\s|---|procfs|ahafs/
 
@@ -32,7 +32,7 @@ def read_mount(fact_name)
           end
 
           def retrieve_sizes_for_mounts
-            output, _status = Open3.capture2('df -P 2>/dev/null')
+            output = Facter::Core::Execution.execute('df -P', logger: log)
             output.split("\n").map do |line|
               next if line =~ /Filesystem|-\s+-\s+-/
 

lib/resolvers/aix/networking_resolver.rb

@@ -15,7 +15,7 @@ def post_resolve(fact_name)
           end
 
           def read_netstat(fact_name)
-            output, _status = Open3.capture2('netstat -rn')
+            output = Facter::Core::Execution.execute('netstat -rn', logger: log)
             output = output.split("\n").select { |line| (line =~ /\s\s[0-9]+.[0-9]+.[0-9]+.[0-9]+|\s\s.*:[0-9a-f]+/) }
             get_primary_interface_info(output)
 

lib/resolvers/aix/os_level_resolver.rb

@@ -14,7 +14,7 @@ def post_resolve(fact_name)
         end
 
         def read_oslevel(fact_name)
-          output, _status = Open3.capture2('/usr/bin/oslevel -s 2>/dev/null')
+          output = Facter::Core::Execution.execute('/usr/bin/oslevel -s', logger: log)
           @fact_list[:build] = output
           @fact_list[:kernel] = 'AIX'
 

lib/resolvers/aix/partitions.rb

@@ -4,7 +4,6 @@ module Facter
   module Resolvers
     module Aix
       class Partitions < BaseResolver
-        @log = Facter::Log.new(self)
         @semaphore = Mutex.new
         @fact_list ||= {}
         class << self
@@ -40,11 +39,9 @@ def query_cudv(fact_name)
           end
 
           def populate_from_lslv(name)
-            stdout, stderr, _status = Open3.capture3("lslv -L #{name}")
-            if stdout.empty?
-              @log.debug(stderr)
-              return
-            end
+            stdout = Facter::Core::Execution.execute("lslv -L #{name}", logger: log)
+
+            return if stdout.empty?
 
             info_hash = extract_info(stdout)
             size_bytes = compute_size(info_hash)

lib/resolvers/aix/utils/odm_query.rb

@@ -8,6 +8,7 @@
 module Facter
   class ODMQuery
     REPOS = %w[CuAt CuDv PdAt PdDv].freeze
+    @log = Facter::Log.new(self)
 
     def initialize
       @query = ''
@@ -29,7 +30,7 @@ def execute
       REPOS.each do |repo|
         break if result && !result.empty?
 
-        result, _stderr, _s = Open3.capture3("#{query} #{repo}")
+        result = Facter::Core::Execution.execute("#{query} #{repo}", logger: @log)
       end
       result
     end

lib/resolvers/augeas_resolver.rb

@@ -14,7 +14,7 @@ def post_resolve(fact_name)
         end
 
         def read_augeas_version(fact_name)
-          output, _status = Open3.capture2('augparse --version 2>&1')
+          output = Facter::Core::Execution.execute('augparse --version 2>&1', logger: log)
           @fact_list[:augeas_version] = Regexp.last_match(1) if output =~ /^augparse (\d+\.\d+\.\d+)/
           @fact_list[fact_name]
         end

lib/resolvers/eos_release_resolver.rb

@@ -18,7 +18,9 @@ def post_resolve(fact_name)
         end
 
         def read_eos_release(fact_name)
-          output, _status = Open3.capture2('cat /etc/Eos-release')
+          output = Util::FileHelper.safe_read('/etc/Eos-release', nil)
+          return @fact_list[fact_name] = nil if output.nil?
+
           output_strings = output.split(' ')
 
           @fact_list[:name] = output_strings[0]

lib/resolvers/freebsd/freebsd_version_resolver.rb

@@ -15,14 +15,12 @@ def post_resolve(fact_name)
           end
 
           def freebsd_version_system_call(fact_name)
-            output, _stderr, status = Open3.capture3('/bin/freebsd-version -kru')
-            return nil unless status.success?
+            output = Facter::Core::Execution.execute('/bin/freebsd-version -kru', logger: log)
+            return if output.empty?
 
             build_fact_list(output)
 
             @fact_list[fact_name]
-          rescue Errno::ENOENT
-            nil
           end
 
           def build_fact_list(output)

lib/resolvers/hostname_resolver.rb

@@ -3,7 +3,6 @@
 module Facter
   module Resolvers
     class Hostname < BaseResolver
-      @log = Facter::Log.new(self)
       @semaphore = Mutex.new
       @fact_list ||= {}
       class << self
@@ -14,7 +13,7 @@ def post_resolve(fact_name)
         end
 
         def retrieve_hostname(fact_name)
-          output, _status = Open3.capture2('hostname')
+          output = Facter::Core::Execution.execute('hostname', logger: log)
 
           # get domain
           domain = read_domain(output)

lib/resolvers/lpar_resolver.rb

@@ -14,8 +14,7 @@ def post_resolve(fact_name)
         end
 
         def read_lpar(fact_name)
-          lpar_cmd = '/usr/bin/lparstat -i'
-          output, _status = Open3.capture2(lpar_cmd)
+          output = Facter::Core::Execution.execute('/usr/bin/lparstat -i', logger: log)
           output.each_line do |line|
             populate_lpar_data(line.split(':').map(&:strip))
           end

lib/resolvers/lsb_release_resolver.rb

@@ -10,7 +10,6 @@ class LsbRelease < BaseResolver
       # :codename
 
       @semaphore = Mutex.new
-      @log = Facter::Log.new(self)
       @fact_list ||= {}
 
       class << self
@@ -27,14 +26,12 @@ def retrieve_facts(fact_name)
         end
 
         def lsb_release_installed?
-          output, stderr, _status = Open3.capture3('which lsb_release')
-          @log.debug(stderr) unless stderr.empty?
+          output = Facter::Core::Execution.execute('which lsb_release', logger: log)
           @fact_list[:lsb_release_installed] = !output.empty?
         end
 
         def read_lsb_release_file
-          output, stderr, _status = Open3.capture3('lsb_release -a')
-          @log.debug(stderr) unless stderr.empty?
+          output = Facter::Core::Execution.execute('lsb_release -a', logger: log)
           build_fact_list(output)
         end
 

lib/resolvers/macosx/dmi_resolver.rb

@@ -4,7 +4,6 @@ module Facter
   module Resolvers
     module Macosx
       class DmiBios < BaseResolver
-        @log = Facter::Log.new(self)
         @semaphore = Mutex.new
         @fact_list ||= {}
 
@@ -19,7 +18,7 @@ def post_resolve(fact_name)
 
           def read_facts
             # OSX only supports the product name
-            output, _status = Open3.capture2('sysctl -n hw.model')
+            output = Facter::Core::Execution.execute('sysctl -n hw.model', logger: log)
             @fact_list[:macosx_model] = output&.strip
           end
         end

lib/resolvers/macosx/filesystems_resolver.rb

@@ -7,7 +7,6 @@ class Filesystems < BaseResolver
         # :macosx_filesystems
         @semaphore = Mutex.new
         @fact_list ||= {}
-        @log = Facter::Log.new(self)
 
         class << self
           private
@@ -17,7 +16,7 @@ def post_resolve(fact_name)
           end
 
           def read_filesystems(fact_name)
-            output, _status = Open3.capture2('mount')
+            output = Facter::Core::Execution.execute('mount', logger: log)
             filesystems = []
             output.each_line do |line|
               filesystem = line.match(/\(([a-z]+)\,*/).to_s