Configuration validation for the most common problems added

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 ("/").

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

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

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

templates/locales/en.yml

@@ -86,8 +86,12 @@ en:
 # Translations for config validation errors
 #-------------------------------------------------------------------------------
     config:
-      base:
+      common:
-        foo: UH OH FOO!
+        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`

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

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],

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")

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