Merge pull request #201 from github/1-5-4

Release 1.5.4

.version

@@ -1 +1 @@
-1.5.3
+1.5.4

doc/CHANGELOG.md

@@ -8,6 +8,17 @@
 </tr>
 </thead><tbody>
 
+<tr valign=top>
+<td>1.5.4</td>
+<td>2018-12-11</td>
+<td>
+<li><a href="https://github.com/github/octocatalog-diff/pull/190">#190</a>: (Enhancement) Additional filtered out cases for compilation directory</li>
+<li><a href="https://github.com/github/octocatalog-diff/pull/195">#195</a>: (Enhancement) Parallel catalog-diff when multiple hostnames are passed</li>
+<li><a href="https://github.com/github/octocatalog-diff/pull/198">#198</a>: (Bug Fix) Portability fixes</li>
+<li><a href="https://github.com/github/octocatalog-diff/pull/200">#200</a>: (Bug Fix) Support name parameter when validating references</li>
+</td>
+</tr>
+
 <tr valign=top>
 <td>1.5.3</td>
 <td>2018-03-05</td>

doc/dev/api/v1/objects/diff.md

@@ -94,9 +94,9 @@ Returns the value of the resource from the new catalog.
     }
   }
 
-  # Demonstrates structure and old_value
+  # Demonstrates structure and new_value
   diff.structure #=> ['parameters', 'content']
-  diff.old_value #=> 'This is the NEW FILE!!!!!'
+  diff.new_value #=> 'This is the NEW FILE!!!!!'
   ```
 
 #### `#old_file` (String)
@@ -107,7 +107,7 @@ Note that this is a pass-through of information provided in the Puppet catalog,
 
 Note also that if the diff represents addition of a resource, this will return `nil`, because the resource does not exist in the old catalog.
 
-#### `#old_file` (String)
+#### `#old_line` (String)
 
 Returns the line number within the Puppet manifest giving rise to the resource as it exists in the old catalog. (See `#old_file` for the filename of the Puppet manifest.)
 

doc/dev/releasing.md

@@ -9,7 +9,7 @@ The project maintainers are responsible for bumping the version number, regenera
 To test the new version of `octocatalog-diff` in the GitHub Puppet repository, check out `github/puppet` and:
 
 - Start a new branch based off master
-- Run `script/update-octocatalog-diff -r <ocd_branch_name>`
+- Run `script/update-octocatalog-diff <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

doc/optionsref.md

@@ -9,7 +9,8 @@
 
 ```
 Usage: octocatalog-diff [command line options]
-    -n, --hostname HOSTNAME          Use PuppetDB facts from last run of hostname
+    -n HOSTNAME1[,HOSTNAME2[,...]],  Use PuppetDB facts from last run of a hostname or a comma separated list of multiple hostnames
+        --hostname
         --basedir DIRNAME            Use an alternate base directory (git checkout of puppet repository)
     -f, --from FROM_BRANCH           Branch you are coming from
     -t, --to TO_BRANCH               Branch you are going to
@@ -856,14 +857,17 @@ Puppet control repo template, the value of this should be 'hieradata', which is
 
   <tr>
     <td valign=top>
-      <pre><code>-n HOSTNAME
---hostname HOSTNAME</code></pre>
+      <pre><code>-n HOSTNAME1[,HOSTNAME2[,...]]
+--hostname HOSTNAME1[,HOSTNAME2[,...]]</code></pre>
     </td>
     <td valign=top>
-      Use PuppetDB facts from last run of hostname
+      Use PuppetDB facts from last run of a hostname or a comma separated list of multiple hostnames
     </td>
     <td valign=top>
-      Set hostname, which is used to look up facts in PuppetDB, and in the header of diff display. (<a href="../lib/octocatalog-diff/cli/options/hostname.rb">hostname.rb</a>)
+      Set hostname, which is used to look up facts in PuppetDB, and in the header of diff display.
+This option can recieve a single hostname, or a comma separated list of
+multiple hostnames, which are split into an Array. Multiple hostnames do not
+work with the `catalog-only` or `bootstrap-then-exit` options. (<a href="../lib/octocatalog-diff/cli/options/hostname.rb">hostname.rb</a>)
     </td>
   </tr>
 

examples/script-overrides/git-extract-submodules/git-extract.sh

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # This script is called from lib/octocatalog-diff/catalog-util/git.rb and is used to
 # archive and extract a certain branch of a git repository into a target directory.

lib/octocatalog-diff/catalog-diff/filter/compilation_dir.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require_relative '../filter'
+require_relative '../../util/util'
 
 module OctocatalogDiff
   module CatalogDiff
@@ -35,43 +36,46 @@ def filtered?(diff, options = {})
 
           # Check for a change where the difference in a parameter exactly corresponds to the difference in the
           # compilation directory.
-          if diff.change? && (diff.old_value.is_a?(String) || diff.new_value.is_a?(String))
-            from_before = nil
-            from_after = nil
-            from_match = false
-            to_before = nil
-            to_after = nil
-            to_match = false
+          if diff.change?
+            o = remove_compilation_dir(diff.old_value, dir2)
+            n = remove_compilation_dir(diff.new_value, dir1)
 
-            if diff.old_value =~ /^(.*)#{dir2}(.*)$/m
-              from_before = Regexp.last_match(1) || ''
-              from_after = Regexp.last_match(2) || ''
-              from_match = true
-            end
-
-            if diff.new_value =~ /^(.*)#{dir1}(.*)$/m
-              to_before = Regexp.last_match(1) || ''
-              to_after = Regexp.last_match(2) || ''
-              to_match = true
-            end
-
-            if from_match && to_match && to_before == from_before && to_after == from_after
+            if o != diff.old_value || n != diff.new_value
               message = "Resource key #{diff.type}[#{diff.title}] #{diff.structure.join(' => ')}"
-              message += ' appears to depend on catalog compilation directory. Suppressed from results.'
+              message += ' may depend on catalog compilation directory, but there may be differences.'
+              message += ' This is included in results for now, but please verify.'
               @logger.warn message
-              return true
             end
 
-            if from_match || to_match
+            if o == n
               message = "Resource key #{diff.type}[#{diff.title}] #{diff.structure.join(' => ')}"
-              message += ' may depend on catalog compilation directory, but there may be differences.'
-              message += ' This is included in results for now, but please verify.'
+              message += ' appears to depend on catalog compilation directory. Suppressed from results.'
               @logger.warn message
+              return true
             end
           end
 
           false
         end
+
+        def remove_compilation_dir(v, dir)
+          value = OctocatalogDiff::Util::Util.deep_dup(v)
+          traverse(value) do |e|
+            e.gsub!(dir, '') if e.respond_to?(:gsub!)
+          end
+          value
+        end
+
+        def traverse(a)
+          case a
+          when Array
+            a.map { |v| traverse(v, &Proc.new) }
+          when Hash
+            traverse(a.values, &Proc.new)
+          else
+            yield a
+          end
+        end
       end
     end
   end

lib/octocatalog-diff/catalog.rb

@@ -331,8 +331,9 @@ def build_resource_hash
         title = normalized_title(resource['title'], resource['type'])
         @resource_hash[resource['type']][title] = resource
 
-        if resource.key?('parameters') && resource['parameters'].key?('alias')
-          @resource_hash[resource['type']][resource['parameters']['alias']] = resource
+        if resource.key?('parameters')
+          @resource_hash[resource['type']][resource['parameters']['alias']] = resource if resource['parameters'].key?('alias')
+          @resource_hash[resource['type']][resource['parameters']['name']] = resource if resource['parameters'].key?('name')
         end
       end
     end

lib/octocatalog-diff/cli.rb

@@ -11,6 +11,7 @@
 require_relative 'version'
 
 require 'logger'
+require 'parallel'
 require 'socket'
 
 module OctocatalogDiff
@@ -116,16 +117,46 @@ def self.cli(argv = ARGV, logger = Logger.new(STDERR), opts = {})
       end
 
       # Compile catalogs and do catalog-diff
-      catalog_diff = OctocatalogDiff::API::V1.catalog_diff(options.merge(logger: logger))
+      node_set = options.delete(:node)
+      node_set = [node_set] unless node_set.is_a?(Array)
+
+      # run multiple node diffs in parallel
+      catalog_diffs = if node_set.size == 1
+        [run_octocatalog_diff(node_set.first, options, logger)]
+      else
+        ::Parallel.map(node_set, in_threads: 4) { |node| run_octocatalog_diff(node, options, logger) }
+      end
+
+      # Return the resulting diff object if requested (generally for testing)
+      # or otherwise return exit code
+      return catalog_diffs.first if opts[:INTEGRATION]
+
+      all_diffs = catalog_diffs.map(&:diffs)
+
+      all_diffs.each do |diff|
+        next unless diff.any?
+        return EXITCODE_SUCCESS_WITH_DIFFS
+      end
+
+      EXITCODE_SUCCESS_NO_DIFFS
+    end
+
+    # Run the octocatalog-diff process for a given node. Return the diffs for a contribution to
+    # the final exit status.
+    # node    - String with the node
+    # options - All of the currently defined options
+    # logger  - Logger object
+    def self.run_octocatalog_diff(node, options, logger)
+      options_copy = options.merge(node: node)
+      catalog_diff = OctocatalogDiff::API::V1.catalog_diff(options_copy.merge(logger: logger))
       diffs = catalog_diff.diffs
 
       # Display diffs
-      printer_obj = OctocatalogDiff::Cli::Printer.new(options, logger)
+      printer_obj = OctocatalogDiff::Cli::Printer.new(options_copy, logger)
       printer_obj.printer(diffs, catalog_diff.from.compilation_dir, catalog_diff.to.compilation_dir)
 
-      # Return the resulting diff object if requested (generally for testing) or otherwise return exit code
-      return catalog_diff if opts[:INTEGRATION]
-      diffs.any? ? EXITCODE_SUCCESS_WITH_DIFFS : EXITCODE_SUCCESS_NO_DIFFS
+      # Return catalog-diff object.
+      catalog_diff
     end
 
     # Parse command line options with 'optparse'. Returns a hash with the parsed arguments.

lib/octocatalog-diff/cli/options.rb

@@ -11,7 +11,7 @@ class Cli
     # This class contains the option parser. 'parse_options' is the external entry point.
     class Options
       # The usage banner.
-      BANNER = 'Usage: catalog-diff -n <hostname> [-f <from environment>] [-t <to environment>]'.freeze
+      BANNER = 'Usage: catalog-diff -n <hostname>[,<hostname>...] [-f <from environment>] [-t <to environment>]'.freeze
 
       # An error class specifically for passing information to the document build task.
       class DocBuildError < RuntimeError; end

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

@@ -1,15 +1,26 @@
 # frozen_string_literal: true
 
 # Set hostname, which is used to look up facts in PuppetDB, and in the header of diff display.
+# This option can recieve a single hostname, or a comma separated list of
+# multiple hostnames, which are split into an Array. Multiple hostnames do not
+# work with the `catalog-only` or `bootstrap-then-exit` options.
 # @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(:hostname) do
   has_weight 1
 
   def parse(parser, options)
-    parser.on('--hostname HOSTNAME', '-n', 'Use PuppetDB facts from last run of hostname') do |hostname|
-      options[:node] = hostname
+    parser.on(
+      '--hostname HOSTNAME1[,HOSTNAME2[,...]]',
+      '-n',
+      'Use PuppetDB facts from last run of a hostname or a comma separated list of multiple hostnames'
+    ) do |hostname|
+      options[:node] = if hostname.include?(',')
+        hostname.split(',')
+      else
+        hostname
+      end
     end
   end
 end

octocatalog-diff.gemspec

@@ -27,6 +27,7 @@ EOF
   s.add_runtime_dependency 'diffy', '>= 3.1.0'
   s.add_runtime_dependency 'httparty', '>= 0.11.0'
   s.add_runtime_dependency 'hashdiff', '>= 0.3.0'
+  s.add_runtime_dependency 'parallel', '>= 1.12.0'
   s.add_runtime_dependency 'rugged', '>= 0.25.0b2'
 
   s.add_development_dependency 'rspec', '~> 3.4.0'

script/fmt

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && cd .. && pwd )"
 RUBOCOP_YML="${DIR}/.rubocop.yml"

script/git-pre-commit

@@ -1,11 +1,13 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # This script is being invoked via <root dir>/.git/hooks, hence the
 # base directory is up two levels, not just one.
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && cd ../.. && pwd )"
 
+cd "$DIR"
+
 # Make sure we can use git correctly
-cd "$DIR" && git rev-parse --verify HEAD >/dev/null 2>&1
+git rev-parse --verify HEAD >/dev/null 2>&1
 if [ $? -ne 0 ]; then
   echo "Unable to determine revision of this git repo"
   exit 1

script/octocatalog-diff-wrapper

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # This script exists to make it easier to test alternate branches of octocatalog-diff.
 # It is intended as a one-for-one replacement of `octocatalog-diff` as installed by the gem.

script/puppet

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && cd .. && pwd )"
 

scripts/env/env.sh

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # This script echoes back the environment. This is used for spec testing
 # and possible debugging.

scripts/git-extract/git-extract.sh

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # This script is called from lib/octocatalog-diff/catalog-util/git.rb and is used to
 # archive and extract a certain branch of a git repository into a target directory.

scripts/puppet/puppet.sh

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # Script to run Puppet. The default implementation here is simply to pass
 # through the command line arguments (which are likely to be numerous when

spec/octocatalog-diff/fixtures/catalogs/compilation-dir-1.json

@@ -53,6 +53,23 @@
         "parameters": {
           "dir": "/path/to/catalog1/onetime"
         }
+      },
+      {
+        "type": "Varies_Due_To_Compilation_Dir_5",
+        "title": "Common Title",
+        "tags": [
+          "ignoreme"
+        ],
+        "exported": false,
+        "parameters": {
+          "dir": {
+            "component": [
+              "path",
+              "/path/to/catalog1/twotimes",
+              "otherpath"
+            ]
+          }
+        }
       }
     ],
     "classes": [

spec/octocatalog-diff/fixtures/catalogs/compilation-dir-2.json

@@ -53,6 +53,19 @@
         "parameters": {
           "dir": "/path/to/catalog2/twotimes"
         }
+      },
+      {
+        "type": "Varies_Due_To_Compilation_Dir_5",
+        "title": "Common Title",
+        "tags": [
+          "ignoreme"
+        ],
+        "exported": false,
+        "parameters": {
+          "dir": {
+            "component": [ "path", "/path/to/catalog2/twotimes", "otherpath" ]
+          }
+        }
       }
     ],
     "classes": [

spec/octocatalog-diff/fixtures/configs/trivial-enc.sh

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 
 # The simplest and least functional ENC you'll ever see
 echo '---'

spec/octocatalog-diff/fixtures/override-scripts/git-extract.sh

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # For test purposes only, this script would ordinarily run a git checkout, but here
 # it's going to generate a directory structure by copying some other fixture.