Merge pull request puppetlabs#2345 from beechtom/2337/config-validator

(puppetlabsGH-2337) Validate config using schemas

Author: lucywyman

lib/bolt/bolt_option_parser.rb

@@ -891,12 +891,12 @@ def initialize(options)
         if ENV.include?(Bolt::Inventory::ENVIRONMENT_VAR)
           raise Bolt::CLIError, "Cannot pass inventory file when #{Bolt::Inventory::ENVIRONMENT_VAR} is set"
         end
-        @options[:inventoryfile] = Pathname.new(File.expand_path(path))
+        @options[:inventoryfile] = File.expand_path(path)
       end
       define('--puppetfile PATH',
              'Specify a Puppetfile to use when installing modules. (default: ~/.puppetlabs/bolt/Puppetfile)',
              'Modules are installed in the current project.') do |path|
-        @options[:puppetfile_path] = Pathname.new(File.expand_path(path))
+        @options[:puppetfile_path] = File.expand_path(path)
       end
       define('--[no-]save-rerun', 'Whether to update the rerun file after this command.') do |save|
         @options[:'save-rerun'] = save

lib/bolt/config.rb

@@ -7,6 +7,7 @@
 require 'bolt/logger'
 require 'bolt/util'
 require 'bolt/config/options'
+require 'bolt/config/validator'
 
 module Bolt
   class UnknownTransportError < Bolt::Error
@@ -73,6 +74,16 @@ def self.from_file(configfile, overrides = {})
       new(project, data, overrides)
     end
 
+    def self.defaults_schema
+      base = OPTIONS.slice(*BOLT_DEFAULTS_OPTIONS)
+      base['inventory-config'][:properties] = TRANSPORT_CONFIG.transform_values(&:schema)
+      base
+    end
+
+    def self.bolt_schema
+      OPTIONS.slice(*BOLT_OPTIONS).merge(TRANSPORT_CONFIG.transform_values(&:schema))
+    end
+
     def self.system_path
       # Lazy-load expensive gem code
       require 'win32/dir' if Bolt::Util.windows?
@@ -106,6 +117,10 @@ def self.load_bolt_defaults_yaml(dir)
         )
       end
 
+      # Validate the config against the schema. This will raise a single error
+      # with all validation errors.
+      Validator.new.validate(data, defaults_schema, filepath)
+
       # Remove project-specific config such as hiera-config, etc.
       project_config = data.slice(*(BOLT_PROJECT_OPTIONS - BOLT_DEFAULTS_OPTIONS))
 
@@ -161,6 +176,10 @@ def self.load_bolt_yaml(dir)
                         msg: "Configuration file #{filepath} is deprecated and will be removed in a future version "\
                         "of Bolt. Use '#{dir + BOLT_DEFAULTS_NAME}' instead." }]
 
+      # Validate the config against the schema. This will raise a single error
+      # with all validation errors.
+      Validator.new.validate(data, bolt_schema, filepath)
+
       { filepath: filepath, data: data, logs: logs, deprecations: deprecations }
     end
 
@@ -271,7 +290,7 @@ def normalize_overrides(options)
 
       # Set console log to debug if in debug mode
       if options[:debug]
-        overrides['log'] = { 'console' => { 'level' => :debug } }
+        overrides['log'] = { 'console' => { 'level' => 'debug' } }
       end
 
       if options[:puppetfile_path]
@@ -280,6 +299,9 @@ def normalize_overrides(options)
 
       overrides['trace'] = opts['trace'] if opts.key?('trace')
 
+      # Validate the overrides
+      Validator.new.validate(overrides, OPTIONS, 'command line')
+
       overrides
     end
 
@@ -397,35 +419,11 @@ def validate
               "is automatically appended to the modulepath and cannot be configured."
       end
 
-      keys = OPTIONS.keys - %w[plugins plugin_hooks puppetdb]
-      keys.each do |key|
-        next unless Bolt::Util.references?(@data[key])
-        valid_keys = TRANSPORT_CONFIG.keys + %w[plugins plugin_hooks puppetdb]
-        raise Bolt::ValidationError,
-              "Found unsupported key _plugin in config setting #{key}. Plugins are only available in "\
-              "#{valid_keys.join(', ')}."
-      end
-
-      unless concurrency.is_a?(Integer) && concurrency > 0
-        raise Bolt::ValidationError,
-              "Concurrency must be a positive Integer, received #{concurrency.class} #{concurrency}"
-      end
-
-      unless compile_concurrency.is_a?(Integer) && compile_concurrency > 0
-        raise Bolt::ValidationError,
-              "Compile concurrency must be a positive Integer, received #{compile_concurrency.class} "\
-              "#{compile_concurrency}"
-      end
-
       compile_limit = 2 * Etc.nprocessors
       unless compile_concurrency < compile_limit
         raise Bolt::ValidationError, "Compilation is CPU-intensive, set concurrency less than #{compile_limit}"
       end
 
-      unless %w[human json rainbow].include? format
-        raise Bolt::ValidationError, "Unsupported format: '#{format}'"
-      end
-
       Bolt::Util.validate_file('hiera-config', @data['hiera-config']) if @data['hiera-config']
       Bolt::Util.validate_file('trusted-external-command', trusted_external) if trusted_external
 

lib/bolt/config/options.rb

@@ -307,7 +307,11 @@ module Options
                        "its value is a map of configuration data. Configurable options are specified by the plugin. "\
                        "Read more about configuring plugins in [Using plugins](using_plugins.md#configuring-plugins).",
           type: Hash,
-          _plugin: true,
+          additionalProperties: {
+            type: Hash,
+            _plugin: true
+          },
+          _plugin: false,
           _example: { "pkcs7" => { "keysize" => 1024 } }
         },
         "puppetdb" => {

lib/bolt/config/transport/base.rb

@@ -2,6 +2,7 @@
 
 require 'bolt/error'
 require 'bolt/util'
+require 'bolt/config/validator'
 require 'bolt/config/transport/options'
 
 module Bolt
@@ -90,6 +91,14 @@ def self.options
           self::OPTIONS
         end
 
+        def self.schema
+          {
+            type:       Hash,
+            properties: self::TRANSPORT_OPTIONS.slice(*self::OPTIONS),
+            _plugin:    true
+          }
+        end
+
         private def defaults
           unless defined? self.class::DEFAULTS
             raise NotImplementedError,
@@ -116,25 +125,7 @@ def self.options
 
         # Validation defaults to just asserting the option types
         private def validate
-          assert_type
-        end
-
-        # Validates that each option is the correct type. Types are loaded from the TRANSPORT_OPTIONS hash.
-        private def assert_type
-          @config.each_pair do |opt, val|
-            types = Array(TRANSPORT_OPTIONS.dig(opt, :type)).compact
-
-            next if val.nil? || types.empty? || types.include?(val.class)
-
-            # Ruby doesn't have a Boolean class, so add it to the types list if TrueClass or FalseClass
-            # are present.
-            if types.include?(TrueClass) || types.include?(FalseClass)
-              types = types - [TrueClass, FalseClass] + ['Boolean']
-            end
-
-            raise Bolt::ValidationError,
-                  "#{opt} must be of type #{types.join(', ')}; received #{val.class} #{val.inspect} "
-          end
+          Bolt::Config::Validator.new.validate(@config.compact, self.class.schema.fetch(:properties))
         end
       end
     end

lib/bolt/config/transport/local.rb

@@ -29,13 +29,6 @@ def self.options
           if @config['interpreters']
             @config['interpreters'] = normalize_interpreters(@config['interpreters'])
           end
-
-          if (run_as_cmd = @config['run-as-command'])
-            unless run_as_cmd.all? { |n| n.is_a?(String) }
-              raise Bolt::ValidationError,
-                    "run-as-command must be an Array of Strings, received #{run_as_cmd.class} #{run_as_cmd.inspect}"
-            end
-          end
         end
       end
     end

lib/bolt/config/transport/ssh.rb

@@ -73,6 +73,14 @@ def self.options
           %w[ssh-command native-ssh].concat(OPTIONS)
         end
 
+        def self.schema
+          {
+            type:       Hash,
+            properties: self::TRANSPORT_OPTIONS.slice(*(self::OPTIONS + self::NATIVE_OPTIONS)),
+            _plugin:    true
+          }
+        end
+
         private def filter(unfiltered)
           # Because we filter before merging config together it's impossible to
           # know whether both ssh-command *and* native-ssh will be specified
@@ -87,12 +95,6 @@ def self.options
           super
 
           if (key_opt = @config['private-key'])
-            unless key_opt.instance_of?(String) || (key_opt.instance_of?(Hash) && key_opt.include?('key-data'))
-              raise Bolt::ValidationError,
-                    "private-key option must be a path to a private key file or a Hash containing the 'key-data', "\
-                    "received #{key_opt.class} #{key_opt}"
-            end
-
             if key_opt.instance_of?(String)
               @config['private-key'] = File.expand_path(key_opt, @project)
 
@@ -114,14 +116,6 @@ def self.options
                   "Unsupported login-shell #{@config['login-shell']}. Supported shells are #{LOGIN_SHELLS.join(', ')}"
           end
 
-          %w[encryption-algorithms host-key-algorithms kex-algorithms mac-algorithms run-as-command].each do |opt|
-            next unless @config.key?(opt)
-            unless @config[opt].all? { |n| n.is_a?(String) }
-              raise Bolt::ValidationError,
-                    "#{opt} must be an Array of Strings, received #{@config[opt].inspect}"
-            end
-          end
-
           if @config['login-shell'] == 'powershell'
             %w[tty run-as].each do |key|
               if @config[key]