From e828719c2f19b7a63d93e4ecf931e63bd0e76607 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Wed, 7 Jan 2015 11:43:14 -0500 Subject: [PATCH 1/5] Add logging to vagrant-login --- plugins/commands/login/client.rb | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/plugins/commands/login/client.rb b/plugins/commands/login/client.rb index 64a8dd1c5..63b01f335 100644 --- a/plugins/commands/login/client.rb +++ b/plugins/commands/login/client.rb @@ -7,11 +7,13 @@ module VagrantPlugins # # @param [Vagrant::Environment] env def initialize(env) - @env = env + @logger = Log4r::Logger.new("vagrant::login::client") + @env = env end # Removes the token, effectively logging the user out. def clear_token + @logger.info("Clearing token") token_path.delete if token_path.file? end @@ -38,6 +40,8 @@ module VagrantPlugins # @param [String] pass # @return [String] token The access token, or nil if auth failed. def login(user, pass) + @logger.info("Logging in '#{user}'") + with_error_handling do url = "#{Vagrant.server_url}/api/v1/authenticate" request = { "user" => { "login" => user, "password" => pass } } @@ -52,9 +56,12 @@ module VagrantPlugins # # @param [String] token def store_token(token) + @logger.info("Storing token in #{token_path}") + token_path.open("w") do |f| f.write(token) end + nil end @@ -65,13 +72,17 @@ module VagrantPlugins # @return [String] def token if ENV["ATLAS_TOKEN"] && !ENV["ATLAS_TOKEN"].empty? + @logger.debug("Using authentication token from environment variable") return ENV["ATLAS_TOKEN"] end if token_path.exist? + @logger.debug("Using authentication token from disk at #{token_path}") return token_path.read.strip end + @logger.debug("No authentication token in environment or #{token_path}") + nil end @@ -80,8 +91,13 @@ module VagrantPlugins def with_error_handling(&block) yield rescue RestClient::Unauthorized + @logger.debug("Unauthorized!") false rescue RestClient::NotAcceptable => e + @logger.debug("Got unacceptable response:") + @logger.debug(e.message) + @logger.debug(e.backtrace.join("\n")) + begin errors = JSON.parse(e.response)["errors"].join("\n") raise Errors::ServerError, errors: errors @@ -89,6 +105,7 @@ module VagrantPlugins raise "An unexpected error occurred: #{e.inspect}" rescue SocketError + @logger.info("Socket error") raise Errors::ServerUnreachable, url: Vagrant.server_url.to_s end From 6b51526ba2ed4566a8f515a30886e1ecc14c7c64 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Wed, 7 Jan 2015 11:43:26 -0500 Subject: [PATCH 2/5] Validate push configuration in the environment --- lib/vagrant/environment.rb | 6 ++++++ plugins/commands/push/command.rb | 5 ----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index b57092b03..7dca5cdb0 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -556,6 +556,7 @@ module Vagrant end strategy, config = pushes[name] + push_registry = Vagrant.plugin("2").manager.pushes klass, _ = push_registry.get(strategy) if klass.nil? @@ -564,6 +565,11 @@ module Vagrant pushes: push_registry.keys end + # Validate the global push configuration and our local push configuration + machine = self.machine(self.machine_names.first, self.default_provider) + machine.action_raw(:config_validate, Vagrant::Action::Builtin::ConfigValidate) + config.validate(machine) + klass.new(self, config).push end diff --git a/plugins/commands/push/command.rb b/plugins/commands/push/command.rb index 902a0c946..dabe74bb6 100644 --- a/plugins/commands/push/command.rb +++ b/plugins/commands/push/command.rb @@ -19,11 +19,6 @@ module VagrantPlugins name = validate_pushes!(@env.pushes, argv[0]) - # Validate the configuration - @env.machine(@env.machine_names.first, @env.default_provider).action_raw( - :config_validate, - Vagrant::Action::Builtin::ConfigValidate) - @logger.debug("'push' environment with strategy: `#{name}'") @env.push(name) From c4eb0261bbd845f6a7d118c418ec108a714c1fef Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Wed, 7 Jan 2015 13:20:53 -0500 Subject: [PATCH 3/5] Add tests for validating push configuration --- .../plugins/commands/push/command_test.rb | 22 ++------ test/unit/vagrant/environment_test.rb | 56 ++++++++++++++++++- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/test/unit/plugins/commands/push/command_test.rb b/test/unit/plugins/commands/push/command_test.rb index 0f649c6c4..06995d51d 100644 --- a/test/unit/plugins/commands/push/command_test.rb +++ b/test/unit/plugins/commands/push/command_test.rb @@ -6,14 +6,11 @@ describe VagrantPlugins::CommandPush::Command do include_context "unit" include_context "command plugin helpers" - let(:iso_env) { isolated_environment } let(:env) do - iso_env.vagrantfile(<<-VF) -Vagrant.configure("2") do |config| - config.vm.box = "nope" -end -VF - iso_env.create_vagrant_env + isolated_environment.tap do |env| + env.vagrantfile("") + env.create_vagrant_env + end end let(:argv) { [] } @@ -38,17 +35,6 @@ VF subject.execute end - it "validates the configuration" do - iso_env.vagrantfile("") - - subject = described_class.new(argv, iso_env.create_vagrant_env) - allow(subject).to receive(:validate_pushes!) - .and_return(:noop) - - expect { subject.execute }.to raise_error( - Vagrant::Errors::ConfigInvalid) - end - it "delegates to Environment#push" do expect(env).to receive(:push).once subject.execute diff --git a/test/unit/vagrant/environment_test.rb b/test/unit/vagrant/environment_test.rb index 02effd7db..84d7975d7 100644 --- a/test/unit/vagrant/environment_test.rb +++ b/test/unit/vagrant/environment_test.rb @@ -991,7 +991,7 @@ VF end def push - !!self.class.class_variable_set(:@@pushed, true) + self.class.class_variable_set(:@@pushed, true) end end end @@ -1005,6 +1005,7 @@ VF environment = isolated_environment do |env| env.vagrantfile(<<-VF.gsub(/^ {10}/, '')) Vagrant.configure("2") do |config| + config.vm.box = "hashicorp/precise64" config.push.define "lolwatbacon" end VF @@ -1015,6 +1016,58 @@ VF .to raise_error(Vagrant::Errors::PushStrategyNotLoaded) end + it "runs global config validation" do + environment = isolated_environment do |env| + env.vagrantfile(<<-VF.gsub(/^ {10}/, '')) + Vagrant.configure("2") do |config| + config.push.define "noop" + end + VF + end + + env = environment.create_vagrant_env + expect { env.push("noop") } + .to raise_error(Vagrant::Errors::ConfigInvalid) + end + + it "runs push config validations" do + config_class = Class.new(Vagrant.plugin("2", :config)) do + def self.validated? + !!class_variable_get(:@@validated) + end + + def validate(machine) + self.class.class_variable_set(:@@validated, true) + {} + end + end + + register_plugin("2") do |plugin| + plugin.name "foo" + + plugin.config(:foo, :push) do + config_class + end + + plugin.push(:foo) do + push_class + end + end + + environment = isolated_environment do |env| + env.vagrantfile(<<-VF.gsub(/^ {10}/, '')) + Vagrant.configure("2") do |config| + config.vm.box = "hashicorp/precise64" + config.push.define "foo" + end + VF + end + + env = environment.create_vagrant_env + env.push("foo") + expect(config_class.validated?).to be(true) + end + it "executes the push action" do register_plugin("2") do |plugin| plugin.name "foo" @@ -1027,6 +1080,7 @@ VF environment = isolated_environment do |env| env.vagrantfile(<<-VF.gsub(/^ {10}/, '')) Vagrant.configure("2") do |config| + config.vm.box = "hashicorp/precise64" config.push.define "foo" end VF From 39233e802fdec8e9181985cc608d7d5a154e8f06 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Wed, 7 Jan 2015 15:51:20 -0500 Subject: [PATCH 4/5] Validate pushes in the global config --- lib/vagrant/environment.rb | 6 --- plugins/commands/push/command.rb | 5 ++ plugins/kernel_v2/config/push.rb | 16 ++++++ .../plugins/commands/push/command_test.rb | 31 +++++++++-- test/unit/vagrant/environment_test.rb | 54 ------------------- 5 files changed, 48 insertions(+), 64 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 7dca5cdb0..b57092b03 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -556,7 +556,6 @@ module Vagrant end strategy, config = pushes[name] - push_registry = Vagrant.plugin("2").manager.pushes klass, _ = push_registry.get(strategy) if klass.nil? @@ -565,11 +564,6 @@ module Vagrant pushes: push_registry.keys end - # Validate the global push configuration and our local push configuration - machine = self.machine(self.machine_names.first, self.default_provider) - machine.action_raw(:config_validate, Vagrant::Action::Builtin::ConfigValidate) - config.validate(machine) - klass.new(self, config).push end diff --git a/plugins/commands/push/command.rb b/plugins/commands/push/command.rb index dabe74bb6..902a0c946 100644 --- a/plugins/commands/push/command.rb +++ b/plugins/commands/push/command.rb @@ -19,6 +19,11 @@ module VagrantPlugins name = validate_pushes!(@env.pushes, argv[0]) + # Validate the configuration + @env.machine(@env.machine_names.first, @env.default_provider).action_raw( + :config_validate, + Vagrant::Action::Builtin::ConfigValidate) + @logger.debug("'push' environment with strategy: `#{name}'") @env.push(name) diff --git a/plugins/kernel_v2/config/push.rb b/plugins/kernel_v2/config/push.rb index e1cb95734..ccba3826d 100644 --- a/plugins/kernel_v2/config/push.rb +++ b/plugins/kernel_v2/config/push.rb @@ -115,6 +115,22 @@ module VagrantPlugins end end + # Validate all pushes + def validate(machine) + errors = { "push" => _detected_errors } + + __compiled_pushes.each do |_, push| + config = push[1] + push_errors = config.validate(machine) + + if push_errors + errors = Vagrant::Config::V2::Util.merge_errors(errors, push_errors) + end + end + + errors + end + # This returns the list of compiled pushes as a hash by name. # # @return [Hash>] diff --git a/test/unit/plugins/commands/push/command_test.rb b/test/unit/plugins/commands/push/command_test.rb index 06995d51d..a97f4caba 100644 --- a/test/unit/plugins/commands/push/command_test.rb +++ b/test/unit/plugins/commands/push/command_test.rb @@ -6,11 +6,14 @@ describe VagrantPlugins::CommandPush::Command do include_context "unit" include_context "command plugin helpers" + let(:iso_env) { isolated_environment } let(:env) do - isolated_environment.tap do |env| - env.vagrantfile("") - env.create_vagrant_env - end + iso_env.vagrantfile(<<-VF) + Vagrant.configure("2") do |config| + config.vm.box = "hashicorp/precise64" + end + VF + iso_env.create_vagrant_env end let(:argv) { [] } @@ -39,6 +42,26 @@ describe VagrantPlugins::CommandPush::Command do expect(env).to receive(:push).once subject.execute end + + it "validates the configuration" do + iso_env.vagrantfile <<-EOH + Vagrant.configure("2") do |config| + config.vm.box = "hashicorp/precise64" + + config.push.define "noop" do |push| + push.bad = "ham" + end + end + EOH + + subject = described_class.new(argv, iso_env.create_vagrant_env) + allow(subject).to receive(:validate_pushes!) + .and_return(:noop) + + expect { subject.execute }.to raise_error(Vagrant::Errors::ConfigInvalid) { |err| + expect(err.message).to include("The following settings shouldn't exist: bad") + } + end end describe "#validate_pushes!" do diff --git a/test/unit/vagrant/environment_test.rb b/test/unit/vagrant/environment_test.rb index 84d7975d7..c5cf8370d 100644 --- a/test/unit/vagrant/environment_test.rb +++ b/test/unit/vagrant/environment_test.rb @@ -1005,7 +1005,6 @@ VF environment = isolated_environment do |env| env.vagrantfile(<<-VF.gsub(/^ {10}/, '')) Vagrant.configure("2") do |config| - config.vm.box = "hashicorp/precise64" config.push.define "lolwatbacon" end VF @@ -1016,58 +1015,6 @@ VF .to raise_error(Vagrant::Errors::PushStrategyNotLoaded) end - it "runs global config validation" do - environment = isolated_environment do |env| - env.vagrantfile(<<-VF.gsub(/^ {10}/, '')) - Vagrant.configure("2") do |config| - config.push.define "noop" - end - VF - end - - env = environment.create_vagrant_env - expect { env.push("noop") } - .to raise_error(Vagrant::Errors::ConfigInvalid) - end - - it "runs push config validations" do - config_class = Class.new(Vagrant.plugin("2", :config)) do - def self.validated? - !!class_variable_get(:@@validated) - end - - def validate(machine) - self.class.class_variable_set(:@@validated, true) - {} - end - end - - register_plugin("2") do |plugin| - plugin.name "foo" - - plugin.config(:foo, :push) do - config_class - end - - plugin.push(:foo) do - push_class - end - end - - environment = isolated_environment do |env| - env.vagrantfile(<<-VF.gsub(/^ {10}/, '')) - Vagrant.configure("2") do |config| - config.vm.box = "hashicorp/precise64" - config.push.define "foo" - end - VF - end - - env = environment.create_vagrant_env - env.push("foo") - expect(config_class.validated?).to be(true) - end - it "executes the push action" do register_plugin("2") do |plugin| plugin.name "foo" @@ -1080,7 +1027,6 @@ VF environment = isolated_environment do |env| env.vagrantfile(<<-VF.gsub(/^ {10}/, '')) Vagrant.configure("2") do |config| - config.vm.box = "hashicorp/precise64" config.push.define "foo" end VF From 0080629a689f80be773b2b7d8289c7be66319ca3 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Wed, 7 Jan 2015 15:51:55 -0500 Subject: [PATCH 5/5] Remove :focus tag --- test/unit/plugins/provisioners/chef/omnibus_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/plugins/provisioners/chef/omnibus_test.rb b/test/unit/plugins/provisioners/chef/omnibus_test.rb index 3242be290..b053f94bc 100644 --- a/test/unit/plugins/provisioners/chef/omnibus_test.rb +++ b/test/unit/plugins/provisioners/chef/omnibus_test.rb @@ -2,7 +2,7 @@ require_relative "../../../base" require Vagrant.source_root.join("plugins/provisioners/chef/omnibus") -describe VagrantPlugins::Chef::Omnibus, :focus do +describe VagrantPlugins::Chef::Omnibus do let(:prefix) { "curl -sL #{described_class.const_get(:OMNITRUCK)}" } let(:version) { :latest }