Merge pull request #5130 from mitchellh/sethvargo/validate_pushes

Actually validate push configurations

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
 

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<Symbol, Array<Class, Object>>]

test/unit/plugins/commands/push/command_test.rb

@@ -9,10 +9,10 @@ describe VagrantPlugins::CommandPush::Command do
   let(:iso_env) { isolated_environment }
   let(:env) do
     iso_env.vagrantfile(<<-VF)
-Vagrant.configure("2") do |config|
-  config.vm.box = "nope"
-end
-VF
+      Vagrant.configure("2") do |config|
+        config.vm.box = "hashicorp/precise64"
+      end
+    VF
     iso_env.create_vagrant_env
   end
 
@@ -38,20 +38,29 @@ VF
       subject.execute
     end
 
+    it "delegates to Environment#push" do
+      expect(env).to receive(:push).once
+      subject.execute
+    end
+
     it "validates the configuration" do
-      iso_env.vagrantfile("")
+      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)
-    end
-
-    it "delegates to Environment#push" do
-      expect(env).to receive(:push).once
-      subject.execute
+      expect { subject.execute }.to raise_error(Vagrant::Errors::ConfigInvalid) { |err|
+        expect(err.message).to include("The following settings shouldn't exist: bad")
+      }
     end
   end
 

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 }

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