haskal
/
vagrant
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix issues with Windows SSH provisioner 

Windows commands that run over SSH are wrapped in a script that writes a
special marker to the two output streams (stdout and stderr).  This
allows Vagrant to consume the output streams.

Unfortunately, this leads to a sort of chicken-and-egg problem where no
commands can be run before a wrapper script exists. For example, you
can't make a destination directory to upload the wrapper script without
first creating a wrapper script to make the directory. :)

This commit changes the behavior of the WinSSH communicator to assume
that the destination directory already exists for provisioning scripts.

It also moves the default `upload_path` from the shell provisioner
config so we can have OS-specific defaults.

Finally, it introduces a Windows-specific #upload method which will
properly use a Windows path separator on a non-Windows host.
This commit is contained in:
Jeff Bonhag 2020-01-10 14:20:42 -05:00
parent f886184011
commit e672e5dc65
No known key found for this signature in database
GPG Key ID: 32966F3FB5AC1129
11 changed files with 237 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
lib/vagrant/util/platform.rb
View File

@ -213,13 +213,20 @@ module Vagrant
return path if !cygwin?
# Replace all "\" with "/", otherwise cygpath doesn't work.
path = path.gsub("\\", "/")
path = unix_windows_path(path)
# Call out to cygpath and gather the result
process = Subprocess.execute("cygpath", "-w", "-l", "-a", path.to_s)
return process.stdout.chomp
end
# This takes any path and converts Windows-style path separators
# to Unix-like path separators.
# @return [String]
def unix_windows_path(path)
path.gsub("\\", "/")
end
# This checks if the filesystem is case sensitive. This is not a
# 100% correct check, since it is possible that the temporary
# directory runs a different filesystem than the root directory.

22
plugins/communicators/ssh/communicator.rb
View File

@ -65,7 +65,7 @@ module VagrantPlugins
while true
ssh_info = @machine.ssh_info
break if ssh_info
sleep 0.5
sleep(0.5)
end
# Got it! Let the user know what we're connecting to.
@ -354,6 +354,11 @@ module VagrantPlugins
wait_for_ready(5)
end
def generate_environment_export(env_key, env_value)
template = machine_config_ssh.export_command_template
template.sub("%ENV_KEY%", env_key).sub("%ENV_VALUE%", env_value) + "\n"
end
protected
# Opens an SSH connection and yields it to a block.
@ -628,7 +633,7 @@ module VagrantPlugins
end
# Set the terminal
ch2.send_data generate_environment_export("TERM", "vt100")
ch2.send_data(generate_environment_export("TERM", "vt100"))
# Set SSH_AUTH_SOCK if we are in sudo and forwarding agent.
# This is to work around often misconfigured boxes where
@ -651,7 +656,7 @@ module VagrantPlugins
@logger.warn("No SSH_AUTH_SOCK found despite forward_agent being set.")
else
@logger.info("Setting SSH_AUTH_SOCK remotely: #{auth_socket}")
ch2.send_data generate_environment_export("SSH_AUTH_SOCK", auth_socket)
ch2.send_data(generate_environment_export("SSH_AUTH_SOCK", auth_socket))
end
end
@ -669,11 +674,11 @@ module VagrantPlugins
data += "printf #{PTY_DELIM_END}\n"
data += "exit $exitcode\n"
data = data.force_encoding('ASCII-8BIT')
ch2.send_data data
ch2.send_data(data)
else
ch2.send_data "printf '#{CMD_GARBAGE_MARKER}'\n(>&2 printf '#{CMD_GARBAGE_MARKER}')\n#{command}\n".force_encoding('ASCII-8BIT')
ch2.send_data("printf '#{CMD_GARBAGE_MARKER}'\n(>&2 printf '#{CMD_GARBAGE_MARKER}')\n#{command}\n".force_encoding('ASCII-8BIT'))
# Remember to exit or this channel will hang open
ch2.send_data "exit\n"
ch2.send_data("exit\n")
end
# Send eof to let server know we're done
@ -759,11 +764,6 @@ module VagrantPlugins
return File.read(path).chomp == source_path.read.chomp
end
def generate_environment_export(env_key, env_value)
template = machine_config_ssh.export_command_template
template.sub("%ENV_KEY%", env_key).sub("%ENV_VALUE%", env_value) + "\n"
end
def create_remote_directory(dir)
execute("mkdir -p \"#{dir}\"")
end

70
plugins/communicators/winssh/communicator.rb
View File

@ -54,7 +54,7 @@ SCRIPT
end
tfile.close
upload(tfile.path, remote_name)
upload(tfile.path, remote_name, mkdir: false)
tfile.delete
base_cmd = shell_cmd(opts.merge(shell: base_cmd))
@ -156,6 +156,74 @@ SCRIPT
@machine.config.winssh
end
def upload(from, to, **opts)
opts = {
mkdir: true
}.merge(opts)
mkdir = opts[:mkdir]
to = Vagrant::Util::Platform.unix_windows_path(to)
@logger.debug("Uploading: #{from} to #{to}")
if File.directory?(from)
if from.end_with?(".")
@logger.debug("Uploading directory contents of: #{from}")
from = from.sub(/\.$/, "")
else
@logger.debug("Uploading full directory container of: #{from}")
to = File.join(to, File.basename(File.expand_path(from)))
end
end
scp_connect do |scp|
uploader = lambda do |path, remote_dest=nil|
if File.directory?(path)
Dir.new(path).each do |entry|
next if entry == "." || entry == ".."
full_path = File.join(path, entry)
dest = File.join(to, path.sub(/^#{Regexp.escape(from)}/, ""))
create_remote_directory(dest) if mkdir
uploader.call(full_path, dest)
end
else
if remote_dest
dest = File.join(remote_dest, File.basename(path))
else
dest = to
if to.end_with?(File::SEPARATOR)
dest = File.join(to, File.basename(path))
end
end
@logger.debug("Ensuring remote directory exists for destination upload")
create_remote_directory(File.dirname(dest)) if mkdir
@logger.debug("Uploading file #{path} to remote #{dest}")
upload_file = File.open(path, "rb")
begin
scp.upload!(upload_file, dest)
ensure
upload_file.close
end
end
end
uploader.call(from)
end
rescue RuntimeError => e
# Net::SCP raises a runtime error for this so the only way we have
# to really catch this exception is to check the message to see if
# it is something we care about. If it isn't, we re-raise.
raise if e.message !~ /Permission denied/
# Otherwise, it is a permission denied, so let's raise a proper
# exception
raise Vagrant::Errors::SCPPermissionDenied,
from: from.to_s,
to: to.to_s
end
def create_remote_directory(dir)
execute("dir \"#{dir}\"\n if errorlevel 1 (mkdir \"#{dir}\")", shell: "cmd")
end
end
end
end

4
plugins/guests/windows/cap/public_key.rb
View File

@ -37,7 +37,7 @@ module VagrantPlugins
# Ensure the user's ssh directory exists
remote_ssh_dir = "#{home_dir}\\.ssh"
comm.execute("dir \"#{remote_ssh_dir}\"\n if errorlevel 1 (mkdir \"#{remote_ssh_dir}\")", shell: "cmd")
comm.create_remote_directory(remote_ssh_dir)
remote_upload_path = "#{temp_dir}\\vagrant-insert-pubkey-#{Time.now.to_i}"
remote_authkeys_path = "#{remote_ssh_dir}\\authorized_keys"
@ -55,7 +55,7 @@ module VagrantPlugins
File.write(keys_file.path, keys.join("\r\n") + "\r\n")
comm.upload(keys_file.path, remote_upload_path)
keys_file.delete
comm.execute <<-EOC.gsub(/^\s*/, ""), shell: "powershell"
comm.execute(<<-EOC.gsub(/^\s*/, ""), shell: "powershell")
Set-Acl "#{remote_upload_path}" (Get-Acl "#{remote_authkeys_path}")
Move-Item -Force "#{remote_upload_path}" "#{remote_authkeys_path}"
EOC

7
plugins/provisioners/shell/config.rb
View File

@ -55,7 +55,7 @@ module VagrantPlugins
@sha384 = nil if @sha384 == UNSET_VALUE
@sha512 = nil if @sha512 == UNSET_VALUE
@env = {} if @env == UNSET_VALUE
@upload_path = "/tmp/vagrant-shell" if @upload_path == UNSET_VALUE
@upload_path = nil if @upload_path == UNSET_VALUE
@privileged = true if @privileged == UNSET_VALUE
@binary = false if @binary == UNSET_VALUE
@keep_color = false if @keep_color == UNSET_VALUE
@ -109,11 +109,6 @@ module VagrantPlugins
errors << I18n.t("vagrant.provisioners.shell.env_must_be_a_hash")
end
# There needs to be a path to upload the script to
if !upload_path
errors << I18n.t("vagrant.provisioners.shell.upload_path_not_set")
end
if !args_valid?
errors << I18n.t("vagrant.provisioners.shell.args_bad_type")
end

26
plugins/provisioners/shell/provisioner.rb
View File

@ -38,6 +38,22 @@ module VagrantPlugins
end
end
def upload_path
if !defined?(@_upload_path)
@_upload_path = config.upload_path.to_s
if @_upload_path.empty?
case @machine.config.vm.communicator
when :winssh
@_upload_path = "C:\\Windows\\Temp\\vagrant-shell"
else
@_upload_path = "/tmp/vagrant-shell"
end
end
end
@_upload_path
end
protected
# This handles outputting the communication data back to the UI
@ -63,10 +79,10 @@ module VagrantPlugins
env = config.env.map { |k,v| "#{k}=#{quote_and_escape(v.to_s)}" }
env = env.join(" ")
command = "chmod +x '#{config.upload_path}'"
command = "chmod +x '#{upload_path}'"
command << " &&"
command << " #{env}" if !env.empty?
command << " #{config.upload_path}#{args}"
command << " #{upload_path}#{args}"
with_script_file do |path|
# Upload the script to the machine
@ -79,10 +95,10 @@ module VagrantPlugins
end
user = info[:username]
comm.sudo("chown -R #{user} #{config.upload_path}",
comm.sudo("chown -R #{user} #{upload_path}",
error_check: false)
comm.upload(path.to_s, config.upload_path)
comm.upload(path.to_s, upload_path)
if config.name
@machine.ui.detail(I18n.t("vagrant.provisioners.shell.running",
@ -114,7 +130,6 @@ module VagrantPlugins
# Upload the script to the machine
@machine.communicate.tap do |comm|
env = config.env.map{|k,v| comm.generate_environment_export(k, v)}.join
upload_path = config.upload_path.to_s
if File.extname(upload_path).empty?
remote_ext = @machine.config.winssh.shell == "powershell" ? "ps1" : "bat"
upload_path << ".#{remote_ext}"
@ -174,7 +189,6 @@ module VagrantPlugins
@machine.communicate.tap do |comm|
# Make sure that the upload path has an extension, since
# having an extension is critical for Windows execution
upload_path = config.upload_path.to_s
if File.extname(upload_path) == ""
upload_path += File.extname(path.to_s)
end

30
test/unit/plugins/communicators/winssh/communicator_test.rb
View File

@ -69,10 +69,10 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do
# by providing to `before`
connection_setup = proc do
allow(connection).to receive(:logger)
allow(connection).to receive(:closed?).and_return false
allow(connection).to receive(:closed?).and_return(false)
allow(connection).to receive(:open_channel).
and_yield(channel).and_return(channel)
allow(channel).to receive(:wait).and_return true
allow(channel).to receive(:wait).and_return(true)
allow(channel).to receive(:close)
allow(command_channel).to receive(:send_data)
allow(command_channel).to receive(:eof!)
@ -88,7 +88,7 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do
allow(channel).to receive(:on_request)
allow(channel).to receive(:on_process)
allow(channel).to receive(:exec).with(anything).
and_yield(command_channel, '').and_return channel
and_yield(command_channel, '').and_return(channel)
expect(command_channel).to receive(:on_request).with('exit-status').
and_yield(nil, exit_data)
# Return mocked net-ssh connection during setup
@ -132,11 +132,11 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do
context "with an invalid shell test" do
before do
expect(exit_data).to receive(:read_long).and_return 1
expect(exit_data).to receive(:read_long).and_return(1)
end
it "returns raises SSHInvalidShell error" do
expect{ communicator.ready? }.to raise_error Vagrant::Errors::SSHInvalidShell
expect{ communicator.ready? }.to raise_error(Vagrant::Errors::SSHInvalidShell)
end
end
end
@ -215,7 +215,7 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do
context "with exit code as non-zero" do
before do
expect(exit_data).to receive(:read_long).and_return 1
expect(exit_data).to receive(:read_long).and_return(1)
end
it "returns false" do
@ -224,7 +224,7 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do
end
end
describe ".upload" do
describe "#upload" do
before do
allow(communicator).to receive(:create_remote_directory)
expect(communicator).to receive(:scp_connect).and_yield(scp)
@ -241,7 +241,9 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do
it "uploads a file if local path is a file" do
file = Tempfile.new('vagrant-test')
begin
expect(scp).to receive(:upload!).with(instance_of(File), 'C:\destination\file')
expect(scp).to receive(:upload!).with(instance_of(File), 'C:/destination/file')
expect(Vagrant::Util::Platform).to receive(:unix_windows_path).with('C:\destination\file').
and_call_original
communicator.upload(file.path, 'C:\destination\file')
ensure
file.delete
@ -251,7 +253,7 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do
it "raises custom error on permission errors" do
file = Tempfile.new('vagrant-test')
begin
expect(scp).to receive(:upload!).with(instance_of(File), 'C:\destination\file').
expect(scp).to receive(:upload!).with(instance_of(File), 'C:/destination/file').
and_raise("Permission denied")
expect{ communicator.upload(file.path, 'C:\destination\file') }.to(
raise_error(Vagrant::Errors::SCPPermissionDenied)
@ -264,7 +266,7 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do
it "does not raise custom error on non-permission errors" do
file = Tempfile.new('vagrant-test')
begin
expect(scp).to receive(:upload!).with(instance_of(File), 'C:\destination\file').
expect(scp).to receive(:upload!).with(instance_of(File), 'C:/destination/file').
and_raise("Some other error")
expect{ communicator.upload(file.path, 'C:\destination\file') }.to raise_error(RuntimeError)
ensure
@ -525,4 +527,12 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do
end
end
end
describe "#shell_execute" do
before(&connection_setup)
it "should not create a directory for the command wrapper script" do
expect(communicator).to receive(:upload).with(anything, anything, hash_including(mkdir: false))
communicator.shell_execute(connection, "dir")
end
end
end

21
test/unit/plugins/provisioners/shell/config_test.rb
View File

@ -7,6 +7,7 @@ describe "VagrantPlugins::Shell::Config" do
let(:machine) { double('machine', env: Vagrant::Environment.new) }
let(:file_that_exists) { File.expand_path(__FILE__) }
let(:unset_value) { Vagrant::Plugin::V2::Config::UNSET_VALUE }
subject { described_class.new }
@ -142,6 +143,15 @@ describe "VagrantPlugins::Shell::Config" do
result = subject.validate(machine)
expect(result["shell provisioner"]).to be_empty
end
it "returns no error if upload_path is unset" do
subject.inline = "script"
subject.upload_path = unset_value
subject.finalize!
result = subject.validate(machine)
expect(result["shell provisioner"]).to be_empty
end
end
describe 'finalize!' do
@ -150,7 +160,7 @@ describe "VagrantPlugins::Shell::Config" do
subject.args = 1
subject.finalize!
expect(subject.args).to eq '1'
expect(subject.args).to eq('1')
end
it 'changes integer args in arrays into strings' do
@ -158,7 +168,14 @@ describe "VagrantPlugins::Shell::Config" do
subject.args = ["string", 1, 2]
subject.finalize!
expect(subject.args).to eq ["string", '1', '2']
expect(subject.args).to eq(["string", '1', '2'])
end
it "no longer sets a default for upload_path" do
subject.upload_path = unset_value
subject.finalize!
expect(subject.upload_path).to eq(nil)
end
context "with sensitive option enabled" do

78
test/unit/plugins/provisioners/shell/provisioner_test.rb
View File

@ -323,4 +323,82 @@ describe "Vagrant::Shell::Provisioner" do
end
end
end
describe "#upload_path" do
context "when upload path is not set" do
let(:vsp) {
VagrantPlugins::Shell::Provisioner.new(machine, config)
}
let(:config) {
double(
:config,
:args => "doesn't matter",
:env => {},
:upload_path => nil,
:remote? => false,
:path => "doesn't matter",
:inline => "doesn't matter",
:binary => false,
:reset => true,
:reboot => false,
)
}
it "should default to /tmp/vagrant-shell" do
expect(vsp.upload_path).to eq("/tmp/vagrant-shell")
end
context "with winssh provisioner" do
before do
allow(machine).to receive_message_chain(:config, :vm, :communicator).and_return(:winssh)
end
it "should default to C:\\Windows\\Temp\\vagrant-shell" do
expect(vsp.upload_path).to eq("C:\\Windows\\Temp\\vagrant-shell")
end
end
end
context "when upload_path is set" do
let(:config) {
double(
:config,
:args => "doesn't matter",
:env => {},
:upload_path => "arbitrary",
:remote? => false,
:path => "doesn't matter",
:inline => "doesn't matter",
:binary => false,
:reset => true,
:reboot => false,
)
}
let(:vsp) {
VagrantPlugins::Shell::Provisioner.new(machine, config)
}
it "should use the value from from config" do
expect(vsp.upload_path).to eq("arbitrary")
end
end
context "with cached value" do
let(:config) { double(:config) }
let(:vsp) {
VagrantPlugins::Shell::Provisioner.new(machine, config)
}
before do
vsp.instance_variable_set(:@_upload_path, "anything")
end
it "should use cached value" do
expect(vsp.upload_path).to eq("anything")
end
end
end
end

8
test/unit/vagrant/util/platform_test.rb
View File

@ -280,7 +280,7 @@ describe Vagrant::Util::Platform do
end
it "should return false when path is within /mnt" do
expect(subject.wsl_path?("/mnt/c")).to be false
expect(subject.wsl_path?("/mnt/c")).to be(false)
end
end
@ -533,5 +533,11 @@ EOF
expect(subject.wsl_drvfs_path?("/home/vagrant/some/path")).to be_falsey
end
end
describe ".unix_windows_path" do
it "takes a windows path and returns a unixy path" do
expect(subject.unix_windows_path("C:\\Temp\\Windows")).to eq("C:/Temp/Windows")
end
end
end
end

2
website/source/docs/vagrantfile/winssh_settings.html.md
View File

@ -74,3 +74,5 @@ being executed.
* `config.winssh.upload_directory` (string) - The upload directory used on the guest
to store scripts for execute. This is set to `C:\Windows\Temp` by default.
Vagrant will not attempt to create this directory, so it must already exist on
the guest.