Merge pull request #6922 from mwrock/negotiate

upgrade to latest winrm and winrm-fs versions

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Next Version (unreleased)
 
+BREAKING CHANGES:
+
+  - The `winrm` communicator now shares the same upload behavior as the `ssh` cummunicator. This change should have no impact to most vagrant operations but may break behavior when uploading directories to an existing destination target. The `file` provisioner should be the only builtin provisioner affected by this change. When uploading a directory and the destination directory exists on the endpoint, the source base directory will be created below the destination directory on the endpoint and the source directory contents will be unzipped to that location. Prior to this release, the contents of the source directory would be unzipped to an existing destination directory without creating the source base directory. This new behavior is more consistent with SCP and other well known shell copy commands.
+
 IMPROVEMENTS:
 
   - provisioners/chef: Add the ability to install on SUSE [GH-6806]

plugins/communicators/winrm/communicator.rb

@@ -142,6 +142,7 @@ module VagrantPlugins
 
         opts[:good_exit] = Array(opts[:good_exit])
         command = wrap_in_scheduled_task(command, opts[:interactive]) if opts[:elevated]
+        @logger.debug("#{opts[:shell]} executing:\n#{command}")
         output = shell.send(opts[:shell], command, &block)
         execution_output(output, opts)
       end

plugins/communicators/winrm/config.rb

@@ -12,6 +12,7 @@ module VagrantPlugins
       attr_accessor :transport
       attr_accessor :ssl_peer_verification
       attr_accessor :execution_time_limit
+      attr_accessor :basic_auth_only
 
       def initialize
         @username               = UNSET_VALUE
@@ -25,12 +26,13 @@ module VagrantPlugins
         @transport              = UNSET_VALUE
         @ssl_peer_verification  = UNSET_VALUE
         @execution_time_limit   = UNSET_VALUE
+        @basic_auth_only        = UNSET_VALUE
       end
 
       def finalize!
         @username = "vagrant"   if @username == UNSET_VALUE
         @password = "vagrant"   if @password == UNSET_VALUE
-        @transport = :plaintext if @transport == UNSET_VALUE
+        @transport = :negotiate if @transport == UNSET_VALUE
         @host = nil           if @host == UNSET_VALUE
         is_ssl = @transport == :ssl
         @port = (is_ssl ? 5986 : 5985)       if @port == UNSET_VALUE
@@ -40,6 +42,7 @@ module VagrantPlugins
         @timeout = 1800        if @timeout == UNSET_VALUE
         @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE
         @execution_time_limit = "PT2H"   if @execution_time_limit == UNSET_VALUE
+        @basic_auth_only = false    if @basic_auth_only == UNSET_VALUE
       end
 
       def validate(machine)
@@ -56,6 +59,9 @@ module VagrantPlugins
         unless @ssl_peer_verification == true || @ssl_peer_verification == false
           errors << "winrm.ssl_peer_verification must be a boolean."
         end
+        unless @basic_auth_only == true || @basic_auth_only == false
+          errors << "winrm.basic_auth_only must be a boolean."
+        end
 
         { "WinRM" => errors }
       end

plugins/communicators/winrm/shell.rb

@@ -54,20 +54,21 @@ module VagrantPlugins
       end
 
       def powershell(command, &block)
-        # Suppress the progress stream from leaking to stderr
-        command = "$ProgressPreference='SilentlyContinue';\r\n" + command
-        command << "\r\n"
         # Ensure an exit code
-        command << "if ($?) { exit 0 } else { if($LASTEXITCODE) { exit $LASTEXITCODE } else { exit 1 } }"
+        command += "\r\nif ($?) { exit 0 } else { if($LASTEXITCODE) { exit $LASTEXITCODE } else { exit 1 } }"
-        execute_shell(command, :powershell, &block)
+        execute_with_rescue(executor.method("run_powershell_script"), command, &block)
       end
 
       def cmd(command, &block)
-        execute_shell(command, :cmd, &block)
+        execute_with_rescue(executor.method("run_cmd"), command, &block)
       end
 
       def wql(query, &block)
-        execute_shell(query, :wql, &block)
+        retryable(tries: @config.max_tries, on: @@exceptions_to_retry_on, sleep: @config.retry_delay) do
+          handle_output(session.method("run_wql"), query, &block)
+        end
+      rescue => e
+        raise_winrm_exception(e, "run_wql", query)
       end
 
       def upload(from, to)
@@ -82,42 +83,35 @@ module VagrantPlugins
 
       protected
 
-      def execute_shell(command, shell=:powershell, &block)
+      def execute_with_rescue(method, command, &block)
-        raise Errors::WinRMInvalidShell, shell: shell unless [:powershell, :cmd, :wql].include?(shell)
+        handle_output(method, command, &block)
-
+      rescue => e
-        begin
+        raise_winrm_exception(e, method.name, command)
-          execute_shell_with_retry(command, shell, &block)
-        rescue => e
-          raise_winrm_exception(e, shell, command)
-        end
       end
 
-      def execute_shell_with_retry(command, shell, &block)
+      def handle_output(execute_method, command, &block)
-        retryable(tries: @config.max_tries, on: @@exceptions_to_retry_on, sleep: @config.retry_delay) do
+        output = execute_method.call(command) do |out, err|
-          @logger.debug("#{shell} executing:\n#{command}")
+          block.call(:stdout, out) if block_given? && out
-          output = session.send(shell, command) do |out, err|
+          block.call(:stderr, err) if block_given? && err
-            block.call(:stdout, out) if block_given? && out
+        end
-            block.call(:stderr, err) if block_given? && err
-          end
 
-          @logger.debug("Output: #{output.inspect}")
+        @logger.debug("Output: #{output.inspect}")
 
-          # Verify that we didn't get a parser error, and if so we should
+        # Verify that we didn't get a parser error, and if so we should
-          # set the exit code to 1. Parse errors return exit code 0 so we
+        # set the exit code to 1. Parse errors return exit code 0 so we
-          # need to do this.
+        # need to do this.
-          if output[:exitcode] == 0
+        if output[:exitcode] == 0
-            (output[:data] || []).each do |data|
+          (output[:data] || []).each do |data|
-              next if !data[:stderr]
+            next if !data[:stderr]
-              if data[:stderr].include?("ParserError")
+            if data[:stderr].include?("ParserError")
-                @logger.warn("Detected ParserError, setting exit code to 1")
+              @logger.warn("Detected ParserError, setting exit code to 1")
-                output[:exitcode] = 1
+              output[:exitcode] = 1
-                break
+              break
-              end
             end
           end
-
-          return output
         end
+
+        return output
       end
 
       def raise_winrm_exception(exception, shell = nil, command = nil)
@@ -170,6 +164,7 @@ module VagrantPlugins
 
         client = ::WinRM::WinRMWebService.new(endpoint, @config.transport.to_sym, endpoint_options)
         client.set_timeout(@config.timeout)
+        client.logger = @logger
         client
       end
 
@@ -177,11 +172,15 @@ module VagrantPlugins
         @session ||= new_session
       end
 
+      def executor
+        @executor ||= session.create_executor
+      end
+
       def endpoint
         case @config.transport.to_sym
         when :ssl
           "https://#{@host}:#{@port}/wsman"
-        when :plaintext
+        when :plaintext, :negotiate
           "http://#{@host}:#{@port}/wsman"
         else
           raise Errors::WinRMInvalidTransport, transport: @config.transport
@@ -193,8 +192,10 @@ module VagrantPlugins
           pass: @password,
           host: @host,
           port: @port,
-          basic_auth_only: true,
+          basic_auth_only: @config.basic_auth_only,
-          no_ssl_peer_verification: !@config.ssl_peer_verification }
+          no_ssl_peer_verification: !@config.ssl_peer_verification,
+          retry_delay: @config.retry_delay,
+          retry_limit: @config.max_tries }
       end
     end #WinShell class
   end

test/unit/plugins/communicators/winrm/shell_test.rb

@@ -6,7 +6,8 @@ require Vagrant.source_root.join("plugins/communicators/winrm/config")
 describe VagrantPlugins::CommunicatorWinRM::WinRMShell do
   include_context "unit"
 
-  let(:session) { double("winrm_session") }
+  let(:session) { double("winrm_session", create_executor: executor) }
+  let(:executor) { double("command_executor") }
   let(:port) { config.transport == :ssl ? 5986 : 5985 }
   let(:config)  {
     VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c|
@@ -14,6 +15,9 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do
       c.password = 'password'
       c.max_tries = 3
       c.retry_delay = 0
+      c.basic_auth_only = false
+      c.retry_delay = 1
+      c.max_tries = 2
       c.finalize!
     end
   }
@@ -26,23 +30,12 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do
 
   describe ".powershell" do
     it "should call winrm powershell" do
-      expect(session).to receive(:powershell).with(/^dir.+/).and_return({ exitcode: 0 })
+      expect(executor).to receive(:run_powershell_script).with(/^dir.+/).and_return({ exitcode: 0 })
       expect(subject.powershell("dir")[:exitcode]).to eq(0)
     end
 
-    it "should retry when a WinRMAuthorizationError is received" do
-      expect(session).to receive(:powershell).with(/^dir.+/).exactly(3).times.and_raise(
-        # Note: The initialize for WinRMAuthorizationError may require a status_code as
-        # the second argument in a future WinRM release. Currently it doesn't track the
-        # status code.
-        WinRM::WinRMAuthorizationError.new("Oh no!! Unauthrorized")
-      )
-      expect { subject.powershell("dir") }.to raise_error(
-        VagrantPlugins::CommunicatorWinRM::Errors::AuthenticationFailed)
-    end
-
     it "should raise an execution error when an exception occurs" do
-      expect(session).to receive(:powershell).with(/^dir.+/).and_raise(
+      expect(executor).to receive(:run_powershell_script).with(/^dir.+/).and_raise(
         StandardError.new("Oh no! a 500 SOAP error!"))
       expect { subject.powershell("dir") }.to raise_error(
         VagrantPlugins::CommunicatorWinRM::Errors::ExecutionError)
@@ -51,11 +44,29 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do
 
   describe ".cmd" do
     it "should call winrm cmd" do
-      expect(session).to receive(:cmd).with("dir").and_return({ exitcode: 0 })
+      expect(executor).to receive(:run_cmd).with("dir").and_return({ exitcode: 0 })
       expect(subject.cmd("dir")[:exitcode]).to eq(0)
     end
   end
 
+  describe ".wql" do
+    it "should call winrm wql" do
+      expect(session).to receive(:run_wql).with("select * from Win32_OperatingSystem").and_return({ exitcode: 0 })
+      expect(subject.wql("select * from Win32_OperatingSystem")[:exitcode]).to eq(0)
+    end
+
+    it "should retry when a WinRMAuthorizationError is received" do
+      expect(session).to receive(:run_wql).with("select * from Win32_OperatingSystem").exactly(2).times.and_raise(
+        # Note: The initialize for WinRMAuthorizationError may require a status_code as
+        # the second argument in a future WinRM release. Currently it doesn't track the
+        # status code.
+        WinRM::WinRMAuthorizationError.new("Oh no!! Unauthrorized")
+      )
+      expect { subject.wql("select * from Win32_OperatingSystem") }.to raise_error(
+        VagrantPlugins::CommunicatorWinRM::Errors::AuthenticationFailed)
+    end
+  end
+
   describe ".endpoint" do
     context 'when transport is :ssl' do
       let(:config)  {
@@ -69,7 +80,19 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do
       end
     end
 
+    context "when transport is :negotiate" do
+      it "should create winrm endpoint address using http" do
+        expect(subject.send(:endpoint)).to eq("http://localhost:5985/wsman")
+      end
+    end
+
     context "when transport is :plaintext" do
+      let(:config)  {
+        VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c|
+          c.transport = :plaintext
+          c.finalize!
+        end
+      }
       it "should create winrm endpoint address using http" do
         expect(subject.send(:endpoint)).to eq("http://localhost:5985/wsman")
       end
@@ -80,7 +103,8 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do
     it "should create endpoint options" do
       expect(subject.send(:endpoint_options)).to eq(
         { user: "username", pass: "password", host: "localhost", port: 5985,
-          basic_auth_only: true, no_ssl_peer_verification: false })
+          basic_auth_only: false, no_ssl_peer_verification: false,
+          retry_delay: 1, retry_limit: 2 })
     end
   end
 

vagrant.gemspec

@@ -29,8 +29,8 @@ Gem::Specification.new do |s|
   s.add_dependency "rb-kqueue", "~> 0.2.0"
   s.add_dependency "rest-client", ">= 1.6.0", "< 2.0"
   s.add_dependency "wdm", "~> 0.1.0"
-  s.add_dependency "winrm", "~> 1.3"
+  s.add_dependency "winrm", "~> 1.6"
-  s.add_dependency "winrm-fs", "~> 0.2.2"
+  s.add_dependency "winrm-fs", "~> 0.3.0"
 
   # We lock this down to avoid compilation issues.
   s.add_dependency "nokogiri", "= 1.6.7.1"

website/source/docs/vagrantfile/winrm_settings.html.md

@@ -51,21 +51,14 @@ to use port 4567 to talk to the guest if there is no other option.
 
 <hr>
 
-`config.winrm.execution_time_limit` - The maximum duration that a WinRM
+`config.winrm.transport` - The transport used for WinRM communication. Valid settings include: `:negotiate`, `ssl`, and `:plaintext`. The default is `:negotiate`.
-task can execute for. This defaults to two hours. The format of this value
-must be in this [Microsoft-documented format](https://msdn.microsoft.com/en-us/library/aa382678.aspx).
 
 <hr>
 
-<strong>Warning:</strong> In order for Vagrant to communicate with a Windows
+`config.winrm.basic_auth_only` - Whether to use Basic Authentication. Defaults to `false`. If set to `true` you should also use the `:plaintext` transport setting and the Windows machine must be confiured appropriately. <strong>Note:</strong> It is strongly recommended that you only use basic authentication for debugging purposes. Credentials will be transferred in plain text.
-guest, you must allow unencrypted WinRM connections on the guest machine
-itself. Some public boxes already have this configured, but if you are
-attempting to `vagrant up` a Windows box and the command hangs at
-`Waiting for WinRM to become available...`, then you will need to run the
-commands below on the guest machine itself, at the box setup stage,
-after provisioning, or through a start up script.
 
-```
+<hr>
-Set-Item WSMan:\localhost\Service\AllowUnencrypted -Value True
+
-Set-Item WSMan:\localhost\Service\Auth\Basic -Value True
+`config.winrm.execution_time_limit` - The maximum duration that a WinRM
-```
+task can execute for. This defaults to two hours. The format of this value
+must be in this [Microsoft-documented format](https://msdn.microsoft.com/en-us/library/aa382678.aspx).