Merge branch 'master' into kpaulisse-remove-time-based-tests

Author: kpaulisse

.version

@@ -1 +1 @@
-0.5.4
+0.5.7

doc/CHANGELOG.md

@@ -8,6 +8,22 @@
 </tr>
 </thead><tbody>
 <tr valign=top>
+<td>0.5.6</td>
+<td>2016-11-16</td>
+<td>
+<ul>
+<li><a href="https://github.com/github/octocatalog-diff/pull/20">https://github.com/github/octocatalog-diff/pull/20</a>: Use modulepath from environment.conf to inform lookup directories for <code>--compare-file-text</code> feature</li>
+</ul>
+</td>
+</tr>
+<tr valign=top>
+<td>0.5.5</td>
+<td>-</td>
+<td>
+Unreleased internal version
+</td>
+</tr>
+<tr valign=top>
 <td>0.5.4</td>
 <td>2016-11-07</td>
 <td>

doc/advanced-bootstrap.md

@@ -31,3 +31,28 @@ The [example configuration file](/examples/octocatalog-diff.cfg.rb) contains an
 # settings[:bootstrap_script] = '/etc/puppetlabs/repo-bootstrap.sh' # Absolute path
 # settings[:bootstrap_script] = 'script/bootstrap' # Relative path
 ```
+
+## Bootstrap environment
+
+When the bootstrap script runs, a limited set of environment variables are passed from the shell running octocatalog-diff. Only these variables are set:
+
+- `HOME`
+- `PATH`
+- `PWD` (set to the base directory of your Puppet checkout)
+- `BASEDIR` (as explicitly set with `--basedir` CLI option or `settings[:basedir]` setting)
+
+If you wish to set additional environment variables for your bootstrap script, you may do so via the `--bootstrap-environment VAR=value` command line flag, or by defining `settings[:bootstrap_environment] = { 'VAR' => 'value' }` in your configuration file.
+
+As an example, consider that your bootstrap script is written in Python, and needs the `PYTHONPATH` variable set to `/usr/local/lib/python-custom`. Even if this environment variable is set when octocatalog-diff is run, it will not be available to the bootstrap script. You may supply it via the command line:
+
+```
+octocatalog-diff --bootstrap-environment PYTHONPATH=/usr/local/lib/python-custom ...
+```
+
+Or you may specify it in your configuration file:
+
+```
+settings[:bootstrap_environment] = {
+  'PYTHONPATH' => '/usr/local/lib/python-custom'
+}
+```

doc/advanced-hiera-path-stripping.md

@@ -37,3 +37,5 @@ bin/octocatalog-diff --hiera-config hiera.yaml --hiera-path-strip /etc/puppetlab
 :yaml:
   :datadir: /var/tmp/puppet-compile-dir-92347829847/environments/%{environment}/hieradata
 ```
+
+:warning: Be sure that you do NOT include a trailing slash on `--hiera-path-strip` or `settings[:hiera_path_strip]`.

doc/configuration-enc.md

@@ -67,3 +67,37 @@ For example, when compiling the catalog for `some-node.github.net`, Puppet will
   ```
 
 Sometimes the ENC script requires credentials or makes other assumptions about the system on which it is running. To be able to run the ENC script on systems other than your Puppet master, you will need to ensure that any such credentials are supplied and other assumptions are met.
+
+## Environment
+
+When the ENC is executed, the following environment variables are set to match the environment of the shell in which octocatalog-diff executes:
+
+- `HOME`
+- `PATH`
+- `PWD` (set to the temporary directory as previously described)
+
+No other environment variables are passed from the shell. If you wish to pass additional environment variables, you must explicitly list them with the `--pass-env-vars` CLI flag or `settings[:pass_env_vars]` array in your configuration file.
+
+As an example, consider that your ENC is written in Python, and needs the `PYTHONPATH` variable set to `/usr/local/lib/python-custom`. Even if this environment variable is set when octocatalog-diff is run, it will not be available to the ENC script. You may pass the variable via the command line:
+
+```
+octocatalog-diff --pass-env-vars PYTHONPATH ...
+```
+
+Or you may specify it in your configuration file:
+
+```
+settings[:pass_env_vars] = [ 'PYTHONPATH' ]
+```
+
+If you wish to specify multiple environment variables to pass:
+
+```
+octocatalog-diff --pass-env-vars PYTHONPATH,SECONDVAR,THIRDVAR ...
+```
+
+or
+
+```
+settings[:pass_env_vars] = [ 'PYTHONPATH', 'SECONDVAR', 'THIRDVAR' ]
+```

examples/octocatalog-diff.cfg.rb

@@ -56,6 +56,7 @@ def self.config
       #      In this case, you desire to strip `/etc/puppetlabs/code` from the beginning of the path,
       #      in order that octocatalog-diff can find your hiera datafiles in the compilation
       #      location, which is {temporary directory}/environments/production/hieradata.
+      #      If you use this, be sure that you do NOT include a trailing slash!
       #
       #      More: https://github.com/github/octocatalog-diff/blob/master/doc/configuration-hiera.md
       ##############################################################################################

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

@@ -197,7 +197,7 @@ def install_hiera_config(logger, options)
 
         # Munge datadir in hiera config file
         obj = YAML.load_file(file_src)
-        %w(yaml json).each do |key|
+        (obj[:backends] || %w(yaml json)).each do |key|
           next unless obj.key?(key.to_sym)
           if options[:hiera_path_strip].is_a?(String)
             next if obj[key.to_sym][:datadir].nil?

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

@@ -21,6 +21,45 @@ def self.convert_file_resources(obj)
         end
       end
 
+      # Internal method: Locate a file that is referenced at puppet:///modules/xxx/yyy using the
+      # 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 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
+
+        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
+
+        nil
+      end
+
+      # Internal method: Parse environment.conf to find the modulepath
+      # @param compilation_dir [String] Compilation directory
+      # @return [Array] Module paths
+      def self.module_path(compilation_dir)
+        environment_conf = File.join(compilation_dir, 'environment.conf')
+        unless File.file?(environment_conf)
+          return [File.join(compilation_dir, 'modules')]
+        end
+
+        # This doesn't support multi-line, continuations with backslash, etc.
+        # Does it need to??
+        if File.read(environment_conf) =~ /^modulepath\s*=\s*(.+)/
+          Regexp.last_match(1).split(/:/).map(&:strip).reject { |x| x =~ /^\$/ }.map { |x| File.join(compilation_dir, x) }
+        else
+          [File.join(compilation_dir, 'modules')]
+        end
+      end
+
       # Internal method: Static method to convert file resources. The compilation directory is
       # required, or else this is a no-op. The passed-in array of resources is modified by this method.
       # @param resources [Array<Hash>] Array of catalog resources
@@ -34,18 +73,15 @@ def self._convert_file_resources(resources, compilation_dir)
         # that compilation_dir/environments/production is pointing at the right place). Otherwise, try to find
         # compilation_dir/modules. If neither of those exist, this code can't run.
         env_dir = File.join(compilation_dir, 'environments', 'production')
-        unless File.directory?(File.join(env_dir, 'modules'))
-          return unless File.directory?(File.join(compilation_dir, 'modules'))
-          env_dir = compilation_dir
-        end
+        modulepaths = module_path(env_dir) + module_path(compilation_dir)
+        modulepaths.select! { |x| File.directory?(x) }
+        return if modulepaths.empty?
 
-        # Modify the resources
+        # At least one existing module path was found! Run the code to modify the resources.
         resources.map! do |resource|
           if resource_convertible?(resource)
-            # Parse the 'source' parameter into a file on disk
-            src = resource['parameters']['source']
-            raise "Bad parameter source #{src}" unless src =~ %r{^puppet:///modules/([^/]+)/(.+)}
-            path = File.join(env_dir, 'modules', Regexp.last_match(1), 'files', Regexp.last_match(2))
+            path = file_path(resource['parameters']['source'], modulepaths)
+            raise Errno::ENOENT, "Unable to resolve '#{resource['parameters']['source']}'!" if path.nil?
 
             if File.file?(path)
               # If the file is found, read its content. If the content is all ASCII, substitute it into
@@ -60,7 +96,10 @@ def self._convert_file_resources(resources, compilation_dir)
               # However, the fact that we found *something* at this location indicates that the catalog
               # is probably correct. Hence, the very general .exist? check.
             else
-              raise Errno::ENOENT, "Unable to find '#{src}' at #{path}!"
+              # This is probably a bug
+              # :nocov:
+              raise "Unable to find '#{resource['parameters']['source']}' at #{path}!"
+              # :nocov:
             end
           end
           resource

lib/octocatalog-diff/catalog/puppetmaster.rb

@@ -68,13 +68,17 @@ def build(logger = Logger.new(StringIO.new))
 
       # Returns a hash of parameters for each supported version of the Puppet Server Catalog API.
       # @return [Hash] Hash of parameters
+      #
+      # Note: The double escaping of the facts here is implemented to correspond to a long standing
+      # bug in the Puppet code. See https://github.com/puppetlabs/puppet/pull/1818 and
+      # https://docs.puppet.com/puppet/latest/http_api/http_catalog.html#parameters for explanation.
       def puppet_catalog_api
         {
           2 => {
             url: "https://#{@options[:puppet_master]}/#{@options[:branch]}/catalog/#{@node}",
             parameters: {
               'facts_format' => 'pson',
-              'facts' => @facts.fudge_timestamp.without('trusted').to_pson,
+              'facts' => CGI.escape(@facts.fudge_timestamp.without('trusted').to_pson),
               'transaction_uuid' => SecureRandom.uuid
             }
           },
@@ -83,7 +87,7 @@ def puppet_catalog_api
             parameters: {
               'environment' => @options[:branch],
               'facts_format' => 'pson',
-              'facts' => @facts.fudge_timestamp.without('trusted').to_pson,
+              'facts' => CGI.escape(@facts.fudge_timestamp.without('trusted').to_pson),
               'transaction_uuid' => SecureRandom.uuid
             }
           }

spec/octocatalog-diff/fixtures/catalogs/catalog-modules-test.json

@@ -0,0 +1,58 @@
+{
+  "document_type": "Catalog",
+  "tags": ["settings","test"],
+  "name": "my.rspec.node",
+  "version": "production",
+  "environment": "production",
+  "resources": [
+    {
+      "type": "Stage",
+      "title": "main",
+      "tags": ["stage"],
+      "exported": false,
+      "parameters": {
+        "name": "main"
+      }
+    },
+    {
+      "type": "Class",
+      "title": "Settings",
+      "tags": ["class","settings"],
+      "exported": false
+    },
+    {
+      "type": "File",
+      "title": "/tmp/foo",
+      "tags": ["file","class"],
+      "file": "/x/modules/modulestest/manifests/init.pp",
+      "line": 37,
+      "exported": false,
+      "parameters": {
+        "backup": false,
+        "mode": "0440",
+        "owner": "root",
+        "group": "root",
+        "source": "puppet:///modules/modulestest/tmp/modulestest"
+      }
+    },
+    {
+      "type": "File",
+      "title": "/tmp/foobaz",
+      "tags": ["file","class"],
+      "file": "/x/modules/modulestest/manifests/init.pp",
+      "line": 37,
+      "exported": false,
+      "parameters": {
+        "backup": false,
+        "ensure": "directory",
+        "mode": "0755",
+        "owner": "root",
+        "group": "root",
+        "source": "puppet:///modules/modulestest/foo"
+      }
+    }
+  ],
+  "classes": [
+    "test"
+  ]
+}