Add Puppet 6 support for computed catalogs

Add support for Puppet >= 6.5.0. The Puppet command Octocatalog-diff
used was removed in 6.0.0 and an equivalent command was added back
in 6.5.0.

With this change, Octocatalog-diff detects the Puppet version and
adjusts the Puppet command line as needed based on the version.

Author: seanmil

config/puppet-versions.json

@@ -19,5 +19,11 @@
     "maximum_version": "5.99.99",
     "additional_gems": [
     ]
+  },
+  {
+    "minimum_version": "6.0.0",
+    "maximum_version": "6.99.99",
+    "additional_gems": [
+    ]
   }
 ]

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

@@ -54,9 +54,21 @@ def setup
         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)
 
+        puppet_version = Gem::Version.new(@options[:puppet_version])
+
         # Node to compile
         cmdline = []
-        cmdline.concat ['master', '--compile', Shellwords.escape(@node)]
+        # The 'puppet master --compile' command was removed in Puppet 6.x and replaced in
+        # Puppet 6.5 with an identically functioning 'puppet catalog compile' command.
+        # From versions 6.0.0 until 6.5.0 there is no compatible invocation method.
+        if puppet_version < Gem::Version.new('6.0.0')
+          cmdline.concat ['master', '--compile', Shellwords.escape(@node)]
+        elsif puppet_version < Gem::Version.new('6.5.0')
+          raise OctocatalogDiff::Errors::PuppetVersionError,
+                'Octocatalog-diff does not support Puppet versions >= 6.0.0 and < 6.5.0'
+        else
+          cmdline.concat ['catalog', 'compile', Shellwords.escape(@node)]
+        end
 
         # storeconfigs?
         if @options[:storeconfigs]
@@ -95,9 +107,16 @@ def setup
           --no-daemonize
           --no-ca
           --color=false
-          --config_version="/bin/echo catalogscript"
         )
 
+        # This config_version parameter causes an error when run with Puppet 6.x. Per
+        # the Puppet configuration settings docs, the below config_version argument
+        # may not actually be valid, but for backward compatibility's sake we'll keep it
+        # for the versions it has always worked with:
+        if puppet_version < Gem::Version.new('6.0.0')
+          cmdline.concat ['--config_version="/bin/echo catalogscript"']
+        end
+
         # Add environment - only make this variable if preserve_environments is used.
         # If preserve_environments is not used, the hard-coded 'production' here matches
         # up with the symlink created under the temporary directory structure.

lib/octocatalog-diff/catalog/computed.rb

@@ -147,7 +147,8 @@ def puppet_command_obj
             puppet_binary: @puppet_binary,
             fact_file: @builddir.fact_file,
             dir: @builddir.tempdir,
-            enc: @builddir.enc
+            enc: @builddir.enc,
+            puppet_version: puppet_version
           )
           OctocatalogDiff::CatalogUtil::Command.new(command_opts)
         end

spec/octocatalog-diff/integration/reference_validation_spec.rb

@@ -126,7 +126,7 @@ def self.catalog_contains_resource(result, type, title)
       expect(@result.exception).to be_a_kind_of(OctocatalogDiff::Errors::CatalogError)
     end
 
-    if OctocatalogDiff::Spec.is_puppet5?
+    if OctocatalogDiff::Spec.major_version >= 5
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
         expect(msg).to match(/Error: Could not find resource 'Exec\[subscribe target\]' in parameter 'subscribe'/)
@@ -159,7 +159,7 @@ def self.catalog_contains_resource(result, type, title)
       expect(@result.exception).to be_a_kind_of(OctocatalogDiff::Errors::CatalogError)
     end
 
-    if OctocatalogDiff::Spec.is_puppet5?
+    if OctocatalogDiff::Spec.major_version >= 5
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
         expect(msg).to match(/Error: Could not find resource 'Exec\[before target\]' in parameter 'before'/)
@@ -187,7 +187,7 @@ def self.catalog_contains_resource(result, type, title)
       expect(@result.exception).to be_a_kind_of(OctocatalogDiff::Errors::CatalogError)
     end
 
-    if OctocatalogDiff::Spec.is_puppet5?
+    if OctocatalogDiff::Spec.major_version >= 5
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
         expect(msg).to match(/Error: Could not find resource 'Test::Foo::Bar\[notify target\]' in parameter 'notify'/)
@@ -215,7 +215,7 @@ def self.catalog_contains_resource(result, type, title)
       expect(@result.exception).to be_a_kind_of(OctocatalogDiff::Errors::CatalogError)
     end
 
-    if OctocatalogDiff::Spec.is_puppet5?
+    if OctocatalogDiff::Spec.major_version >= 5
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
         expect(msg).to match(/Error: Could not find resource 'Exec\[require target\]' in parameter 'require'/)
@@ -239,7 +239,7 @@ def self.catalog_contains_resource(result, type, title)
       @result = OctocatalogDiff::Spec.reference_validation_catalog('broken-subscribe', %w(before notify require))
     end
 
-    if OctocatalogDiff::Spec.is_puppet5?
+    if OctocatalogDiff::Spec.major_version >= 5
       it 'should raise CatalogError' do
         expect(@result.exception).to be_a_kind_of(OctocatalogDiff::Errors::CatalogError)
       end
@@ -295,7 +295,7 @@ def self.catalog_contains_resource(result, type, title)
       expect(@result.exception).to be_a_kind_of(OctocatalogDiff::Errors::CatalogError)
     end
 
-    if OctocatalogDiff::Spec.is_puppet5?
+    if OctocatalogDiff::Spec.major_version >= 5
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
         expect(msg).to match(/Error: Could not find resource 'Exec\[before alias target\]' in parameter 'before'/)

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

@@ -3,6 +3,7 @@
 require_relative '../spec_helper'
 require 'fileutils'
 require OctocatalogDiff::Spec.require_path('/catalog-util/command')
+require OctocatalogDiff::Spec.require_path('/errors')
 
 describe OctocatalogDiff::CatalogUtil::Command do
   describe '#initialize' do
@@ -73,6 +74,23 @@
       expect { testobj.puppet_command }.to raise_error(Errno::ENOENT, /Puppet binary.*doesn't exist/)
     end
 
+    it 'should use "master --compile" when Puppet version is 5.x' do
+      testobj = OctocatalogDiff::CatalogUtil::Command.new(@default_opts.merge(puppet_version: '5.5.20'))
+      result = testobj.puppet_command
+      expect(result).to match(/master --compile/)
+    end
+
+    it 'should use "catalog compile" when Puppet version is 6.x' do
+      testobj = OctocatalogDiff::CatalogUtil::Command.new(@default_opts.merge(puppet_version: '6.5.0'))
+      result = testobj.puppet_command
+      expect(result).to match(/catalog compile/)
+    end
+
+    it 'should raise an error when Puppet version is 6.4' do
+      testobj = OctocatalogDiff::CatalogUtil::Command.new(@default_opts.merge(puppet_version: '6.4.0'))
+      expect { testobj.puppet_command }.to raise_error(OctocatalogDiff::Errors::PuppetVersionError, /does not support/)
+    end
+
     it 'should include --storeconfigs and --storeconfigs_backend when storeconfigs is enabled' do
       testobj = OctocatalogDiff::CatalogUtil::Command.new(@default_opts.merge(storeconfigs: true))
       result = testobj.puppet_command
@@ -134,6 +152,18 @@
       expect(result).to match(/--facts_terminus=facter/)
     end
 
+    it 'should include config_version when Puppet version < 6' do
+      testobj = OctocatalogDiff::CatalogUtil::Command.new(@default_opts.merge(puppet_version: '5.5.20'))
+      result = testobj.puppet_command
+      expect(result).to match(%r{--config_version="/bin/echo catalogscript"})
+    end
+
+    it 'should not include config_version when Puppet version >= 6' do
+      testobj = OctocatalogDiff::CatalogUtil::Command.new(@default_opts.merge(puppet_version: '6.18.0'))
+      result = testobj.puppet_command
+      expect(result).not_to match(/--config_version=/)
+    end
+
     it 'should raise error when invalid facts terminus is specified' do
       testobj = OctocatalogDiff::CatalogUtil::Command.new(@default_opts.merge(facts_terminus: 'chicken'))
       expect { testobj.puppet_command }.to raise_error(ArgumentError, /Unrecognized facts_terminus setting/)

spec/octocatalog-diff/tests/catalog/computed_spec.rb

@@ -70,7 +70,11 @@
           expect(@logger_str.string).to match(%r{Symlinked.*environments/production ->.*/repos/default})
           expect(@logger_str.string).to match(/Installed hiera.yaml from/)
           expect(@logger_str.string).to match(/Installed fact file at/)
-          expect(@logger_str.string).to match(/puppet master --compile rspec-node.github.net/)
+          if OctocatalogDiff::Spec.major_version >= 6
+            expect(@logger_str.string).to match(/puppet catalog compile rspec-node.github.net/)
+          else
+            expect(@logger_str.string).to match(/puppet master --compile rspec-node.github.net/)
+          end
           expect(@logger_str.string).to match(/Catalog succeeded on try 1/)
         end
       end
@@ -150,7 +154,11 @@
         it 'should have the correct log messages' do
           expect(@logger_str.string).to match(%r{Symlinked.*environments/production ->.*/repos/failing-catalog})
           expect(@logger_str.string).to match(/Installed fact file at/)
-          expect(@logger_str.string).to match(/puppet master --compile rspec-node.github.net/)
+          if OctocatalogDiff::Spec.major_version >= 6
+            expect(@logger_str.string).to match(/puppet catalog compile rspec-node.github.net/)
+          else
+            expect(@logger_str.string).to match(/puppet master --compile rspec-node.github.net/)
+          end
           expect(@logger_str.string).to match(/Catalog failed on try 1/)
         end
       end

spec/octocatalog-diff/tests/spec_helper.rb

@@ -288,9 +288,9 @@ def self.mock_puppetdb_fact_response(hostname)
       facts_in['values'].keys.map { |k| { 'name' => k, 'value' => facts_in['values'][k] } }.to_json
     end
 
-    # Determine if puppet version is Puppet 5 or not
-    def self.is_puppet5?
-      puppet_version && puppet_version >= '5.0.0'
+    # Helper functions to determine which major version of Puppet we are working with:
+    def self.major_version
+      puppet_version && puppet_version.split('.')[0].to_i
     end
   end
 end