diff --git a/plugins/communicators/ssh/communicator.rb b/plugins/communicators/ssh/communicator.rb index 9ef4b3e09..373db189a 100644 --- a/plugins/communicators/ssh/communicator.rb +++ b/plugins/communicators/ssh/communicator.rb @@ -9,6 +9,7 @@ require 'log4r' require 'net/ssh' require 'net/ssh/proxy/command' require 'net/scp' +require 'net/sftp' require 'vagrant/util/ansi_escape_code_remover' require 'vagrant/util/file_mode' @@ -290,15 +291,47 @@ module VagrantPlugins def upload(from, to) @logger.debug("Uploading: #{from} to #{to}") - scp_connect do |scp| - if File.directory?(from) - # Recursively upload directories - scp.upload!(from, to, recursive: true) + if File.directory?(from) + if from.end_with?(".") + @logger.debug("Uploading directory contents of: #{from}") + from = from.sub(/\.$/, "") else - # Open file read only to fix issue [GH-1036] - scp.upload!(File.open(from, "r"), to) + @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) + 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) + create_remote_directory(dest) + dest = File.join(to, File.basename(path)) + end + end + @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 @@ -731,6 +764,10 @@ module VagrantPlugins template.sub("%ENV_KEY%", env_key).sub("%ENV_VALUE%", env_value) + "\n" end + def create_remote_directory(dir) + execute("mkdir -p \"#{dir}\"") + end + def machine_config_ssh @machine.config.ssh end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index f515bed9e..61d09475c 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -108,6 +108,13 @@ module VagrantPlugins # @return [FixNum] Total size transfered from host to guest def upload(from, to) file_manager = WinRM::FS::FileManager.new(connection) + if from.is_a?(String) && File.directory?(from) + if from.end_with?(".") + from = from[0, from.length - 1] + else + to = File.join(to, File.basename(File.expand_path(from))) + end + end if from.is_a?(Array) # Preserve return FixNum of bytes transfered return_bytes = 0 diff --git a/plugins/provisioners/file/provisioner.rb b/plugins/provisioners/file/provisioner.rb index e763b54d2..4cc5d7888 100644 --- a/plugins/provisioners/file/provisioner.rb +++ b/plugins/provisioners/file/provisioner.rb @@ -6,40 +6,23 @@ module VagrantPlugins source = File.expand_path(config.source) destination = expand_guest_path(config.destination) - # if source is a directory, make it then trim destination with dirname - # Make sure the remote path exists + # If the source is a directory determine if any path modifications + # need to be applied to the source for upload behavior. If the original + # source value ends with a "." or if the original source does not end + # with a "." but the original destination ends with a file separator + # then append a "." character to the new source. This ensures that + # the contents of the directory are uploaded to the destination and + # not folder itself. if File.directory?(source) - # We need to make sure the actual destination folder - # also exists before uploading, otherwise - # you will get nested folders - # - # https://serverfault.com/questions/538368/make-scp-always-overwrite-or-create-directory - # https://unix.stackexchange.com/questions/292641/get-scp-path-behave-like-rsync-path/292732 - command = "mkdir -p \"%s\"" % destination - if !destination.end_with?(File::SEPARATOR) && - !source.end_with?("#{File::SEPARATOR}.") - # We also need to append a '/.' to the source folder so we copy - # the contents rather than the folder itself, in case a users - # destination folder differs from its source - # - # If source is set as `source/` it will lose the trailing - # slash due to how `File.expand_path` works, so we don't need - # a conditional for that case. - if @machine.config.vm.communicator == :winrm - # windows needs an array of paths because of the - # winrm-fs function Vagrant is using to upload file/folder. - source = Dir["#{source}#{File::SEPARATOR}*"] - else - source << "#{File::SEPARATOR}." - end + if config.source.end_with?(".") || + (!config.destination.end_with?(File::SEPARATOR) && + !config.source.end_with?("#{File::SEPARATOR}.")) + source = File.join(source, ".") end - else - command = "mkdir -p \"%s\"" % File.dirname(destination) end - comm.execute(command) @machine.ui.detail(I18n.t("vagrant.actions.vm.provision.file.locations", - src: source, dst: destination)) + src: config.source, dst: config.destination)) # now upload the file comm.upload(source, destination) end diff --git a/test/unit/plugins/communicators/ssh/communicator_test.rb b/test/unit/plugins/communicators/ssh/communicator_test.rb index d43cec3ea..0cde8b32d 100644 --- a/test/unit/plugins/communicators/ssh/communicator_test.rb +++ b/test/unit/plugins/communicators/ssh/communicator_test.rb @@ -497,12 +497,39 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do describe ".upload" do before do expect(communicator).to receive(:scp_connect).and_yield(scp) + allow(communicator).to receive(:create_remote_directory) end - it "uploads a directory if local path is a directory" do - Dir.mktmpdir('vagrant-test') do |dir| - expect(scp).to receive(:upload!).with(dir, '/destination', recursive: true) - communicator.upload(dir, '/destination') + context "directory uploads" do + let(:test_dir) { @dir } + let(:test_file) { File.join(test_dir, "test-file") } + let(:dir_name) { File.basename(test_dir) } + let(:file_name) { File.basename(test_file) } + + before do + @dir = Dir.mktmpdir("vagrant-test") + FileUtils.touch(test_file) + end + + after { FileUtils.rm_rf(test_dir) } + + it "uploads directory when directory path provided" do + expect(scp).to receive(:upload!).with(instance_of(File), + File.join("", "destination", dir_name, file_name)) + communicator.upload(test_dir, "/destination") + end + + it "uploads contents of directory when dot suffix provided on directory" do + expect(scp).to receive(:upload!).with(instance_of(File), + File.join("", "destination", file_name)) + communicator.upload(File.join(test_dir, "."), "/destination") + end + + it "creates directories before upload" do + expect(communicator).to receive(:create_remote_directory).with( + /#{Regexp.escape(File.join("", "destination", dir_name))}/) + allow(scp).to receive(:upload!) + communicator.upload(test_dir, "/destination") end end @@ -516,6 +543,17 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do end end + it "uploads file to directory if destination ends with file separator" do + file = Tempfile.new('vagrant-test') + begin + expect(scp).to receive(:upload!).with(instance_of(File), "/destination/dir/#{File.basename(file.path)}") + expect(communicator).to receive(:create_remote_directory).with("/destination/dir/") + communicator.upload(file.path, "/destination/dir/") + ensure + file.delete + end + end + it "raises custom error on permission errors" do file = Tempfile.new('vagrant-test') begin @@ -609,7 +647,7 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do end it "includes the default cipher array for encryption" do - cipher_array = %w(aes256-ctr aes192-ctr aes128-ctr + cipher_array = %w(aes256-ctr aes192-ctr aes128-ctr aes256-cbc aes192-cbc aes128-cbc rijndael-cbc@lysator.liu.se blowfish-ctr blowfish-cbc cast128-ctr cast128-cbc diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 0c1a57ca6..57874160d 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -33,13 +33,17 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do describe "#upload" do let(:fm) { double("file_manager") } + + before do + allow(WinRM::FS::FileManager).to receive(:new).with(connection) + .and_return(fm) + end + it "should call file_manager.upload for each passed in path" do from = ["/path", "/path/folder", "/path/folder/file.py"] to = "/destination" size = 80 - allow(WinRM::FS::FileManager).to receive(:new).with(connection) - .and_return(fm) allow(fm).to receive(:upload).and_return(size) expect(fm).to receive(:upload).exactly(from.size).times @@ -51,13 +55,34 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do to = "/destination" size = 80 - allow(WinRM::FS::FileManager).to receive(:new).with(connection) - .and_return(fm) allow(fm).to receive(:upload).and_return(size) expect(fm).to receive(:upload).exactly(1).times expect(subject.upload(from, to)).to eq(size) end + + context "when source is a directory" do + let(:source) { "path/sourcedir" } + + before do + allow(File).to receive(:directory?).with(/#{Regexp.escape(source)}/).and_return(true) + end + + it "should add source directory name to destination" do + expect(fm).to receive(:upload) do |from, to| + expect(to).to include("sourcedir") + end + subject.upload(source, "/dest") + end + + it "should not add source directory name to destination when source ends with '.'" do + source << "/." + expect(fm).to receive(:upload) do |from, to| + expect(to).to eq("/dest") + end + subject.upload(source, "/dest") + end + end end describe ".powershell" do diff --git a/test/unit/plugins/communicators/winssh/communicator_test.rb b/test/unit/plugins/communicators/winssh/communicator_test.rb index f0d36e825..8da9b2196 100644 --- a/test/unit/plugins/communicators/winssh/communicator_test.rb +++ b/test/unit/plugins/communicators/winssh/communicator_test.rb @@ -226,12 +226,14 @@ describe VagrantPlugins::CommunicatorWinSSH::Communicator do describe ".upload" do before do + allow(communicator).to receive(:create_remote_directory) expect(communicator).to receive(:scp_connect).and_yield(scp) end it "uploads a directory if local path is a directory" do Dir.mktmpdir('vagrant-test') do |dir| - expect(scp).to receive(:upload!).with(dir, 'C:\destination', recursive: true) + FileUtils.touch(File.join(dir, "test-file")) + expect(scp).to receive(:upload!).with(an_instance_of(File), /test-file/) communicator.upload(dir, 'C:\destination') end end diff --git a/test/unit/plugins/provisioners/file/provisioner_test.rb b/test/unit/plugins/provisioners/file/provisioner_test.rb index 0aedb8fe8..7f3c954ba 100644 --- a/test/unit/plugins/provisioners/file/provisioner_test.rb +++ b/test/unit/plugins/provisioners/file/provisioner_test.rb @@ -34,8 +34,6 @@ describe VagrantPlugins::FileUpload::Provisioner do allow(config).to receive(:source).and_return("/source") allow(config).to receive(:destination).and_return("/foo/bar") - expect(communicator).to receive(:execute).with("mkdir -p \"/foo\"") - subject.provision end @@ -43,8 +41,6 @@ describe VagrantPlugins::FileUpload::Provisioner do allow(config).to receive(:source).and_return("/source") allow(config).to receive(:destination).and_return("/foo bar/bar") - expect(communicator).to receive(:execute).with("mkdir -p \"/foo bar\"") - subject.provision end @@ -52,8 +48,6 @@ describe VagrantPlugins::FileUpload::Provisioner do allow(config).to receive(:source).and_return("/source/file.sh") allow(config).to receive(:destination).and_return("/foo/bar/file.sh") - expect(communicator).to receive(:execute).with("mkdir -p \"/foo/bar\"") - subject.provision end @@ -105,21 +99,12 @@ describe VagrantPlugins::FileUpload::Provisioner do subject.provision end - it "sends an array of files and folders if winrm and destination doesn't end with file separator" do - files = ["/source/file.py", "/source/folder"] - allow(Dir).to receive(:[]).and_return(files) - allow(config).to receive(:source).and_return("/source") - allow(config).to receive(:destination).and_return("/foo/bar") + it "appends a '/.' to expanded source if defined in original source" do + allow(config).to receive(:source).and_return("/source/.") allow(File).to receive(:directory?).with("/source").and_return(true) - allow(machine.config.vm).to receive(:communicator).and_return(:winrm) + allow(config).to receive(:destination).and_return("/foo/bar") - expect(guest).to receive(:capability?). - with(:shell_expand_guest_path).and_return(true) - expect(guest).to receive(:capability). - with(:shell_expand_guest_path, "/foo/bar").and_return("/foo/bar") - - expect(communicator).to receive(:upload) - .with(files, "/foo/bar") + expect(communicator).to receive(:upload).with("/source/.", "/foo/bar") subject.provision end