Merge pull request #98 from github/kpaulisse-git-checkout-script

Separate scripts and commands and make override-able

Author: kpaulisse

doc/advanced-script-override.md

@@ -0,0 +1,51 @@
+# Overriding external scripts
+
+## Background
+
+During normal operation, `octocatalog-diff` runs certain scripts or commands from the underlying operating system. For example, it may run `git` to check out a certain code branch, and run `puppet` to build a catalog.
+
+Each external script is found within the [`scripts`](/scripts) directory.
+
+## How to override scripts
+
+### Command line option
+
+It is possible to override these scripts with customized versions. To do this, specify a directory that contains replacement scripts via the command line:
+
+```
+octocatalog-diff [other options] --override-script-path /path/to/scripts ...
+```
+
+### Configuration file
+
+You can also specify this option via a [configuration file](/doc/configuration.md) setting:
+
+```
+settings[:override_script_path] = '/path/to/scripts'
+```
+
+### Writing replacement scripts
+
+Within the override script path you've configured, place a file with the same name as the built-in script. For example, if you wish to override the `git-extract.sh` script with a custom version, also name your script `git-extract.sh`. (Do NOT create subdirectories within the override directory.)
+
+If you specify an override script path but a particular script is not present there, octocatalog-diff will default to the built-in script. This means that you do not need to create unmodified copies of the built-in scripts. Only override the scripts you need to change.
+
+### Notes
+
+Please note that these scripts are considered part of octocatalog-diff, and not part of your Puppet codebase. Therefore, the path to your scripts must be an absolute path, and we do not support (or intend to support) using multiple script directories during the same run of octocatalog-diff.
+
+## Explanation of scripts
+
+This is an explanation of the [existing scripts supplied by octocatalog-diff](/scripts):
+
+- [`env.sh`](/scripts/env)
+
+    Prints out the environment. This is currently only used for spec tests.
+
+- [`git-extract.sh`](/scripts/git-extract)
+
+    Extracts a specified branch from the git repository into a specified target directory.
+
+- [`puppet.sh`](/scripts/puppet)
+
+    Runs puppet (with additional command line arguments), generally used to compile a catalog or determine the Puppet version.

doc/advanced.md

@@ -24,6 +24,7 @@ See also:
 - [Using `octocatalog-diff` without git](/doc/advanced-using-without-git.md)
 - [Catalog validation](/doc/advanced-catalog-validation.md)
 - [Environment setup](/doc/advanced-environments.md)
+- [Overriding built-in octocatalog-diff scripts](/doc/advanced-script-override.md)
 
 ### Controlling output
 

doc/optionsref.md

@@ -139,6 +139,8 @@ Usage: octocatalog-diff [command line options]
                                      SSL client certificate to connect to PE ENC
         --pe-enc-ssl-client-key FILENAME
                                      SSL client key to connect to PE ENC
+        --override-script-path DIRNAME
+                                     Directory with scripts to override built-ins
         --no-ignore-tags             Disable ignoring based on tags
         --ignore-tags STRING1[,STRING2[,...]]
                                      Specify tags to ignore
@@ -730,6 +732,19 @@ array of differences, where each difference is an array (the octocatalog-diff 0.
     </td>
   </tr>
 
+  <tr>
+    <td valign=top>
+      <pre><code>--override-script-path DIRNAME</code></pre>
+    </td>
+    <td valign=top>
+      Directory with scripts to override built-ins
+    </td>
+    <td valign=top>
+      Provide an optional directory to override default built-in scripts such as git checkout
+and puppet version determination. (<a href="../lib/octocatalog-diff/cli/options/override_script_path.rb">override_script_path.rb</a>)
+    </td>
+  </tr>
+
   <tr>
     <td valign=top>
       <pre><code>--parallel

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

@@ -82,8 +82,8 @@ def self.bootstrap_directory_parallelizer(options, logger)
 
       # Performs the actual bootstrap of a directory. Intended to be called by bootstrap_directory_parallelizer
       # above, or as part of the parallelized catalog build process from util/catalogs.
+      # @param options [Hash] Directory options: branch, path, tag
       # @param logger [Logger] Logger object
-      # @param dir_opts [Hash] Directory options: branch, path, tag
       def self.bootstrap_directory(options, logger)
         raise ArgumentError, ':path must be supplied' unless options[:path]
         FileUtils.mkdir_p(options[:path]) unless Dir.exist?(options[:path])
@@ -96,11 +96,11 @@ def self.bootstrap_directory(options, logger)
 
       # Perform git checkout
       # @param logger [Logger] Logger object
-      # @param dir_opts [Hash] Directory options: branch, path, tag
-      def self.git_checkout(logger, dir_opts)
-        logger.debug("Begin git checkout #{dir_opts[:basedir]}:#{dir_opts[:branch]} -> #{dir_opts[:path]}")
-        OctocatalogDiff::CatalogUtil::Git.check_out_git_archive(dir_opts.merge(logger: logger))
-        logger.debug("Success git checkout #{dir_opts[:basedir]}:#{dir_opts[:branch]} -> #{dir_opts[:path]}")
+      # @param options [Hash] Options (need to contain: basedir, branch, path)
+      def self.git_checkout(logger, options)
+        logger.debug("Begin git checkout #{options[:basedir]}:#{options[:branch]} -> #{options[:path]}")
+        OctocatalogDiff::CatalogUtil::Git.check_out_git_archive(options.merge(logger: logger))
+        logger.debug("Success git checkout #{options[:basedir]}:#{options[:branch]} -> #{options[:path]}")
       rescue OctocatalogDiff::Errors::GitCheckoutError => exc
         logger.error("Git checkout error: #{exc}")
         raise OctocatalogDiff::Errors::BootstrapError, exc

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

@@ -21,19 +21,41 @@ def initialize(options = {}, logger = nil)
 
         @node = options[:node]
         raise ArgumentError, 'Node must be specified to compile catalog' if @node.nil? || !@node.is_a?(String)
+
+        # To be initialized on-demand
+        @puppet_argv = nil
+        @puppet_binary = nil
+      end
+
+      # Retrieve puppet_command, puppet_binary, puppet_argv
+      def puppet_argv
+        setup
+        @puppet_argv
+      end
+
+      def puppet_binary
+        setup
+        @puppet_binary
       end
 
-      # Build up the command line to run Puppet
       def puppet_command
-        cmdline = []
+        setup
+        [@puppet_binary, @puppet_argv].flatten.join(' ')
+      end
+
+      private
+
+      # Build up the command line to run Puppet
+      def setup
+        return if @puppet_binary && @puppet_argv
 
         # Where is the puppet binary?
-        puppet = @options[:puppet_binary]
-        raise ArgumentError, 'Puppet binary was not supplied' if puppet.nil?
-        raise Errno::ENOENT, "Puppet binary #{puppet} doesn't exist" unless File.file?(puppet)
-        cmdline << puppet
+        @puppet_binary = @options[:puppet_binary]
+        raise ArgumentError, 'Puppet binary was not supplied' if @puppet_binary.nil?
+        raise Errno::ENOENT, "Puppet binary #{@puppet_binary} doesn't exist" unless File.file?(@puppet_binary)
 
         # Node to compile
+        cmdline = []
         cmdline.concat ['master', '--compile', Shellwords.escape(@node)]
 
         # storeconfigs?
@@ -99,11 +121,9 @@ def puppet_command
         override_and_append_commandline_with_user_supplied_arguments(cmdline)
 
         # Return full command
-        cmdline.join(' ')
+        @puppet_argv = cmdline
       end
 
-      private
-
       # Private: Mutate the command line with arguments that were passed directly from the
       # user. This appends new arguments and overwrites existing arguments.
       # @param cmdline [Array] Existing command line - mutated by this method

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

@@ -1,12 +1,9 @@
 # frozen_string_literal: true
 
-require 'fileutils'
-require 'open3'
 require 'rugged'
-require 'shellwords'
-require 'tempfile'
 
 require_relative '../errors'
+require_relative '../util/scriptrunner'
 
 module OctocatalogDiff
   module CatalogUtil
@@ -24,6 +21,7 @@ def self.check_out_git_archive(options = {})
         path = options.fetch(:path)
         dir = options.fetch(:basedir)
         logger = options.fetch(:logger)
+        override_script_path = options.fetch(:override_script_path, nil)
 
         # Validate parameters
         if dir.nil? || !File.directory?(dir)
@@ -34,31 +32,26 @@ def self.check_out_git_archive(options = {})
         end
 
         # Create and execute checkout script
-        script = create_git_checkout_script(branch, path)
-        logger.debug("Begin git archive #{dir}:#{branch} -> #{path}")
-        output, status = Open3.capture2e(script, chdir: dir)
-        unless status.exitstatus.zero?
-          raise OctocatalogDiff::Errors::GitCheckoutError, "Git archive #{branch}->#{path} failed: #{output}"
-        end
-        logger.debug("Success git archive #{dir}:#{branch}")
-      end
+        sr_opts = {
+          logger: logger,
+          default_script: 'git-extract/git-extract.sh',
+          override_script_path: override_script_path
+        }
+        script = OctocatalogDiff::Util::ScriptRunner.new(sr_opts)
 
-      # Create the temporary file used to interact with the git command line.
-      # To get the options working correctly (-o pipefail in particular) this needs to run under
-      # bash. It's just creating a script, rather than figuring out all the shell escapes...
-      # @param branch [String] Branch name
-      # @param path [String] Target directory
-      # @return [String] Name of script
-      def self.create_git_checkout_script(branch, path)
-        tmp_script = Tempfile.new(['git-checkout', '.sh'])
-        tmp_script.write "#!/bin/bash\n"
-        tmp_script.write "set -euf -o pipefail\n"
-        tmp_script.write "git archive --format=tar #{Shellwords.escape(branch)} | \\\n"
-        tmp_script.write "  ( cd #{Shellwords.escape(path)} && tar -xf - )\n"
-        tmp_script.close
-        FileUtils.chmod 0o755, tmp_script.path
-        at_exit { FileUtils.rm_f tmp_script.path if File.exist?(tmp_script.path) }
-        tmp_script.path
+        sr_run_opts = {
+          :working_dir             => dir,
+          :pass_env_vars           => options[:pass_env_vars],
+          'OCD_GIT_EXTRACT_BRANCH' => branch,
+          'OCD_GIT_EXTRACT_TARGET' => path
+        }
+
+        begin
+          script.run(sr_run_opts)
+          logger.debug("Success git archive #{dir}:#{branch}")
+        rescue OctocatalogDiff::Util::ScriptRunner::ScriptException
+          raise OctocatalogDiff::Errors::GitCheckoutError, "Git archive #{branch}->#{path} failed: #{script.output}"
+        end
       end
 
       # Determine the SHA of origin/master (or any other branch really) in the git repo

lib/octocatalog-diff/catalog/computed.rb

@@ -2,14 +2,14 @@
 
 require 'fileutils'
 require 'json'
-require 'open3'
 require 'stringio'
 
 require_relative '../catalog-util/bootstrap'
 require_relative '../catalog-util/builddir'
 require_relative '../catalog-util/command'
-require_relative '../util/puppetversion'
 require_relative '../catalog-util/facts'
+require_relative '../util/puppetversion'
+require_relative '../util/scriptrunner'
 
 module OctocatalogDiff
   class Catalog
@@ -160,37 +160,60 @@ def build_catalog(logger = nil)
 
       # Get the command to compile the catalog
       # @return [String] Puppet command line
-      def puppet_command(options = @opts)
-        return @puppet_command if @puppet_command
-        raise ArgumentError, '"puppet_binary" was not passed to OctocatalogDiff::Catalog::Computed' unless @puppet_binary
-        command_opts = options.merge(
-          node: @node,
-          compilation_dir: @builddir.tempdir,
-          parser: options.fetch(:parser, :default),
-          puppet_binary: @puppet_binary,
-          fact_file: @builddir.fact_file,
-          dir: @builddir.tempdir,
-          enc: @builddir.enc
-        )
-        command = OctocatalogDiff::CatalogUtil::Command.new(command_opts)
-        @puppet_command = command.puppet_command
+      def puppet_command
+        puppet_command_obj.puppet_command
+      end
+
+      def puppet_command_obj
+        @puppet_command_obj ||= begin
+          raise ArgumentError, '"puppet_binary" was not passed to OctocatalogDiff::Catalog::Computed' unless @puppet_binary
+
+          command_opts = @opts.merge(
+            node: @node,
+            compilation_dir: @builddir.tempdir,
+            parser: @opts.fetch(:parser, :default),
+            puppet_binary: @puppet_binary,
+            fact_file: @builddir.fact_file,
+            dir: @builddir.tempdir,
+            enc: @builddir.enc
+          )
+          OctocatalogDiff::CatalogUtil::Command.new(command_opts)
+        end
       end
 
       # Private method: Actually execute puppet
       # @return [Hash] { stdout, stderr, exitcode }
-      def exec_puppet
+      def exec_puppet(logger)
         # This is the environment provided to the puppet command.
-        env = {
-          'HOME' => ENV['HOME'],
-          'PATH' => ENV['PATH'],
-          'PWD' => @builddir.tempdir
-        }
+        env = {}
         @pass_env_vars.each { |var| env[var] ||= ENV[var] }
-        out, err, status = Open3.capture3(env, puppet_command, unsetenv_others: true, chdir: @builddir.tempdir)
+
+        # This is the Puppet command itself
+        env['OCD_PUPPET_BINARY'] = @puppet_command_obj.puppet_binary
+
+        # Additional passed-in options
+        sr_run_opts = env.merge(
+          logger: logger,
+          working_dir: @builddir.tempdir,
+          argv: @puppet_command_obj.puppet_argv
+        )
+
+        # Set up the ScriptRunner
+        scriptrunner = OctocatalogDiff::Util::ScriptRunner.new(
+          default_script: 'puppet/puppet.sh',
+          override_script_path: @opts[:override_script_path]
+        )
+
+        begin
+          scriptrunner.run(sr_run_opts)
+        rescue OctocatalogDiff::Util::ScriptRunner::ScriptException => exc
+          logger.warn "Puppet command failed: #{exc.message}" if logger
+        end
+
         {
-          stdout: out,
-          stderr: err,
-          exitcode: status.exitstatus
+          stdout: scriptrunner.stdout,
+          stderr: scriptrunner.stderr,
+          exitcode: scriptrunner.exitcode
         }
       end
 
@@ -216,7 +239,7 @@ def run_puppet(logger)
           @retries = retry_num
           time_begin = Time.now
           logger.debug("(#{@tag}) Try #{1 + retry_num} executing Puppet #{puppet_version}: #{puppet_command}")
-          result = exec_puppet
+          result = exec_puppet(logger)
 
           # Success
           if (result[:exitcode]).zero?

lib/octocatalog-diff/cli/options/override_script_path.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+# Provide an optional directory to override default built-in scripts such as git checkout
+# and puppet version determination.
+# @param parser [OptionParser object] The OptionParser argument
+# @param options [Hash] Options hash being constructed; this is modified in this method.
+OctocatalogDiff::Cli::Options::Option.newoption(:override_script_path) do
+  has_weight 385
+
+  def parse(parser, options)
+    parser.on('--override-script-path DIRNAME', 'Directory with scripts to override built-ins') do |dir|
+      unless dir.start_with?('/')
+        raise ArgumentError, 'Absolute path is required for --override-script-path'
+      end
+
+      unless File.directory?(dir)
+        raise Errno::ENOENT, 'Invalid --override-script-path'
+      end
+
+      options[:override_script_path] = dir
+    end
+  end
+end