fixes #19254 - check Puppet version is supported by modules (#213)

Using the Puppet version requirements from metadata.json in the available modules, the running Puppet version is validated to ensure it is compatible. The user can skip this with `--skip-puppet-version-check` if they wish.
theforeman · Jun 28, 2017 · c55f9e4 · c55f9e4
1 parent f8005be
commit c55f9e4
Show file tree

Hide file tree

Showing 13 changed files with 208 additions and 17 deletions.
diff --git a/README.md b/README.md
@@ -1125,6 +1125,7 @@ Other exit codes that can be returned:
 * '27' means that kafo found found scenario configuration error that prevents installation from continuing
 * '28' means that a value is missing for a parameter given on the command line
 * '29' means that effective user that ran the installer does not have permission to update the answer file
+* '30' means that the version of Puppet is incompatible with a module, according to its [metadata.json](https://docs.puppet.com/puppet/latest/modules_metadata.html)
 * '130' user interrupt (^C)
 
 ## Running Puppet Profiling

diff --git a/lib/kafo/configuration.rb b/lib/kafo/configuration.rb
@@ -29,7 +29,8 @@ class Configuration
         :hook_dirs            => [],
         :custom               => {},
         :low_priority_modules => [],
-        :verbose_log_level    => 'info'
+        :verbose_log_level    => 'info',
+        :skip_puppet_version_check => false
     }
 
     def initialize(file, persist = true)
@@ -166,11 +167,19 @@ class { '::kafo_configure::dump_values':
         @logger.debug result
         unless $?.exitstatus == 0
           log = app[:log_dir] + '/' + app[:log_name]
-          puts "Could not get default values, check log file at #{log} for more information"
-          @logger.error command
-          @logger.error result
-          @logger.error 'Could not get default values, cannot continue'
-          KafoConfigure.exit(:defaults_error)
+          if (version_mismatch = /kafo_configure::puppet_version_failure: (.+?\))/.match(result))
+            puts version_mismatch[1]
+            puts "Cannot continue due to incompatible version of Puppet. Use --skip-puppet-version-check to disable this check."
+            @logger.error version_mismatch[1]
+            @logger.error 'Incompatible version of Puppet used, cannot continue'
+            KafoConfigure.exit(:puppet_version_error)
+          else
+            puts "Could not get default values, check log file at #{log} for more information"
+            @logger.error command
+            @logger.error result
+            @logger.error 'Could not get default values, cannot continue'
+            KafoConfigure.exit(:defaults_error)
+          end
         end
         @logger.info "... finished"
 

diff --git a/lib/kafo/exit_handler.rb b/lib/kafo/exit_handler.rb
@@ -19,7 +19,8 @@ def error_codes
           :unset_scenario => 26,
           :scenario_error => 27,
           :missing_argument => 28,
-          :insufficient_permissions => 29
+          :insufficient_permissions => 29,
+          :puppet_version_error => 30
       }
     end
 

diff --git a/lib/kafo/kafo_configure.rb b/lib/kafo/kafo_configure.rb
@@ -308,6 +308,7 @@ def set_app_options
       self.class.app_option ['-p', '--profile'], :flag, 'Run puppet in profile mode?',
                             :default => false
       self.class.app_option ['-s', '--skip-checks-i-know-better'], :flag, 'Skip all system checks', :default => false
+      self.class.app_option ['--skip-puppet-version-check'], :flag, 'Skip check for compatible Puppet versions', :default => false
       self.class.app_option ['-v', '--verbose'], :flag, 'Display log on STDOUT instead of progressbar'
       self.class.app_option ['-l', '--verbose-log-level'], 'LEVEL', 'Log level for verbose mode output',
                             :default => 'info'

diff --git a/lib/kafo/puppet_command.rb b/lib/kafo/puppet_command.rb
@@ -6,22 +6,16 @@ def initialize(command, options = [], puppet_config = nil, configuration = KafoC
       @command = command
       @puppet_config = puppet_config
 
-      # Expand the modules_path to work around the fact that Puppet doesn't
-      # allow modulepath to contain relative (i.e ..) directory references as
-      # of 2.7.23.
-      @options = options.push("--modulepath #{File.expand_path(modules_path)}")
+      @options = options.push("--modulepath #{modules_path.join(':')}")
       @options.push("--config=#{puppet_config.config_path}") if puppet_config
       @logger  = KafoConfigure.logger
-    end
-
-    def add_progress
-      %{$kafo_add_progress="#{!KafoConfigure.verbose}"}
+      @puppet_version_check = !configuration.app[:skip_puppet_version_check]
     end
 
     def command
       @puppet_config.write_config if @puppet_config
       result = [
-          "echo '$kafo_config_file=\"#{@configuration.config_file}\" #{add_progress} #{@command}'",
+          manifest,
           '|',
           "RUBYLIB=#{[@configuration.kafo_modules_dir, ::ENV['RUBYLIB']].join(File::PATH_SEPARATOR)}",
           "#{puppet_path} apply #{@options.join(' ')} #{@suffix}",
@@ -44,11 +38,65 @@ def self.search_puppet_path(bin_name)
 
     private
 
+    def manifest
+      %{echo '
+        $kafo_config_file="#{@configuration.config_file}"
+        #{add_progress}
+        #{generate_version_checks.join("\n") if @puppet_version_check}
+        #{@command}
+      '}
+    end
+
+    def add_progress
+      %{$kafo_add_progress="#{!KafoConfigure.verbose}"}
+    end
+
+    def generate_version_checks
+      checks = []
+      modules_path.each do |modulepath|
+        Dir[File.join(modulepath, '*', 'metadata.json')].sort.each do |metadata_json|
+          metadata = JSON.load(File.read(metadata_json))
+          next unless metadata['requirements'] && metadata['requirements'].is_a?(Array)
+
+          metadata['requirements'].select { |req| req['name'] == 'puppet' && req['version_requirement'] }.each do |req|
+            checks << versioncmp(metadata['name'], req['version_requirement'])
+          end
+        end
+      end
+      checks
+    end
+
+    def versioncmp(id, version_req)
+      # Parse the common ">= x.y < x.y" version requirement to support pre-Puppet 4.5
+      # checks with versioncmp. Newer versions use SemVerRange for full support.
+      if (version_match = /\A>=\s*([0-9\.]+)(?:\s+<\s*([0-9\.]+))?/.match(version_req))
+        minimum = %{minimum => "#{version_match[1]}",}
+        maximum = version_match[2] ? %{maximum => "#{version_match[2]}",} : ''
+      else
+        minimum = ''
+        maximum = ''
+      end
+
+      # SemVerRange is isolated inside a defined type to prevent parse errors on old versions
+      <<-EOS
+        if versioncmp($::puppetversion, "4.5.0") >= 0 {
+          kafo_configure::puppet_version_semver { "#{id}":
+            requirement => "#{version_req}",
+          }
+        } else {
+          kafo_configure::puppet_version_versioncmp { "#{id}":
+            #{minimum}
+            #{maximum}
+          }
+        }
+      EOS
+    end
+
     def modules_path
       [
           @configuration.module_dirs,
           @configuration.kafo_modules_dir,
-      ].flatten.join(':')
+      ].flatten
     end
 
     def puppet_path

diff --git a/modules/kafo_configure/manifests/puppet_version_semver.pp b/modules/kafo_configure/manifests/puppet_version_semver.pp
@@ -0,0 +1,5 @@
+define kafo_configure::puppet_version_semver($requirement) {
+  unless SemVer($facts['puppetversion']) =~ SemVerRange($requirement) {
+    fail("kafo_configure::puppet_version_failure: Puppet ${facts['puppetversion']} does not meet requirements for ${title} ($requirement)")
+  }
+}
diff --git a/modules/kafo_configure/manifests/puppet_version_versioncmp.pp b/modules/kafo_configure/manifests/puppet_version_versioncmp.pp
@@ -0,0 +1,9 @@
+define kafo_configure::puppet_version_versioncmp($minimum = undef, $maximum = undef) {
+  if $minimum and versioncmp($minimum, $::puppetversion) > 0 {
+    fail("kafo_configure::puppet_version_failure: Puppet ${puppetversion} does not meet minimum requirement for ${title} (version $minimum)")
+  }
+
+  if $maximum and versioncmp($maximum, $::puppetversion) < 0 {
+    fail("kafo_configure::puppet_version_failure: Puppet ${puppetversion} does not meet maximum requirement for ${title} (version $maximum)")
+  }
+}
diff --git a/test/acceptance/kafo_configure_test.rb b/test/acceptance/kafo_configure_test.rb
@@ -137,5 +137,39 @@ module Kafo
         File.read("#{INSTALLER_HOME}/testing").must_equal '1.0'
       end
     end
+
+    describe 'with Puppet version requirements' do
+      it 'must run if they are met' do
+        add_metadata('basic')
+        code, out, err = run_command 'bin/kafo-configure'
+        code.exitstatus.must_equal 0
+        File.exist?("#{INSTALLER_HOME}/testing").must_equal true
+      end
+
+      it 'must fail if minimum version is not met' do
+        add_metadata('with_minimum_puppet')
+        code, out, err = run_command 'bin/kafo-configure'
+        code.exitstatus.must_equal 30
+        out.must_match /^Puppet [0-9\.]+ does not meet (\w+ )?requirements? for theforeman-testing/
+        out.must_include 'Use --skip-puppet-version-check to disable this check'
+        File.exist?("#{INSTALLER_HOME}/testing").must_equal false
+      end
+
+      it 'must fail if maximum version is not met' do
+        add_metadata('with_maximum_puppet')
+        code, out, err = run_command 'bin/kafo-configure'
+        code.exitstatus.must_equal 30
+        out.must_match /^Puppet [0-9\.]+ does not meet (\w+ )?requirements? for theforeman-testing/
+        out.must_include 'Use --skip-puppet-version-check to disable this check'
+        File.exist?("#{INSTALLER_HOME}/testing").must_equal false
+      end
+
+      it 'must run with --skip-puppet-version-check' do
+        add_metadata('with_maximum_puppet')
+        code, out, err = run_command 'bin/kafo-configure --skip-puppet-version-check'
+        code.exitstatus.must_equal 0
+        File.exist?("#{INSTALLER_HOME}/testing").must_equal true
+      end
+    end
   end
 end
diff --git a/test/acceptance/test_helper.rb b/test/acceptance/test_helper.rb
@@ -63,3 +63,8 @@ def add_module_data(name = 'basic')
   FileUtils.mkdir_p TEST_MODULE_PATH
   FileUtils.cp_r File.expand_path("../../fixtures/module_data/#{name}", __FILE__) + '/.', TEST_MODULE_PATH
 end
+
+def add_metadata(name = 'basic')
+  FileUtils.mkdir_p TEST_MODULE_PATH
+  FileUtils.cp File.expand_path("../../fixtures/metadata/#{name}.json", __FILE__), File.join(TEST_MODULE_PATH, 'metadata.json')
+end
diff --git a/test/fixtures/metadata/basic.json b/test/fixtures/metadata/basic.json
@@ -0,0 +1,18 @@
+{
+  "name": "theforeman-testing",
+  "version": "0.0.1",
+  "author": "The Foreman",
+  "license": "GPL-3.0",
+  "summary": "Testing module",
+  "source": "https://github.com/theforeman/kafo",
+  "project_page": "https://github.com/theforeman/kafo",
+  "issues_url": "http://projects.theforeman.org/projects/kafo/issues/",
+  "tags": ["kafo"],
+  "dependencies": [],
+  "requirements": [
+    {
+      "name": "puppet",
+      "version_requirement": ">= 3.0.0 < 999.0.0"
+    }
+  ]
+}
diff --git a/test/fixtures/metadata/with_maximum_puppet.json b/test/fixtures/metadata/with_maximum_puppet.json
@@ -0,0 +1,18 @@
+{
+  "name": "theforeman-testing",
+  "version": "0.0.1",
+  "author": "The Foreman",
+  "license": "GPL-3.0",
+  "summary": "Testing module",
+  "source": "https://github.com/theforeman/kafo",
+  "project_page": "https://github.com/theforeman/kafo",
+  "issues_url": "http://projects.theforeman.org/projects/kafo/issues/",
+  "tags": ["kafo"],
+  "dependencies": [],
+  "requirements": [
+    {
+      "name": "puppet",
+      "version_requirement": ">= 1.0.0 < 2.0.0"
+    }
+  ]
+}
diff --git a/test/fixtures/metadata/with_minimum_puppet.json b/test/fixtures/metadata/with_minimum_puppet.json
@@ -0,0 +1,18 @@
+{
+  "name": "theforeman-testing",
+  "version": "0.0.1",
+  "author": "The Foreman",
+  "license": "GPL-3.0",
+  "summary": "Testing module",
+  "source": "https://github.com/theforeman/kafo",
+  "project_page": "https://github.com/theforeman/kafo",
+  "issues_url": "http://projects.theforeman.org/projects/kafo/issues/",
+  "tags": ["kafo"],
+  "dependencies": [],
+  "requirements": [
+    {
+      "name": "puppet",
+      "version_requirement": ">= 999.0.0 < 100.0.0"
+    }
+  ]
+}
diff --git a/test/kafo/puppet_command_test.rb b/test/kafo/puppet_command_test.rb
@@ -12,6 +12,7 @@ module Kafo
       describe "with defaults" do
         specify { pc.command.must_be_kind_of String }
         specify { pc.command.must_include 'puppet apply --modulepath /' }
+        specify { pc.command.wont_include 'kafo_configure::puppet_version' }
 
         specify { KafoConfigure.stub(:verbose, false) { pc.command.must_include '$kafo_add_progress="true"' } }
         specify { KafoConfigure.stub(:verbose, true) { pc.command.must_include '$kafo_add_progress="false"' } }
@@ -31,6 +32,29 @@ module Kafo
           end
         end
       end
+
+      describe "with version checks" do
+        specify do
+          pc.stub(:modules_path, ['/modules']) do
+            Dir.stub(:[], ['./test/fixtures/metadata/basic.json']) do
+              pc.command.must_include 'kafo_configure::puppet_version_semver { "theforeman-testing":'
+              pc.command.must_include 'requirement => ">= 3.0.0 < 999.0.0"'
+              pc.command.must_include 'kafo_configure::puppet_version_versioncmp { "theforeman-testing":'
+              pc.command.must_include 'minimum => "3.0.0",'
+              pc.command.must_include 'maximum => "999.0.0",'
+            end
+          end
+        end
+
+        specify do
+          KafoConfigure.config.app[:skip_puppet_version_check] = true
+          pc.stub(:modules_path, ['/modules']) do
+            Dir.stub(:[], ['./test/fixtures/metadata/basic.json']) do
+              pc.command.wont_include 'kafo_configure::puppet_version'
+            end
+          end
+        end
+      end
     end
 
     describe '.search_puppet_path' do