(PUP-10985) Output facts existing only in Facter 4 via diff action

This commit aims to improve the `puppet facts diff` CLI command by:
- showing all differences between Facter 3 and Facter 4 (including facts
  available only in Facter 4)
- sorting output results in alphabetical order for better human readability
- allowing users to remove certain facts from output (via the `--exclude
  <regex>` CLI option)
- including a CLI option to see results in a fully structured form
- adding various code improvements

Author: luchihoratiu

lib/puppet/face/facts.rb

@@ -2,19 +2,19 @@
 require 'puppet/node/facts'
 require 'puppet/util/fact_dif'
 
-EXCLUDE_LIST = %w[ facterversion
-  load_averages\..*
-  processors\.speed
-  swapfree swapfree_mb
-  memoryfree memoryfree_mb
-  memory\.swap\.available_bytes memory\.swap\.used_bytes
-  memory\.swap\.available memory\.swap\.capacity memory\.swap\.used
-  memory\.system\.available_bytes memory\.system\.used_bytes
-  memory\.system\.available memory\.system\.capacity memory\.system\.used
-  mountpoints\..*\.available.* mountpoints\..*\.capacity mountpoints\..*\.used.*
-  sp_uptime system_profiler\.uptime
-  uptime uptime_days uptime_hours uptime_seconds
-  system_uptime\.uptime system_uptime\.days system_uptime\.hours system_uptime\.seconds
+EXCLUDE_LIST = %w[ ^facterversion$
+  ^load_averages\..*$
+  ^processors\.speed$
+  ^swapfree$ ^swapfree_mb$
+  ^memoryfree$ ^memoryfree_mb$
+  ^memory\.swap\.available_bytes$ ^memory\.swap\.used_bytes$
+  ^memory\.swap\.available$ ^memory\.swap\.capacity$ ^memory\.swap\.used$
+  ^memory\.system\.available_bytes$ ^memory\.system\.used_bytes$
+  ^memory\.system\.available$ ^memory\.system\.capacity$ ^memory\.system\.used$
+  ^mountpoints\..*\.available.*$ ^mountpoints\..*\.capacity$ ^mountpoints\..*\.used.*$
+  ^sp_uptime$ ^system_profiler\.uptime$
+  ^uptime$ ^uptime_days$ ^uptime_hours$ ^uptime_seconds$
+  ^system_uptime\.uptime$ ^system_uptime\.days$ ^system_uptime\.hours$ ^system_uptime\.seconds$
 ]
 
 Puppet::Indirector::Face.define(:facts, '0.0.1') do
@@ -117,7 +117,18 @@
     $ puppet facts diff
     EOT
 
+    option("--structured") do
+      default_to { false }
+      summary _("Render the different facts as structured.")
+    end
+
+    option("--exclude " + _("<regex>")) do
+      summary _("Regex used to exclude specific facts from diff.")
+    end
+
     when_invoked do |*args|
+      options = args.pop
+
       Puppet.settings.preferred_run_mode = :agent
       Puppet::Node::Facts.indirection.terminus_class = :facter
 
@@ -133,7 +144,8 @@
         facter_3_result = Puppet::Util::Execution.execute("#{puppet_show_cmd} --no-facterng #{cmd_flags}")
         facter_ng_result = Puppet::Util::Execution.execute("#{puppet_show_cmd} --facterng #{cmd_flags}")
 
-        fact_diff = FactDif.new(facter_3_result, facter_ng_result, EXCLUDE_LIST)
+        exclude_list = options[:exclude].nil? ? EXCLUDE_LIST : EXCLUDE_LIST + [ options[:exclude] ]
+        fact_diff = FactDif.new(facter_3_result, facter_ng_result, exclude_list, options[:structured])
         fact_diff.difs
       else
         Puppet.warning _("Already using Facter 4. To use `puppet facts diff` remove facterng from the .conf file or run `puppet config set facterng false`.")
@@ -234,4 +246,3 @@
     end
   end
 end
-

lib/puppet/util/fact_dif.rb

@@ -1,15 +1,24 @@
 require 'json'
 
 class FactDif
-  def initialize(old_output, new_output, exclude_list = [])
+  def initialize(old_output, new_output, exclude_list, save_structured)
     @c_facter = JSON.parse(old_output)
     @next_facter = JSON.parse(new_output)
     @exclude_list = exclude_list
+    @save_structured = save_structured
+    @flat_diff = []
     @diff = {}
   end
 
   def difs
-    search_hash(@c_facter, [])
+    search_hash(((@c_facter.to_a - @next_facter.to_a) | (@next_facter.to_a - @c_facter.to_a)).to_h)
+
+    @flat_diff.sort_by { |a| a[0] }.each do |pair|
+      fact_path = pair[0]
+      value = pair[1]
+      compare(fact_path, value, @c_facter)
+      compare(fact_path, value, @next_facter)
+    end
 
     @diff
   end
@@ -28,35 +37,45 @@ def search_hash(sh, path = [])
         path.pop
       end
     else
-      compare(path, sh)
+      @flat_diff.push([path.dup, sh])
     end
   end
 
-  def compare(fact_path, old_value)
-    new_value = @next_facter.dig(*fact_path)
-    if different?(new_value, old_value) && !excluded?(fact_path.join('.'))
-      @diff[fact_path.join('.')] = { new_value: new_value, old_value: old_value }
+  def compare(fact_path, given_value, compared_hash)
+    compared_value = compared_hash.dig(*fact_path)
+    if different?(compared_value, given_value) && !excluded?(fact_path.join('.'))
+      fact_path = fact_path.map{|f| f.to_s.include?('.') ? "\"#{f}\"" : f}.join('.') unless @save_structured
+      if compared_hash == @c_facter
+        bury(*fact_path, { :new_value => given_value, :old_value => compared_value }, @diff)
+      else
+        bury(*fact_path, { :new_value => compared_value, :old_value => given_value }, @diff)
+      end
+    end
+  end
+
+  def bury(*paths, value, hash)
+    if paths.count > 1
+      path = paths.shift
+      hash[path] = Hash.new unless hash.key?(path)
+      bury(*paths, value, hash[path])
+    else
+      hash[*paths] = value
     end
   end
 
   def different?(new, old)
-    if old.is_a?(String) && new.is_a?(String)
+    if old.is_a?(String) && new.is_a?(String) && (old.include?(',') || new.include?(','))
       old_values = old.split(',')
       new_values = new.split(',')
 
-      diff = old_values - new_values
-      # also add new entries only available in Facter 4
-      diff.concat(new_values - old_values)
-
-      return true if diff.any?
-
-      return false
+      diff = (old_values - new_values) | (new_values - old_values)
+      return diff.size.positive?
     end
 
     old != new
   end
 
   def excluded?(fact_name)
-    @exclude_list.any? {|excluded_fact| fact_name =~ /^#{excluded_fact}$/}
+    @exclude_list.any? {|excluded_fact| fact_name =~ /#{excluded_fact}/}
   end
 end