From f2666a9b74f26601111299d03ab9385f33f4daea Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 10 Dec 2011 13:21:45 -0800 Subject: [PATCH] Config validation is back in. --- BRANCH_TODO | 1 - lib/vagrant/action/builtin.rb | 15 +++++++++------ lib/vagrant/action/general/validate.rb | 2 +- lib/vagrant/config/base.rb | 12 ++---------- lib/vagrant/config/ssh.rb | 2 +- lib/vagrant/config/top.rb | 11 +++-------- lib/vagrant/config/vagrant.rb | 2 +- lib/vagrant/config/vm.rb | 6 +++--- lib/vagrant/config/vm/provisioner.rb | 2 +- 9 files changed, 21 insertions(+), 32 deletions(-) diff --git a/BRANCH_TODO b/BRANCH_TODO index a9886b3c1..8c997cf1a 100644 --- a/BRANCH_TODO +++ b/BRANCH_TODO @@ -1,6 +1,5 @@ This is a TODO file for only this branch (config-overhaul) -* Config validation * Only allow one kind of vagrantfile to be loaded (i.e. if both Vagrantfile and vagrantfile exist, throw an error) * Nicer error if can't setup home directory diff --git a/lib/vagrant/action/builtin.rb b/lib/vagrant/action/builtin.rb index ad1922b06..1da12a7d2 100644 --- a/lib/vagrant/action/builtin.rb +++ b/lib/vagrant/action/builtin.rb @@ -8,6 +8,7 @@ module Vagrant # provision - Provisions a running VM registry.register(:provision) do Builder.new do + use General::Validate use VM::CheckAccessible use VM::Provision end @@ -17,6 +18,7 @@ module Vagrant # environment. registry.register(:start) do Builder.new do + use General::Validate use VM::CheckAccessible use VM::CleanMachineFolder use VM::ClearForwardedPorts @@ -37,6 +39,7 @@ module Vagrant # a restart if fails. registry.register(:halt) do Builder.new do + use General::Validate use VM::CheckAccessible use VM::DiscardState use VM::Halt @@ -46,6 +49,7 @@ module Vagrant # suspend - Suspends the VM registry.register(:suspend) do Builder.new do + use General::Validate use VM::CheckAccessible use VM::Suspend end @@ -54,6 +58,7 @@ module Vagrant # resume - Resume a VM registry.register(:resume) do Builder.new do + use General::Validate use VM::CheckAccessible use VM::Resume end @@ -62,6 +67,7 @@ module Vagrant # reload - Halts then restarts the VM registry.register(:reload) do Builder.new do + use General::Validate use VM::CheckAccessible use registry.get(:halt) use registry.get(:start) @@ -71,6 +77,7 @@ module Vagrant # up - Imports, prepares, then starts a fresh VM. registry.register(:up) do Builder.new do + use General::Validate use VM::CheckAccessible use VM::CheckBox use VM::Import @@ -83,6 +90,7 @@ module Vagrant # destroy - Halts, cleans up, and destroys an existing VM registry.register(:destroy) do Builder.new do + use General::Validate use VM::CheckAccessible use registry.get(:halt), :force => true use VM::ProvisionerCleanup @@ -96,6 +104,7 @@ module Vagrant # package - Export and package the VM registry.register(:package) do Builder.new do + use General::Validate use VM::CheckAccessible use registry.get(:halt) use VM::ClearForwardedPorts @@ -129,12 +138,6 @@ module Vagrant use Box::Package end end - - # Other callbacks. There will be more of these in the future. For - # now, these are limited to what are needed internally. -# registry.register(:before_action_run, Builder.new do -# use General::Validate -# end) end end end diff --git a/lib/vagrant/action/general/validate.rb b/lib/vagrant/action/general/validate.rb index ec87d1823..ab2b53aea 100644 --- a/lib/vagrant/action/general/validate.rb +++ b/lib/vagrant/action/general/validate.rb @@ -10,7 +10,7 @@ module Vagrant end def call(env) - @env["config"].validate! if !@env.has_key?("validate") || @env["validate"] + @env[:vm].config.validate!(@env[:vm].env) if !@env.has_key?("validate") || @env["validate"] @app.call(@env) end end diff --git a/lib/vagrant/config/base.rb b/lib/vagrant/config/base.rb index a789f85ac..5a079969d 100644 --- a/lib/vagrant/config/base.rb +++ b/lib/vagrant/config/base.rb @@ -29,14 +29,6 @@ module Vagrant end end - # A helper to access the environment that this configuration is for. - # This is obtained by getting the env from the {Top}. - # - # @return [Vagrant::Envrionment] - def env - top.env - end - # Allows setting options from a hash. By default this simply calls # the `#{key}=` method on the config class with the value, which is # the expected behavior most of the time. @@ -51,7 +43,7 @@ module Vagrant # method and add any errors to the `errors` object given. # # @param [ErrorRecorder] errors - def validate(errors); end + def validate(env, errors); end # Converts the configuration to a raw hash by calling `#to_hash` # on all instance variables (if it can) and putting them into @@ -76,7 +68,7 @@ module Vagrant # 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 [:@env, :@top].include?(iv.to_sym) + acc[iv.to_s[1..-1]] = instance_variable_get(iv) unless iv.to_sym == :@top acc end end diff --git a/lib/vagrant/config/ssh.rb b/lib/vagrant/config/ssh.rb index e9a067d5e..c4ca33ca0 100644 --- a/lib/vagrant/config/ssh.rb +++ b/lib/vagrant/config/ssh.rb @@ -22,7 +22,7 @@ module Vagrant @forward_x11 = false end - def validate(errors) + def validate(env, errors) [:username, :host, :forwarded_port_key, :max_tries, :timeout, :private_key_path].each do |field| errors.add(I18n.t("vagrant.config.common.error_empty", :field => field)) if !instance_variable_get("@#{field}".to_sym) end diff --git a/lib/vagrant/config/top.rb b/lib/vagrant/config/top.rb index ce90d2bdd..b80ed5083 100644 --- a/lib/vagrant/config/top.rb +++ b/lib/vagrant/config/top.rb @@ -9,9 +9,6 @@ module Vagrant class Top < Base @@configures = {} if !defined?(@@configures) - # The environment that this configuration is for. - attr_reader :env - class << self # The list of registered configuration classes as well as the key # they're registered under. @@ -27,14 +24,12 @@ module Vagrant end end - def initialize(env=nil) + def initialize self.class.configures_list.each do |key, klass| config = klass.new config.top = self instance_variable_set("@#{key}".to_sym, config) end - - @env = env end # Validates the configuration classes of this instance and raises an @@ -42,13 +37,13 @@ module Vagrant # class, the method you want to implement is {Base#validate}. This is # the method that checks all the validation, not one which defines # validation rules. - def validate! + def validate!(env) # 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) + send(key.to_sym).validate(env, recorder) container[key.to_sym] = recorder if !recorder.errors.empty? container end diff --git a/lib/vagrant/config/vagrant.rb b/lib/vagrant/config/vagrant.rb index 3b991bd98..4ba66db92 100644 --- a/lib/vagrant/config/vagrant.rb +++ b/lib/vagrant/config/vagrant.rb @@ -7,7 +7,7 @@ module Vagrant attr_accessor :host attr_accessor :ssh_session_cache - def validate(errors) + def validate(env, errors) [:dotfile_name, :host].each do |field| errors.add(I18n.t("vagrant.config.common.error_empty", :field => field)) if !instance_variable_get("@#{field}".to_sym) end diff --git a/lib/vagrant/config/vm.rb b/lib/vagrant/config/vm.rb index 6ec6c5b0b..25ff2c73c 100644 --- a/lib/vagrant/config/vm.rb +++ b/lib/vagrant/config/vm.rb @@ -94,11 +94,11 @@ module Vagrant defined_vms[name].push_proc(&block) end - def validate(errors) + def validate(env, errors) errors.add(I18n.t("vagrant.config.vm.box_missing")) if !box - errors.add(I18n.t("vagrant.config.vm.box_not_found", :name => box)) if box && !box_url && !env.box + errors.add(I18n.t("vagrant.config.vm.box_not_found", :name => box)) if box && !box_url && !env.boxes.find(box) errors.add(I18n.t("vagrant.config.vm.boot_mode_invalid")) if ![:vrdp, :gui].include?(boot_mode.to_sym) - errors.add(I18n.t("vagrant.config.vm.base_mac_invalid")) if env.box && !base_mac + errors.add(I18n.t("vagrant.config.vm.base_mac_invalid")) if env.boxes.find(box) && !base_mac shared_folders.each do |name, options| if !File.directory?(File.expand_path(options[:hostpath].to_s, env.root_path)) diff --git a/lib/vagrant/config/vm/provisioner.rb b/lib/vagrant/config/vm/provisioner.rb index 933701787..4052c09c1 100644 --- a/lib/vagrant/config/vm/provisioner.rb +++ b/lib/vagrant/config/vm/provisioner.rb @@ -36,7 +36,7 @@ module Vagrant block.call(@config) if block end - def validate(errors) + def validate(env, errors) if !provisioner # If we don't have a provisioner then the whole thing is invalid. errors.add(I18n.t("vagrant.config.vm.provisioner_not_found", :shortcut => shortcut))