Add fact file dir

.gitignore

@@ -36,6 +36,7 @@ build-iPhoneSimulator/
 ## Environment normalization:
 /.bundle/
 /vendor/bundle
+/vendor/cache
 /lib/bundler/man/
 
 # for a library or gem, you might want to ignore these files since the code is

README.md

@@ -26,6 +26,8 @@ There are some [limitations](doc/limitations.md) to a catalog-based approach, me
 - Obtain catalog over the network using the API to query a Puppet Master / PuppetServer (Puppet 3.x through 6.x supported)
 - Read catalog from a JSON file
 
+Facts can be sourced from PuppetDB, a single fact file (`--fact-file`), or a per-node fact directory (`--fact-dir`).
+
 ## Example
 
 Here is simulated output from running `octocatalog-diff` to compare the Puppet catalog changes between the master branch and the Puppet code in the current working directory:

doc/CHANGELOG.md

@@ -12,6 +12,7 @@
 <td>2.1.0</td>
 <td>2020-02-18</td>
 <td>
+<li>(Enhancement) Add --fact-dir option for per-node YAML/JSON facts and enforce mutual exclusivity with PuppetDB and --fact-file</li>
 <li><a href="https://github.com/github/octocatalog-diff/pull/240">#240</a>: (Enhancement) Run CI against Puppet 7 and Ruby 3</li>
 <li><a href="https://github.com/github/octocatalog-diff/pull/241">#241</a>: (Bug Fix) Fix indent and numbering in several docs</li>
 <li><a href="https://github.com/github/octocatalog-diff/pull/242">#242</a>: (Enhancement) Handle JSON facts structured as name/values</li>

doc/advanced-environment-variables.md

@@ -67,4 +67,4 @@ 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`.)
+This variable specifies the directory path where puppet fact files are stored. (Fact files must be named `<fqdn>.yaml`, `<fqdn>.yml`, or `<fqdn>.json` where `<fqdn>` is specified when running `octocatalog-diff`.) For CLI usage, prefer `--fact-dir` for directory-based fact files.

doc/basic.md

@@ -14,7 +14,7 @@ You are required to provide the following information, either as a command line
 
 - The node name whose catalogs you wish to compile. Use `-n HOSTNAME` on the command line.
 
-- Facts, which can either be retrieved from [PuppetDB](/doc/configuration-puppetdb.md) or via the `--fact-file` command line option. See the usage examples below.
+- Facts, which can either be retrieved from [PuppetDB](/doc/configuration-puppetdb.md) or via the `--fact-file` or `--fact-dir` command line options. See the usage examples below.
 
 ## Examples
 
@@ -40,11 +40,23 @@ git pull
 octocatalog-diff -n SomeNodeName.yourdomain.com --fact-file /tmp/SomeNodeName.yourdomain.com.yaml
 ```
 
+### Using a fact directory
+
+If you have per-node fact files stored in a directory (with filenames like `<fqdn>.yaml` or `<fqdn>.json`), you can point octocatalog-diff at the directory and it will select the matching file for the node.
+
+```
+# Copy the fact file for SomeNodeName.yourdomain.com into /tmp/facts/SomeNodeName.yourdomain.com.yaml
+cd Puppet_Checkout_Directory
+git checkout master
+git pull
+octocatalog-diff -n SomeNodeName.yourdomain.com --fact-dir /tmp/facts
+```
+
 ## Using hiera
 
 This example demonstrates how to point octocatalog-diff at your Hiera configuration file. The Hiera configuration file for your site might be found in `/etc/puppet/hiera.yaml` (for Puppet 3.x) or `/etc/puppetlabs/puppet/hiera.yaml` (for Puppet 4.x).
 
-Note that you will either need to configure the PuppetDB URL or specify a `--fact-file` for this to work.
+Note that you will either need to configure the PuppetDB URL or specify a `--fact-file` or `--fact-dir` for this to work.
 
 ```
 # Copy the fact file for SomeNodeName.yourdomain.com into /tmp/SomeNodeName.yourdomain.com.yaml

doc/optionsref.md

@@ -34,6 +34,9 @@ Usage: octocatalog-diff [command line options]
         --ignore "Type1[Title1],Type2[Title2],..."
                                      More resources to ignore in format type[title]
         --[no-]include-tags          Include changes to tags in the diff output
+        --fact-dir STRING            Fact directory globally
+        --to-fact-dir STRING         Fact directory for the to branch
+        --from-fact-dir STRING       Fact directory for the from branch
         --fact-file STRING           Override fact globally
         --to-fact-file STRING        Override fact for the to branch
         --from-fact-file STRING      Override fact for the from branch
@@ -68,9 +71,9 @@ Usage: octocatalog-diff [command line options]
                                      References to validate
         --[no-]compare-file-text[=force]
                                      Compare text, not source location, of file resources
+        --[no-]storeconfigs          Enable integration with puppetdb for collected resources
         --storeconfigs-backend TERMINUS
                                      Set the terminus used for storeconfigs
-        --[no-]storeconfigs          Enable integration with puppetdb for collected resources
         --retry-failed-catalog N     Retry building a failed catalog N times
         --no-enc                     Disable ENC
         --enc PATH                   Path to ENC script, relative to checkout directory or absolute
@@ -543,6 +546,18 @@ with `--preserve-environments`. (<a href="../lib/octocatalog-diff/cli/options/en
     </td>
   </tr>
 
+  <tr>
+    <td valign=top>
+      <pre><code>--fact-dir STRING</code></pre>
+    </td>
+    <td valign=top>
+      Fact directory globally
+    </td>
+    <td valign=top>
+      Allow a directory of per-node fact files to be provided, to avoid pulling facts from PuppetDB. (<a href="../lib/octocatalog-diff/cli/options/fact_dir.rb">fact_dir.rb</a>)
+    </td>
+  </tr>
+
   <tr>
     <td valign=top>
       <pre><code>--fact-file STRING</code></pre>
@@ -688,6 +703,18 @@ with `--preserve-environments`. (<a href="../lib/octocatalog-diff/cli/options/en
     </td>
   </tr>
 
+  <tr>
+    <td valign=top>
+      <pre><code>--from-fact-dir STRING</code></pre>
+    </td>
+    <td valign=top>
+      Fact directory for the from branch
+    </td>
+    <td valign=top>
+      Allow a directory of per-node fact files to be provided, to avoid pulling facts from PuppetDB. (<a href="../lib/octocatalog-diff/cli/options/fact_dir.rb">fact_dir.rb</a>)
+    </td>
+  </tr>
+
   <tr>
     <td valign=top>
       <pre><code>--from-fact-file STRING</code></pre>
@@ -1749,6 +1776,18 @@ with `--preserve-environments`. (<a href="../lib/octocatalog-diff/cli/options/en
     </td>
   </tr>
 
+  <tr>
+    <td valign=top>
+      <pre><code>--to-fact-dir STRING</code></pre>
+    </td>
+    <td valign=top>
+      Fact directory for the to branch
+    </td>
+    <td valign=top>
+      Allow a directory of per-node fact files to be provided, to avoid pulling facts from PuppetDB. (<a href="../lib/octocatalog-diff/cli/options/fact_dir.rb">fact_dir.rb</a>)
+    </td>
+  </tr>
+
   <tr>
     <td valign=top>
       <pre><code>--to-fact-file STRING</code></pre>

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

@@ -19,6 +19,10 @@ def initialize(options, logger = nil)
         # Environment variable recognition
         @options[:puppetdb_url] ||= ENV['PUPPETDB_URL'] if ENV['PUPPETDB_URL']
         @options[:puppet_fact_dir] ||= ENV['PUPPET_FACT_DIR'] if ENV['PUPPET_FACT_DIR']
+
+        if @options[:fact_file_dir] && @options[:puppetdb_url]
+          raise ArgumentError, '--fact-dir and --puppetdb-url are mutually exclusive'
+        end
       end
 
       # Compute facts if needed and then return them
@@ -34,14 +38,37 @@ def facts
       # @return [OctocatalogDiff::Facts] Facts object
       def facts_from_file(filename)
         @logger.debug("Retrieving facts from #{filename}") unless @logger.nil?
+        backend = filename =~ /\.json$/ ? :json : :yaml
         opts = {
           node: @options[:node],
-          backend: :yaml,
+          backend: backend,
           fact_file_string: File.read(filename)
         }
         OctocatalogDiff::Facts.new(opts)
       end
 
+      # Retrieve facts from a directory of fact files, selecting by node name
+      # @param dir [String] Directory containing fact files
+      # @param node [String] Node name
+      # @return [OctocatalogDiff::Facts] Facts object
+      def facts_from_dir(dir, node)
+        filename = fact_file_for_node(dir, node)
+        return facts_from_file(filename) if filename
+        nil
+      end
+
+      # Determine the fact file name for a node (yaml or json)
+      # @param dir [String] Directory containing fact files
+      # @param node [String] Node name
+      # @return [String, nil] Filename if found
+      def fact_file_for_node(dir, node)
+        ['.yaml', '.yml', '.json'].each do |ext|
+          filename = File.join(dir, node + ext)
+          return filename if File.file?(filename)
+        end
+        nil
+      end
+
       # Retrieve facts from PuppetDB. Either options[:puppetdb_url] or ENV['PUPPETDB_URL']
       # needs to be set for this to work. Node name must also be set in options.
       # @return [OctocatalogDiff::Facts] Facts object
@@ -75,15 +102,21 @@ def compute_facts
 
         error_node_not_provided if @options[:node].nil?
 
+        if @options[:fact_file_dir] && File.directory?(@options[:fact_file_dir])
+          facts = facts_from_dir(@options[:fact_file_dir], @options[:node])
+          return facts if facts
+        end
+
         if @options[:puppet_fact_dir] && File.directory?(@options[:puppet_fact_dir])
           filename = File.join(@options[:puppet_fact_dir], @options[:node] + '.yaml')
           return facts_from_file(filename) if File.file?(filename)
         end
 
         return facts_from_puppetdb if @options[:puppetdb_url]
 
-        message = 'Unable to compute facts for node. Please use "--fact-file FILENAME" option' \
-                  ' or set one of these environment variables: PUPPET_FACT_DIR or PUPPETDB_URL.'
+        message = 'Unable to compute facts for node. Please use "--fact-file FILENAME" or' \
+                  ' "--fact-dir DIRECTORY" option, or set one of these environment variables:' \
+                  ' PUPPET_FACT_DIR or PUPPETDB_URL.'
         raise ArgumentError, message
       end
     end

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

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+# Allow a directory of per-node fact files to be provided, to avoid pulling facts from PuppetDB.
+# @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(:fact_dir) do
+  has_weight 149
+
+  def parse(parser, options)
+    OctocatalogDiff::Cli::Options.option_globally_or_per_branch(
+      parser: parser,
+      options: options,
+      cli_name: 'fact-dir',
+      option_name: 'fact_file_dir',
+      desc: 'Fact directory',
+      datatype: '',
+      validator: lambda do |fact_dir|
+        base_dir = options[:basedir]
+        path = fact_dir.start_with?('/') ? File.expand_path(fact_dir) : File.expand_path(File.join(base_dir.to_s, fact_dir))
+        raise Errno::ENOENT, "Fact directory #{path} does not exist" unless Dir.exist?(path)
+      end,
+      translator: lambda do |fact_dir|
+        base_dir = options[:basedir]
+        fact_dir.start_with?('/') ? File.expand_path(fact_dir) : File.expand_path(File.join(base_dir.to_s, fact_dir))
+      end,
+      post_process: lambda do |opts|
+        if opts[:puppetdb_url]
+          raise ArgumentError, '--fact-dir and --puppetdb-url are mutually exclusive'
+        end
+        if opts[:to_facts] || opts[:from_facts] || opts[:facts] || opts[:fact_file]
+          raise ArgumentError, '--fact-dir and --fact-file are mutually exclusive'
+        end
+      end
+    )
+  end
+end

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

@@ -29,6 +29,9 @@ def parse(parser, options)
         end
       end,
       post_process: lambda do |opts|
+        if opts[:fact_file_dir]
+          raise ArgumentError, '--fact-file and --fact-dir are mutually exclusive'
+        end
         unless options[:node]
           %w[to_facts from_facts facts].each do |opt|
             next unless opts[opt.to_sym] && opts[opt.to_sym].node

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

@@ -15,6 +15,9 @@ def parse(parser, options)
       # support HTTP begrudgingly as well.
       obj = URI.parse(url)
       raise ArgumentError, 'PuppetDB URL must be http or https' unless obj.is_a?(URI::HTTPS) || obj.is_a?(URI::HTTP)
+      if options[:fact_file_dir] || options[:from_fact_file_dir] || options[:to_fact_file_dir]
+        raise ArgumentError, '--puppetdb-url and --fact-dir are mutually exclusive'
+      end
       options[:puppetdb_url] = url
     end
   end

spec/octocatalog-diff/fixtures/fact-dirs/json/rspec-node.xyz.github.net.json

@@ -0,0 +1,7 @@
+{
+  "apt_update_last_success":1458162123,
+  "architecture":"amd64",
+  "datacenter":"xyz",
+  "fqdn":"rspec-node.xyz.github.net",
+  "_timestamp":"2014-12-02 14:56:20 -0600"
+}

spec/octocatalog-diff/tests/catalog-util/facts_spec.rb

@@ -32,6 +32,43 @@
     end
   end
 
+  context 'facts from fact directory' do
+    describe '#facts' do
+      it 'should read a YAML file from fact dir' do
+        fact_dir = OctocatalogDiff::Spec.fixture_path('facts')
+        testobj = OctocatalogDiff::CatalogUtil::Facts.new(node: 'valid-facts', fact_file_dir: fact_dir)
+        answer = OctocatalogDiff::Facts.new(
+          backend: :yaml,
+          fact_file_string: File.read(OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml')),
+          node: 'valid-facts'
+        )
+        expect(testobj.facts.facts).to eq(answer.facts)
+      end
+
+      it 'should read a JSON file from fact dir' do
+        fact_dir = OctocatalogDiff::Spec.fixture_path('fact-dirs/json')
+        testobj = OctocatalogDiff::CatalogUtil::Facts.new(node: 'rspec-node.xyz.github.net', fact_file_dir: fact_dir)
+        answer = OctocatalogDiff::Facts.new(
+          backend: :json,
+          fact_file_string: File.read(OctocatalogDiff::Spec.fixture_path('fact-dirs/json/rspec-node.xyz.github.net.json')),
+          node: 'rspec-node.xyz.github.net'
+        )
+        expect(testobj.facts.facts).to eq(answer.facts)
+      end
+
+      it 'should raise an error when puppetdb_url is also provided' do
+        fact_dir = OctocatalogDiff::Spec.fixture_path('facts')
+        expect do
+          OctocatalogDiff::CatalogUtil::Facts.new(
+            node: 'valid-facts',
+            fact_file_dir: fact_dir,
+            puppetdb_url: 'http://localhost:8080'
+          )
+        end.to raise_error(ArgumentError)
+      end
+    end
+  end
+
   context 'facts from YAML' do
     describe '#facts' do
       it 'should read a YAML file and build facts' do

spec/octocatalog-diff/tests/cli/options/fact_dir_spec.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require_relative '../options_helper'
+
+describe OctocatalogDiff::Cli::Options do
+  describe '#opt_fact_dir' do
+    let(:fact_dir) { OctocatalogDiff::Spec.fixture_path('facts') }
+
+    it 'should set fact_file_dir for to/from when provided' do
+      result = run_optparse(['--fact-dir', fact_dir])
+      expect(result[:to_fact_file_dir]).to eq(fact_dir)
+      expect(result[:from_fact_file_dir]).to eq(fact_dir)
+    end
+
+    it 'should allow branch-specific values' do
+      result = run_optparse(['--to-fact-dir', fact_dir])
+      expect(result[:to_fact_file_dir]).to eq(fact_dir)
+      expect(result.key?(:from_fact_file_dir)).to be(false)
+    end
+
+    it 'should resolve relative paths against basedir' do
+      result = run_optparse(['--fact-dir', 'spec/octocatalog-diff/fixtures/facts'], basedir: Dir.pwd)
+      expect(result[:to_fact_file_dir]).to eq(fact_dir)
+      expect(result[:from_fact_file_dir]).to eq(fact_dir)
+    end
+
+    it 'should throw error if --puppetdb-url is also provided' do
+      expect do
+        run_optparse(['--fact-dir', fact_dir, '--puppetdb-url', 'http://localhost:8080'])
+      end.to raise_error(ArgumentError)
+    end
+  end
+end

spec/octocatalog-diff/tests/cli/options/fact_file_spec.rb

@@ -47,6 +47,14 @@
         run_optparse(['--fact-file', fact_file])
       end.to raise_error(Errno::ENOENT)
     end
+
+    it 'should throw error if --fact-dir is also provided' do
+      expect do
+        fact_file = OctocatalogDiff::Spec.fixture_path('facts/facts.yaml')
+        fact_dir = OctocatalogDiff::Spec.fixture_path('facts')
+        run_optparse(['--fact-file', fact_file, '--fact-dir', fact_dir])
+      end.to raise_error(ArgumentError)
+    end
   end
 
   describe '#opt_to_fact_file' do