Merge branch 'master' into kpaulisse-hiera-5

Author: kpaulisse

.version

@@ -1 +1 @@
-1.4.1
+1.5.0

README.md

@@ -53,6 +53,7 @@ The example above reflects the changes in the Puppet catalog from switching an u
 - [Requirements](/doc/requirements.md)
 - [Limitations](/doc/limitations.md)
 - [List of all command line options](/doc/optionsref.md)
+- [Environment variables](/doc/advanced-environment-variables.md)
 
 ### Project
 

doc/CHANGELOG.md

@@ -8,6 +8,15 @@
 </tr>
 </thead><tbody>
 <tr valign=top>
+<td>1.5.0</td>
+<td>2017-10-18</td>
+<td>
+<li><a href="https://github.com/github/octocatalog-diff/pull/151">#151</a>: (Enhancement) Support text differences in files where `source` is an array</li>
+<li><a href="https://github.com/github/octocatalog-diff/pull/153">#153</a>: (Enhancement) Support for hiera 5</li>
+<li><a href="https://github.com/github/octocatalog-diff/pull/152">#152</a>: (Internal) Better temporary directory handling</li>
+</td>
+</tr>
+<tr valign=top>
 <td>1.4.1</td>
 <td>2017-10-02</td>
 <td>

doc/advanced-environment-variables.md

@@ -0,0 +1,70 @@
+# Environment variables
+
+The following environment variables have special meaning to octocatalog-diff:
+
+### `OCTOCATALOG_DIFF_CONFIG_FILE`
+
+This environment variable is used to locate the configuration file for the CLI. The use of configuration files is described generally in:
+
+- [Configuration](/doc/configuration.md)
+
+### `OCTOCATALOG_DIFF_CUSTOM_VERSION`
+
+When set, the `octocatalog-diff` CLI will display this as the version number within debugging, instead of the version number in the package. This is most useful if you want to use or include a git SHA in the version number.
+
+```
+$ export OCTOCATALOG_DIFF_CUSTOM_VERSION="@$(git rev-parse HEAD)"
+$ octocatalog-diff -d ...
+D, [2017-10-12T08:57:46.454738 #35205] DEBUG -- : Running octocatalog-diff @504d7f3c91267e5193beb103caae5d4d8cebfee3 with ruby 2.3.1
+...
+```
+
+### `OCTOCATALOG_DIFF_DEVELOPER_PATH`
+
+When set, instead of loading libraries from the system or bundler location, libraries will be loaded from the specified value of this environment variable. This is used internally for development as we point users to unreleased code for debugging or testing.
+
+```
+$ export OCTOCATALOG_DIFF_DEVELOPER_PATH=$HOME/git-checkouts/octocatalog-diff
+$ octocatalog-diff ...
+```
+
+### `OCTOCATALOG_DIFF_TEMPDIR`
+
+When set:
+
+- `octocatalog-diff` will create all of its temporary directories within the specified directory.
+- `octocatalog-diff` will not attempt to remove any temporary directories it creates.
+
+This is useful in the following situations:
+
+- You are calling `octocatalog-diff` from within another program which is highly parallelized, and `at_exit` handlers are difficult to implement. Instead of figuring that all out (if it can even be figured out), you create a temporary directory before the parallelized logic, and remove it afterwards.
+
+- You wish to debug intermediate output. For example, you may be instructed to set this variable and send some of the output to the project maintainers if you request assistance.
+
+This variable is used internally for the parallelized logic for catalog compilation, but the value set from the environment will override any internal usage.
+
+### `OCTOCATALOG_DIFF_VERSION`
+
+This variable is used when building the gem, to override the default version. This is used for internal testing of `octocatalog-diff` before public releases. This variable is not useful outside the build context.
+
+### `PUPPETDB_HOST`
+
+This variable specifies the fully qualified domain name or IP address of the PuppetDB server.
+
+Note: If `PUPPETDB_URL` is specified, then `PUPPETDB_HOST` is not consulted.
+
+### `PUPPETDB_PORT`
+
+This variable specifies the port number of the PuppetDB server.
+
+Note: If `PUPPETDB_URL` is specified, then `PUPPETDB_PORT` is not consulted.
+
+### `PUPPETDB_URL`
+
+This variable specifies the URL to the PuppetDB server.
+
+Example: `https://puppetdb.example.net:8081`
+
+### `PUPPET_FACT_DIR`
+
+This variable specifies the directory path where puppet fact files are stored. (Fact files must be named `<fqdn>.yaml` where `<fqdn>` is specified when running `octocatalog-diff`.)

doc/dev/releasing.md

@@ -6,24 +6,13 @@ The project maintainers are responsible for bumping the version number, regenera
 
 *This procedure is performed by a GitHubber.*
 
-To test the new version of `octocatalog-diff` in the GitHub Puppet repository:
+To test the new version of `octocatalog-diff` in the GitHub Puppet repository, check out `github/puppet` and:
 
-0. In a checkout of the GitHub Puppet repository, start a new branch based off master.
-0. In the `octocatalog-diff` checkout:
-  - Ensure that the desired branch is checked out.
-  - Choose a unique internal version number which has never been used in CI. A good guideline is that if you're planning to release a version `0.6.0` then for these tests, use `0.6.0a`, `0.6.0b`, ...
-  - Build the gem using your internal version number:
-
-    ```
-    OCTOCATALOG_DIFF_VERSION=0.6.0a rake gem:force-build
-    ```
-  - Run the task to install the gem into your Puppet checkout:
-
-    ```
-    OCTOCATALOG_DIFF_VERSION=0.6.0a rake gem:localinstall
-    ```
-
-0. Back in the Puppet checkout, ensure that the changes are as expected (updates to Gemfile / Gemfile.lock, addition of new gem). Push the change and build appropriate CI job(s) to validate the changes.
+- Start a new branch based off master
+- Run `script/update-octocatalog-diff -r <ocd_branch_name>`
+- Confirm and commit the result
+- Make sure all CI jobs pass
+- Run the `puppet-catalog-diff` CI job and make sure it passes and shows expected results
 
 ## Merging one PR
 

lib/octocatalog-diff/catalog-util/builddir.rb

@@ -4,6 +4,7 @@
 
 require_relative '../facts'
 require_relative 'enc'
+require_relative '../util/util'
 
 module OctocatalogDiff
   module CatalogUtil
@@ -38,9 +39,7 @@ class BuildDir
       # @param options [Hash] Options for class; see above description
       def initialize(options = {}, logger = nil)
         @options = options.dup
-        @tempdir = Dir.mktmpdir('ocd-builddir-')
-        at_exit { FileUtils.rm_rf(@tempdir) if File.directory?(@tempdir) }
-
+        @tempdir = OctocatalogDiff::Util::Util.temp_dir('ocd-builddir-')
         @factdir = nil
         @enc = nil
         @fact_file = nil

lib/octocatalog-diff/catalog-util/fileresources.rb

@@ -27,18 +27,20 @@ def self.convert_file_resources(obj, environment = 'production')
       # module path that is specified within the environment.conf file (assuming the default 'modules'
       # directory doesn't exist or the module isn't found in there). If the file can't be found then
       # this returns nil which may trigger an error.
-      # @param src [String] A file reference: puppet:///modules/xxx/yyy
+      # @param src_in [String|Array] A file reference: puppet:///modules/xxx/yyy
       # @param modulepaths [Array] Cached module path
       # @return [String] File system path to referenced file
-      def self.file_path(src, modulepaths)
-        unless src =~ %r{^puppet:///modules/([^/]+)/(.+)}
-          raise ArgumentError, "Bad parameter source #{src}"
-        end
+      def self.file_path(src_in, modulepaths)
+        valid_sources = [src_in].flatten.select { |line| line =~ %r{\Apuppet:///modules/([^/]+)/(.+)} }
+        return unless valid_sources.any?
 
-        path = File.join(Regexp.last_match(1), 'files', Regexp.last_match(2))
-        modulepaths.each do |mp|
-          file = File.join(mp, path)
-          return file if File.exist?(file)
+        valid_sources.each do |src|
+          src =~ %r{\Apuppet:///modules/([^/]+)/(.+)}
+          path = File.join(Regexp.last_match(1), 'files', Regexp.last_match(2))
+          modulepaths.each do |mp|
+            file = File.join(mp, path)
+            return file if File.exist?(file)
+          end
         end
 
         nil
@@ -125,9 +127,14 @@ def self.resource_convertible?(resource)
                        !resource['parameters'].nil? && \
                        resource['parameters'].key?('source') && \
                        !resource['parameters'].key?('content') && \
-                       resource['parameters']['source'] =~ %r{^puppet:///modules/([^/]+)/(.+)}
+                       valid_sources?(resource)
+
         false
       end
+
+      def self.valid_sources?(resource)
+        [resource['parameters']['source']].flatten.select { |line| line =~ %r{\Apuppet:///modules/([^/]+)/(.+)} }.any?
+      end
     end
   end
 end

lib/octocatalog-diff/catalog/computed.rb

@@ -11,6 +11,7 @@
 require_relative '../catalog-util/facts'
 require_relative '../util/puppetversion'
 require_relative '../util/scriptrunner'
+require_relative '../util/util'
 
 module OctocatalogDiff
   class Catalog
@@ -69,23 +70,6 @@ def convert_file_resources(dry_run = false)
         OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(self, environment)
       end
 
-      private
-
-      # Private method: Clean up a checkout directory, if it exists
-      def cleanup_checkout_dir(checkout_dir, logger)
-        return unless File.directory?(checkout_dir)
-        logger.debug("Cleaning up temporary directory #{checkout_dir}")
-        # Sometimes this seems to break when handling the recursive removal when running under
-        # a parallel environment. Trap and ignore the errors here if we don't care about them.
-        begin
-          FileUtils.remove_entry_secure checkout_dir
-          # :nocov:
-        rescue Errno::ENOTEMPTY, Errno::ENOENT => exc
-          logger.debug "cleanup_checkout_dir(#{checkout_dir}) logged #{exc.class} - this can be ignored"
-          # :nocov:
-        end
-      end
-
       # Private method: Bootstrap a directory
       def bootstrap(logger)
         return if @builddir
@@ -99,9 +83,7 @@ def bootstrap(logger)
           tmphash[:basedir] = @options[:bootstrapped_dir]
         elsif @options[:branch] == '.'
           if @options[:bootstrap_current]
-            tmphash[:basedir] =  Dir.mktmpdir('ocd-bootstrap-basedir-')
-            at_exit { cleanup_checkout_dir(tmphash[:basedir], logger) }
-
+            tmphash[:basedir] = OctocatalogDiff::Util::Util.temp_dir('ocd-bootstrap-basedir-')
             FileUtils.cp_r File.join(@options[:basedir], '.'), tmphash[:basedir]
 
             o = @options.reject { |k, _v| k == :branch }.merge(path: tmphash[:basedir])
@@ -110,10 +92,8 @@ def bootstrap(logger)
             tmphash[:basedir] = @options[:basedir]
           end
         else
-          checkout_dir = Dir.mktmpdir('ocd-bootstrap-checkout-')
-          at_exit { cleanup_checkout_dir(checkout_dir, logger) }
-          tmphash[:basedir] = checkout_dir
-          OctocatalogDiff::CatalogUtil::Bootstrap.bootstrap_directory(@options.merge(path: checkout_dir), logger)
+          tmphash[:basedir] = OctocatalogDiff::Util::Util.temp_dir('ocd-bootstrap-checkout-')
+          OctocatalogDiff::CatalogUtil::Bootstrap.bootstrap_directory(@options.merge(path: tmphash[:basedir]), logger)
         end
 
         # Create and populate the temporary directory

lib/octocatalog-diff/util/parallel.rb

@@ -6,6 +6,7 @@
 # this instead executes the tasks serially, but provides the same API as the parallel tasks.
 
 require 'stringio'
+require_relative 'util'
 
 module OctocatalogDiff
   module Util
@@ -107,13 +108,14 @@ def self.run_tasks(task_array, logger = nil, parallelized = true, raise_exceptio
       # @return [Exception] First exception encountered by a child process; returns nil if no exceptions encountered.
       def self.run_tasks_parallel(result, task_array, logger)
         pidmap = {}
-        ipc_tempdir = Dir.mktmpdir('ocd-ipc-')
+        ipc_tempdir = OctocatalogDiff::Util::Util.temp_dir('ocd-ipc-')
 
         # Child process forking
         task_array.each_with_index do |task, index|
           # simplecov doesn't see this because it's forked
           # :nocov:
           this_pid = fork do
+            ENV['OCTOCATALOG_DIFF_TEMPDIR'] ||= ipc_tempdir
             task_result = execute_task(task, logger)
             File.open(File.join(ipc_tempdir, "#{Process.pid}.dat"), 'w') { |f| f.write Marshal.dump(task_result) }
             Kernel.exit! 0 # Kernel.exit! avoids at_exit from parents being triggered by children exiting
@@ -157,17 +159,6 @@ def self.run_tasks_parallel(result, task_array, logger)
             # If the process doesn't exist, that's fine.
           end
         end
-
-        retries = 0
-        while File.directory?(ipc_tempdir) && retries < 10
-          retries += 1
-          begin
-            FileUtils.remove_entry_secure ipc_tempdir
-          rescue Errno::ENOTEMPTY, Errno::ENOENT # rubocop:disable Lint/HandleExceptions
-            # Errno::ENOTEMPTY will trigger a retry because the directory exists
-            # Errno::ENOENT will break the loop because the directory won't exist next time it's checked
-          end
-        end
       end
 
       # Utility method! Not intended to be called from outside this class.

lib/octocatalog-diff/util/scriptrunner.rb

@@ -6,6 +6,8 @@
 require 'open3'
 require 'shellwords'
 
+require_relative 'util'
+
 module OctocatalogDiff
   module Util
     # This is a utility class to execute a built-in script.
@@ -91,14 +93,7 @@ def log(priority, message, logger = @logger)
       # @return [String] Path to tempfile containing script
       def temp_script(script)
         raise Errno::ENOENT, "Script '#{script}' not found" unless File.file?(script)
-        temp_dir = Dir.mktmpdir('ocd-scriptrunner-')
-        at_exit do
-          begin
-            FileUtils.remove_entry_secure temp_dir
-          rescue Errno::ENOENT # rubocop:disable Lint/HandleExceptions
-            # OK if the directory doesn't exist since we're trying to remove it anyway
-          end
-        end
+        temp_dir = OctocatalogDiff::Util::Util.temp_dir('ocd-scriptrunner')
         temp_file = File.join(temp_dir, File.basename(script))
         File.open(temp_file, 'w') { |f| f.write(File.read(script)) }
         FileUtils.chmod 0o755, temp_file