From 80c05460ab20249ee3b6c44a0960ef48dc411fb2 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 16 Dec 2019 17:10:12 -0800 Subject: [PATCH 1/2] Call hooks before and after each action if they are available --- lib/vagrant/action/warden.rb | 9 ++ test/unit/vagrant/action/warden_test.rb | 120 +++++++++++++++--------- 2 files changed, 85 insertions(+), 44 deletions(-) diff --git a/lib/vagrant/action/warden.rb b/lib/vagrant/action/warden.rb index 222d19ea3..105b6f928 100644 --- a/lib/vagrant/action/warden.rb +++ b/lib/vagrant/action/warden.rb @@ -47,7 +47,16 @@ module Vagrant raise Errors::VagrantInterrupt if env[:interrupted] action = @actions.shift @logger.info("Calling IN action: #{action}") + + if !action.is_a?(Proc) && env[:hook] + hook_name = action.class.name.split("::").last. + gsub(/([a-z])([A-Z])/, '\1_\2').gsub('-', '_').downcase + end + + env[:hook].call("before_#{hook_name}".to_sym) if hook_name @stack.unshift(action).first.call(env) + env[:hook].call("after_#{hook_name}".to_sym) if hook_name + raise Errors::VagrantInterrupt if env[:interrupted] @logger.info("Calling OUT action: #{action}") rescue SystemExit diff --git a/test/unit/vagrant/action/warden_test.rb b/test/unit/vagrant/action/warden_test.rb index 9c668e371..d119f871a 100644 --- a/test/unit/vagrant/action/warden_test.rb +++ b/test/unit/vagrant/action/warden_test.rb @@ -1,6 +1,48 @@ require File.expand_path("../../../base", __FILE__) describe Vagrant::Action::Warden do + class ActionOne + def initialize(app, env) + @app = app + end + + def call(env) + @app.call(env) + end + + def recover(env) + env[:recover] << 1 + end + end + + class ActionTwo + def initialize(app, env) + @app = app + end + + def call(env) + @app.call(env) + end + + def recover(env) + env[:recover] << 2 + end + end + + class ExitAction + def initialize(app, env) + @app = app + end + + def call(env) + @app.call(env) + end + + def recover(env) + env[:recover] = true + end + end + let(:data) { { data: [] } } let(:instance) { described_class.new } @@ -18,38 +60,10 @@ describe Vagrant::Action::Warden do end it "starts a recovery sequence when an exception is raised" do - class Action - def initialize(app, env) - @app = app - end - - def call(env) - @app.call(env) - end - - def recover(env) - env[:recover] << 1 - end - end - - class ActionTwo - def initialize(app, env) - @app = app - end - - def call(env) - @app.call(env) - end - - def recover(env) - env[:recover] << 2 - end - end - error_proc = Proc.new { raise "ERROR!" } data = { recover: [] } - instance = described_class.new([Action, ActionTwo, error_proc], data) + instance = described_class.new([ActionOne, ActionTwo, error_proc], data) # The error should be raised back up expect { instance.call(data) }. @@ -63,25 +77,11 @@ describe Vagrant::Action::Warden do end it "does not do a recovery sequence if SystemExit is raised" do - class Action - def initialize(app, env) - @app = app - end - - def call(env) - @app.call(env) - end - - def recover(env) - env[:recover] = true - end - end - # Make a proc that just calls "abort" which raises a # SystemExit exception. error_proc = Proc.new { abort } - instance = described_class.new([Action, error_proc], data) + instance = described_class.new([ExitAction, error_proc], data) # The SystemExit should come through expect { instance.call(data) }.to raise_error(SystemExit) @@ -89,4 +89,36 @@ describe Vagrant::Action::Warden do # The recover should not have been called expect(data.key?(:recover)).not_to be end + + context "when hook is defined" do + let(:hook) { double("hook") } + + before do + data[:hook] = hook + allow(hook).to receive(:call) + end + + it "should receive a before hook call" do + expect(hook).to receive(:call).with(:before_action_one) + described_class.new([ActionOne], data).call(data) + end + + it "should receive an after hook call" do + expect(hook).to receive(:call).with(:after_action_one) + described_class.new([ActionOne], data).call(data) + end + + it "should not receive any hook calls for proc instances" do + expect(hook).not_to receive(:call) + described_class.new([proc{|*_| :testing }], data).call(data) + end + + it "should receive before and after calls for each class" do + expect(hook).to receive(:call).with(:before_action_one) + expect(hook).to receive(:call).with(:after_action_one) + expect(hook).to receive(:call).with(:before_action_two) + expect(hook).to receive(:call).with(:after_action_two) + described_class.new([ActionOne, proc{|*_| :testing }, ActionTwo], data).call(data) + end + end end From 25659a6f6bc09d3020912837736a76ed028610d9 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 16 Dec 2019 17:11:05 -0800 Subject: [PATCH 2/2] Support properly setting up synced folders on Catalina Since the root file system is marked as read-only, attempting to link the shared directory to `/vagrant` will fail. If the guest path is on the root file system and APFS is used, create the link as a firmlink instead. --- .../darwin/cap/mount_vmware_shared_folder.rb | 101 +++++++++-- .../cap/mount_vmware_shared_folder_test.rb | 167 ++++++++++++++++++ 2 files changed, 254 insertions(+), 14 deletions(-) create mode 100644 test/unit/plugins/guests/darwin/cap/mount_vmware_shared_folder_test.rb diff --git a/plugins/guests/darwin/cap/mount_vmware_shared_folder.rb b/plugins/guests/darwin/cap/mount_vmware_shared_folder.rb index cc9eff562..be4090aed 100644 --- a/plugins/guests/darwin/cap/mount_vmware_shared_folder.rb +++ b/plugins/guests/darwin/cap/mount_vmware_shared_folder.rb @@ -1,3 +1,5 @@ +require "securerandom" + module VagrantPlugins module GuestDarwin module Cap @@ -5,31 +7,102 @@ module VagrantPlugins # we seem to be unable to ask 'mount -t vmhgfs' to mount the roots # of specific shares, so instead we symlink from what is already - # mounted by the guest tools + # mounted by the guest tools # (ie. the behaviour of the VMware_fusion provider prior to 0.8.x) def self.mount_vmware_shared_folder(machine, name, guestpath, options) + # Use this variable to determine which machines + # have been registered with after hook + @apply_firmlinks ||= Hash.new{ |h, k| h[k] = {bootstrap: false, content: []} } + machine.communicate.tap do |comm| - # clear prior symlink - if comm.test("test -L \"#{guestpath}\"", sudo: true) - comm.sudo("rm -f \"#{guestpath}\"") + # check if we are dealing with an APFS root container + if comm.test("test -d /System/Volumes/Data") + parts = Pathname.new(guestpath).descend.to_a + firmlink = parts[1].to_s + firmlink.slice!(0, 1) if firmlink.start_with?("/") + if parts.size > 2 + guestpath = File.join("/System/Volumes/Data", guestpath) + else + guestpath = nil + end end - # clear prior directory if exists - if comm.test("test -d \"#{guestpath}\"", sudo: true) - comm.sudo("rm -Rf \"#{guestpath}\"") + # Remove existing symlink or directory if defined + if guestpath + if comm.test("test -L \"#{guestpath}\"") + comm.sudo("rm -f \"#{guestpath}\"") + elsif comm.test("test -d \"#{guestpath}\"") + comm.sudo("rm -Rf \"#{guestpath}\"") + end + + # create intermediate directories if needed + intermediate_dir = File.dirname(guestpath) + if intermediate_dir != "/" + comm.sudo("mkdir -p \"#{intermediate_dir}\"") + end + + comm.sudo("ln -s \"/Volumes/VMware Shared Folders/#{name}\" \"#{guestpath}\"") end - # create intermediate directories if needed - intermediate_dir = File.dirname(guestpath) - if !comm.test("test -d \"#{intermediate_dir}\"", sudo: true) - comm.sudo("mkdir -p \"#{intermediate_dir}\"") - end + if firmlink && !system_firmlink?(firmlink) + if guestpath.nil? + guestpath = "/Volumes/VMware Shared Folders/#{name}" + else + guestpath = File.join("/System/Volumes/Data", firmlink) + end - # finally make the symlink - comm.sudo("ln -s \"/Volumes/VMware Shared Folders/#{name}\" \"#{guestpath}\"") + share_line = "#{firmlink}\t#{guestpath}" + + # Check if the line is already defined. If so, bail since we are done + if !comm.test("[[ \"$( /etc/synthetic.conf") + if @apply_firmlinks[:bootstrap] + # Re-bootstrap the root container to pick up firmlink updates + comm.sudo("/System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B") + end + } + hook.prepend(action) + end + end + @apply_firmlinks[machine.id][:content] << share_line + end end end + + # Check if firmlink is provided by the system + # + # @param [String] firmlink Firmlink path + # @return [Boolean] + def self.system_firmlink?(firmlink) + if !@_firmlinks + if File.exist?("/usr/share/firmlinks") + @_firmlinks = File.readlines("/usr/share/firmlinks").map do |line| + line.split.first + end + else + @_firmlinks = [] + end + end + firmlink = "/#{firmlink}" if !firmlink.start_with?("/") + @_firmlinks.include?(firmlink) + end + + # @private + # Reset the cached values for capability. This is not considered a public + # API and should only be used for testing. + def self.reset! + instance_variables.each(&method(:remove_instance_variable)) + end end end end diff --git a/test/unit/plugins/guests/darwin/cap/mount_vmware_shared_folder_test.rb b/test/unit/plugins/guests/darwin/cap/mount_vmware_shared_folder_test.rb new file mode 100644 index 000000000..71dfbd829 --- /dev/null +++ b/test/unit/plugins/guests/darwin/cap/mount_vmware_shared_folder_test.rb @@ -0,0 +1,167 @@ +require_relative "../../../../base" + +describe "VagrantPlugins::GuestDarwin::Cap::MountVmwareSharedFolder" do + let(:described_class) do + VagrantPlugins::GuestDarwin::Plugin + .components + .guest_capabilities[:darwin] + .get(:mount_vmware_shared_folder) + end + + let(:machine) { double("machine", communicate: communicator, id: "MACHINE_ID") } + let(:communicator) { double("communicator") } + + before do + allow(communicator).to receive(:test) + allow(communicator).to receive(:sudo) + allow(VagrantPlugins::GuestDarwin::Plugin).to receive(:action_hook) + end + + describe ".mount_vmware_shared_folder" do + let(:name) { "-vagrant" } + let(:guestpath) { "/vagrant" } + let(:options) { {} } + + before do + allow(described_class).to receive(:system_firmlink?) + described_class.reset! + end + + after { described_class. + mount_vmware_shared_folder(machine, name, guestpath, options) } + + context "with APFS root container" do + before do + expect(communicator).to receive(:test).with("test -d /System/Volumes/Data").and_return(true) + end + + it "should check for existing entry" do + expect(communicator).to receive(:test).with(/synthetic\.conf/) + end + + it "should register an action hook" do + expect(VagrantPlugins::GuestDarwin::Plugin).to receive(:action_hook).with(:apfs_firmlinks, :after_synced_folders) + end + + context "with guest path within existing directory" do + let(:guestpath) { "/Users/vagrant/workspace" } + + it "should test if guest path is a symlink" do + expect(communicator).to receive(:test).with(/test -L/) + end + + it "should remove guest path if it is a symlink" do + expect(communicator).to receive(:test).with(/test -L/).and_return(true) + expect(communicator).to receive(:sudo).with(/rm -f/) + end + + it "should not test if guest path is a directory if guest path is symlink" do + expect(communicator).to receive(:test).with(/test -L/).and_return(true) + expect(communicator).not_to receive(:test).with(/test -d/) + end + + it "should test if guest path is directory if not a symlink" do + expect(communicator).to receive(:test).with(/test -d/) + end + + it "should remove guest path if it is a directory" do + expect(communicator).to receive(:test).with(/test -d/).and_return(true) + expect(communicator).to receive(:sudo).with(/rm -Rf/) + end + + it "should create the symlink to the vmware folder" do + expect(communicator).to receive(:sudo).with(/ln -s/) + end + + it "should create the symlink within the writable APFS container" do + expect(communicator).to receive(:sudo).with(%r{ln -s .+/System/Volumes/Data.+}) + end + + it "should register an action hook" do + expect(VagrantPlugins::GuestDarwin::Plugin).to receive(:action_hook).with(:apfs_firmlinks, :after_synced_folders) + end + + context "when firmlink is provided by the system" do + before { expect(described_class).to receive(:system_firmlink?).and_return(true) } + + it "should not register an action hook" do + expect(VagrantPlugins::GuestDarwin::Plugin).not_to receive(:action_hook).with(:apfs_firmlinks, :after_synced_folders) + end + end + end + end + + context "with non-APFS root container" do + before do + expect(communicator).to receive(:test).with("test -d /System/Volumes/Data").and_return(false) + end + + it "should test if guest path is a symlink" do + expect(communicator).to receive(:test).with(/test -L/) + end + + it "should remove guest path if it is a symlink" do + expect(communicator).to receive(:test).with(/test -L/).and_return(true) + expect(communicator).to receive(:sudo).with(/rm -f/) + end + + it "should not test if guest path is a directory if guest path is symlink" do + expect(communicator).to receive(:test).with(/test -L/).and_return(true) + expect(communicator).not_to receive(:test).with(/test -d/) + end + + it "should test if guest path is directory if not a symlink" do + expect(communicator).to receive(:test).with(/test -d/) + end + + it "should remove guest path if it is a directory" do + expect(communicator).to receive(:test).with(/test -d/).and_return(true) + expect(communicator).to receive(:sudo).with(/rm -Rf/) + end + + it "should create the symlink to the vmware folder" do + expect(communicator).to receive(:sudo).with(/ln -s/) + end + + it "should not register an action hook" do + expect(VagrantPlugins::GuestDarwin::Plugin).not_to receive(:action_hook).with(:apfs_firmlinks, :after_synced_folders) + end + end + end + + describe ".system_firmlink?" do + before { described_class.reset! } + + context "when file does not exist" do + before { allow(File).to receive(:exist?).with("/usr/share/firmlinks").and_return(false) } + + it "should always return false" do + expect(described_class.system_firmlink?("test")).to be_falsey + end + end + + context "when file does exist" do + let(:content) { + ["/Users\tUsers", + "/usr/local\tusr/local"] + } + + before do + expect(File).to receive(:exist?).with("/usr/share/firmlinks").and_return(true) + expect(File).to receive(:readlines).with("/usr/share/firmlinks").and_return(content) + end + + it "should return true when firmlink exists" do + expect(described_class.system_firmlink?("/Users")).to be_truthy + end + + it "should return true when firmlink is not prefixed with /" do + expect(described_class.system_firmlink?("Users")).to be_truthy + end + + it "should return false when firmlink does not exist" do + expect(described_class.system_firmlink?("/testing")).to be_falsey + end + end + end +end