From a99d32f60af94073bf676808c1910756c4c61269 Mon Sep 17 00:00:00 2001 From: Peter Ericson Date: Tue, 3 Jun 2014 13:49:19 +1000 Subject: [PATCH 01/22] Add WinRM over SSL support --- Gemfile | 2 ++ plugins/communicators/winrm/communicator.rb | 1 + plugins/communicators/winrm/shell.rb | 10 +++++++--- vagrant.gemspec | 3 ++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 15f245819..7820e506f 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,8 @@ source "https://rubygems.org" gemspec +# f4ece43: Allow the user to disable SSL peer verification. +gem 'winrm', git: 'https://github.com/WinRb/WinRM.git', ref: 'f4ece4384df2768bc7756c3c5c336db65b05c674' if File.exist?(File.expand_path("../../vagrant-spec", __FILE__)) gem 'vagrant-spec', path: "../vagrant-spec" diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index 7609f16b3..29737f6ac 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/plugins/communicators/winrm/communicator.rb @@ -111,6 +111,7 @@ module VagrantPlugins port: winrm_info[:port], timeout_in_seconds: @machine.config.winrm.timeout, max_tries: @machine.config.winrm.max_tries, + ssl: @machine.config.winrm.ssl, ) end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 353c0e84d..c6d626448 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -38,17 +38,19 @@ module VagrantPlugins attr_reader :port attr_reader :timeout_in_seconds attr_reader :max_tries + attr_reader :ssl def initialize(host, username, password, options = {}) @logger = Log4r::Logger.new("vagrant::communication::winrmshell") @logger.debug("initializing WinRMShell") @host = host - @port = options[:port] || 5985 + @port = options[:port] || options[:ssl] ? 5986 : 5985 @username = username @password = password @timeout_in_seconds = options[:timeout_in_seconds] || 60 @max_tries = options[:max_tries] || 20 + @ssl = options[:ssl] || false end def powershell(command, &block) @@ -119,6 +121,7 @@ module VagrantPlugins @logger.info(" - Host: #{@host}") @logger.info(" - Port: #{@port}") @logger.info(" - Username: #{@username}") + @logger.info(" - SSL: #{@ssl}") client = ::WinRM::WinRMWebService.new(endpoint, :plaintext, endpoint_options) client.set_timeout(@timeout_in_seconds) @@ -131,7 +134,7 @@ module VagrantPlugins end def endpoint - "http://#{@host}:#{@port}/wsman" + "http#{@ssl ? 's' : ''}://#{@host}:#{@port}/wsman" end def endpoint_options @@ -140,7 +143,8 @@ module VagrantPlugins host: @host, port: @port, operation_timeout: @timeout_in_seconds, - basic_auth_only: true } + basic_auth_only: true, + no_ssl_peer_verification: true } end end #WinShell class end diff --git a/vagrant.gemspec b/vagrant.gemspec index 0afe7e766..4c4beebf6 100644 --- a/vagrant.gemspec +++ b/vagrant.gemspec @@ -28,7 +28,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.1.3" + # 1.1.4 (Unreleased) + #s.add_dependency "winrm", "~> 1.1.3" # We lock this down to avoid compilation issues. s.add_dependency "nokogiri", "= 1.6.3.1" From 4455d2401bd708c1025400a5e329f68bcb3b51f7 Mon Sep 17 00:00:00 2001 From: Peter Ericson Date: Tue, 3 Jun 2014 14:03:07 +1000 Subject: [PATCH 02/22] Add ssl option to winrm config.rb --- plugins/communicators/winrm/config.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 155ac0567..c7f13c041 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -8,6 +8,7 @@ module VagrantPlugins attr_accessor :guest_port attr_accessor :max_tries attr_accessor :timeout + attr_accessor :ssl def initialize @username = UNSET_VALUE @@ -17,6 +18,7 @@ module VagrantPlugins @guest_port = UNSET_VALUE @max_tries = UNSET_VALUE @timeout = UNSET_VALUE + @ssl = UNSET_VALUE end def finalize! @@ -27,6 +29,7 @@ module VagrantPlugins @guest_port = 5985 if @guest_port == UNSET_VALUE @max_tries = 20 if @max_tries == UNSET_VALUE @timeout = 1800 if @timeout == UNSET_VALUE + @ssl = false if @ssl == UNSET_VALUE end def validate(machine) From 2f52a9c15ccbd14fe9ac56ce09a39c47efd06146 Mon Sep 17 00:00:00 2001 From: Peter Ericson Date: Tue, 3 Jun 2014 14:03:29 +1000 Subject: [PATCH 03/22] Add winrmssl port forward (port 5986) --- plugins/kernel_v2/config/vm.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index da2440c52..c5a9e1f6c 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -400,9 +400,15 @@ module VagrantPlugins id: "winrm", auto_correct: true end - end - - if !@__networks["forwarded_port-ssh"] + if !@__networks["forwarded_port-winrmssl"] + network :forwarded_port, + guest: 5986, + host: 55986, + host_ip: "127.0.0.1", + id: "winrmssl", + auto_correct: true + end + elsif !@__networks["forwarded_port-ssh"] network :forwarded_port, guest: 22, host: 2222, From 52d8fddf38701dbc3aa1981d0ba38e13aa2cdae9 Mon Sep 17 00:00:00 2001 From: Peter Ericson Date: Tue, 3 Jun 2014 14:19:21 +1000 Subject: [PATCH 04/22] shell.rb: fix precedence --- plugins/communicators/winrm/shell.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index c6d626448..8889819b2 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -45,7 +45,7 @@ module VagrantPlugins @logger.debug("initializing WinRMShell") @host = host - @port = options[:port] || options[:ssl] ? 5986 : 5985 + @port = options[:port] || (options[:ssl] ? 5986 : 5985) @username = username @password = password @timeout_in_seconds = options[:timeout_in_seconds] || 60 From 243cc5dc37183628f07887a6a75d47731cec5b9e Mon Sep 17 00:00:00 2001 From: Peter Ericson Date: Tue, 3 Jun 2014 14:26:40 +1000 Subject: [PATCH 05/22] config.rb: set default port based on @ssl --- plugins/communicators/winrm/config.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index c7f13c041..09ec5fb62 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -25,8 +25,8 @@ module VagrantPlugins @username = "vagrant" if @username == UNSET_VALUE @password = "vagrant" if @password == UNSET_VALUE @host = nil if @host == UNSET_VALUE - @port = 5985 if @port == UNSET_VALUE - @guest_port = 5985 if @guest_port == UNSET_VALUE + @port = (@ssl ? 5986 : 5985) if @port == UNSET_VALUE + @guest_port = (@ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE @max_tries = 20 if @max_tries == UNSET_VALUE @timeout = 1800 if @timeout == UNSET_VALUE @ssl = false if @ssl == UNSET_VALUE From 2b9626f19c54a27fee0608f3504f767529fcc726 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Tue, 22 Jul 2014 17:30:58 -0400 Subject: [PATCH 06/22] Use Winrm 1.2.0 and fix tests --- Gemfile | 2 - .../plugins/communicators/winrm/shell_test.rb | 2 +- test/unit/plugins/kernel_v2/config/vm_test.rb | 41 +++++++------------ vagrant.gemspec | 3 +- 4 files changed, 17 insertions(+), 31 deletions(-) diff --git a/Gemfile b/Gemfile index 7820e506f..15f245819 100644 --- a/Gemfile +++ b/Gemfile @@ -1,8 +1,6 @@ source "https://rubygems.org" gemspec -# f4ece43: Allow the user to disable SSL peer verification. -gem 'winrm', git: 'https://github.com/WinRb/WinRM.git', ref: 'f4ece4384df2768bc7756c3c5c336db65b05c674' if File.exist?(File.expand_path("../../vagrant-spec", __FILE__)) gem 'vagrant-spec', path: "../vagrant-spec" diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 477108b97..3948cf336 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -51,7 +51,7 @@ 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, - operation_timeout: 60, basic_auth_only: true }) + operation_timeout: 60, basic_auth_only: true, no_ssl_peer_verification: true }) end end diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 828753ad0..4683b01b7 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -23,6 +23,13 @@ describe VagrantPlugins::Kernel_V2::VMConfig do end end + def find_network(name) + network_definitions = subject.networks.map do |n| + n[1] + end + network_definitions.find {|n| n[:id] == name} + end + before do env = double("env") env.stub(root_path: nil) @@ -121,28 +128,6 @@ describe VagrantPlugins::Kernel_V2::VMConfig do end end - describe "#define" do - it "should allow regular names" do - subject.define "foo" - subject.finalize! - - assert_valid - end - - [ - "foo [1]", - "bar {2}", - "foo/bar", - ].each do |name| - it "should disallow names with brackets" do - subject.define name - subject.finalize! - - assert_invalid - end - end - end - describe "#guest" do it "is nil by default" do subject.finalize! @@ -183,16 +168,20 @@ describe VagrantPlugins::Kernel_V2::VMConfig do subject.finalize! n = subject.networks expect(n.length).to eq(2) + + # WinRM HTTP expect(n[0][0]).to eq(:forwarded_port) expect(n[0][1][:guest]).to eq(5985) expect(n[0][1][:host]).to eq(55985) expect(n[0][1][:host_ip]).to eq("127.0.0.1") expect(n[0][1][:id]).to eq("winrm") + # WinRM HTTPS expect(n[1][0]).to eq(:forwarded_port) - expect(n[1][1][:guest]).to eq(22) - expect(n[1][1][:host]).to eq(2222) - expect(n[1][1][:id]).to eq("ssh") + expect(n[1][1][:guest]).to eq(5986) + expect(n[1][1][:host]).to eq(55986) + expect(n[1][1][:host_ip]).to eq("127.0.0.1") + expect(n[1][1][:id]).to eq("winrmssl") end it "allows overriding SSH" do @@ -215,7 +204,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do subject.finalize! n = subject.networks - expect(n.length).to eq(2) + expect(n.length).to eq(1) expect(n[0][0]).to eq(:forwarded_port) expect(n[0][1][:guest]).to eq(22) expect(n[0][1][:host]).to eq(14100) diff --git a/vagrant.gemspec b/vagrant.gemspec index 4c4beebf6..4198452c4 100644 --- a/vagrant.gemspec +++ b/vagrant.gemspec @@ -28,8 +28,7 @@ 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" - # 1.1.4 (Unreleased) - #s.add_dependency "winrm", "~> 1.1.3" + s.add_dependency "winrm", "~> 1.2" # We lock this down to avoid compilation issues. s.add_dependency "nokogiri", "= 1.6.3.1" From 1beb221bf371d3269d911a676cbaa49aaa58ddc2 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Tue, 22 Jul 2014 18:28:50 -0400 Subject: [PATCH 07/22] Rename no_ssl_peer_verification to ssl_peer_verification, and make it configurable --- plugins/communicators/winrm/config.rb | 21 ++++++++++++------- plugins/communicators/winrm/shell.rb | 18 +++++++++------- .../plugins/communicators/winrm/shell_test.rb | 2 +- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 09ec5fb62..37259e3b3 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -11,14 +11,15 @@ module VagrantPlugins attr_accessor :ssl def initialize - @username = UNSET_VALUE - @password = UNSET_VALUE - @host = UNSET_VALUE - @port = UNSET_VALUE - @guest_port = UNSET_VALUE - @max_tries = UNSET_VALUE - @timeout = UNSET_VALUE - @ssl = UNSET_VALUE + @username = UNSET_VALUE + @password = UNSET_VALUE + @host = UNSET_VALUE + @port = UNSET_VALUE + @guest_port = UNSET_VALUE + @max_tries = UNSET_VALUE + @timeout = UNSET_VALUE + @ssl = UNSET_VALUE + @ssl_peer_verification = UNSET_VALUE end def finalize! @@ -30,6 +31,7 @@ module VagrantPlugins @max_tries = 20 if @max_tries == UNSET_VALUE @timeout = 1800 if @timeout == UNSET_VALUE @ssl = false if @ssl == UNSET_VALUE + @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE end def validate(machine) @@ -41,6 +43,9 @@ module VagrantPlugins errors << "winrm.guest_port cannot be nil." if @guest_port.nil? errors << "winrm.max_tries cannot be nil." if @max_tries.nil? errors << "winrm.timeout cannot be nil." if @timeout.nil? + unless @ssl_peer_verification == true || @ssl_peer_verification == false + errors << "winrm.ssl_peer_verification must be a boolean." + end { "WinRM" => errors } end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 8889819b2..f851d3805 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -39,18 +39,20 @@ module VagrantPlugins attr_reader :timeout_in_seconds attr_reader :max_tries attr_reader :ssl + attr_reader :ssl_peer_verification def initialize(host, username, password, options = {}) @logger = Log4r::Logger.new("vagrant::communication::winrmshell") @logger.debug("initializing WinRMShell") - @host = host - @port = options[:port] || (options[:ssl] ? 5986 : 5985) - @username = username - @password = password - @timeout_in_seconds = options[:timeout_in_seconds] || 60 - @max_tries = options[:max_tries] || 20 - @ssl = options[:ssl] || false + @host = host + @port = options[:port] || (options[:ssl] ? 5986 : 5985) + @username = username + @password = password + @timeout_in_seconds = options[:timeout_in_seconds] || 60 + @max_tries = options[:max_tries] || 20 + @ssl = options[:ssl] || false + @ssl_peer_verification = options[:ssl_peer_verification] || true end def powershell(command, &block) @@ -144,7 +146,7 @@ module VagrantPlugins port: @port, operation_timeout: @timeout_in_seconds, basic_auth_only: true, - no_ssl_peer_verification: true } + no_ssl_peer_verification: !@ssl_peer_verification } 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 3948cf336..6a61f67b0 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -51,7 +51,7 @@ 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, - operation_timeout: 60, basic_auth_only: true, no_ssl_peer_verification: true }) + operation_timeout: 60, basic_auth_only: true, no_ssl_peer_verification: false }) end end From b3480049adccb14e9a78ac1b3c95074a3df06b69 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Tue, 22 Jul 2014 19:30:29 -0400 Subject: [PATCH 08/22] DRY the Shell: Don't duplicate the Config, especially since there were differing default values --- plugins/communicators/winrm/communicator.rb | 8 ++--- plugins/communicators/winrm/config.rb | 2 +- plugins/communicators/winrm/shell.rb | 33 +++++++------------ .../communicators/winrm/communicator_test.rb | 2 +- .../plugins/communicators/winrm/shell_test.rb | 12 +++++-- 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index 29737f6ac..4bd6e47d4 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/plugins/communicators/winrm/communicator.rb @@ -106,12 +106,8 @@ module VagrantPlugins WinRMShell.new( winrm_info[:host], - @machine.config.winrm.username, - @machine.config.winrm.password, - port: winrm_info[:port], - timeout_in_seconds: @machine.config.winrm.timeout, - max_tries: @machine.config.winrm.max_tries, - ssl: @machine.config.winrm.ssl, + winrm_info[:port], + @machine.config.winrm ) end diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 37259e3b3..7ac28404e 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -29,7 +29,7 @@ module VagrantPlugins @port = (@ssl ? 5986 : 5985) if @port == UNSET_VALUE @guest_port = (@ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE @max_tries = 20 if @max_tries == UNSET_VALUE - @timeout = 1800 if @timeout == UNSET_VALUE + @timeout = 60 if @timeout == UNSET_VALUE @ssl = false if @ssl == UNSET_VALUE @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index f851d3805..7b3f98ae5 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -32,27 +32,17 @@ module VagrantPlugins ] attr_reader :logger - attr_reader :username - attr_reader :password attr_reader :host attr_reader :port - attr_reader :timeout_in_seconds - attr_reader :max_tries - attr_reader :ssl - attr_reader :ssl_peer_verification + attr_reader :config - def initialize(host, username, password, options = {}) + def initialize(host, port, config) @logger = Log4r::Logger.new("vagrant::communication::winrmshell") @logger.debug("initializing WinRMShell") @host = host - @port = options[:port] || (options[:ssl] ? 5986 : 5985) - @username = username - @password = password - @timeout_in_seconds = options[:timeout_in_seconds] || 60 - @max_tries = options[:max_tries] || 20 - @ssl = options[:ssl] || false - @ssl_peer_verification = options[:ssl_peer_verification] || true + @port = port + @config = config end def powershell(command, &block) @@ -122,11 +112,11 @@ module VagrantPlugins @logger.info("Attempting to connect to WinRM...") @logger.info(" - Host: #{@host}") @logger.info(" - Port: #{@port}") - @logger.info(" - Username: #{@username}") - @logger.info(" - SSL: #{@ssl}") + @logger.info(" - Username: #{@config.username}") + @logger.info(" - Transport: #{@config.transport}") - client = ::WinRM::WinRMWebService.new(endpoint, :plaintext, endpoint_options) - client.set_timeout(@timeout_in_seconds) + client = ::WinRM::WinRMWebService.new(endpoint, transport.to_sym, endpoint_options) + client.set_timeout(@config.timeout) client.toggle_nori_type_casting(:off) #we don't want coersion of types client end @@ -140,13 +130,12 @@ module VagrantPlugins end def endpoint_options - { user: @username, - pass: @password, + { user: @config.username, + pass: @config.password, host: @host, port: @port, - operation_timeout: @timeout_in_seconds, basic_auth_only: true, - no_ssl_peer_verification: !@ssl_peer_verification } + no_ssl_peer_verification: !@config.ssl_peer_verification } end end #WinShell class end diff --git a/test/unit/plugins/communicators/winrm/communicator_test.rb b/test/unit/plugins/communicators/winrm/communicator_test.rb index 1433f4317..c73106900 100644 --- a/test/unit/plugins/communicators/winrm/communicator_test.rb +++ b/test/unit/plugins/communicators/winrm/communicator_test.rb @@ -101,6 +101,6 @@ describe VagrantPlugins::CommunicatorWinRM::Communicator do expect(shell).to receive(:download).with("from", "to") subject.download("from", "to") end - end + end end diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 6a61f67b0..5b694b62d 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -1,14 +1,22 @@ require File.expand_path("../../../../base", __FILE__) require Vagrant.source_root.join("plugins/communicators/winrm/shell") +require Vagrant.source_root.join("plugins/communicators/winrm/config") describe VagrantPlugins::CommunicatorWinRM::WinRMShell do include_context "unit" let(:session) { double("winrm_session") } + let(:config) { + VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c| + c.username = 'username' + c.password = 'password' + c.finalize! + end + } subject do - described_class.new('localhost', 'username', 'password').tap do |comm| + described_class.new('localhost', 5985, config).tap do |comm| allow(comm).to receive(:new_session).and_return(session) end end @@ -51,7 +59,7 @@ 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, - operation_timeout: 60, basic_auth_only: true, no_ssl_peer_verification: false }) + basic_auth_only: true, no_ssl_peer_verification: false }) end end From 072bb26a3078caa4d465d518db6e3a1c170f0c21 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Tue, 22 Jul 2014 19:58:27 -0400 Subject: [PATCH 09/22] Change @ssl to @transport --- plugins/communicators/winrm/config.rb | 11 +++++---- plugins/communicators/winrm/errors.rb | 4 ++++ plugins/communicators/winrm/shell.rb | 11 +++++++-- templates/locales/comm_winrm.yml | 2 ++ .../plugins/communicators/winrm/shell_test.rb | 23 +++++++++++++++---- 5 files changed, 40 insertions(+), 11 deletions(-) diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 7ac28404e..0e7db6f63 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -8,7 +8,7 @@ module VagrantPlugins attr_accessor :guest_port attr_accessor :max_tries attr_accessor :timeout - attr_accessor :ssl + attr_accessor :transport def initialize @username = UNSET_VALUE @@ -18,19 +18,20 @@ module VagrantPlugins @guest_port = UNSET_VALUE @max_tries = UNSET_VALUE @timeout = UNSET_VALUE - @ssl = UNSET_VALUE + @transport = UNSET_VALUE @ssl_peer_verification = UNSET_VALUE end def finalize! @username = "vagrant" if @username == UNSET_VALUE @password = "vagrant" if @password == UNSET_VALUE + @transport = :ssl if @transport == UNSET_VALUE @host = nil if @host == UNSET_VALUE - @port = (@ssl ? 5986 : 5985) if @port == UNSET_VALUE - @guest_port = (@ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE + is_ssl = @transport == :ssl + @port = (is_ssl ? 5986 : 5985) if @port == UNSET_VALUE + @guest_port = (is_ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE @max_tries = 20 if @max_tries == UNSET_VALUE @timeout = 60 if @timeout == UNSET_VALUE - @ssl = false if @ssl == UNSET_VALUE @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE end diff --git a/plugins/communicators/winrm/errors.rb b/plugins/communicators/winrm/errors.rb index 552b8a300..90c3c6dda 100644 --- a/plugins/communicators/winrm/errors.rb +++ b/plugins/communicators/winrm/errors.rb @@ -29,6 +29,10 @@ module VagrantPlugins class WinRMFileTransferError < WinRMError error_key(:winrm_file_transfer_error) end + + class WinRMInvalidTransport < WinRMError + error_key(:invalid_transport) + end end end end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 7b3f98ae5..cd34b01fe 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -115,7 +115,7 @@ module VagrantPlugins @logger.info(" - Username: #{@config.username}") @logger.info(" - Transport: #{@config.transport}") - client = ::WinRM::WinRMWebService.new(endpoint, transport.to_sym, endpoint_options) + client = ::WinRM::WinRMWebService.new(endpoint, @config.transport.to_sym, endpoint_options) client.set_timeout(@config.timeout) client.toggle_nori_type_casting(:off) #we don't want coersion of types client @@ -126,7 +126,14 @@ module VagrantPlugins end def endpoint - "http#{@ssl ? 's' : ''}://#{@host}:#{@port}/wsman" + case @config.transport + when :ssl + "https://#{@host}:#{@port}/wsman" + when :plaintext + "http://#{@host}:#{@port}/wsman" + else + raise Errors::WinRMInvalidTransport, transport: @config.transport + end end def endpoint_options diff --git a/templates/locales/comm_winrm.yml b/templates/locales/comm_winrm.yml index 9de47a9cf..8800fbbb0 100644 --- a/templates/locales/comm_winrm.yml +++ b/templates/locales/comm_winrm.yml @@ -29,6 +29,8 @@ en: Message: %{message} invalid_shell: |- %{shell} is not a supported type of Windows shell. + invalid_transport: |- + %{transport} is not a supported WinRM transport. winrm_not_ready: |- The box is not able to report an address for WinRM to connect to yet. WinRM cannot access this Vagrant environment. Please wait for the diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 5b694b62d..18d2dd021 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -7,6 +7,7 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do include_context "unit" let(:session) { double("winrm_session") } + let(:port) { config.transport == :ssl ? 5986 : 5985 } let(:config) { VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c| c.username = 'username' @@ -16,7 +17,7 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do } subject do - described_class.new('localhost', 5985, config).tap do |comm| + described_class.new('localhost', port, config).tap do |comm| allow(comm).to receive(:new_session).and_return(session) end end @@ -50,15 +51,29 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do end describe ".endpoint" do - it "should create winrm endpoint address" do - expect(subject.send(:endpoint)).to eq("http://localhost:5985/wsman") + context 'when transport is :ssl' do + it "should create winrm endpoint address using https" do + expect(subject.send(:endpoint)).to eq("https://localhost:5986/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 end end describe ".endpoint_options" do it "should create endpoint options" do expect(subject.send(:endpoint_options)).to eq( - { user: "username", pass: "password", host: "localhost", port: 5985, + { user: "username", pass: "password", host: "localhost", port: 5986, basic_auth_only: true, no_ssl_peer_verification: false }) end end From 02f4adc895580315e04afd075b55956de98cf984 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Wed, 23 Jul 2014 14:56:49 -0400 Subject: [PATCH 10/22] Fix broken variable references --- plugins/communicators/winrm/shell.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index cd34b01fe..6c55ff45f 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -81,7 +81,7 @@ module VagrantPlugins end def execute_shell_with_retry(command, shell, &block) - retryable(tries: @max_tries, on: @@exceptions_to_retry_on, sleep: 10) do + retryable(tries: @config.max_tries, on: @@exceptions_to_retry_on, sleep: 10) do @logger.debug("#{shell} executing:\n#{command}") output = session.send(shell, command) do |out, err| block.call(:stdout, out) if block_given? && out @@ -96,8 +96,8 @@ module VagrantPlugins # If the error is a 401, we can return a more specific error message if winrm_exception.message.include?("401") raise Errors::AuthError, - user: @username, - password: @password, + user: @config.username, + password: @config.password, endpoint: endpoint, message: winrm_exception.message end @@ -126,7 +126,7 @@ module VagrantPlugins end def endpoint - case @config.transport + case @config.transport.to_sym when :ssl "https://#{@host}:#{@port}/wsman" when :plaintext From 2caaf82ae0ac9975a1195d7ac95c2d088ff5a83b Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Wed, 23 Jul 2014 15:04:33 -0400 Subject: [PATCH 11/22] Change default transport back to :plaintext, for backwards compatibility --- plugins/communicators/winrm/config.rb | 6 +++--- .../unit/plugins/communicators/winrm/shell_test.rb | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 0e7db6f63..ea381ae0e 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -23,9 +23,9 @@ module VagrantPlugins end def finalize! - @username = "vagrant" if @username == UNSET_VALUE - @password = "vagrant" if @password == UNSET_VALUE - @transport = :ssl if @transport == UNSET_VALUE + @username = "vagrant" if @username == UNSET_VALUE + @password = "vagrant" if @password == UNSET_VALUE + @transport = :plaintext if @transport == UNSET_VALUE @host = nil if @host == UNSET_VALUE is_ssl = @transport == :ssl @port = (is_ssl ? 5986 : 5985) if @port == UNSET_VALUE diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 18d2dd021..05e8351b4 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -52,18 +52,18 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do describe ".endpoint" do context 'when transport is :ssl' do + let(:config) { + VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c| + c.transport = :ssl + c.finalize! + end + } it "should create winrm endpoint address using https" do expect(subject.send(:endpoint)).to eq("https://localhost:5986/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 @@ -73,7 +73,7 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do describe ".endpoint_options" do it "should create endpoint options" do expect(subject.send(:endpoint_options)).to eq( - { user: "username", pass: "password", host: "localhost", port: 5986, + { user: "username", pass: "password", host: "localhost", port: 5985, basic_auth_only: true, no_ssl_peer_verification: false }) end end From 62ddd92768c3e7cb01eee745b44574d47ce53aac Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Wed, 23 Jul 2014 16:05:06 -0400 Subject: [PATCH 12/22] Only forward one port for winrm --- plugins/kernel_v2/config/vm.rb | 8 ----- test/unit/plugins/kernel_v2/config/vm_test.rb | 31 ++++++++++--------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index c5a9e1f6c..a06478096 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -400,14 +400,6 @@ module VagrantPlugins id: "winrm", auto_correct: true end - if !@__networks["forwarded_port-winrmssl"] - network :forwarded_port, - guest: 5986, - host: 55986, - host_ip: "127.0.0.1", - id: "winrmssl", - auto_correct: true - end elsif !@__networks["forwarded_port-ssh"] network :forwarded_port, guest: 22, diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 4683b01b7..073e88e22 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -167,7 +167,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do subject.communicator = "winrm" subject.finalize! n = subject.networks - expect(n.length).to eq(2) + expect(n.length).to eq(1) # WinRM HTTP expect(n[0][0]).to eq(:forwarded_port) @@ -175,13 +175,6 @@ describe VagrantPlugins::Kernel_V2::VMConfig do expect(n[0][1][:host]).to eq(55985) expect(n[0][1][:host_ip]).to eq("127.0.0.1") expect(n[0][1][:id]).to eq("winrm") - - # WinRM HTTPS - expect(n[1][0]).to eq(:forwarded_port) - expect(n[1][1][:guest]).to eq(5986) - expect(n[1][1][:host]).to eq(55986) - expect(n[1][1][:host_ip]).to eq("127.0.0.1") - expect(n[1][1][:id]).to eq("winrmssl") end it "allows overriding SSH" do @@ -203,12 +196,22 @@ describe VagrantPlugins::Kernel_V2::VMConfig do guest: 22, host: 14100, id: "winrm" subject.finalize! - n = subject.networks - expect(n.length).to eq(1) - expect(n[0][0]).to eq(:forwarded_port) - expect(n[0][1][:guest]).to eq(22) - expect(n[0][1][:host]).to eq(14100) - expect(n[0][1][:id]).to eq("winrm") + winrm_network = find_network 'winrm' + expect(winrm_network[:guest]).to eq(22) + expect(winrm_network[:host]).to eq(14100) + expect(winrm_network[:id]).to eq("winrm") + end + + it "allows overriding WinRM SSL" do + subject.communicator = :winrmssl + subject.network "forwarded_port", + guest: 22, host: 14100, id: "winrmssl" + subject.finalize! + + winrmssl_network = find_network 'winrmssl' + expect(winrmssl_network[:guest]).to eq(22) + expect(winrmssl_network[:host]).to eq(14100) + expect(winrmssl_network[:id]).to eq("winrmssl") end it "turns all forwarded port ports to ints" do From 24f919c4d3173c6498131a91a42e95cb8f468d9c Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Fri, 25 Jul 2014 17:00:10 -0400 Subject: [PATCH 13/22] Fix accessors used by in communicator.rb --- plugins/communicators/winrm/config.rb | 3 ++- plugins/communicators/winrm/shell.rb | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index ea381ae0e..6e4a44338 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -9,6 +9,7 @@ module VagrantPlugins attr_accessor :max_tries attr_accessor :timeout attr_accessor :transport + attr_accessor :ssl_peer_verification def initialize @username = UNSET_VALUE @@ -31,7 +32,7 @@ module VagrantPlugins @port = (is_ssl ? 5986 : 5985) if @port == UNSET_VALUE @guest_port = (is_ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE @max_tries = 20 if @max_tries == UNSET_VALUE - @timeout = 60 if @timeout == UNSET_VALUE + @timeout = 1800 if @timeout == UNSET_VALUE @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 6c55ff45f..2114e944e 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -34,6 +34,8 @@ module VagrantPlugins attr_reader :logger attr_reader :host attr_reader :port + attr_reader :username + attr_reader :password attr_reader :config def initialize(host, port, config) @@ -42,6 +44,8 @@ module VagrantPlugins @host = host @port = port + @username = config.username + @password = config.password @config = config end @@ -137,8 +141,8 @@ module VagrantPlugins end def endpoint_options - { user: @config.username, - pass: @config.password, + { user: @username, + pass: @password, host: @host, port: @port, basic_auth_only: true, From e787291717cb3aa66fded5799fc97b3b49c8fc58 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Mon, 8 Dec 2014 12:41:11 -0500 Subject: [PATCH 14/22] Use pre-release gem version w/ better errors --- vagrant.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vagrant.gemspec b/vagrant.gemspec index 4198452c4..b032b8f73 100644 --- a/vagrant.gemspec +++ b/vagrant.gemspec @@ -28,7 +28,7 @@ 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.2" + s.add_dependency "winrm", "= 1.3.0.dev.2" # We lock this down to avoid compilation issues. s.add_dependency "nokogiri", "= 1.6.3.1" From ba7b964b1ee4bf5c0f592716c30991b88aac227b Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Mon, 8 Dec 2014 19:09:11 -0500 Subject: [PATCH 15/22] Better error handling for WinRM (using winrm v1.3.0.dev.2) --- plugins/communicators/winrm/communicator.rb | 83 +++++++++++++++++++-- plugins/communicators/winrm/errors.rb | 37 ++++++++- plugins/communicators/winrm/shell.rb | 55 ++++++++++---- templates/locales/comm_winrm.yml | 43 ++++++++++- 4 files changed, 195 insertions(+), 23 deletions(-) diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index 4bd6e47d4..e354df5ee 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/plugins/communicators/winrm/communicator.rb @@ -25,22 +25,95 @@ module VagrantPlugins @logger.info("Initializing WinRMCommunicator") end + def wait_for_ready(timeout) + Timeout.timeout(timeout) do + # Wait for winrm_info to be ready + winrm_info = nil + while true + winrm_info = Helper.winrm_info(@machine) + break if winrm_info + sleep 0.5 + end + + # Got it! Let the user know what we're connecting to. + @machine.ui.detail("WinRM address: #{shell.host}:#{shell.port}") + @machine.ui.detail("WinRM username: #{shell.username}") + @machine.ui.detail("WinRM transport: #{shell.config.transport}") + + last_message = nil + last_message_repeat_at = 0 + while true + message = nil + begin + begin + standard_shell = shell.cmd('echo vagrant') + powershell = shell.cmd('@PowerShell Write-Host vagrant') + + unless standard_shell[:exitcode] == 0 || powershell[:exitcode] == 0 + raise Errors::InvalidShell + end + return true if shell(true).sane? + rescue Vagrant::Errors::VagrantError => e + @logger.info("WinRM not ready: #{e.inspect}") + raise + end + rescue Errors::ConnectionTimeout + message = "Connection timeout." + rescue Errors::AuthenticationFailed + message = "Authentication failure." + rescue Errors::Disconnected + message = "Remote connection disconnect." + rescue Errors::ConnectionRefused + message = "Connection refused." + rescue Errors::ConnectionReset + message = "Connection reset." + rescue Errors::HostDown + message = "Host appears down." + rescue Errors::NoRoute + message = "Host unreachable." + rescue Errors::TransientError => e + # Any other retriable errors + message = e.message + end + + # If we have a message to show, then show it. We don't show + # repeated messages unless they've been repeating longer than + # 10 seconds. + if message + message_at = Time.now.to_f + show_message = true + if last_message == message + show_message = (message_at - last_message_repeat_at) > 10.0 + end + + if show_message + @machine.ui.detail("Warning: #{message} Retrying...") + last_message = message + last_message_repeat_at = message_at + end + end + end + end + rescue Timeout::Error + return false + end + def ready? @logger.info("Checking whether WinRM is ready...") - Timeout.timeout(@machine.config.winrm.timeout) do + result = Timeout.timeout(@machine.config.winrm.timeout) do shell(true).powershell("hostname") end @logger.info("WinRM is ready!") return true - rescue Vagrant::Errors::VagrantError => e - # We catch a `VagrantError` which would signal that something went - # wrong expectedly in the `connect`, which means we didn't connect. + rescue Errors::TransientError => e + # We catch a `TransientError` which would signal that something went + # that might work if we wait and retry. @logger.info("WinRM not up: #{e.inspect}") # We reset the shell to trigger calling of winrm_finder again. - # This resolves a problem when using vSphere where the ssh_info was not refreshing + # This resolves a problem when using vSphere where the winrm_info was not refreshing # thus never getting the correct hostname. @shell = nil return false diff --git a/plugins/communicators/winrm/errors.rb b/plugins/communicators/winrm/errors.rb index 90c3c6dda..6a5df1be1 100644 --- a/plugins/communicators/winrm/errors.rb +++ b/plugins/communicators/winrm/errors.rb @@ -6,8 +6,11 @@ module VagrantPlugins error_namespace("vagrant_winrm.errors") end - class AuthError < WinRMError - error_key(:auth_error) + class TransientError < WinRMError + end + + class AuthenticationFailed < WinRMError + error_key(:authentication_failed) end class ExecutionError < WinRMError @@ -30,9 +33,37 @@ module VagrantPlugins error_key(:winrm_file_transfer_error) end - class WinRMInvalidTransport < WinRMError + class InvalidTransport < WinRMError error_key(:invalid_transport) end + + class SSLError < WinRMError + error_key(:ssl_error) + end + + class ConnectionTimeout < TransientError + error_key(:connection_timeout) + end + + class Disconnected < TransientError + error_key(:disconnected) + end + + class ConnectionRefused < TransientError + error_key(:connection_refused) + end + + class ConnectionReset < TransientError + error_key(:connection_reset) + end + + class HostDown < TransientError + error_key(:host_down) + end + + class NoRoute < TransientError + error_key(:no_route) + end end end end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 2114e944e..3bdd869db 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -96,20 +96,49 @@ module VagrantPlugins end end - def raise_winrm_exception(winrm_exception, shell, command) - # If the error is a 401, we can return a more specific error message - if winrm_exception.message.include?("401") - raise Errors::AuthError, - user: @config.username, - password: @config.password, - endpoint: endpoint, - message: winrm_exception.message - end + def raise_winrm_exception(exception, shell = nil, command = nil) + case exception + when WinRM::WinRMAuthorizationError + raise Errors::AuthenticationFailed, + user: @config.username, + password: @config.password, + endpoint: endpoint, + message: exception.message + when WinRM::WinRMHTTPTransportError + case exception.response_code + # If the error is a 401, we can return a more specific error message + when 401 + raise Errors::AuthenticationFailed, + user: @config.username, + password: @config.password, + endpoint: endpoint, + message: exception.message + end - raise Errors::ExecutionError, - shell: shell, - command: command, - message: winrm_exception.message + raise Errors::ExecutionError, + shell: shell, + command: command, + message: exception.message + when OpenSSL::SSL::SSLError + raise Errors::SSLError, message: exception.message + when HTTPClient::TimeoutError + raise Errors::ConnectionTimeout, message: exception.message + when Errno::ECONNREFUSED + # This is raised if we failed to connect the max amount of times + raise Errors::ConnectionRefused + when Errno::ECONNRESET + # This is raised if we failed to connect the max number of times + # due to an ECONNRESET. + raise Errors::ConnectionReset + when Errno::EHOSTDOWN + # This is raised if we get an ICMP DestinationUnknown error. + raise Errors::HostDown + when Errno::EHOSTUNREACH + # This is raised if we can't work out how to route traffic. + raise Errors::NoRoute + else + raise + end end def new_session diff --git a/templates/locales/comm_winrm.yml b/templates/locales/comm_winrm.yml index 8800fbbb0..0f01a2755 100644 --- a/templates/locales/comm_winrm.yml +++ b/templates/locales/comm_winrm.yml @@ -1,11 +1,10 @@ en: vagrant_winrm: errors: - auth_error: |- + authentication_failed: |- An authorization error occurred while connecting to WinRM. User: %{user} - Password: %{password} Endpoint: %{endpoint} Message: %{message} winrm_bad_exit_status: |- @@ -31,6 +30,13 @@ en: %{shell} is not a supported type of Windows shell. invalid_transport: |- %{transport} is not a supported WinRM transport. + ssl_error: |- + An SSL error occurred while connecting to WinRM. This usually + occurs when you are using a self-signed certificate and have + not set the WinRM `ssl_peer_verification` config setting to false. + + Message: %{message} + winrm_not_ready: |- The box is not able to report an address for WinRM to connect to yet. WinRM cannot access this Vagrant environment. Please wait for the @@ -41,3 +47,36 @@ en: From: %{from} To: %{to} Message: %{message} + + connection_refused: |- + WinRM connection was refused! This usually happens if the VM failed to + boot properly. Some steps to try to fix this: First, try reloading your + VM with `vagrant reload`, since a simple restart sometimes fixes things. + If that doesn't work, destroy your VM and recreate it with a `vagrant destroy` + followed by a `vagrant up`. If that doesn't work, contact a Vagrant + maintainer (support channels listed on the website) for more assistance. + connection_reset: |- + WinRM connection was reset! This usually happens when the machine is + taking too long to reboot. First, try reloading your machine with + `vagrant reload`, since a simple restart sometimes fixes things. + If that doesn't work, destroy your machine and recreate it with + a `vagrant destroy` followed by a `vagrant up`. If that doesn't work, + contact support. + connection_timeout: |- + Vagrant timed out while attempting to connect via WinRM. This usually + means that the VM booted, but there are issues with the WinRM configuration + or network connectivity issues. Please try to `vagrant reload` or + `vagrant up` again. + disconnected: |- + The WinRM connection was unexpectedly closed by the remote end. This + usually indicates that WinRM within the guest machine was unable to + properly start up. Please boot the VM in GUI mode to check whether + it is booting properly. + no_route: |- + While attempting to connect with WinRM, a "no route to host" (EHOSTUNREACH) + error was received. Please verify your network settings are correct + and try again. + host_down: |- + While attempting to connect with WinRM, a "host is down" (EHOSTDOWN) + error was received. Please verify your WinRM settings are correct + and try again. From 24de8a1fb7fdc22806bab803745bd37d935cc6be Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Tue, 9 Dec 2014 13:51:45 -0500 Subject: [PATCH 16/22] Just use ready? --- plugins/communicators/winrm/communicator.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index e354df5ee..9c02c48c8 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/plugins/communicators/winrm/communicator.rb @@ -46,13 +46,7 @@ module VagrantPlugins message = nil begin begin - standard_shell = shell.cmd('echo vagrant') - powershell = shell.cmd('@PowerShell Write-Host vagrant') - - unless standard_shell[:exitcode] == 0 || powershell[:exitcode] == 0 - raise Errors::InvalidShell - end - return true if shell(true).sane? + return true if ready? rescue Vagrant::Errors::VagrantError => e @logger.info("WinRM not ready: #{e.inspect}") raise From e7e50d39d915f2fd09ea94b0c2a9ec61be2b7509 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Wed, 10 Dec 2014 15:15:07 -0500 Subject: [PATCH 17/22] Fix tests - all pass but auth retry test is extremely slow --- plugins/communicators/winrm/shell.rb | 7 +++++-- .../communicators/winrm/communicator_test.rb | 9 +++++++-- test/unit/plugins/communicators/winrm/shell_test.rb | 9 +++++---- test/unit/plugins/kernel_v2/config/vm_test.rb | 13 +++++++------ 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 3bdd869db..700408940 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -105,7 +105,7 @@ module VagrantPlugins endpoint: endpoint, message: exception.message when WinRM::WinRMHTTPTransportError - case exception.response_code + case exception.status_code # If the error is a 401, we can return a more specific error message when 401 raise Errors::AuthenticationFailed, @@ -137,7 +137,10 @@ module VagrantPlugins # This is raised if we can't work out how to route traffic. raise Errors::NoRoute else - raise + raise Errors::ExecutionError, + shell: shell, + command: command, + message: exception.message end end diff --git a/test/unit/plugins/communicators/winrm/communicator_test.rb b/test/unit/plugins/communicators/winrm/communicator_test.rb index c73106900..1afaffffe 100644 --- a/test/unit/plugins/communicators/winrm/communicator_test.rb +++ b/test/unit/plugins/communicators/winrm/communicator_test.rb @@ -28,11 +28,16 @@ describe VagrantPlugins::CommunicatorWinRM::Communicator do expect(subject.ready?).to be_true end - it "returns false if hostname command fails to execute without error" do - expect(shell).to receive(:powershell).with("hostname").and_raise(Vagrant::Errors::VagrantError) + it "returns false if hostname command fails with a transient error" do + expect(shell).to receive(:powershell).with("hostname").and_raise(VagrantPlugins::CommunicatorWinRM::Errors::TransientError) expect(subject.ready?).to be_false end + it "raises an error if hostname command fails with an unknown error" do + expect(shell).to receive(:powershell).with("hostname").and_raise(Vagrant::Errors::VagrantError) + expect { subject.ready? }.to raise_error(Vagrant::Errors::VagrantError) + end + it "raises timeout error when hostname command takes longer then winrm timeout" do expect(shell).to receive(:powershell).with("hostname") do sleep 2 # winrm.timeout = 1 diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 05e8351b4..7a402e3cb 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -28,11 +28,12 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do expect(subject.powershell("dir")[:exitcode]).to eq(0) end - it "should raise auth error when exception message contains 401" do - expect(session).to receive(:powershell).with(/^dir.+/).and_raise( - StandardError.new("Oh no! a 401 SOAP error!")) + it "should raise auth error when WinRM exception has a response code of 401" do + # The default settings might an account lockout - 20 auth failures! + expect(session).to receive(:powershell).with(/^dir.+/).exactly(20).times.and_raise( + WinRM::WinRMHTTPTransportError.new("Oh no!!", 401)) expect { subject.powershell("dir") }.to raise_error( - VagrantPlugins::CommunicatorWinRM::Errors::AuthError) + VagrantPlugins::CommunicatorWinRM::Errors::AuthenticationFailed) end it "should raise an execution error when an exception occurs" do diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 073e88e22..3028e2552 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -167,14 +167,15 @@ describe VagrantPlugins::Kernel_V2::VMConfig do subject.communicator = "winrm" subject.finalize! n = subject.networks - expect(n.length).to eq(1) + expect(n.length).to eq(2) # WinRM HTTP - expect(n[0][0]).to eq(:forwarded_port) - expect(n[0][1][:guest]).to eq(5985) - expect(n[0][1][:host]).to eq(55985) - expect(n[0][1][:host_ip]).to eq("127.0.0.1") - expect(n[0][1][:id]).to eq("winrm") + network = find_network("winrm") + # expect(n[0][0]).to eq(:forwarded_port) + expect(network[:guest]).to eq(5985) + expect(network[:host]).to eq(55985) + expect(network[:host_ip]).to eq("127.0.0.1") + expect(network[:id]).to eq("winrm") end it "allows overriding SSH" do From b5a94774053bb2314ab64cb0d292cda63aa14aaa Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Thu, 11 Dec 2014 13:39:24 -0500 Subject: [PATCH 18/22] Fix default forwarded ports --- plugins/kernel_v2/config/vm.rb | 7 +++++++ test/unit/plugins/kernel_v2/config/vm_test.rb | 18 +++++++++++------- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index a06478096..fd08f436a 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -399,6 +399,13 @@ module VagrantPlugins host_ip: "127.0.0.1", id: "winrm", auto_correct: true + + network :forwarded_port, + guest: 5986, + host: 55986, + host_ip: "127.0.0.1", + id: "winrm-ssl", + auto_correct: true end elsif !@__networks["forwarded_port-ssh"] network :forwarded_port, diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 3028e2552..50f185c9c 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -169,13 +169,17 @@ describe VagrantPlugins::Kernel_V2::VMConfig do n = subject.networks expect(n.length).to eq(2) - # WinRM HTTP - network = find_network("winrm") - # expect(n[0][0]).to eq(:forwarded_port) - expect(network[:guest]).to eq(5985) - expect(network[:host]).to eq(55985) - expect(network[:host_ip]).to eq("127.0.0.1") - expect(network[:id]).to eq("winrm") + expect(n[0][0]).to eq(:forwarded_port) + expect(n[0][1][:guest]).to eq(5985) + expect(n[0][1][:host]).to eq(55985) + expect(n[0][1][:host_ip]).to eq("127.0.0.1") + expect(n[0][1][:id]).to eq("winrm") + + expect(n[1][0]).to eq(:forwarded_port) + expect(n[1][1][:guest]).to eq(5986) + expect(n[1][1][:host]).to eq(55986) + expect(n[1][1][:host_ip]).to eq("127.0.0.1") + expect(n[1][1][:id]).to eq("winrm-ssl") end it "allows overriding SSH" do From 234adaae637e8565fff5490fb554742ed5deb431 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Thu, 11 Dec 2014 13:55:49 -0500 Subject: [PATCH 19/22] WinRM SSL support --- plugins/communicators/winrm/communicator.rb | 7 +--- plugins/communicators/winrm/config.rb | 32 +++++++++----- plugins/communicators/winrm/shell.rb | 42 +++++++++++-------- plugins/kernel_v2/config/vm.rb | 11 +++-- .../communicators/winrm/communicator_test.rb | 2 +- .../plugins/communicators/winrm/shell_test.rb | 34 ++++++++++++--- test/unit/plugins/kernel_v2/config/vm_test.rb | 37 ++++++++++++---- vagrant.gemspec | 2 +- 8 files changed, 114 insertions(+), 53 deletions(-) diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index 7609f16b3..4bd6e47d4 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/plugins/communicators/winrm/communicator.rb @@ -106,11 +106,8 @@ module VagrantPlugins WinRMShell.new( winrm_info[:host], - @machine.config.winrm.username, - @machine.config.winrm.password, - port: winrm_info[:port], - timeout_in_seconds: @machine.config.winrm.timeout, - max_tries: @machine.config.winrm.max_tries, + winrm_info[:port], + @machine.config.winrm ) end diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 155ac0567..6e4a44338 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -8,25 +8,32 @@ module VagrantPlugins attr_accessor :guest_port attr_accessor :max_tries attr_accessor :timeout + attr_accessor :transport + attr_accessor :ssl_peer_verification def initialize - @username = UNSET_VALUE - @password = UNSET_VALUE - @host = UNSET_VALUE - @port = UNSET_VALUE - @guest_port = UNSET_VALUE - @max_tries = UNSET_VALUE - @timeout = UNSET_VALUE + @username = UNSET_VALUE + @password = UNSET_VALUE + @host = UNSET_VALUE + @port = UNSET_VALUE + @guest_port = UNSET_VALUE + @max_tries = UNSET_VALUE + @timeout = UNSET_VALUE + @transport = UNSET_VALUE + @ssl_peer_verification = UNSET_VALUE end def finalize! - @username = "vagrant" if @username == UNSET_VALUE - @password = "vagrant" if @password == UNSET_VALUE + @username = "vagrant" if @username == UNSET_VALUE + @password = "vagrant" if @password == UNSET_VALUE + @transport = :plaintext if @transport == UNSET_VALUE @host = nil if @host == UNSET_VALUE - @port = 5985 if @port == UNSET_VALUE - @guest_port = 5985 if @guest_port == UNSET_VALUE + is_ssl = @transport == :ssl + @port = (is_ssl ? 5986 : 5985) if @port == UNSET_VALUE + @guest_port = (is_ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE @max_tries = 20 if @max_tries == UNSET_VALUE @timeout = 1800 if @timeout == UNSET_VALUE + @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE end def validate(machine) @@ -38,6 +45,9 @@ module VagrantPlugins errors << "winrm.guest_port cannot be nil." if @guest_port.nil? errors << "winrm.max_tries cannot be nil." if @max_tries.nil? errors << "winrm.timeout cannot be nil." if @timeout.nil? + unless @ssl_peer_verification == true || @ssl_peer_verification == false + errors << "winrm.ssl_peer_verification must be a boolean." + end { "WinRM" => errors } end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 353c0e84d..1f9e6796c 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -32,23 +32,21 @@ module VagrantPlugins ] attr_reader :logger - attr_reader :username - attr_reader :password attr_reader :host attr_reader :port - attr_reader :timeout_in_seconds - attr_reader :max_tries + attr_reader :username + attr_reader :password + attr_reader :config - def initialize(host, username, password, options = {}) + def initialize(host, port, config) @logger = Log4r::Logger.new("vagrant::communication::winrmshell") @logger.debug("initializing WinRMShell") - @host = host - @port = options[:port] || 5985 - @username = username - @password = password - @timeout_in_seconds = options[:timeout_in_seconds] || 60 - @max_tries = options[:max_tries] || 20 + @host = host + @port = port + @username = config.username + @password = config.password + @config = config end def powershell(command, &block) @@ -87,7 +85,7 @@ module VagrantPlugins end def execute_shell_with_retry(command, shell, &block) - retryable(tries: @max_tries, on: @@exceptions_to_retry_on, sleep: 10) do + retryable(tries: @config.max_tries, on: @@exceptions_to_retry_on, sleep: 10) do @logger.debug("#{shell} executing:\n#{command}") output = session.send(shell, command) do |out, err| block.call(:stdout, out) if block_given? && out @@ -118,10 +116,11 @@ module VagrantPlugins @logger.info("Attempting to connect to WinRM...") @logger.info(" - Host: #{@host}") @logger.info(" - Port: #{@port}") - @logger.info(" - Username: #{@username}") + @logger.info(" - Username: #{@config.username}") + @logger.info(" - Transport: #{@config.transport}") - client = ::WinRM::WinRMWebService.new(endpoint, :plaintext, endpoint_options) - client.set_timeout(@timeout_in_seconds) + client = ::WinRM::WinRMWebService.new(endpoint, @config.transport.to_sym, endpoint_options) + client.set_timeout(@config.timeout) client.toggle_nori_type_casting(:off) #we don't want coersion of types client end @@ -131,7 +130,14 @@ module VagrantPlugins end def endpoint - "http://#{@host}:#{@port}/wsman" + case @config.transport.to_sym + when :ssl + "https://#{@host}:#{@port}/wsman" + when :plaintext + "http://#{@host}:#{@port}/wsman" + else + raise Errors::WinRMInvalidTransport, transport: @config.transport + end end def endpoint_options @@ -139,8 +145,8 @@ module VagrantPlugins pass: @password, host: @host, port: @port, - operation_timeout: @timeout_in_seconds, - basic_auth_only: true } + basic_auth_only: true, + no_ssl_peer_verification: !@config.ssl_peer_verification } end end #WinShell class end diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index da2440c52..fd08f436a 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -399,10 +399,15 @@ module VagrantPlugins host_ip: "127.0.0.1", id: "winrm", auto_correct: true - end - end - if !@__networks["forwarded_port-ssh"] + network :forwarded_port, + guest: 5986, + host: 55986, + host_ip: "127.0.0.1", + id: "winrm-ssl", + auto_correct: true + end + elsif !@__networks["forwarded_port-ssh"] network :forwarded_port, guest: 22, host: 2222, diff --git a/test/unit/plugins/communicators/winrm/communicator_test.rb b/test/unit/plugins/communicators/winrm/communicator_test.rb index 1433f4317..c73106900 100644 --- a/test/unit/plugins/communicators/winrm/communicator_test.rb +++ b/test/unit/plugins/communicators/winrm/communicator_test.rb @@ -101,6 +101,6 @@ describe VagrantPlugins::CommunicatorWinRM::Communicator do expect(shell).to receive(:download).with("from", "to") subject.download("from", "to") end - end + end end diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 477108b97..95fbb5c69 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -1,14 +1,23 @@ require File.expand_path("../../../../base", __FILE__) require Vagrant.source_root.join("plugins/communicators/winrm/shell") +require Vagrant.source_root.join("plugins/communicators/winrm/config") describe VagrantPlugins::CommunicatorWinRM::WinRMShell do include_context "unit" let(:session) { double("winrm_session") } + let(:port) { config.transport == :ssl ? 5986 : 5985 } + let(:config) { + VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c| + c.username = 'username' + c.password = 'password' + c.finalize! + end + } subject do - described_class.new('localhost', 'username', 'password').tap do |comm| + described_class.new('localhost', port, config).tap do |comm| allow(comm).to receive(:new_session).and_return(session) end end @@ -20,7 +29,8 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do end it "should raise auth error when exception message contains 401" do - expect(session).to receive(:powershell).with(/^dir.+/).and_raise( + config.winrm.max_tries = 2 + expect(session).to receive(:powershell).with(/^dir.+/).twice.and_raise( StandardError.new("Oh no! a 401 SOAP error!")) expect { subject.powershell("dir") }.to raise_error( VagrantPlugins::CommunicatorWinRM::Errors::AuthError) @@ -42,8 +52,22 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do end describe ".endpoint" do - it "should create winrm endpoint address" do - expect(subject.send(:endpoint)).to eq("http://localhost:5985/wsman") + context 'when transport is :ssl' do + let(:config) { + VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c| + c.transport = :ssl + c.finalize! + end + } + it "should create winrm endpoint address using https" do + expect(subject.send(:endpoint)).to eq("https://localhost:5986/wsman") + end + end + + context "when transport is :plaintext" do + it "should create winrm endpoint address using http" do + expect(subject.send(:endpoint)).to eq("http://localhost:5985/wsman") + end end end @@ -51,7 +75,7 @@ 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, - operation_timeout: 60, basic_auth_only: true }) + basic_auth_only: true, no_ssl_peer_verification: false }) end end diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 828753ad0..14f43205d 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -23,6 +23,13 @@ describe VagrantPlugins::Kernel_V2::VMConfig do end end + def find_network(name) + network_definitions = subject.networks.map do |n| + n[1] + end + network_definitions.find {|n| n[:id] == name} + end + before do env = double("env") env.stub(root_path: nil) @@ -183,6 +190,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do subject.finalize! n = subject.networks expect(n.length).to eq(2) + expect(n[0][0]).to eq(:forwarded_port) expect(n[0][1][:guest]).to eq(5985) expect(n[0][1][:host]).to eq(55985) @@ -190,9 +198,10 @@ describe VagrantPlugins::Kernel_V2::VMConfig do expect(n[0][1][:id]).to eq("winrm") expect(n[1][0]).to eq(:forwarded_port) - expect(n[1][1][:guest]).to eq(22) - expect(n[1][1][:host]).to eq(2222) - expect(n[1][1][:id]).to eq("ssh") + expect(n[1][1][:guest]).to eq(5986) + expect(n[1][1][:host]).to eq(55986) + expect(n[1][1][:host_ip]).to eq("127.0.0.1") + expect(n[1][1][:id]).to eq("winrm-ssl") end it "allows overriding SSH" do @@ -214,12 +223,22 @@ describe VagrantPlugins::Kernel_V2::VMConfig do guest: 22, host: 14100, id: "winrm" subject.finalize! - n = subject.networks - expect(n.length).to eq(2) - expect(n[0][0]).to eq(:forwarded_port) - expect(n[0][1][:guest]).to eq(22) - expect(n[0][1][:host]).to eq(14100) - expect(n[0][1][:id]).to eq("winrm") + winrm_network = find_network 'winrm' + expect(winrm_network[:guest]).to eq(22) + expect(winrm_network[:host]).to eq(14100) + expect(winrm_network[:id]).to eq("winrm") + end + + it "allows overriding WinRM SSL" do + subject.communicator = :winrmssl + subject.network "forwarded_port", + guest: 22, host: 14100, id: "winrmssl" + subject.finalize! + + winrmssl_network = find_network 'winrmssl' + expect(winrmssl_network[:guest]).to eq(22) + expect(winrmssl_network[:host]).to eq(14100) + expect(winrmssl_network[:id]).to eq("winrmssl") end it "turns all forwarded port ports to ints" do diff --git a/vagrant.gemspec b/vagrant.gemspec index 0afe7e766..b032b8f73 100644 --- a/vagrant.gemspec +++ b/vagrant.gemspec @@ -28,7 +28,7 @@ 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.1.3" + s.add_dependency "winrm", "= 1.3.0.dev.2" # We lock this down to avoid compilation issues. s.add_dependency "nokogiri", "= 1.6.3.1" From 2b810c3cccbffc89e834a272beb81dd79a7c5629 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Tue, 20 Jan 2015 16:03:09 -0500 Subject: [PATCH 20/22] WinRM 1.3 released w/ proper SSL support --- vagrant.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vagrant.gemspec b/vagrant.gemspec index b032b8f73..7fe457f22 100644 --- a/vagrant.gemspec +++ b/vagrant.gemspec @@ -28,7 +28,7 @@ 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.0.dev.2" + s.add_dependency "winrm", "~> 1.3" # We lock this down to avoid compilation issues. s.add_dependency "nokogiri", "= 1.6.3.1" From 5d5e13bc0f8d93311910c5f466e5f6a306cb5460 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Wed, 28 Jan 2015 10:47:03 -0500 Subject: [PATCH 21/22] Change authorization error tests to match WinRM 1.3 --- plugins/communicators/winrm/shell.rb | 10 +--------- test/unit/plugins/communicators/winrm/shell_test.rb | 2 +- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 2e9c35d5a..ae14e0f23 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -22,6 +22,7 @@ module VagrantPlugins @@exceptions_to_retry_on = [ HTTPClient::KeepAliveDisconnected, WinRM::WinRMHTTPTransportError, + WinRM::WinRMAuthorizationError, Errno::EACCES, Errno::EADDRINUSE, Errno::ECONNREFUSED, @@ -124,15 +125,6 @@ module VagrantPlugins message: exception.message when WinRM::WinRMHTTPTransportError case exception.status_code - # If the error is a 401, we can return a more specific error message - when 401 - raise Errors::AuthenticationFailed, - user: @config.username, - password: @config.password, - endpoint: endpoint, - message: exception.message - end - raise Errors::ExecutionError, shell: shell, command: command, diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 7a402e3cb..22a50072d 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -31,7 +31,7 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do it "should raise auth error when WinRM exception has a response code of 401" do # The default settings might an account lockout - 20 auth failures! expect(session).to receive(:powershell).with(/^dir.+/).exactly(20).times.and_raise( - WinRM::WinRMHTTPTransportError.new("Oh no!!", 401)) + WinRM::WinRMAuthorizationError.new("Oh no!!", 401)) expect { subject.powershell("dir") }.to raise_error( VagrantPlugins::CommunicatorWinRM::Errors::AuthenticationFailed) end From 39882957eea13c9bba09c8cebbccf7576e071ae1 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Wed, 28 Jan 2015 13:40:22 -0500 Subject: [PATCH 22/22] Add retry_delay setting to speed up test --- plugins/communicators/winrm/config.rb | 20 +++++++++++-------- plugins/communicators/winrm/shell.rb | 3 +-- .../plugins/communicators/winrm/shell_test.rb | 13 ++++++++---- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 6e4a44338..79901d503 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -7,6 +7,7 @@ module VagrantPlugins attr_accessor :port attr_accessor :guest_port attr_accessor :max_tries + attr_accessor :retry_delay attr_accessor :timeout attr_accessor :transport attr_accessor :ssl_peer_verification @@ -18,6 +19,7 @@ module VagrantPlugins @port = UNSET_VALUE @guest_port = UNSET_VALUE @max_tries = UNSET_VALUE + @retry_delay = UNSET_VALUE @timeout = UNSET_VALUE @transport = UNSET_VALUE @ssl_peer_verification = UNSET_VALUE @@ -31,20 +33,22 @@ module VagrantPlugins is_ssl = @transport == :ssl @port = (is_ssl ? 5986 : 5985) if @port == UNSET_VALUE @guest_port = (is_ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE - @max_tries = 20 if @max_tries == UNSET_VALUE - @timeout = 1800 if @timeout == UNSET_VALUE + @max_tries = 20 if @max_tries == UNSET_VALUE + @retry_delay = 2 if @retry_delay == UNSET_VALUE + @timeout = 1800 if @timeout == UNSET_VALUE @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE end def validate(machine) errors = [] - errors << "winrm.username cannot be nil." if @username.nil? - errors << "winrm.password cannot be nil." if @password.nil? - errors << "winrm.port cannot be nil." if @port.nil? - errors << "winrm.guest_port cannot be nil." if @guest_port.nil? - errors << "winrm.max_tries cannot be nil." if @max_tries.nil? - errors << "winrm.timeout cannot be nil." if @timeout.nil? + errors << "winrm.username cannot be nil." if @username.nil? + errors << "winrm.password cannot be nil." if @password.nil? + errors << "winrm.port cannot be nil." if @port.nil? + errors << "winrm.guest_port cannot be nil." if @guest_port.nil? + errors << "winrm.max_tries cannot be nil." if @max_tries.nil? + errors << "winrm.retry_delay cannot be nil." if @max_tries.nil? + errors << "winrm.timeout cannot be nil." if @timeout.nil? unless @ssl_peer_verification == true || @ssl_peer_verification == false errors << "winrm.ssl_peer_verification must be a boolean." end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index ae14e0f23..d9d5f28ce 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -88,7 +88,7 @@ module VagrantPlugins end def execute_shell_with_retry(command, shell, &block) - retryable(tries: @config.max_tries, on: @@exceptions_to_retry_on, sleep: 10) do + 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 @@ -124,7 +124,6 @@ module VagrantPlugins endpoint: endpoint, message: exception.message when WinRM::WinRMHTTPTransportError - case exception.status_code raise Errors::ExecutionError, shell: shell, command: command, diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 22a50072d..de7b1dc19 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -12,6 +12,8 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c| c.username = 'username' c.password = 'password' + c.max_tries = 3 + c.retry_delay = 0 c.finalize! end } @@ -28,10 +30,13 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do expect(subject.powershell("dir")[:exitcode]).to eq(0) end - it "should raise auth error when WinRM exception has a response code of 401" do - # The default settings might an account lockout - 20 auth failures! - expect(session).to receive(:powershell).with(/^dir.+/).exactly(20).times.and_raise( - WinRM::WinRMAuthorizationError.new("Oh no!!", 401)) + 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