diff --git a/CHANGELOG.md b/CHANGELOG.md index b5c05b376..32d9d793a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## 0.6.0 (unreleased) + - Configuration is now validated so improper input can be found in + Vagrantfiles. - Fixed issue with not detecting Vagrantfile at root directory ("/"). - Vagrant now gives a nice error message if there is a syntax error in any Vagrantfile. [GH-154] diff --git a/lib/vagrant/config.rb b/lib/vagrant/config.rb index d08e1907d..94740d76e 100644 --- a/lib/vagrant/config.rb +++ b/lib/vagrant/config.rb @@ -36,6 +36,7 @@ module Vagrant config_object ||= config run_procs!(config_object) + config_object.validate! config_object end end @@ -91,6 +92,23 @@ module Vagrant @env = env end + + # Validates the configuration classes of this instance and raises an + # exception if they are invalid. + def validate! + # Validate each of the configured classes and store the results into + # a hash. + errors = self.class.configures_list.inject({}) do |container, data| + key, _ = data + recorder = ErrorRecorder.new + send(key.to_sym).validate(recorder) + container[key.to_sym] = recorder if !recorder.errors.empty? + container + end + + return if errors.empty? + raise Errors::ConfigValidationFailed.new(:messages => Util::TemplateRenderer.render("config/validation_failed", :errors => errors)) + end end end end diff --git a/lib/vagrant/config/base.rb b/lib/vagrant/config/base.rb index daa83a831..1bb997926 100644 --- a/lib/vagrant/config/base.rb +++ b/lib/vagrant/config/base.rb @@ -18,7 +18,16 @@ module Vagrant end end - # Converts the configuration to a raw hash. + # Called by {Top} after the configuration is loaded to validate + # the configuaration objects. Subclasses should implement this + # method and add any errors to the `errors` object given. + # + # @param [ErrorRecorder] errors + def validate(errors); end + + # Converts the configuration to a raw hash by calling `#to_hash` + # on all instance variables (if it can) and putting them into + # a hash. def to_hash instance_variables_hash.inject({}) do |acc, data| k,v = data @@ -28,11 +37,15 @@ module Vagrant end end + # Converts to JSON, with the `json_class` field set so that when + # the JSON is parsed back, it can be loaded back into the proper class. + # See {json_create}. def to_json(*a) result = { 'json_class' => self.class.name } result.merge(instance_variables_hash).to_json(*a) end + # Returns the instance variables as a hash of key-value pairs. def instance_variables_hash instance_variables.inject({}) do |acc, iv| acc[iv.to_s[1..-1]] = instance_variable_get(iv) unless iv.to_sym == :@env diff --git a/lib/vagrant/config/error_recorder.rb b/lib/vagrant/config/error_recorder.rb new file mode 100644 index 000000000..ae8991e3d --- /dev/null +++ b/lib/vagrant/config/error_recorder.rb @@ -0,0 +1,21 @@ +module Vagrant + class Config + # A class which is passed into the various {Base#validate} methods and + # can be used as a helper to add error messages about a single config + # class. + class ErrorRecorder + attr_reader :errors + + def initialize + @errors = [] + end + + # Adds an error to the list of errors. The message key must be a key + # to an I18n translatable error message. Opts can be specified as + # interpolation variables for the message. + def add(message_key, opts=nil) + @errors << I18n.t(message_key, opts) + end + end + end +end diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index ed0f9c8de..23846a999 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -94,6 +94,11 @@ module Vagrant error_key(:cli_missing_env) end + class ConfigValidationFailed < VagrantError + status_code(42) + error_key(:config_validation) + end + class DownloaderFileDoesntExist < VagrantError status_code(37) error_key(:file_missing, "vagrant.downloaders.file") diff --git a/templates/config/validation_failed.erb b/templates/config/validation_failed.erb new file mode 100644 index 000000000..3e8d27b09 --- /dev/null +++ b/templates/config/validation_failed.erb @@ -0,0 +1,7 @@ +<% errors.each do |key, container| -%> +<%= key %>: +<% container.errors.each do |error| -%> +* <%= error %> +<% end -%> + +<% end -%> diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 743b45045..818b2d8b5 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -8,6 +8,11 @@ en: base_vm_not_found: The base VM with the name '%{name}' was not found. box_not_found: Box '%{name}' could not be found. cli_missing_env: This command requires that a Vagrant environment be properly passed in as the last parameter. + config_validation: |- + There was a problem with the configuration of Vagrant. The error messages + or messages are printed below: + + %{messages} interrupted: Vagrant exited after cleanup due to external interrupt. multi_vm_required: A multi-vm environment is required for name specification to this command. multi_vm_target_required: `vagrant %{command}` requires a specific VM name to target in a multi-VM environment. @@ -77,6 +82,13 @@ en: vm_creation_required: VM must be created before running this command. Run `vagrant up` first. vm_not_found: A VM by the name of %{name} was not found. +#------------------------------------------------------------------------------- +# Translations for config validation errors +#------------------------------------------------------------------------------- + config: + base: + foo: UH OH FOO! + #------------------------------------------------------------------------------- # Translations for commands. e.g. `vagrant x` #------------------------------------------------------------------------------- diff --git a/test/vagrant/config/error_recorder_test.rb b/test/vagrant/config/error_recorder_test.rb new file mode 100644 index 000000000..90be44ce7 --- /dev/null +++ b/test/vagrant/config/error_recorder_test.rb @@ -0,0 +1,24 @@ +require "test_helper" + +class ConfigErrorsTest < Test::Unit::TestCase + setup do + @klass = Vagrant::Config::ErrorRecorder + @instance = @klass.new + end + + should "not have any errors to start" do + assert @instance.errors.empty? + end + + should "add errors" do + key = "vagrant.test.errors.test_key" + @instance.add(key) + assert_equal I18n.t(key), @instance.errors.first + end + + should "interpolate error messages if options given" do + key = "vagrant.test.errors.test_key_with_interpolation" + @instance.add(key, :key => "hey") + assert_equal I18n.t(key, :key => "hey"), @instance.errors.first + end +end diff --git a/test/vagrant/config_test.rb b/test/vagrant/config_test.rb index 1163b0e0f..21d5dcbff 100644 --- a/test/vagrant/config_test.rb +++ b/test/vagrant/config_test.rb @@ -120,7 +120,9 @@ class ConfigTest < Test::Unit::TestCase end should "run the proc stack with the config when execute is called" do - @klass.expects(:run_procs!).with(@klass.config).once + seq = sequence('seq') + @klass.expects(:run_procs!).with(@klass.config).once.in_sequence(seq) + @klass.config.expects(:validate!).once.in_sequence(seq) @klass.execute! end @@ -188,5 +190,29 @@ class ConfigTest < Test::Unit::TestCase assert_equal instance, config.send(key) end end + + context "validation" do + should "do nothing if no errors are added" do + valid_class = Class.new(@klass::Base) + @klass::Top.configures(:subconfig, valid_class) + instance = @klass::Top.new + assert_nothing_raised { instance.validate! } + end + + should "raise an exception if there are errors" do + invalid_class = Class.new(@klass::Base) do + def validate(errors) + errors.add("vagrant.test.errors.test_key") + end + end + + @klass::Top.configures(:subconfig, invalid_class) + instance = @klass::Top.new + + assert_raises(Vagrant::Errors::ConfigValidationFailed) { + instance.validate! + } + end + end end end