(SIMP-4076) Ensure that puppet code is valid (#86)

* Prior to bootstrap, we now ensure that the site.pp and site module
  code is valid so that we don't have confusing delays after waiting for
  multiple failing Puppet runs.
* Clarified the message when bootstrap is locked
* Ensured that backtraces are not displayed to the user on a known
  failure case

SIMP-4550 #close
SIMP-4551 #comment Ready for test
SIMP-4036 #close

build/rubygem-simp-cli.spec

@@ -100,6 +100,14 @@ EOM
 %doc %{gemdir}/doc
 
 %changelog
+* Fri Mar 16 2018 Trevor Vaughan <[email protected]> - 4.0.5
+- Prior to bootstrap, we now ensure that the site.pp and site module
+  code is valid so that we don't have confusing delays after waiting for
+  multiple failing Puppet runs.
+- Clarified the message when bootstrap is locked
+- Ensured that backtraces are not displayed to the user on known
+  bootstrap failure cases
+
 * Mon Mar 12 2018 Liz Nemsick <[email protected]> - 4.0.5
 - Set the ownership and permissions of files generated by simp cli,
   instead of allowing them to be set to those of the root user.

lib/simp/cli.rb

@@ -111,6 +111,9 @@ def self.start(args = ARGV)
           e.backtrace.first(10).each{|l| $stderr.puts l }
         end
         result = 1
+      rescue Simp::Cli::ProcessingError => e
+        $stderr.puts "\n\033[31m#{e.message}\033[39m\n\n"
+        result = 1
       rescue => e
         $stderr.puts "\n\033[31m#{e.message}\033[39m\n\n"
         e.backtrace.first(10).each{|l| $stderr.puts l }

lib/simp/cli/commands/bootstrap.rb

@@ -9,6 +9,7 @@ class Simp::Cli::Commands::Bootstrap < Simp::Cli
   require 'timeout'
   require 'facter'
   require File.expand_path( '../defaults', File.dirname(__FILE__) )
+  require File.expand_path( '../errors', File.dirname(__FILE__) )
   HighLine.colorize_strings
 
   @start_time = Time.now
@@ -78,13 +79,14 @@ def self.run(args = [])
     super
     return if @help_requested
 
-    check_for_start_lock
-    set_up_simp_environment
-
     # Open log file
     logfilepath = File.dirname(File.expand_path(@bootstrap_log))
     FileUtils.mkpath(logfilepath) unless File.exists?(logfilepath)
     @logfile = File.open(@bootstrap_log, 'w')
+
+    check_for_start_lock
+    set_up_simp_environment
+
     FileUtils.mkdir(@bootstrap_backup)
 
     # Print intro
@@ -107,6 +109,7 @@ def self.run(args = [])
 
     ensure_puppet_processes_stopped
     handle_existing_puppet_certs
+    validate_site_puppet_code
     configure_bootstrap_puppetserver
 
     # - Firstrun is tagged and run against the bootstrap puppetserver port, 8150.
@@ -165,6 +168,40 @@ def self.check_for_start_lock
     end
   end
 
+  # Do a quick validation that the code in the malleable SIMP spaces is not
+  # going to cause the compilation to fail out of the box.
+  def self.validate_site_puppet_code
+    info('Validating site puppet code', 'cyan')
+
+    errors = []
+
+    site_pp = File.join(::Utils.puppet_info[:config]['codedir'], 'environments','simp','manifests','site.pp')
+
+    if File.exist?(site_pp)
+      msg = %x{puppet parser validate #{site_pp} 2>&1}
+      unless $?.success?
+        errors << msg.strip
+      end
+    end
+
+    site_module = File.join(::Utils.puppet_info[:config]['codedir'], 'environments','simp','modules','site')
+
+    if File.directory?(site_module)
+      msg = %x{puppet parser validate #{site_module} 2>&1}
+      unless $?.success?
+        errors << msg.strip
+      end
+    end
+
+    unless errors.empty?
+      fail(
+        "Puppet code validation failed\n" +
+          "Please fix your manifests and try again\n" +
+          "  * #{errors.join("\n  * ")}"
+        )
+    end
+  end
+
   # Configure an initial, bootstrap puppetserver service listening on 8150
   # - Many of our modules depend on server_facts, which require a running puppetserver.
   #   Otherwise, puppet applys would suffice.
@@ -545,8 +582,13 @@ def self.warn(message, options=nil, console_prefix='> ')
     log_and_say("WARNING: #{message}", options, console_prefix)
   end
 
-  def self.error(message, options=nil, console_prefix='> ')
+  def self.error(message, options='red.bold', console_prefix='> ')
+    log_and_say("ERROR: #{message}", options, console_prefix)
+  end
+
+  def self.fail(message, options='red.bold', console_prefix='> ')
     log_and_say("ERROR: #{message}", options, console_prefix)
+    raise Simp::Cli::ProcessingError.new("bootstrap processing terminated")
   end
 
   def self.log_and_say(message, options, console_prefix, log_to_console = true)
@@ -564,5 +606,4 @@ def self.log_and_say(message, options, console_prefix, log_to_console = true)
       end
     end
   end
-
 end

lib/simp/cli/config/items/action/warn_lockout_risk_action.rb

@@ -19,7 +19,7 @@ def initialize
 
 #{'#'*72}
 Per security policy, SIMP, by default, disables login via ssh for all
-users, including 'root', and beginning with SIMP 6.0.0 (when 
+users, including 'root', and beginning with SIMP 6.0.0 (when
 useradd::securetty is empty), disables root logins at the console.  So,
 to prevent lockout in systems for which no administrative user account
 has yet been created or both console access is not available and the
@@ -70,11 +70,11 @@ class userx_user (
 addressed any other issues identified in this file, you can remove
 this file and continue with 'simp bootstrap'.
 DOC
-   end
+    end
 
     def apply
       warn( "\nWARNING: #{@warning_message_brief}", [:RED] )
-      warn( "See #{Simp::Cli::BOOTSTRAP_START_LOCK_FILE} for details", [:RED] )
+      warn( "See #{@warning_file} for details", [:RED] )
 
       # append/create file that will prevent bootstrap from running until problem
       # is fixed
@@ -87,7 +87,7 @@ def apply
 
     def apply_summary
       if @applied_status == :failed
-        "'simp bootstrap' has been locked due to potential login lockout."
+        "'simp bootstrap' has been locked due to potential login lockout.\n  * See #{@warning_file} for details"
       else
         "Check for login lockout risk #{@applied_status}"
       end

lib/simp/cli/errors.rb

@@ -0,0 +1,8 @@
+module Simp; end
+
+class Simp::Cli
+  # Any command processing error that terminates the command
+  # operation but for which a backtrace is not required
+  class ProcessingError < StandardError; end
+end
+

spec/lib/simp/cli/config/items/action/warn_lockout_risk_action_spec.rb

@@ -25,8 +25,8 @@
       actual_message = IO.read(@warning_file)
       expect( actual_message).to eq @ci.warning_message
       expected_summary =
-        "'simp bootstrap' has been locked due to potential login lockout."
-      expect( @ci.apply_summary ).to eq expected_summary
+        %r{^'simp bootstrap' has been locked due to potential login lockout\.\n  \* See /.+/simp_bootstrap_start_lock for details$}
+      expect( @ci.apply_summary ).to match expected_summary
     end
 
     it "appends warning file" do