Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow timeout option for WinRM commands #27

Merged
merged 2 commits into from
Sep 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions lib/train-winrm/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@

require "train"
require "train/plugins"
# This module may need to directly require WinRM to reference its exception classes
require "winrm" unless defined?(WinRM)

module TrainPlugins
module WinRM
Expand Down Expand Up @@ -108,15 +110,47 @@ def file_via_connection(path)
Train::File::Remote::Windows.new(self, path)
end

def run_command_via_connection(command, &data_handler)
def run_command_via_connection(command, opts = {}, &data_handler)
return if command.nil?

logger.debug("[WinRM] #{self} (#{command})")
out = ""
response = nil
james-stocks marked this conversation as resolved.
Show resolved Hide resolved
timeout = opts[:timeout]&.to_i

# Run the command in a thread, to support timing out the command
james-stocks marked this conversation as resolved.
Show resolved Hide resolved
thr = Thread.new do
# Surface any exceptions in this thread back to this method
Thread.current.report_on_exception = false
Thread.current.abort_on_exception = true
begin
response = session.run(command) do |stdout, _|
yield(stdout) if data_handler && stdout
out << stdout if stdout
end
rescue ::WinRM::WinRMHTTPTransportError => e
# If this command hits timeout, there is also a potential race in the HTTP transport
# where decryption is attempted on an empty message.
raise e unless timeout && e.to_s == "Could not decrypt NTLM message. ()."
rescue RuntimeError => e
# Ref: https://github.com/WinRb/WinRM/issues/315
# If this command hits timeout, calling close with the command currently running causes
# a RuntimeError error in WinRM's cleanup code. This specific error can be ignored.
# The command will be terminated and further commands can be sent on the connection.
raise e unless timeout && e.to_s == "opts[:shell_id] is required"
Copy link
Author

@james-stocks james-stocks Sep 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WinRb/WinRM#316 addresses this but is not yet released in a gem. Even once it is available, there's a chance there'll just be another exception we need to catch and dismiss.

end
end

response = session.run(command) do |stdout, _|
yield(stdout) if data_handler && stdout
out << stdout if stdout
if timeout
res = thr.join(timeout)
unless res
msg = "PowerShell command '(#{command})' reached timeout (#{timeout}s)"
logger.info("[WinRM] #{msg}")
close
raise Train::CommandTimeoutReached.new msg
end
else
thr.join
end

CommandResult.new(out, response.stderr, response.exitcode)
Expand Down
4 changes: 2 additions & 2 deletions test/unit/connection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
# We need to test run_command b/c run_command_via_connection is private.
winrm.run_command("test") do |data|
called = true
data.must_equal "testdata"
_(data).must_equal "testdata"
end
called.must_equal true
_(called).must_equal true
end
end

Expand Down
28 changes: 14 additions & 14 deletions test/unit/transport_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,47 +24,47 @@
let(:winrm) { cls.new({ host: "dummy", logger: Logger.new(STDERR, level: :info) }) }

it "can be instantiated (with valid config)" do
winrm.wont_be_nil
_(winrm).wont_be_nil
end

it "configures the host" do
winrm.options[:host].must_equal "dummy"
_(winrm.options[:host]).must_equal "dummy"
end

it "has default endpoint" do
winrm.options[:endpoint].must_be_nil
_(winrm.options[:endpoint]).must_be_nil
end

it "has default path set" do
winrm.options[:path].must_equal "/wsman"
_(winrm.options[:path]).must_equal "/wsman"
end

it "has default ssl set" do
winrm.options[:ssl].must_equal false
_(winrm.options[:ssl]).must_equal false
end

it "has default self_signed set" do
winrm.options[:self_signed].must_equal false
_(winrm.options[:self_signed]).must_equal false
end

it "has default rdp_port set" do
winrm.options[:rdp_port].must_equal 3389
_(winrm.options[:rdp_port]).must_equal 3389
end

it "has default winrm_transport set" do
winrm.options[:winrm_transport].must_equal :negotiate
_(winrm.options[:winrm_transport]).must_equal :negotiate
end

it "has default winrm_disable_sspi set" do
winrm.options[:winrm_disable_sspi].must_equal false
_(winrm.options[:winrm_disable_sspi]).must_equal false
end

it "has default winrm_basic_auth_only set" do
winrm.options[:winrm_basic_auth_only].must_equal false
_(winrm.options[:winrm_basic_auth_only]).must_equal false
end

it "has default user" do
winrm.options[:user].must_equal "administrator"
_(winrm.options[:user]).must_equal "administrator"
end
end

Expand All @@ -73,21 +73,21 @@
let(:connection) { winrm.connection }
it "without ssl genrates uri" do
conf[:host] = "dummy_host"
connection.uri.must_equal "winrm://administrator@http://dummy_host:5985/wsman:3389"
_(connection.uri).must_equal "winrm://administrator@http://dummy_host:5985/wsman:3389"
end

it "without ssl genrates uri" do
conf[:ssl] = true
conf[:host] = "dummy_host_ssl"
connection.uri.must_equal "winrm://administrator@https://dummy_host_ssl:5986/wsman:3389"
_(connection.uri).must_equal "winrm://administrator@https://dummy_host_ssl:5986/wsman:3389"
end
end

describe "options validation" do
let(:winrm) { cls.new(conf) }
it "raises an error when a non-supported winrm_transport is specificed" do
conf[:winrm_transport] = "invalid"
proc { winrm.connection }.must_raise Train::ClientError
_(proc { winrm.connection }).must_raise Train::ClientError
end
end
end