Merge branch 'master' into glob_modulepath_support

Author: ahayworth

lib/octocatalog-diff/catalog-diff/differ.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'diffy'
+require 'digest'
 require 'hashdiff'
 require 'json'
 require 'set'
@@ -11,6 +12,8 @@
 require_relative '../util/util'
 require_relative 'filter'
 
+HashDiff = Hashdiff unless defined? HashDiff
+
 module OctocatalogDiff
   module CatalogDiff
     # Calculate the difference between two Puppet catalogs.
@@ -263,7 +266,7 @@ def filter_and_cleanup(catalog_resources)
 
             # Handle parameters
             if k == 'parameters'
-              cleansed_param = cleanse_parameters_hash(v)
+              cleansed_param = cleanse_parameters_hash(v, resource.fetch('sensitive_parameters', []))
               hsh[k] = cleansed_param unless cleansed_param.nil? || cleansed_param.empty?
             elsif k == 'tags'
               # The order of tags is unimportant. Sort this array to avoid false diffs if order changes.
@@ -456,10 +459,18 @@ def ignored?(diff)
 
       # Cleanse parameters of filtered attributes.
       # @param parameters_hash [Hash] Hash of parameters
+      # @param sensitive_parameters [Array] Array of sensitive parameters
       # @return [Hash] Cleaned parameters hash (original input hash is not altered)
-      def cleanse_parameters_hash(parameters_hash)
+      def cleanse_parameters_hash(parameters_hash, sensitive_parameters)
         result = parameters_hash.dup
 
+        # hides sensitive params. We still need to know if there's a going to
+        # be a diff, so we hash the value.
+        sensitive_parameters.each do |p|
+          md5 = Digest::MD5.hexdigest Marshal.dump(result[p])
+          result[p] = 'Sensitive [md5sum ' + md5 + ']'
+        end
+
         # 'before' and 'require' handle internal Puppet ordering but do not affect what
         # happens on the target machine. Don't consider these for the purpose of catalog diff.
         result.delete('before')

lib/octocatalog-diff/util/parallel.rb

@@ -129,22 +129,26 @@ def self.run_tasks_parallel(result, task_array, logger)
 
         # Waiting for children and handling results
         while pidmap.any?
-          this_pid, exit_obj = Process.wait2(0)
-          next unless this_pid && pidmap.key?(this_pid)
-          index = pidmap[this_pid][:index]
-          exitstatus = exit_obj.exitstatus
-          raise "PID=#{this_pid} exited abnormally: #{exit_obj.inspect}" if exitstatus.nil?
-          raise "PID=#{this_pid} exited with status #{exitstatus}" unless exitstatus.zero?
-
-          input = File.read(File.join(ipc_tempdir, "#{this_pid}.dat"))
-          result[index] = Marshal.load(input) # rubocop:disable Security/MarshalLoad
-          time_delta = Time.now - pidmap[this_pid][:start_time]
-          pidmap.delete(this_pid)
-
-          logger.debug "PID=#{this_pid} completed in #{time_delta} seconds, #{input.length} bytes"
-
-          next if result[index].status
-          return result[index].exception
+          pidmap.each do |pid|
+            status = Process.waitpid2(pid[0], Process::WNOHANG)
+            next if status.nil?
+            this_pid, exit_obj = status
+            next unless this_pid && pidmap.key?(this_pid)
+            index = pidmap[this_pid][:index]
+            exitstatus = exit_obj.exitstatus
+            raise "PID=#{this_pid} exited abnormally: #{exit_obj.inspect}" if exitstatus.nil?
+            raise "PID=#{this_pid} exited with status #{exitstatus}" unless exitstatus.zero?
+
+            input = File.read(File.join(ipc_tempdir, "#{this_pid}.dat"))
+            result[index] = Marshal.load(input) # rubocop:disable Security/MarshalLoad
+            time_delta = Time.now - pidmap[this_pid][:start_time]
+            pidmap.delete(this_pid)
+
+            logger.debug "PID=#{this_pid} completed in #{time_delta} seconds, #{input.length} bytes"
+
+            next if result[index].status
+            return result[index].exception
+          end
         end
 
         logger.debug 'All child processes completed with no exceptions raised'

spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb

@@ -382,6 +382,30 @@
           result = testobj.catalog1
           expect(result.first['title']).to eq('/etc/foo')
         end
+
+        it 'should hide sensitive parameters' do
+          json_hash = {
+            'document_type' => 'Catalog',
+            'data' => {
+              'name' => 'rspec-node.github.net',
+              'tags' => [],
+              'resources' => [
+                {
+                  'type' => 'File',
+                  'title' => 'verysecretfile',
+                  'parameters' => {
+                    'content' => 'secret1'
+                  },
+                  'sensitive_parameters' => ['content']
+                }
+              ]
+            }
+          }
+          catalog = OctocatalogDiff::Catalog.create(json: JSON.generate(json_hash))
+          testobj = OctocatalogDiff::CatalogDiff::Differ.new(@options, catalog, @empty_puppet_catalog)
+          result = testobj.catalog1
+          expect(result.first['parameters']['content']).to eq('Sensitive [md5sum e52d98c459819a11775936d8dfbb7929]')
+        end
       end
 
       describe '#diff' do

spec/octocatalog-diff/tests/util/parallel_spec.rb

@@ -16,6 +16,65 @@
   end
 
   context 'with parallel processing' do
+    it 'should only Process.wait() its own children' do
+      class Foo
+        def one(arg, _logger = nil)
+          'one ' + arg
+        end
+
+        def two(arg, _logger = nil)
+          'two ' + arg
+        end
+
+        def dont_wait_me_bro(sleep_for = 1)
+          # do we need a rescue block here?
+          pid = fork do
+            sleep sleep_for
+            Kernel.exit! 0 # Kernel.exit! avoids at_exit from parents being triggered by children exiting
+          end
+          pid
+        end
+
+        def wait_on_me(pid)
+          status = nil
+          # just in case status never equals anything
+          count = 100
+          while status.nil? || count > 0
+            count -= 1
+            status = Process.waitpid2(pid, Process::WNOHANG)
+          end
+          status
+        end
+      end
+
+      c = Foo.new
+      # start my non-parallel process first
+      just_a_guy = c.dont_wait_me_bro
+
+      one = OctocatalogDiff::Util::Parallel::Task.new(method: c.method(:one), args: 'abc', description: 'test1')
+      two = OctocatalogDiff::Util::Parallel::Task.new(method: c.method(:two), args: 'def', description: 'test2')
+      result = OctocatalogDiff::Util::Parallel.run_tasks([one, two], nil, true)
+      expect(result).to be_a_kind_of(Array)
+      expect(result.size).to eq(2)
+
+      one_result = result[0]
+      expect(one_result).to be_a_kind_of(OctocatalogDiff::Util::Parallel::Result)
+      expect(one_result.status).to eq(true)
+      expect(one_result.exception).to eq(nil)
+      expect(one_result.output).to match(/^one abc/)
+
+      two_result = result[1]
+      expect(two_result).to be_a_kind_of(OctocatalogDiff::Util::Parallel::Result)
+      expect(two_result.status).to eq(true)
+      expect(two_result.exception).to eq(nil)
+      expect(two_result.output).to match(/^two def/)
+
+      # just_a_guy should still be need to be waited
+      result = c.wait_on_me(just_a_guy)
+      expect(result).to be_a_kind_of(Array)
+      # test result and check for error conditions
+    end
+
     it 'should parallelize and return task results' do
       class Foo
         def one(arg, _logger = nil)