Merge branch 'kpaulisse-hiera-simplification' into kpaulisse-octocatalog-diff-0-5-4

Author: Kevin Paulisse

doc/advanced-hiera-path-stripping.md

@@ -0,0 +1,39 @@
+# Configuring octocatalog-diff to use Hiera path stripping
+
+This is a different, and potentially more complex, alternative to `hiera-path` / `settings[:hiera_path]` described in the [Configuring octocatalog-diff to use Hiera](/doc/configuration-hiera.md) document. Unless you have a very good reason, you should prefer to use the instructions in that document instead of using the more complicated option that is described herein.
+
+The command line option `--hiera-path-strip PATH` allows you to manipulate directory paths for the JSON or YAML hiera backends. This setting only has an effect on the copy of hiera.yaml that is copied into the temporary compilation directory. This does not make any changes to the actual source hiera.yaml file on your system or in your checkout.
+
+For example, perhaps your production hiera.yaml file has entries such as the following:
+
+```
+---
+:backends:
+  - yaml
+:hierarchy:
+  - "nodes/%{::trusted.certname}"
+  - common
+
+:yaml:
+  :datadir: /etc/puppetlabs/code/environments/%{environment}/hieradata
+```
+
+However, when you run octocatalog-diff on a machine that is not a Puppet master, the hiera data will not actually be found in `/etc/puppetlabs/code/environments/production/hieradata`, but rather in a directory called `hieradata` relative to the checkout of your Puppet code.
+
+Specifying `--hiera-path-strip PATH` causes octocatalog-diff to munge the datadir for the YAML and JSON configuration. The correct command in this case is now:
+
+```
+bin/octocatalog-diff --hiera-config hiera.yaml --hiera-path-strip /etc/puppetlabs/code
+```
+
+```
+---
+:backends:
+  - yaml
+:hierarchy:
+  - "nodes/%{::trusted.certname}"
+  - common
+
+:yaml:
+  :datadir: /var/tmp/puppet-compile-dir-92347829847/environments/%{environment}/hieradata
+```

doc/configuration-hiera.md

@@ -10,7 +10,7 @@ Before you start, please understand how octocatalog-diff compiles a catalog:
 - It compiles the catalog, based on the temporary directory, for environment=production
 - It removes the temporary directory
 
-## Configuring the path to hiera.yaml
+## Configuring the location of hiera.yaml
 
 The command line option `--hiera-config PATH` allows you to set the path to hiera.yaml.
 
@@ -21,22 +21,26 @@ You may specify this as either an absolute or a relative path.
   octocatalog-diff knows to use a relative path when the supplied path for `--hiera-config` does not start with a `/`.
 
     ```
-    bin/octocatalog-diff --hiera-config config/hiera.yaml ...
+    bin/octocatalog-diff --hiera-config hiera.yaml ...
     ```
 
-  The path is relative to a checkout of your Puppet repository. As per the example in the introduction, say that octocatalog-diff is using a temporary directory of `/var/tmp/puppet-compile-dir-92347829847` when compiling a Puppet catalog. With the setting above, it will copy `config/hiera.yaml` (relative to your Puppet checkout) into the temporary directory.
+  The path is relative to a checkout of your Puppet repository. With the setting above, it will use the file named `hiera.yaml` that is at the top level
+  of your Puppet checkout.
 
-  If you use Puppet to manage your hiera.yaml file on Puppet masters, perhaps it is found in one of the modules in your code. In that case, you may use syntax like:
+  Perhaps your hiera.yaml file is in a subdirectory of your Puppet checkout. In that case, just use the relative directory path. Be sure not to add a leading `/` though,
+  because you don't want octocatalog-diff to treat it as an absolute path. In the following example, suppose you have a top level directory called `config` and your
+  `hiera.yaml` file is contained within it. You could then use:
 
     ```
-    bin/octocatalog-diff --hiera-config modules/puppet/files/hiera.yaml ...
+    bin/octocatalog-diff --hiera-config config/hiera.yaml ...
     ```
 
   If you are specifying the hiera.yaml path in the [configuration file](/doc/configuration.md), you will instead set the variable like this:
 
     ```
+    settings[:hiera_config] = 'hiera.yaml'
+    (or)
     settings[:hiera_config] = 'config/hiera.yaml'
-    settings[:hiera_config] = 'modules/puppet/files/hiera.yaml'
     ```
 
   octocatalog-diff will fail if you specify a hiera configuration location that cannot be opened.
@@ -57,47 +61,35 @@ You may specify this as either an absolute or a relative path.
     settings[:hiera_config] = '/etc/puppetlabs/puppet/hiera.yaml'
     ```
 
-  Please note that octocatalog-diff will copy the file from the specified location into the compile directory. Since this hiera.yaml file is not copied from your Puppet repo, there is no way to compile the "to" and "from" branches using different hiera.yaml files. Furthermore, you are responsible for getting this file into place on any machine that will run octocatalog-diff.
+  Please note that octocatalog-diff will copy the file from the specified location into the compile directory. Since this hiera.yaml file is not copied from your Puppet repo, there is no way to compile the "to" and "from" branches using different hiera.yaml files. Furthermore, you are responsible for getting this file into place on any machine that will run octocatalog-diff. An absolute path may make octocatalog-diff work correctly on your Puppet master servers, but the structure may differ on other machines where you wish to run the utility.
 
   We strongly recommend that you version-control your hiera.yaml file within your Puppet repository, and use the relative path option described above.
 
+## Configuring the directory in your repository in which hiera data files are found
+
+The command line option `--hiera-path PATH` allows you to set the directory path, relative to the checkout of your Puppet repository, of your Hiera YAML/JSON data files.
+
+If you are using the out-of-the-box Puppet Enterprise configuration, or the [Puppet Control Repo template](https://github.com/puppetlabs/control-repo), then the correct setting here is simply 'hieradata'.
+
+You must specify this as a relative path. octocatalog-diff knows to use a relative path when the supplied path for `--hiera-path` does not start with a `/`.
+
+  ```
+  bin/octocatalog-diff --hiera-path hieradata ...
+  ```
+
+The path is relative to a checkout of your Puppet repository. With the setting above, it will look for Hiera data in a directory called `hieradata` that is at the top level
+of your Puppet checkout.
+
+If you are specifying the Hiera data path in the [configuration file](/doc/configuration.md), you will instead set the variable like this:
+
+  ```
+  settings[:hiera_path] = 'hieradata'
+  ```
+
+octocatalog-diff will fail if you specify a path that is not a directory.
+
 ## Configuring the prefix path to strip
 
-The command line option `--hiera-path-strip PATH` allows you to manipulate directory paths for the JSON or YAML hiera backends. This setting only has an effect on the copy of hiera.yaml that is copied into the temporary compilation directory. This does not make any changes to the actual source hiera.yaml file on your system or in your checkout.
-
-For example, perhaps your production hiera.yaml file has entries such as the following:
-
-```
-:backends:
-  - yaml
-:yaml:
-  :datadir: /var/lib/puppet/environments/%{::environment}/hieradata
-:hierarchy:
-  - servers/%{::fqdn}
-  - platform/%{::virtual}
-  - datacenter/%{::datacenter}
-  - os/%{::operatingsystem}
-  - common
-```
-
-However, when you run octocatalog-diff, the hiera data will not actually be found in `/var/lib/puppet/environments/production/hieradata`, but rather in a directory called `environments/production/hieradata` relative to the checkout of your Puppet code.
-
-Specifying `--hiera-path-strip PATH` causes octocatalog-diff will rewrite the datadir for the YAML and JSON configuration. In the example above, the correct setting is `--hiera-path-strip /var/lib/puppet`, which will result in the following configuration in the hiera.yaml file:
-
-```
-bin/octocatalog-diff --hiera-config environments/production/config/hiera.yaml --hiera-path-strip /var/lib/puppet
-```
-
-```
-# This is the temporary hiera.yaml file used for octocatalog-diff catalog compilation
-:backends:
-  - yaml
-:yaml:
-  :datadir: /var/tmp/puppet-compile-dir-92347829847/environments/%{::environment}/hieradata
-:hierarchy:
-  - servers/%{::fqdn}
-  - platform/%{::virtual}
-  - datacenter/%{::datacenter}
-  - os/%{::operatingsystem}
-  - common
-```
+This is a different, and potentially more complex, alternative to `hiera-path` / `settings[:hiera_path]` described in the prior section. Unless you have a very good reason, you should prefer to use the instructions above.
+
+If you need to use the more complex path strip alternative, see: [Configuring octocatalog-diff to use Hiera path stripping](/doc/advanced-hiera-path-stripping.md).

examples/octocatalog-diff.cfg.rb

@@ -34,21 +34,37 @@ def self.config
       ##############################################################################################
 
       # settings[:hiera_config] = '/etc/puppetlabs/puppet/hiera.yaml' # Absolute path
-      # settings[:hiera_config] = 'environments/production/config/hiera.yaml' # Relative path
+      settings[:hiera_config] = 'hiera.yaml' # Relative path, assumes hiera.yaml at top of repo
 
       ##############################################################################################
+      # hiera_path
       # hiera_path_strip
-      #   Portion of the `:datadir:` to strip (used for JSON and YAML data sources). For
-      #   example, perhaps your hiera.yaml file contains this code:
-      #     :yaml:
-      #       :datadir: /var/lib/puppet/environments/%{::environment}/hieradata
-      #   In this case, you desire to strip `/var/lib/puppet` 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.
-      #   More: https://github.com/github/octocatalog-diff/blob/master/doc/configuration-hiera.md
+      #   These control the setup of the 'datadir' when you are using the JSON or YAML data source.
+      #   There are two ways to configure this setting - do one or the other but not both.
+      #
+      #   1. (EASIEST METHOD)
+      #      You can specify the path to the hieradata relative to the checkout of your Puppet repo.
+      #      This may be the most straightforward to configure. For example, if your Hiera data YAML
+      #      and JSON files are found under a `hieradata` directory in the top level of your Puppet
+      #      repo, simply set `settings[:hiera_path] = 'hieradata'` and you're done!
+      #
+      #   2. (MORE COMPLEX METHOD)
+      #      You can specify a string that will be stripped off the existing defined data directory
+      #      in the hiera.yaml file. For example, perhaps your hiera.yaml file contains this code:
+      #       :yaml:
+      #         :datadir: /etc/puppetlabs/code/environments/%{environment}/hieradata
+      #      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.
+      #
+      #      More: https://github.com/github/octocatalog-diff/blob/master/doc/configuration-hiera.md
       ##############################################################################################
 
-      # settings[:hiera_path_strip] = '/var/lib/puppet'
+      # This should work out-of-the-box with a default Puppet Enterprise or Puppet Control Repo setup.
+      settings[:hiera_path] = 'hieradata'
+
+      # If you want to use the 'strip' method described above, this may work.
+      # settings[:hiera_path_strip] = '/etc/puppetlabs/code'
 
       ##############################################################################################
       # puppetdb_url

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

@@ -39,7 +39,8 @@ class Cli
         compare_file_text: true,
         display_datatype_changes: true,
         parallel: true,
-        suppress_absent_file_details: true
+        suppress_absent_file_details: true,
+        hiera_path: 'hieradata'
       }.freeze
 
       # This method is the one to call externally. It is possible to specify alternate
@@ -75,6 +76,8 @@ def self.cli(argv = ARGV, logger = Logger.new(STDERR), opts = {})
         # Note: do NOT use 'options[k] ||= v' here because if the value of options[k] is boolean(false)
         # it will then be overridden. Whereas the intent is to define values only for those keys that don't exist.
         DEFAULT_OPTIONS.each { |k, v| options[k] = v unless options.key?(k) }
+        veto_with_none_options = %w(hiera_path hiera_path_strip)
+        veto_with_none_options.each { |x| options.delete(x.to_sym) if options[x.to_sym] == :none }
 
         # Fact overrides come in here - 'options' is modified
         setup_fact_overrides(options)

lib/octocatalog-diff/catalog-diff/cli/options/hiera_path.rb

@@ -0,0 +1,36 @@
+# Specify the path to the Hiera data directory (relative to the top level Puppet checkout). For Puppet Enterprise and the
+# Puppet control repo template, the value of this should be 'hieradata', which is the default.
+# @param parser [OptionParser object] The OptionParser argument
+# @param options [Hash] Options hash being constructed; this is modified in this method.
+OctocatalogDiff::CatalogDiff::Cli::Options::Option.newoption(:hiera_path) do
+  has_weight 181
+
+  def parse(parser, options)
+    parser.on('--hiera-path PATH', 'Path to hiera data directory, relative to top directory of repository') do |path_in|
+      if options.key?(:hiera_path_strip) && options[:hiera_path_strip] != :none
+        raise ArgumentError, '--hiera-path and --hiera-path-strip are mutually exclusive'
+      end
+
+      if options[:hiera_path] == :none
+        raise ArgumentError, '--hiera-path and --no-hiera-path are mutually exclusive'
+      end
+
+      options[:hiera_path] = path_in
+
+      if options[:hiera_path].start_with?('/')
+        raise ArgumentError, '--hiera-path PATH must be a relative path not an absolute path'
+      end
+
+      options[:hiera_path].sub!(%r{/+$}, '')
+      raise ArgumentError, '--hiera-path must not be empty' if options[:hiera_path].empty?
+    end
+
+    parser.on('--no-hiera-path', 'Do not use any default hiera path settings') do
+      if options[:hiera_path].is_a?(String)
+        raise ArgumentError, '--hiera-path and --no-hiera-path are mutually exclusive'
+      end
+
+      options[:hiera_path] = :none
+    end
+  end
+end

lib/octocatalog-diff/catalog-diff/cli/options/hiera_path_strip.rb

@@ -2,11 +2,26 @@
 # @param parser [OptionParser object] The OptionParser argument
 # @param options [Hash] Options hash being constructed; this is modified in this method.
 OctocatalogDiff::CatalogDiff::Cli::Options::Option.newoption(:hiera_path_strip) do
-  has_weight 181
+  has_weight 182
 
   def parse(parser, options)
     parser.on('--hiera-path-strip PATH', 'Path prefix to strip when munging hiera.yaml') do |path_in|
+      if options.key?(:hiera_path) && options[:hiera_path] != :none
+        raise ArgumentError, '--hiera-path and --hiera-path-strip are mutually exclusive'
+      end
+
+      if options[:hiera_path_strip] == :none
+        raise ArgumentError, '--hiera-path and --no-hiera-path are mutually exclusive'
+      end
+
       options[:hiera_path_strip] = path_in
     end
+
+    parser.on('--no-hiera-path-strip', 'Do not use any default hiera path strip settings') do
+      if options[:hiera_path_strip].is_a?(String)
+        raise ArgumentError, '--hiera-path and --no-hiera-path are mutually exclusive'
+      end
+      options[:hiera_path_strip] = :none
+    end
   end
 end

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

@@ -27,6 +27,7 @@ class BuildDir
       # :enc [String] ENC script file (can be relative or absolute path)
       # :pe_enc_url [String] ENC URL (for Puppet Enterprise node classification service)
       # :hiera_config [String] hiera configuration file (relative to base directory)
+      # :hiera_path [String] relative path to hiera data files (mutually exclusive with :hiera_path_strip)
       # :hiera_path_strip [String] string to strip off the beginning of :datadir
       # :puppetdb_ssl_ca [String] Path to SSL CA certificate
       # :puppetdb_ssl_client_key [String] String representation of SSL client key
@@ -52,9 +53,8 @@ def initialize(options = {}, logger = nil)
           install_puppetdb_conf(logger, options[:puppetdb_url], options[:puppetdb_server_url_timeout])
           install_routes_yaml(logger)
         end
-        unless options[:hiera_config].nil?
-          install_hiera_config(logger, options[:hiera_config], options[:hiera_path_strip])
-        end
+        install_hiera_config(logger, options) unless options[:hiera_config].nil?
+
         @fact_file = install_fact_file(logger, options) if @facts_terminus == 'yaml'
         @enc = install_enc(logger) unless options[:enc].nil? && options[:pe_enc_url].nil?
         install_ssl(logger, options) if options[:puppetdb_ssl_ca] || options[:puppetdb_ssl_client_cert]
@@ -179,10 +179,10 @@ def install_enc(logger)
       end
 
       # Install hiera config file
-      # @param hiera_config [String] Path to file, relative to checkout
-      # @param hiera_path_strip [String] Prefix to strip off when munging file
-      def install_hiera_config(logger, hiera_config, hiera_path_strip)
+      # @param options [Hash] Options hash
+      def install_hiera_config(logger, options)
         # Validate hiera config file
+        hiera_config = options[:hiera_config]
         unless hiera_config.is_a?(String)
           raise ArgumentError, "Called install_hiera_config with a #{hiera_config.class} argument"
         end
@@ -199,13 +199,23 @@ def install_hiera_config(logger, hiera_config, hiera_path_strip)
         obj = YAML.load_file(file_src)
         %w(yaml json).each do |key|
           next unless obj.key?(key.to_sym)
-          next if obj[key.to_sym][:datadir].nil?
-          unless hiera_path_strip.nil?
-            rexp1 = Regexp.new('^' + hiera_path_strip)
+          if options[:hiera_path_strip].is_a?(String)
+            next if obj[key.to_sym][:datadir].nil?
+            rexp1 = Regexp.new('^' + options[:hiera_path_strip])
             obj[key.to_sym][:datadir].sub!(rexp1, @tempdir)
+          elsif options[:hiera_path]
+            obj[key.to_sym][:datadir] = File.join(@tempdir, 'environments', 'production', options[:hiera_path])
           end
           rexp2 = Regexp.new('%{(::)?environment}')
           obj[key.to_sym][:datadir].sub!(rexp2, 'production')
+
+          # Make sure the dirctory exists. If not, log a warning. This is *probably* a setup error, but we don't
+          # want it to be fatal in case (for example) someone is doing an octocatalog-diff to verify moving this
+          # directory around or even setting up Hiera for the very first time.
+          unless File.directory?(obj[key.to_sym][:datadir])
+            message = "WARNING: Hiera datadir for #{key} doesn't seem to exist at #{obj[key.to_sym][:datadir]}"
+            logger.warn message
+          end
         end
 
         # Write properly formatted hiera config file into temporary directory