diff --git a/CHANGELOG.md b/CHANGELOG.md index 6701bab89..66e98b4c2 100644 --- a/CHANGELOG.md +++ b/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] diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index e22a4f9b0..fe884d272 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/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 diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 3387ca142..9c2d7fe84 100644 --- a/plugins/communicators/winrm/config.rb +++ b/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 diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 656a8b670..f55c8fb75 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/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 } }" - execute_shell(command, :powershell, &block) + command += "\r\nif ($?) { exit 0 } else { if($LASTEXITCODE) { exit $LASTEXITCODE } else { exit 1 } }" + 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) - raise Errors::WinRMInvalidShell, shell: shell unless [:powershell, :cmd, :wql].include?(shell) - - begin - execute_shell_with_retry(command, shell, &block) - rescue => e - raise_winrm_exception(e, shell, command) - end + def execute_with_rescue(method, command, &block) + handle_output(method, command, &block) + rescue => e + raise_winrm_exception(e, method.name, command) end - def execute_shell_with_retry(command, shell, &block) - retryable(tries: @config.max_tries, on: @@exceptions_to_retry_on, sleep: @config.retry_delay) do - @logger.debug("#{shell} executing:\n#{command}") - output = session.send(shell, command) do |out, err| - block.call(:stdout, out) if block_given? && out - block.call(:stderr, err) if block_given? && err - end + def handle_output(execute_method, command, &block) + output = execute_method.call(command) do |out, err| + block.call(:stdout, out) if block_given? && out + 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 - # set the exit code to 1. Parse errors return exit code 0 so we - # need to do this. - if output[:exitcode] == 0 - (output[:data] || []).each do |data| - next if !data[:stderr] - if data[:stderr].include?("ParserError") - @logger.warn("Detected ParserError, setting exit code to 1") - output[:exitcode] = 1 - break - end + # 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 + # need to do this. + if output[:exitcode] == 0 + (output[:data] || []).each do |data| + next if !data[:stderr] + if data[:stderr].include?("ParserError") + @logger.warn("Detected ParserError, setting exit code to 1") + output[:exitcode] = 1 + break 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, - no_ssl_peer_verification: !@config.ssl_peer_verification } + basic_auth_only: @config.basic_auth_only, + no_ssl_peer_verification: !@config.ssl_peer_verification, + retry_delay: @config.retry_delay, + retry_limit: @config.max_tries } end end #WinShell class end diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index de7b1dc19..5cf1f6810 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/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 diff --git a/vagrant.gemspec b/vagrant.gemspec index 158f55418..f600731c6 100644 --- a/vagrant.gemspec +++ b/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-fs", "~> 0.2.2" + s.add_dependency "winrm", "~> 1.6" + s.add_dependency "winrm-fs", "~> 0.3.0" # We lock this down to avoid compilation issues. s.add_dependency "nokogiri", "= 1.6.7.1" diff --git a/website/source/docs/vagrantfile/winrm_settings.html.md b/website/source/docs/vagrantfile/winrm_settings.html.md index 80d7c806a..046d1854f 100644 --- a/website/source/docs/vagrantfile/winrm_settings.html.md +++ b/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.