From b3480049adccb14e9a78ac1b3c95074a3df06b69 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Tue, 22 Jul 2014 19:30:29 -0400 Subject: [PATCH] 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