From ee69ecfab838fc3094bf122b88f7311e443bee40 Mon Sep 17 00:00:00 2001 From: Trevor Vaughan Date: Tue, 27 Mar 2018 09:21:03 -0400 Subject: [PATCH] (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 | 8 +++ lib/simp/cli.rb | 3 ++ lib/simp/cli/commands/bootstrap.rb | 51 +++++++++++++++++-- .../items/action/warn_lockout_risk_action.rb | 8 +-- lib/simp/cli/errors.rb | 8 +++ .../action/warn_lockout_risk_action_spec.rb | 4 +- 6 files changed, 71 insertions(+), 11 deletions(-) create mode 100644 lib/simp/cli/errors.rb diff --git a/build/rubygem-simp-cli.spec b/build/rubygem-simp-cli.spec index f2d6bba5..6b960445 100644 --- a/build/rubygem-simp-cli.spec +++ b/build/rubygem-simp-cli.spec @@ -100,6 +100,14 @@ EOM %doc %{gemdir}/doc %changelog +* Fri Mar 16 2018 Trevor Vaughan - 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 - 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. diff --git a/lib/simp/cli.rb b/lib/simp/cli.rb index d0a3e1a2..20ed94bf 100644 --- a/lib/simp/cli.rb +++ b/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 } diff --git a/lib/simp/cli/commands/bootstrap.rb b/lib/simp/cli/commands/bootstrap.rb index c0c67b54..26c14016 100755 --- a/lib/simp/cli/commands/bootstrap.rb +++ b/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 diff --git a/lib/simp/cli/config/items/action/warn_lockout_risk_action.rb b/lib/simp/cli/config/items/action/warn_lockout_risk_action.rb index 796a900b..ebe86938 100644 --- a/lib/simp/cli/config/items/action/warn_lockout_risk_action.rb +++ b/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 diff --git a/lib/simp/cli/errors.rb b/lib/simp/cli/errors.rb new file mode 100644 index 00000000..3102d56e --- /dev/null +++ b/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 + diff --git a/spec/lib/simp/cli/config/items/action/warn_lockout_risk_action_spec.rb b/spec/lib/simp/cli/config/items/action/warn_lockout_risk_action_spec.rb index c41cc5a2..ac37f917 100644 --- a/spec/lib/simp/cli/config/items/action/warn_lockout_risk_action_spec.rb +++ b/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