From 23c08f2daa732812743aff1cb94d6af5ac6ff194 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 5 Sep 2010 11:26:38 -0700 Subject: [PATCH] Configuration validation for the most common problems added --- CHANGELOG.md | 2 ++ lib/vagrant/config/ssh.rb | 8 ++++++++ lib/vagrant/config/vagrant.rb | 8 +++++++- lib/vagrant/config/vm.rb | 10 ++++++++++ templates/locales/en.yml | 8 ++++++-- test/test_helper.rb | 2 +- test/vagrant/action/vm/share_folders_test.rb | 8 ++++---- test/vagrant/config/vagrant_test.rb | 5 ----- test/vagrant/config_test.rb | 5 +++++ 9 files changed, 43 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32d9d793a..94d8fb1fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## 0.6.0 (unreleased) + - Remove `config.ssh.password`. It hasn't been used for a few versions + now and was only kept around to avoid exceptions in Vagrantfiles. - Configuration is now validated so improper input can be found in Vagrantfiles. - Fixed issue with not detecting Vagrantfile at root directory ("/"). diff --git a/lib/vagrant/config/ssh.rb b/lib/vagrant/config/ssh.rb index a694f3a5c..fc0a3f3d3 100644 --- a/lib/vagrant/config/ssh.rb +++ b/lib/vagrant/config/ssh.rb @@ -15,6 +15,14 @@ module Vagrant def private_key_path File.expand_path(@private_key_path, env.root_path) end + + def validate(errors) + [:username, :host, :port, :forwarded_port_key, :max_tries, :timeout, :private_key_path].each do |field| + errors.add("vagrant.config.common.error_empty", :field => field) if !instance_variable_get("@#{field}".to_sym) + end + + errors.add("vagrant.config.ssh.private_key_missing", :path => private_key_path) if !File.file?(private_key_path) + end end end end diff --git a/lib/vagrant/config/vagrant.rb b/lib/vagrant/config/vagrant.rb index 79ccfa35a..5fa56bf3d 100644 --- a/lib/vagrant/config/vagrant.rb +++ b/lib/vagrant/config/vagrant.rb @@ -13,7 +13,13 @@ module Vagrant end def home - @home ? File.expand_path(@home) : nil + File.expand_path(@home) + end + + def validate(errors) + [:dotfile_name, :home, :host].each do |field| + errors.add("vagrant.config.common.error_empty", :field => field) if !instance_variable_get("@#{field}".to_sym) + end end end end diff --git a/lib/vagrant/config/vm.rb b/lib/vagrant/config/vm.rb index 64758aae1..f54aef258 100644 --- a/lib/vagrant/config/vm.rb +++ b/lib/vagrant/config/vm.rb @@ -94,6 +94,16 @@ module Vagrant defined_vms[name.to_sym].options.merge!(options) defined_vms[name.to_sym].push_proc(&block) end + + def validate(errors) + shared_folders.each do |name, options| + if !File.directory?(File.expand_path(options[:hostpath], env.root_path)) + errors.add("vagrant.config.vm.shared_folder_hostpath_missing", + :name => name, + :path => options[:hostpath]) + end + end + end end end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 818b2d8b5..351a035f6 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -86,8 +86,12 @@ en: # Translations for config validation errors #------------------------------------------------------------------------------- config: - base: - foo: UH OH FOO! + common: + error_empty: `%{field}` must be filled in. + ssh: + private_key_missing: "`private_key_path` file must exist: %{path}" + vm: + shared_folder_hostpath_missing: "Shared folder host path for '%{name}' doesn't exist: %{path}" #------------------------------------------------------------------------------- # Translations for commands. e.g. `vagrant x` diff --git a/test/test_helper.rb b/test/test_helper.rb index 07dbb7f26..122005ead 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -37,7 +37,7 @@ class Test::Unit::TestCase config.ssh.forwarded_port_key = "ssh" config.ssh.max_tries = 10 config.ssh.timeout = 10 - config.ssh.private_key_path = '~/foo' + config.ssh.private_key_path = File.expand_path("keys/vagrant", Vagrant.source_root) config.vm.box = "foo" config.vm.box_url = nil diff --git a/test/vagrant/action/vm/share_folders_test.rb b/test/vagrant/action/vm/share_folders_test.rb index 1e570a354..d5bc5e832 100644 --- a/test/vagrant/action/vm/share_folders_test.rb +++ b/test/vagrant/action/vm/share_folders_test.rb @@ -14,6 +14,10 @@ class ShareFoldersVMActionTest < Test::Unit::TestCase @internal_vm = mock("internal") @vm.stubs(:vm).returns(@internal_vm) + # No validation for this test since its a nightmare due to all the + # nonexistent shared folders. + Vagrant::Config::Top.any_instance.stubs(:validate!) + @instance = @klass.new(@app, @env) end @@ -47,10 +51,6 @@ class ShareFoldersVMActionTest < Test::Unit::TestCase end context "collecting shared folders" do - setup do - File.stubs(:expand_path).returns("baz") - end - should "return a hash of the shared folders" do data = { "foo" => %W[bar baz], diff --git a/test/vagrant/config/vagrant_test.rb b/test/vagrant/config/vagrant_test.rb index 87da78ebe..c9feafe0e 100644 --- a/test/vagrant/config/vagrant_test.rb +++ b/test/vagrant/config/vagrant_test.rb @@ -5,11 +5,6 @@ class ConfigVagrantTest < Test::Unit::TestCase @config = Vagrant::Config::VagrantConfig.new end - should "return nil if home is nil" do - File.expects(:expand_path).never - assert @config.home.nil? - end - should "expand the path if home is not nil" do @config.home = "foo" File.expects(:expand_path).with("foo").once.returns("result") diff --git a/test/vagrant/config_test.rb b/test/vagrant/config_test.rb index 21d5dcbff..519a1eb0e 100644 --- a/test/vagrant/config_test.rb +++ b/test/vagrant/config_test.rb @@ -9,6 +9,9 @@ class ConfigTest < Test::Unit::TestCase setup do @env = mock_environment @instance = @klass.new(@env) + + # Don't want validation to occur for these tests + @klass::Top.any_instance.stubs(:validate!) end should "initially have an empty queue" do @@ -80,6 +83,7 @@ class ConfigTest < Test::Unit::TestCase context "resetting" do setup do + @klass::Top.any_instance.stubs(:validate!) @klass.run { |config| } @klass.execute! end @@ -111,6 +115,7 @@ class ConfigTest < Test::Unit::TestCase context "initializing" do setup do @klass.reset! + @klass::Top.any_instance.stubs(:validate!) end should "add the given block to the proc stack" do