From afd2a28f6034cce84f197445a1c15a69a830c679 Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 30 Jul 2020 14:35:44 -0500 Subject: [PATCH 01/14] Add synced_folder_capability --- lib/vagrant/machine.rb | 3 +++ lib/vagrant/plugin/v2/components.rb | 6 ++++++ lib/vagrant/plugin/v2/manager.rb | 14 ++++++++++++++ lib/vagrant/plugin/v2/plugin.rb | 12 ++++++++++++ lib/vagrant/plugin/v2/synced_folder.rb | 8 ++++++++ plugins/kernel_v2/config/vm.rb | 16 +++++++++++++++- .../smb/cap/default_fstab_modification.rb | 11 +++++++++++ plugins/synced_folders/smb/plugin.rb | 5 +++++ 8 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 plugins/synced_folders/smb/cap/default_fstab_modification.rb diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index 981e9175a..f8e6bd6cb 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -1,4 +1,5 @@ require_relative "util/ssh" +require_relative "action/builtin/mixin_synced_folders" require "digest/md5" require "thread" @@ -10,6 +11,8 @@ module Vagrant # API for querying the state and making state changes to the machine, which # is backed by any sort of provider (VirtualBox, VMware, etc.). class Machine + include Vagrant::Action::Builtin::MixinSyncedFolders + # The box that is backing this machine. # # @return [Box] diff --git a/lib/vagrant/plugin/v2/components.rb b/lib/vagrant/plugin/v2/components.rb index d7c64d370..973c6cee2 100644 --- a/lib/vagrant/plugin/v2/components.rb +++ b/lib/vagrant/plugin/v2/components.rb @@ -64,6 +64,11 @@ module Vagrant # @return [Registry>] attr_reader :synced_folders + # This contains all the registered synced folder capabilities. + # + # @return [Hash] + attr_reader :synced_folder_capabilities + def initialize # The action hooks hash defaults to [] @action_hooks = Hash.new { |h, k| h[k] = [] } @@ -78,6 +83,7 @@ module Vagrant @provider_capabilities = Hash.new { |h, k| h[k] = Registry.new } @pushes = Registry.new @synced_folders = Registry.new + @synced_folder_capabilities = Hash.new { |h, k| h[k] = Registry.new } end end end diff --git a/lib/vagrant/plugin/v2/manager.rb b/lib/vagrant/plugin/v2/manager.rb index bba37ae04..0927958ad 100644 --- a/lib/vagrant/plugin/v2/manager.rb +++ b/lib/vagrant/plugin/v2/manager.rb @@ -257,7 +257,21 @@ module Vagrant end end end + + # This returns all the registered synced folder capabilities. + # + # @return [Hash] + def synced_folder_capabilities + results = Hash.new { |h, k| h[k] = Registry.new } + @registered.each do |plugin| + plugin.components.synced_folder_capabilities.each do |synced_folder, caps| + results[synced_folder].merge!(caps) + end + end + + results + end # This registers a plugin. This should _NEVER_ be called by the public # and should only be called from within Vagrant. Vagrant will # automatically register V2 plugins when a name is set on the diff --git a/lib/vagrant/plugin/v2/plugin.rb b/lib/vagrant/plugin/v2/plugin.rb index 9bc166268..7ec9f6644 100644 --- a/lib/vagrant/plugin/v2/plugin.rb +++ b/lib/vagrant/plugin/v2/plugin.rb @@ -247,6 +247,18 @@ module Vagrant nil end + # Defines a capability for the given synced folder. The block should return + # a class/module that has a method with the capability name, ready + # to be executed. This means that if it is an instance method, + # the block should return an instance of the class. + # + # @param [String] host The name of the synced folder + # @param [String] cap The name of the capability + def self.synced_folder_capability(synced_folder, cap, &block) + components.synced_folder_capabilities[synced_folder.to_sym].register(cap.to_sym, &block) + nil + end + # Returns the internal data associated with this plugin. This # should NOT be called by the general public. # diff --git a/lib/vagrant/plugin/v2/synced_folder.rb b/lib/vagrant/plugin/v2/synced_folder.rb index 9747b9cc3..06b4ec623 100644 --- a/lib/vagrant/plugin/v2/synced_folder.rb +++ b/lib/vagrant/plugin/v2/synced_folder.rb @@ -3,6 +3,8 @@ module Vagrant module V2 # This is the base class for a synced folder implementation. class SyncedFolder + include CapabilityHost + # This is called early when the synced folder is set to determine # if this implementation can be used for this machine. This should # return true or false. @@ -54,6 +56,12 @@ module Vagrant # @param [Hash] opts def cleanup(machine, opts) end + + def _initialize(machine, synced_folder_type) + plugins = Vagrant.plugin("2").manager.synced_folders + capabilities = Vagrant.plugin("2").manager.synced_folder_capabilities + initialize_capabilities!(synced_folder_type, plugins, capabilities, machine) + end end end end diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 14dc2de80..4bf64f355 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -508,7 +508,6 @@ module VagrantPlugins def finalize! # Defaults - @allow_fstab_modification = true if @allow_fstab_modification == UNSET_VALUE @allowed_synced_folder_types = nil if @allowed_synced_folder_types == UNSET_VALUE @base_mac = nil if @base_mac == UNSET_VALUE @base_address = nil if @base_address == UNSET_VALUE @@ -726,6 +725,21 @@ module VagrantPlugins def validate(machine, ignore_provider=nil) errors = _detected_errors + if @allow_fstab_modification == UNSET_VALUE + plugins = Vagrant.plugin("2").manager.synced_folders + machine.synced_folders.each do |type, _| + instance = plugins[type.to_sym][0].new + instance._initialize(machine, type) + if instance.capability?(:default_fstab_modification) + if instance.capability(:default_fstab_modification) == false + @allow_fstab_modification = false + break + end + end + end + @allow_fstab_modification = true if @allow_fstab_modification == UNSET_VALUE + end + if !box && !clone && !machine.provider_options[:box_optional] errors << I18n.t("vagrant.config.vm.box_missing") end diff --git a/plugins/synced_folders/smb/cap/default_fstab_modification.rb b/plugins/synced_folders/smb/cap/default_fstab_modification.rb new file mode 100644 index 000000000..12f54ef02 --- /dev/null +++ b/plugins/synced_folders/smb/cap/default_fstab_modification.rb @@ -0,0 +1,11 @@ +module VagrantPlugins + module SyncedFolderSMB + module Cap + module DefaultFstabModification + def self.default_fstab_modification(machine) + return false + end + end + end + end +end diff --git a/plugins/synced_folders/smb/plugin.rb b/plugins/synced_folders/smb/plugin.rb index bfb58a5b2..8115c377e 100644 --- a/plugins/synced_folders/smb/plugin.rb +++ b/plugins/synced_folders/smb/plugin.rb @@ -23,6 +23,11 @@ module VagrantPlugins SyncedFolder end + synced_folder_capability("smb", "default_fstab_modification") do + require_relative "cap/default_fstab_modification" + Cap::DefaultFstabModification + end + protected def self.init! From cde39e26ba0c9cdfe8ef308de5b9794a0cafbaa0 Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 30 Jul 2020 16:11:21 -0500 Subject: [PATCH 02/14] Make mount options a synced_folder capability --- .../action/builtin/mixin_synced_folders.rb | 6 ++++- lib/vagrant/machine.rb | 25 ++++++++++++++++++- .../cap/mount_virtualbox_shared_folder.rb | 2 +- .../linux/cap/persist_mount_shared_folder.rb | 4 +-- plugins/kernel_v2/config/vm.rb | 8 +++--- .../providers/virtualbox/cap/mount_options.rb | 23 +++++++++++++++++ plugins/providers/virtualbox/plugin.rb | 5 ++++ plugins/synced_folders/unix_mount_helpers.rb | 12 --------- 8 files changed, 63 insertions(+), 22 deletions(-) create mode 100644 plugins/providers/virtualbox/cap/mount_options.rb diff --git a/lib/vagrant/action/builtin/mixin_synced_folders.rb b/lib/vagrant/action/builtin/mixin_synced_folders.rb index 1209f5d69..ad7c4d528 100644 --- a/lib/vagrant/action/builtin/mixin_synced_folders.rb +++ b/lib/vagrant/action/builtin/mixin_synced_folders.rb @@ -36,7 +36,11 @@ module Vagrant # Find the proper implementation ordered.each do |_, key, impl| - return key if impl.new.usable?(machine) + begin + return key if impl.new.usable?(machine) + rescue + # Don't do anything. If an error is raised, the impl is not usable + end end return nil diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index f8e6bd6cb..7f57c8309 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -11,7 +11,7 @@ module Vagrant # API for querying the state and making state changes to the machine, which # is backed by any sort of provider (VirtualBox, VMware, etc.). class Machine - include Vagrant::Action::Builtin::MixinSyncedFolders + extend Vagrant::Action::Builtin::MixinSyncedFolders # The box that is backing this machine. # @@ -619,6 +619,29 @@ module Vagrant end end + # This returns the set of shared folders that should be done for + # this machine. It returns the folders in a hash keyed by the + # implementation class for the synced folders. + # + # @return [Hash>] + def synced_folders + self.class.synced_folders(self) + end + + # Returns the SyncedFolder object associated with this machine. + # + # @return [List] + def synced_folder_types + return @synced_folder_types if defined?(@synced_folder_types) + plugins = Vagrant.plugin("2").manager.synced_folders + @synced_folder_types = synced_folders.map { |type, folders| + impl = plugins[type][0].new() + impl._initialize(self, type) + [type, impl] + }.to_h + @synced_folder_types + end + protected # Returns the path to the file that stores the UID. diff --git a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb index a2a206745..fcc4b0343 100644 --- a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb +++ b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb @@ -16,7 +16,7 @@ module VagrantPlugins builtin_mount_type = "-cit #{VB_MOUNT_TYPE}" addon_mount_type = "-t #{VB_MOUNT_TYPE}" - mount_options, mount_uid, mount_gid = mount_options(machine, name, guest_path, options) + mount_options, mount_uid, mount_gid = machine.synced_folder_types[:virtualbox].capability(:mount_options, name, guestpath, options) mount_command = "mount #{addon_mount_type} -o #{mount_options} #{name} #{guest_path}" # Create the guest path if it doesn't exist diff --git a/plugins/guests/linux/cap/persist_mount_shared_folder.rb b/plugins/guests/linux/cap/persist_mount_shared_folder.rb index 076cd18b0..9e7dda16d 100644 --- a/plugins/guests/linux/cap/persist_mount_shared_folder.rb +++ b/plugins/guests/linux/cap/persist_mount_shared_folder.rb @@ -22,7 +22,7 @@ module VagrantPlugins end export_folders = fstab_folders.map do |name, data| guest_path = Shellwords.escape(data[:guestpath]) - mount_options, mount_uid, mount_gid = mount_options(machine, name, guest_path, data) + mount_options, mount_uid, mount_gid = machine.synced_folder_types[:virtualbox].capability(:mount_options, name, guest_path, data) mount_options = "#{mount_options},nofail" { name: name, @@ -45,4 +45,4 @@ module VagrantPlugins end end end -end \ No newline at end of file +end diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 4bf64f355..451fa77c6 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -727,11 +727,9 @@ module VagrantPlugins if @allow_fstab_modification == UNSET_VALUE plugins = Vagrant.plugin("2").manager.synced_folders - machine.synced_folders.each do |type, _| - instance = plugins[type.to_sym][0].new - instance._initialize(machine, type) - if instance.capability?(:default_fstab_modification) - if instance.capability(:default_fstab_modification) == false + machine.synced_folder_types.each do |_, inst| + if inst.capability?(:default_fstab_modification) + if inst.capability(:default_fstab_modification) == false @allow_fstab_modification = false break end diff --git a/plugins/providers/virtualbox/cap/mount_options.rb b/plugins/providers/virtualbox/cap/mount_options.rb new file mode 100644 index 000000000..1639ba8e9 --- /dev/null +++ b/plugins/providers/virtualbox/cap/mount_options.rb @@ -0,0 +1,23 @@ +require_relative "../../../synced_folders/unix_mount_helpers" + +module VagrantPlugins + module ProviderVirtualBox + module Cap + module MountOptions + extend VagrantPlugins::SyncedFolder::UnixMountHelpers + + def self.mount_options(machine, name, guest_path, options) + mount_options = options.fetch(:mount_options, []) + detected_ids = detect_owner_group_ids(machine, guest_path, mount_options, options) + mount_uid = detected_ids[:uid] + mount_gid = detected_ids[:gid] + + mount_options << "uid=#{mount_uid}" + mount_options << "gid=#{mount_gid}" + mount_options = mount_options.join(',') + return mount_options, mount_uid, mount_gid + end + end + end + end +end diff --git a/plugins/providers/virtualbox/plugin.rb b/plugins/providers/virtualbox/plugin.rb index 8a44e613e..e193040ca 100644 --- a/plugins/providers/virtualbox/plugin.rb +++ b/plugins/providers/virtualbox/plugin.rb @@ -68,6 +68,11 @@ module VagrantPlugins require_relative "cap" Cap end + + synced_folder_capability(:virtualbox, "mount_options") do + require_relative "cap/mount_options" + Cap::MountOptions + end end autoload :Action, File.expand_path("../action", __FILE__) diff --git a/plugins/synced_folders/unix_mount_helpers.rb b/plugins/synced_folders/unix_mount_helpers.rb index f49462bb0..d96077091 100644 --- a/plugins/synced_folders/unix_mount_helpers.rb +++ b/plugins/synced_folders/unix_mount_helpers.rb @@ -99,18 +99,6 @@ module VagrantPlugins fi EOH end - - def mount_options(machine, name, guest_path, options) - mount_options = options.fetch(:mount_options, []) - detected_ids = detect_owner_group_ids(machine, guest_path, mount_options, options) - mount_uid = detected_ids[:uid] - mount_gid = detected_ids[:gid] - - mount_options << "uid=#{mount_uid}" - mount_options << "gid=#{mount_gid}" - mount_options = mount_options.join(',') - return mount_options, mount_uid, mount_gid - end end end end From de28cec95f426ffe60bf1f271ccdb6b8026a47eb Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 30 Jul 2020 16:14:07 -0500 Subject: [PATCH 03/14] Add mount options cap for smb --- .../synced_folders/smb/cap/mount_options.rb | 36 +++++++++++++++++++ plugins/synced_folders/smb/plugin.rb | 5 +++ plugins/synced_folders/unix_mount_helpers.rb | 14 ++++++++ 3 files changed, 55 insertions(+) create mode 100644 plugins/synced_folders/smb/cap/mount_options.rb diff --git a/plugins/synced_folders/smb/cap/mount_options.rb b/plugins/synced_folders/smb/cap/mount_options.rb new file mode 100644 index 000000000..aa1687da5 --- /dev/null +++ b/plugins/synced_folders/smb/cap/mount_options.rb @@ -0,0 +1,36 @@ +require_relative "../../unix_mount_helpers" + +module VagrantPlugins + module SyncedFolderSMB + module Cap + module MountOptions + extend VagrantPlugins::SyncedFolder::UnixMountHelpers + + def self.mount_options(machine, name, guest_path, options) + mount_options = options.fetch(:mount_options, []) + detected_ids = detect_owner_group_ids(machine, guest_path, mount_options, options) + mount_uid = detected_ids[:uid] + mount_gid = detected_ids[:gid] + + mnt_opts = [] + if machine.env.host.capability?(:smb_mount_options) + mnt_opts += machine.env.host.capability(:smb_mount_options) + else + mnt_opts << "sec=ntlmssp" + end + + mnt_opts << "credentials=/etc/smb_creds_#{name}" + mnt_opts << "uid=#{mount_uid}" + mnt_opts << "gid=#{mount_gid}" + if !ENV['VAGRANT_DISABLE_SMBMFSYMLINKS'] + mnt_opts << "mfsymlinks" + end + mnt_opts = merge_mount_options(mnt_opts, options[:mount_options] || []) + + mount_options = mnt_opts.join(",") + return mount_options, mount_uid, mount_gid + end + end + end + end +end diff --git a/plugins/synced_folders/smb/plugin.rb b/plugins/synced_folders/smb/plugin.rb index 8115c377e..5fbe74b08 100644 --- a/plugins/synced_folders/smb/plugin.rb +++ b/plugins/synced_folders/smb/plugin.rb @@ -28,6 +28,11 @@ module VagrantPlugins Cap::DefaultFstabModification end + synced_folder_capability("smb", "mount_options") do + require_relative "cap/mount_options" + Cap::MountOptions + end + protected def self.init! diff --git a/plugins/synced_folders/unix_mount_helpers.rb b/plugins/synced_folders/unix_mount_helpers.rb index d96077091..2cb0932d7 100644 --- a/plugins/synced_folders/unix_mount_helpers.rb +++ b/plugins/synced_folders/unix_mount_helpers.rb @@ -99,6 +99,20 @@ module VagrantPlugins fi EOH end + + def merge_mount_options(base, overrides) + base = base.join(",").split(",") + overrides = overrides.join(",").split(",") + b_kv = Hash[base.map{|item| item.split("=", 2) }] + o_kv = Hash[overrides.map{|item| item.split("=", 2) }] + merged = {}.tap do |opts| + (b_kv.keys + o_kv.keys).uniq.each do |key| + opts[key] = o_kv.fetch(key, b_kv[key]) + end + end + merged.map do |key, value| + [key, value].compact.join("=") + end end end end From faad9f27174a07afa40b1affc5313a16de91b092 Mon Sep 17 00:00:00 2001 From: sophia Date: Fri, 31 Jul 2020 11:56:30 -0500 Subject: [PATCH 04/14] Add tests --- lib/vagrant/plugin/v2/plugin.rb | 2 +- plugins/synced_folders/unix_mount_helpers.rb | 1 + .../mount_virtual_box_shared_folder_test.rb | 141 +------------- .../cap/persist_mount_shared_folder_test.rb | 15 +- test/unit/plugins/kernel_v2/config/vm_test.rb | 11 +- .../virtualbox/cap/mount_options_test.rb | 179 ++++++++++++++++++ .../smb/caps/mount_options_test.rb | 67 +++++++ test/unit/vagrant/machine_test.rb | 11 ++ test/unit/vagrant/plugin/v2/manager_test.rb | 18 ++ 9 files changed, 301 insertions(+), 144 deletions(-) create mode 100644 test/unit/plugins/providers/virtualbox/cap/mount_options_test.rb create mode 100644 test/unit/plugins/synced_folders/smb/caps/mount_options_test.rb diff --git a/lib/vagrant/plugin/v2/plugin.rb b/lib/vagrant/plugin/v2/plugin.rb index 7ec9f6644..84f1c707a 100644 --- a/lib/vagrant/plugin/v2/plugin.rb +++ b/lib/vagrant/plugin/v2/plugin.rb @@ -252,7 +252,7 @@ module Vagrant # to be executed. This means that if it is an instance method, # the block should return an instance of the class. # - # @param [String] host The name of the synced folder + # @param [String] synced_folder The name of the synced folder # @param [String] cap The name of the capability def self.synced_folder_capability(synced_folder, cap, &block) components.synced_folder_capabilities[synced_folder.to_sym].register(cap.to_sym, &block) diff --git a/plugins/synced_folders/unix_mount_helpers.rb b/plugins/synced_folders/unix_mount_helpers.rb index 2cb0932d7..a6921ce29 100644 --- a/plugins/synced_folders/unix_mount_helpers.rb +++ b/plugins/synced_folders/unix_mount_helpers.rb @@ -113,6 +113,7 @@ module VagrantPlugins merged.map do |key, value| [key, value].compact.join("=") end + end end end end diff --git a/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb b/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb index e46bfe164..fcaabf5d9 100644 --- a/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb +++ b/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb @@ -23,9 +23,11 @@ describe "VagrantPlugins::GuestLinux::Cap::MountVirtualBoxSharedFolder" do } end let(:cap){ caps.get(:mount_virtualbox_shared_folder) } + let(:mount_options_cap){ double("opts") } before do allow(machine).to receive(:communicate).and_return(comm) + allow(machine).to receive(:synced_folder_types).and_return( { :virtualbox => mount_options_cap } ) end after do @@ -40,159 +42,27 @@ describe "VagrantPlugins::GuestLinux::Cap::MountVirtualBoxSharedFolder" do end it "generates the expected default mount command" do - expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) - expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") + expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_uid},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) end it "automatically chown's the mounted directory on guest" do - expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) - expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") + expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_uid},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) expect(comm).to receive(:sudo).with("chown #{mount_uid}:#{mount_gid} #{mount_guest_path}") cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) end - context "with owner user ID explicitly defined" do - - before do - expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") - end - - context "with user ID provided as Integer" do - let(:mount_owner){ 2000 } - - it "generates the expected mount command using mount_owner directly" do - expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_owner},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) - expect(comm).to receive(:sudo).with("chown #{mount_owner}:#{mount_gid} #{mount_guest_path}") - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - end - - context "with user ID provided as String" do - let(:mount_owner){ "2000" } - - it "generates the expected mount command using mount_owner directly" do - expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_owner},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) - expect(comm).to receive(:sudo).with("chown #{mount_owner}:#{mount_gid} #{mount_guest_path}") - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - end - - end - - context "with owner group ID explicitly defined" do - - before do - expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) - end - - context "with owner group ID provided as Integer" do - let(:mount_group){ 2000 } - - it "generates the expected mount command using mount_group directly" do - expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_uid},gid=#{mount_group} #{mount_name} #{mount_guest_path}", anything) - expect(comm).to receive(:sudo).with("chown #{mount_uid}:#{mount_group} #{mount_guest_path}") - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - end - - context "with owner group ID provided as String" do - let(:mount_group){ "2000" } - - it "generates the expected mount command using mount_group directly" do - expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_uid},gid=#{mount_group} #{mount_name} #{mount_guest_path}", anything) - expect(comm).to receive(:sudo).with("chown #{mount_uid}:#{mount_group} #{mount_guest_path}") - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - end - - end - - context "with non-existent default owner group" do - - it "fetches the effective group ID of the user" do - expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) - expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_raise(Vagrant::Errors::VirtualBoxMountFailed, {command: '', output: ''}) - expect(comm).to receive(:execute).with("id -g #{mount_owner}", anything).and_yield(:stdout, "1").and_return(0) - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - end - - context "with non-existent owner group" do - - it "raises an error" do - expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) - expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_raise(Vagrant::Errors::VirtualBoxMountFailed, {command: '', output: ''}) - expect do - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end.to raise_error Vagrant::Errors::VirtualBoxMountFailed - end - end - - context "with read-only option defined" do - - it "does not chown mounted guest directory" do - expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) - expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") - expect(comm).to receive(:sudo).with("mount -t vboxsf -o ro,uid=#{mount_uid},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) - expect(comm).not_to receive(:sudo).with("chown #{mount_uid}:#{mount_gid} #{mount_guest_path}") - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options.merge(mount_options: ["ro"])) - end - end - context "with upstart init" do it "emits mount event" do expect(comm).to receive(:sudo).with(/initctl emit/) + expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) end end - context "with custom mount options" do - - let(:ui){ double(:ui) } - before do - allow(ui).to receive(:warn) - allow(machine).to receive(:ui).and_return(ui) - end - - context "with uid defined" do - let(:options_uid){ '1234' } - - it "should only include uid defined within mount options" do - expect(comm).not_to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) - expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") - expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{options_uid},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options.merge(mount_options: ["uid=#{options_uid}"])) - end - end - - context "with gid defined" do - let(:options_gid){ '1234' } - - it "should only include gid defined within mount options" do - expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) - expect(comm).not_to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") - expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_uid},gid=#{options_gid} #{mount_name} #{mount_guest_path}", anything) - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options.merge(mount_options: ["gid=#{options_gid}"])) - end - end - - context "with uid and gid defined" do - let(:options_gid){ '1234' } - let(:options_uid){ '1234' } - - it "should only include uid and gid defined within mount options" do - expect(comm).not_to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) - expect(comm).not_to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{options_gid}:") - expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{options_uid},gid=#{options_gid} #{mount_name} #{mount_guest_path}", anything) - cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options.merge(mount_options: ["gid=#{options_gid}", "uid=#{options_uid}"])) - end - end - end - context "with guest builtin vboxsf module" do let(:vbox_stderr){ <<-EOF mount.vboxsf cannot be used with mainline vboxsf; instead use: @@ -201,6 +71,7 @@ mount.vboxsf cannot be used with mainline vboxsf; instead use: EOF } it "should perform guest mount using builtin module" do + expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) expect(comm).to receive(:sudo).with(/mount -t vboxsf/, any_args).and_yield(:stderr, vbox_stderr).and_return(1) expect(comm).to receive(:sudo).with(/mount -cit/, any_args) cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) diff --git a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb index f4d6f58c7..bf3eacffb 100644 --- a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb +++ b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb @@ -9,10 +9,15 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do let(:machine) { double("machine") } let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) } + let(:options_gid){ '1234' } + let(:options_uid){ '1234' } let(:cap){ caps.get(:persist_mount_shared_folder) } + let(:mount_options_cap){ double("opts") } before do allow(machine).to receive(:communicate).and_return(comm) + allow(machine).to receive(:synced_folder_types).and_return( { :virtualbox => mount_options_cap } ) + allow(mount_options_cap).to receive(:capability).with(:mount_options, any_args).and_return(["uid=#{options_uid},gid=#{options_gid}", options_uid, options_gid]) end after do @@ -20,12 +25,10 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do end describe ".persist_mount_shared_folder" do - let(:options_gid){ '1234' } - let(:options_uid){ '1234' } let (:fstab_folders) { [ - ["test1", {:guestpath=>"/test1", :hostpath=>"/my/host/path", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=1234", "gid=1234"] }], - ["vagrant", {:guestpath=>"/vagrant", :hostpath=>"/my/host/vagrant", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=1234", "gid=1234"] }] + ["test1", {:guestpath=>"/test1", :hostpath=>"/my/host/path", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=#{options_uid}", "gid=#{options_gid}"] }], + ["vagrant", {:guestpath=>"/vagrant", :hostpath=>"/my/host/vagrant", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=#{options_uid}", "gid=#{options_gid}"] }] ]} let(:ui){ double(:ui) } @@ -37,8 +40,8 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do end it "inserts folders into /etc/fstab" do - expected_entry_vagrant = "vagrant /vagrant vboxsf uid=1234,gid=1234,nofail 0 0" - expected_entry_test = "test1 /test1 vboxsf uid=1234,gid=1234,nofail 0 0" + expected_entry_vagrant = "vagrant /vagrant vboxsf uid=#{options_uid},gid=#{options_gid},nofail 0 0" + expected_entry_test = "test1 /test1 vboxsf uid=#{options_uid},gid=#{options_gid},nofail 0 0" expect(cap).to receive(:remove_vagrant_managed_fstab) expect(comm).to receive(:sudo).with(/#{expected_entry_test}\n#{expected_entry_vagrant}/) cap.persist_mount_shared_folder(machine, fstab_folders, "vboxsf") diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index ebcef38a3..0084d7c95 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -37,7 +37,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do allow(machine).to receive(:env).and_return(env) allow(machine).to receive(:provider_config).and_return(nil) allow(machine).to receive(:provider_options).and_return({}) - + allow(machine).to receive(:synced_folder_types).and_return([]) allow(provider).to receive(:capability?).with(:validate_disk_ext).and_return(true) allow(provider).to receive(:capability).with(:validate_disk_ext, "vdi").and_return(true) allow(provider).to receive(:capability?).with(:set_default_disk_ext).and_return(true) @@ -51,7 +51,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do assert_valid end - it "validates disables_host_modification option" do + it "validates disables_host_modification option" do subject.allow_hosts_modification = true subject.finalize! assert_valid @@ -65,6 +65,13 @@ describe VagrantPlugins::Kernel_V2::VMConfig do assert_invalid end + it "does not check for fstab caps if already set" do + expect(machine).to_not receive(:synced_folder_types) + subject.allow_fstab_modification = true + subject.finalize! + assert_valid + end + describe "#base_mac" do it "defaults properly" do subject.finalize! diff --git a/test/unit/plugins/providers/virtualbox/cap/mount_options_test.rb b/test/unit/plugins/providers/virtualbox/cap/mount_options_test.rb new file mode 100644 index 000000000..430c409c4 --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/cap/mount_options_test.rb @@ -0,0 +1,179 @@ +require_relative "../../../../base" + +describe "VagrantPlugins::ProviderVirtualBox::Cap::MountOptions" do + let(:caps) do + VagrantPlugins::ProviderVirtualBox::Plugin + .components + .synced_folder_capabilities[:virtualbox] + end + + let(:machine) { double("machine") } + let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) } + let(:mount_owner){ "vagrant" } + let(:mount_group){ "vagrant" } + let(:mount_uid){ "1000" } + let(:mount_gid){ "1000" } + let(:mount_name){ "vagrant" } + let(:mount_guest_path){ "/vagrant" } + let(:folder_options) do + { + owner: mount_owner, + group: mount_group, + hostpath: "/host/directory/path" + } + end + let(:cap){ caps.get(:mount_options) } + + before do + allow(machine).to receive(:communicate).and_return(comm) + end + + after do + comm.verify_expectations! + end + + describe ".mount_options" do + + before do + allow(comm).to receive(:sudo).with(any_args) + allow(comm).to receive(:execute).with(any_args) + end + + context "with owner user ID explicitly defined" do + + before do + expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") + end + + context "with user ID provided as Integer" do + let(:mount_owner){ 2000 } + it "generates the expected mount command using mount_owner directly" do + out_mount_options, out_mount_uid, out_mount_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + expect(out_mount_options).to eq("uid=#{mount_owner},gid=#{mount_gid}") + expect(out_mount_uid).to eq(mount_owner) + expect(out_mount_gid).to eq(mount_gid) + end + end + + context "with user ID provided as String" do + let(:mount_owner){ "2000" } + it "generates the expected mount command using mount_owner directly" do + out_mount_options, out_mount_uid, out_mount_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + expect(out_mount_options).to eq("uid=#{mount_owner},gid=#{mount_gid}") + expect(out_mount_uid).to eq(mount_owner) + expect(out_mount_gid).to eq(mount_gid) + end + end + + end + + context "with owner group ID explicitly defined" do + + before do + expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + end + + context "with owner group ID provided as Integer" do + let(:mount_group){ 2000 } + + it "generates the expected mount command using mount_group directly" do + out_mount_options, out_mount_uid, out_mount_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + expect(out_mount_options).to eq("uid=#{mount_uid},gid=#{mount_group}") + expect(out_mount_uid).to eq(mount_uid) + expect(out_mount_gid).to eq(mount_group) + end + end + + context "with owner group ID provided as String" do + let(:mount_group){ "2000" } + + it "generates the expected mount command using mount_group directly" do + out_mount_options, out_mount_uid, out_mount_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + expect(out_mount_options).to eq("uid=#{mount_uid},gid=#{mount_group}") + expect(out_mount_uid).to eq(mount_uid) + expect(out_mount_gid).to eq(mount_group) + end + end + + end + + context "with non-existent default owner group" do + it "fetches the effective group ID of the user" do + expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_raise(Vagrant::Errors::VirtualBoxMountFailed, {command: '', output: ''}) + expect(comm).to receive(:execute).with("id -g #{mount_owner}", anything).and_yield(:stdout, "1").and_return(0) + out_mount_options, out_mount_uid, out_mount_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + expect(out_mount_options).to eq("uid=#{mount_uid},gid=1") + end + end + + context "with non-existent owner group" do + it "raises an error" do + expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_raise(Vagrant::Errors::VirtualBoxMountFailed, {command: '', output: ''}) + expect do + cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + end.to raise_error Vagrant::Errors::VirtualBoxMountFailed + end + end + + context "with read-only option defined" do + it "does not chown mounted guest directory" do + expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") + out_mount_options, out_mount_uid, out_mount_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options.merge(mount_options: ["ro"])) + expect(out_mount_options).to eq("ro,uid=#{mount_uid},gid=#{mount_gid}") + expect(out_mount_uid).to eq(mount_uid) + expect(out_mount_gid).to eq(mount_gid) + end + end + + context "with custom mount options" do + let(:ui){ double(:ui) } + before do + allow(ui).to receive(:warn) + allow(machine).to receive(:ui).and_return(ui) + end + + context "with uid defined" do + let(:options_uid){ '1234' } + + it "should only include uid defined within mount options" do + expect(comm).not_to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") + out_mount_options, out_mount_uid, out_mount_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options.merge(mount_options: ["uid=#{options_uid}"]) ) + expect(out_mount_options).to eq("uid=#{options_uid},gid=#{mount_gid}") + expect(out_mount_uid).to eq(options_uid) + expect(out_mount_gid).to eq(mount_gid) + end + end + + context "with gid defined" do + let(:options_gid){ '1234' } + + it "should only include gid defined within mount options" do + expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + expect(comm).not_to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") + out_mount_options, out_mount_uid, out_mount_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options.merge(mount_options: ["gid=#{options_gid}"]) ) + expect(out_mount_options).to eq("uid=#{mount_uid},gid=#{options_gid}") + expect(out_mount_uid).to eq(mount_uid) + expect(out_mount_gid).to eq(options_gid) + end + end + + context "with uid and gid defined" do + let(:options_gid){ '1234' } + let(:options_uid){ '1234' } + + it "should only include uid and gid defined within mount options" do + expect(comm).not_to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + expect(comm).not_to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{options_gid}:") + out_mount_options, out_mount_uid, out_mount_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options.merge(mount_options: ["uid=#{options_uid}", "gid=#{options_gid}"]) ) + expect(out_mount_options).to eq("uid=#{options_uid},gid=#{options_gid}") + expect(out_mount_uid).to eq(options_uid) + expect(out_mount_gid).to eq(options_gid) + end + end + end + end +end diff --git a/test/unit/plugins/synced_folders/smb/caps/mount_options_test.rb b/test/unit/plugins/synced_folders/smb/caps/mount_options_test.rb new file mode 100644 index 000000000..90f6472a5 --- /dev/null +++ b/test/unit/plugins/synced_folders/smb/caps/mount_options_test.rb @@ -0,0 +1,67 @@ +require_relative "../../../../base" + +require_relative "../../../../../../plugins/synced_folders/smb/cap/mount_options" + +describe VagrantPlugins::SyncedFolderSMB::Cap::MountOptions do + + let(:caps) do + VagrantPlugins::SyncedFolderSMB::Plugin + .components + .synced_folder_capabilities[:smb] + end + let(:cap){ caps.get(:mount_options) } + + let(:machine) { double("machine") } + let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) } + let(:mount_owner){ "vagrant" } + let(:mount_group){ "vagrant" } + let(:mount_uid){ "1000" } + let(:mount_gid){ "1000" } + let(:mount_name){ "vagrant" } + let(:mount_guest_path){ "/vagrant" } + let(:folder_options) do + { + owner: mount_owner, + group: mount_group, + hostpath: "/host/directory/path" + } + end + + before do + allow(machine).to receive(:communicate).and_return(comm) + allow(machine).to receive_message_chain(:env, :host, :capability?).with(:smb_mount_options).and_return(false) + ENV['VAGRANT_DISABLE_SMBMFSYMLINKS'] = "1" + end + + describe ".mount_options" do + it "generates the expected default mount command" do + expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") + out_opts, out_uid, out_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + expect(out_opts).to eq("sec=ntlmssp,credentials=/etc/smb_creds_vagrant,uid=1000,gid=1000") + expect(out_uid).to eq(mount_uid) + expect(out_gid).to eq(mount_gid) + end + + it "includes provided mount options" do + expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_yield(:stdout, "vagrant:x:#{mount_gid}:") + folder_options[:mount_options] =["ro"] + out_opts, out_uid, out_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + expect(out_opts).to eq("sec=ntlmssp,credentials=/etc/smb_creds_vagrant,uid=1000,gid=1000,ro") + expect(out_uid).to eq(mount_uid) + expect(out_gid).to eq(mount_gid) + end + + context "with non-existent owner group" do + it "raises an error" do + expect(comm).to receive(:execute).with("id -u #{mount_owner}", anything).and_yield(:stdout, mount_uid) + expect(comm).to receive(:execute).with("id -g #{mount_group}", anything).and_yield(:stdout, mount_gid) + expect(comm).to receive(:execute).with("getent group #{mount_group}", anything).and_raise(Vagrant::Errors::VirtualBoxMountFailed, {command: '', output: ''}) + expect do + cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + end.to raise_error Vagrant::Errors::VirtualBoxMountFailed + end + end + end +end diff --git a/test/unit/vagrant/machine_test.rb b/test/unit/vagrant/machine_test.rb index 913dc5e9c..c2003a87a 100644 --- a/test/unit/vagrant/machine_test.rb +++ b/test/unit/vagrant/machine_test.rb @@ -1006,5 +1006,16 @@ describe Vagrant::Machine do end end end + + describe "#synced_folder_types" do + it "returns a map of initialized synced folders" do + allow(instance).to receive(:synced_folders).and_return( {:smb => {}, :virtualbox => {}} ) + folders = instance.synced_folder_types + folders.each do |k, v| + expect([:smb, :virtualbox]).to include(k) + expect(v.respond_to?(:capability)).to eq(true) + end + end + end end end diff --git a/test/unit/vagrant/plugin/v2/manager_test.rb b/test/unit/vagrant/plugin/v2/manager_test.rb index c145b4b28..5dbcc11a5 100644 --- a/test/unit/vagrant/plugin/v2/manager_test.rb +++ b/test/unit/vagrant/plugin/v2/manager_test.rb @@ -357,4 +357,22 @@ describe Vagrant::Plugin::V2::Manager do expect(instance.synced_folders[:foo]).to eq(["bar", 10]) expect(instance.synced_folders[:bar]).to eq(["baz", 50]) end + + it "should enumerate registered synced_folder_capabilities classes" do + pA = plugin do |p| + p.synced_folder_capability("foo", "foo") { "bar" } + end + + pB = plugin do |p| + p.synced_folder_capability("bar", "bar") { "baz" } + end + + instance.register(pA) + instance.register(pB) + + expect(instance.synced_folder_capabilities.to_hash.length).to eq(2) + expect(instance.synced_folder_capabilities[:foo][:foo]).to eq("bar") + expect(instance.synced_folder_capabilities[:bar][:bar]).to eq("baz") + end + end From 2239ee8582c9f81bb7805285529ee9b811a95ff8 Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 11 Aug 2020 09:44:12 -0500 Subject: [PATCH 05/14] Create Hash with type attribute --- .../action/builtin/mixin_synced_folders.rb | 15 +++++++++------ lib/vagrant/machine.rb | 14 -------------- lib/vagrant/util/typed_hash.rb | 10 ++++++++++ .../linux/cap/mount_virtualbox_shared_folder.rb | 2 +- .../linux/cap/persist_mount_shared_folder.rb | 2 +- plugins/kernel_v2/config/vm.rb | 2 +- 6 files changed, 22 insertions(+), 23 deletions(-) create mode 100644 lib/vagrant/util/typed_hash.rb diff --git a/lib/vagrant/action/builtin/mixin_synced_folders.rb b/lib/vagrant/action/builtin/mixin_synced_folders.rb index ad7c4d528..725a062cf 100644 --- a/lib/vagrant/action/builtin/mixin_synced_folders.rb +++ b/lib/vagrant/action/builtin/mixin_synced_folders.rb @@ -2,6 +2,7 @@ require "json" require "set" require 'vagrant/util/scoped_hash_override' +require 'vagrant/util/typed_hash' module Vagrant module Action @@ -36,11 +37,7 @@ module Vagrant # Find the proper implementation ordered.each do |_, key, impl| - begin - return key if impl.new.usable?(machine) - rescue - # Don't do anything. If an error is raised, the impl is not usable - end + return key if impl.new.usable?(machine) end return nil @@ -128,7 +125,7 @@ module Vagrant end config_folders = config.synced_folders - folders = {} + folders = Vagrant::Util::TypedHash.new() # Determine all the synced folders as well as the implementation # they're going to use. @@ -194,6 +191,12 @@ module Vagrant folders[impl_name] = new_fs end + + folders.types = folders.map { |type, folders| + impl = plugins[type][0].new() + impl._initialize(machine, type) + [type, impl] + }.to_h return folders end diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index 7f57c8309..c92ceb52f 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -628,20 +628,6 @@ module Vagrant self.class.synced_folders(self) end - # Returns the SyncedFolder object associated with this machine. - # - # @return [List] - def synced_folder_types - return @synced_folder_types if defined?(@synced_folder_types) - plugins = Vagrant.plugin("2").manager.synced_folders - @synced_folder_types = synced_folders.map { |type, folders| - impl = plugins[type][0].new() - impl._initialize(self, type) - [type, impl] - }.to_h - @synced_folder_types - end - protected # Returns the path to the file that stores the UID. diff --git a/lib/vagrant/util/typed_hash.rb b/lib/vagrant/util/typed_hash.rb new file mode 100644 index 000000000..14360f1aa --- /dev/null +++ b/lib/vagrant/util/typed_hash.rb @@ -0,0 +1,10 @@ +module Vagrant + module Util + class TypedHash < Hash + + # Types available in the Hash + attr_accessor :types + + end + end +end diff --git a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb index fcc4b0343..87efb8c82 100644 --- a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb +++ b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb @@ -16,7 +16,7 @@ module VagrantPlugins builtin_mount_type = "-cit #{VB_MOUNT_TYPE}" addon_mount_type = "-t #{VB_MOUNT_TYPE}" - mount_options, mount_uid, mount_gid = machine.synced_folder_types[:virtualbox].capability(:mount_options, name, guestpath, options) + mount_options, mount_uid, mount_gid = machine.synced_folders.types[:virtualbox].capability(:mount_options, name, guest_path, options) mount_command = "mount #{addon_mount_type} -o #{mount_options} #{name} #{guest_path}" # Create the guest path if it doesn't exist diff --git a/plugins/guests/linux/cap/persist_mount_shared_folder.rb b/plugins/guests/linux/cap/persist_mount_shared_folder.rb index 9e7dda16d..b7a239874 100644 --- a/plugins/guests/linux/cap/persist_mount_shared_folder.rb +++ b/plugins/guests/linux/cap/persist_mount_shared_folder.rb @@ -22,7 +22,7 @@ module VagrantPlugins end export_folders = fstab_folders.map do |name, data| guest_path = Shellwords.escape(data[:guestpath]) - mount_options, mount_uid, mount_gid = machine.synced_folder_types[:virtualbox].capability(:mount_options, name, guest_path, data) + mount_options, _, _ = machine.synced_folders.types[:virtualbox].capability(:mount_options, name, guest_path, data) mount_options = "#{mount_options},nofail" { name: name, diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 451fa77c6..6415f58a8 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -727,7 +727,7 @@ module VagrantPlugins if @allow_fstab_modification == UNSET_VALUE plugins = Vagrant.plugin("2").manager.synced_folders - machine.synced_folder_types.each do |_, inst| + machine.synced_folders.types.each do |_, inst| if inst.capability?(:default_fstab_modification) if inst.capability(:default_fstab_modification) == false @allow_fstab_modification = false From bc898e385da2852739b5e6c30e909537ad16fc7b Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 11 Aug 2020 14:40:46 -0500 Subject: [PATCH 06/14] Update tests for synced folder typed hash --- .../action/builtin/mixin_synced_folders.rb | 17 ++++++++++++----- plugins/synced_folders/nfs/synced_folder.rb | 4 +++- test/unit/plugins/commands/port/command_test.rb | 1 + .../cap/mount_virtual_box_shared_folder_test.rb | 2 +- .../cap/persist_mount_shared_folder_test.rb | 2 +- test/unit/plugins/kernel_v2/config/vm_test.rb | 4 ++-- .../action/prepare_nfs_settings_test.rb | 1 + .../shared/action_synced_folders_context.rb | 4 ++++ .../action/builtin/mixin_synced_folders_test.rb | 8 ++++++++ test/unit/vagrant/machine_test.rb | 11 ----------- 10 files changed, 33 insertions(+), 21 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_synced_folders.rb b/lib/vagrant/action/builtin/mixin_synced_folders.rb index 725a062cf..83c6f33a2 100644 --- a/lib/vagrant/action/builtin/mixin_synced_folders.rb +++ b/lib/vagrant/action/builtin/mixin_synced_folders.rb @@ -37,7 +37,7 @@ module Vagrant # Find the proper implementation ordered.each do |_, key, impl| - return key if impl.new.usable?(machine) + return key if impl.new.usable?(machine, raise_error=false) end return nil @@ -191,12 +191,11 @@ module Vagrant folders[impl_name] = new_fs end - folders.types = folders.map { |type, folders| impl = plugins[type][0].new() impl._initialize(machine, type) [type, impl] - }.to_h + }.to_h return folders end @@ -246,7 +245,7 @@ module Vagrant protected def cached_synced_folders(machine) - JSON.parse(machine.data_dir.join("synced_folders").read).tap do |r| + folders = JSON.parse(machine.data_dir.join("synced_folders").read, object_class: Vagrant::Util::TypedHash).tap do |r| # We have to do all sorts of things to make the proper things # symbols and r.keys.each do |k| @@ -261,11 +260,19 @@ module Vagrant r.delete(k) end end + folders.types = folders.map { |type, folders| + impl = plugins[type][0].new() + impl._initialize(machine, type) + [type, impl] + }.to_h || {} + folders rescue Errno::ENOENT # If the file doesn't exist, we probably just have a machine created # by a version of Vagrant that didn't cache shared folders. Report no # shared folders to be safe. - return {} + folders = Vagrant::Util::TypedHash.new() + folders.types = {} + folders end end end diff --git a/plugins/synced_folders/nfs/synced_folder.rb b/plugins/synced_folders/nfs/synced_folder.rb index d0134d68c..827412879 100644 --- a/plugins/synced_folders/nfs/synced_folder.rb +++ b/plugins/synced_folders/nfs/synced_folder.rb @@ -30,7 +30,9 @@ module VagrantPlugins if !machine.config.nfs.functional return false end - return true if machine.env.host.capability(:nfs_installed) + if machine.env.host.capability?(:nfs_installed) + return true if machine.env.host.capability(:nfs_installed) + end return false if !raise_error raise Vagrant::Errors::NFSNotSupported end diff --git a/test/unit/plugins/commands/port/command_test.rb b/test/unit/plugins/commands/port/command_test.rb index 2bf2eeca6..257a99be9 100644 --- a/test/unit/plugins/commands/port/command_test.rb +++ b/test/unit/plugins/commands/port/command_test.rb @@ -29,6 +29,7 @@ describe VagrantPlugins::CommandPort::Command do before do allow(machine).to receive(:state).and_return(state) + allow(machine).to receive_message_chain(:synced_folders).and_return( double(:types => []) ) allow(subject).to receive(:with_target_vms) { |&block| block.call(machine) } end diff --git a/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb b/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb index fcaabf5d9..dd9e05bc1 100644 --- a/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb +++ b/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb @@ -27,7 +27,7 @@ describe "VagrantPlugins::GuestLinux::Cap::MountVirtualBoxSharedFolder" do before do allow(machine).to receive(:communicate).and_return(comm) - allow(machine).to receive(:synced_folder_types).and_return( { :virtualbox => mount_options_cap } ) + allow(machine).to receive_message_chain(:synced_folders, :types).and_return( { :virtualbox => mount_options_cap } ) end after do diff --git a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb index bf3eacffb..b607bc044 100644 --- a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb +++ b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb @@ -16,7 +16,7 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do before do allow(machine).to receive(:communicate).and_return(comm) - allow(machine).to receive(:synced_folder_types).and_return( { :virtualbox => mount_options_cap } ) + allow(machine).to receive_message_chain(:synced_folders, :types).and_return( { :virtualbox => mount_options_cap } ) allow(mount_options_cap).to receive(:capability).with(:mount_options, any_args).and_return(["uid=#{options_uid},gid=#{options_gid}", options_uid, options_gid]) end diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 0084d7c95..f556dc222 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -37,7 +37,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do allow(machine).to receive(:env).and_return(env) allow(machine).to receive(:provider_config).and_return(nil) allow(machine).to receive(:provider_options).and_return({}) - allow(machine).to receive(:synced_folder_types).and_return([]) + allow(machine).to receive_message_chain(:synced_folders, :types).and_return( {} ) allow(provider).to receive(:capability?).with(:validate_disk_ext).and_return(true) allow(provider).to receive(:capability).with(:validate_disk_ext, "vdi").and_return(true) allow(provider).to receive(:capability?).with(:set_default_disk_ext).and_return(true) @@ -736,7 +736,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do context "WSL host paths" do let(:valid_path){ "/mnt/c/path" } let(:invalid_path){ "/home/vagrant/path" } - let(:synced_folder_impl){ double("synced_folder_impl", new: double("synced_folder_inst", usable?: true)) } + let(:synced_folder_impl){ double("synced_folder_impl", new: double("synced_folder_inst", usable?: true, _initialize: true)) } let(:fs_config){ double("fs_config", vm: double("fs_vm", allowed_synced_folder_types: nil)) } let(:plugin){ double("plugin", manager: manager) } let(:manager){ double("manager", synced_folders: {sf_impl: [synced_folder_impl, 1]}) } diff --git a/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb b/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb index 2a854671b..1510fdbb0 100644 --- a/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb +++ b/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb @@ -29,6 +29,7 @@ describe VagrantPlugins::ProviderVirtualBox::Action::PrepareNFSSettings do before do env[:test] = true allow(machine.env).to receive(:host) { host } + allow(host).to receive(:capability?).with(:nfs_installed) { true } allow(host).to receive(:capability).with(:nfs_installed) { true } # We don't care about smb support so return not installed allow(host).to receive(:capability?).with(:smb_installed).and_return(false) diff --git a/test/unit/support/shared/action_synced_folders_context.rb b/test/unit/support/shared/action_synced_folders_context.rb index b468cecce..d036deeae 100644 --- a/test/unit/support/shared/action_synced_folders_context.rb +++ b/test/unit/support/shared/action_synced_folders_context.rb @@ -10,6 +10,10 @@ shared_context "synced folder actions" do raise "#{name}: usable" if raise_error && !usable usable end + + define_method(:_initialize) do |machine, type| + true + end end end end diff --git a/test/unit/vagrant/action/builtin/mixin_synced_folders_test.rb b/test/unit/vagrant/action/builtin/mixin_synced_folders_test.rb index e07223753..adaf6af3a 100644 --- a/test/unit/vagrant/action/builtin/mixin_synced_folders_test.rb +++ b/test/unit/vagrant/action/builtin/mixin_synced_folders_test.rb @@ -125,6 +125,7 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do expect(result[:nfs]).to eq({ "nfs" => folders["nfs"].merge(__vagrantfile: true), }) + expect(result.types.keys).to eq([:default, :nfs]) end it "should return the proper set of folders of a custom config" do @@ -140,6 +141,7 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do expect(result[:default]).to eq({ "bar" => other_folders["bar"], }) + expect(result.types.keys).to eq([:default]) end it "should error if an explicit type is unusable" do @@ -157,6 +159,7 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do result = subject.synced_folders(machine) expect(result.length).to eq(1) expect(result[:default].length).to eq(1) + expect(result.types.keys).to eq([:default]) end it "should scope hash override the settings" do @@ -168,11 +171,13 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do result = subject.synced_folders(machine) expect(result[:nfs]["root"][:foo]).to eql("bar") + expect(result.types.keys).to eq([:nfs]) end it "returns {} if cached read with no cache" do result = subject.synced_folders(machine, cached: true) expect(result).to eql({}) + expect(result.types).to eq({}) end it "should be able to save and retrieve cached versions" do @@ -196,6 +201,7 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do "root" => old_folders["root"].merge(__vagrantfile: true), }) expect(result[:nfs]).to eq({ "nfs" => old_folders["nfs"].merge(__vagrantfile: true) }) + expect(result.types.keys).to eq([:default, :nfs]) end it "should be able to save and retrieve cached versions" do @@ -227,6 +233,7 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do expect(result[:nfs]).to eq({ "baz" => { type: "nfs", __vagrantfile: true } }) + expect(result.types.keys).to eq([:default, :nfs]) end it "should remove items from the vagrantfile that were removed" do @@ -253,6 +260,7 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do expect(result[:nfs]).to eq({ "baz" => { type: "nfs", __vagrantfile: true } }) + expect(result.types.keys).to eq([:default, :nfs]) end end diff --git a/test/unit/vagrant/machine_test.rb b/test/unit/vagrant/machine_test.rb index c2003a87a..913dc5e9c 100644 --- a/test/unit/vagrant/machine_test.rb +++ b/test/unit/vagrant/machine_test.rb @@ -1006,16 +1006,5 @@ describe Vagrant::Machine do end end end - - describe "#synced_folder_types" do - it "returns a map of initialized synced folders" do - allow(instance).to receive(:synced_folders).and_return( {:smb => {}, :virtualbox => {}} ) - folders = instance.synced_folder_types - folders.each do |k, v| - expect([:smb, :virtualbox]).to include(k) - expect(v.respond_to?(:capability)).to eq(true) - end - end - end end end From 1a3136b9febc44dc5e971b10c9ef26eebd8d8220 Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 11 Aug 2020 16:20:48 -0500 Subject: [PATCH 07/14] Allow types to be specified when instantiating a typed hash --- lib/vagrant/action/builtin/mixin_synced_folders.rb | 4 +--- lib/vagrant/util/typed_hash.rb | 5 +++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_synced_folders.rb b/lib/vagrant/action/builtin/mixin_synced_folders.rb index 83c6f33a2..bd4367c0f 100644 --- a/lib/vagrant/action/builtin/mixin_synced_folders.rb +++ b/lib/vagrant/action/builtin/mixin_synced_folders.rb @@ -270,9 +270,7 @@ module Vagrant # If the file doesn't exist, we probably just have a machine created # by a version of Vagrant that didn't cache shared folders. Report no # shared folders to be safe. - folders = Vagrant::Util::TypedHash.new() - folders.types = {} - folders + Vagrant::Util::TypedHash.new(types: {}) end end end diff --git a/lib/vagrant/util/typed_hash.rb b/lib/vagrant/util/typed_hash.rb index 14360f1aa..5b8ebc8f4 100644 --- a/lib/vagrant/util/typed_hash.rb +++ b/lib/vagrant/util/typed_hash.rb @@ -5,6 +5,11 @@ module Vagrant # Types available in the Hash attr_accessor :types + def initialize(**opts) + if opts[:types] + @types = opts[:types] + end + end end end end From 7577c11eb6e1406fe7c9a8ec258967b97ca32848 Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 12 Aug 2020 14:14:20 -0500 Subject: [PATCH 08/14] Persist synced folders in action --- .../action/builtin/mixin_synced_folders.rb | 2 +- lib/vagrant/action/builtin/synced_folders.rb | 10 ++++++ .../linux/cap/persist_mount_shared_folder.rb | 33 +++++++++++-------- plugins/hosts/linux/cap/rdp.rb | 1 - plugins/providers/virtualbox/synced_folder.rb | 8 ----- 5 files changed, 31 insertions(+), 23 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_synced_folders.rb b/lib/vagrant/action/builtin/mixin_synced_folders.rb index bd4367c0f..81c238fc8 100644 --- a/lib/vagrant/action/builtin/mixin_synced_folders.rb +++ b/lib/vagrant/action/builtin/mixin_synced_folders.rb @@ -37,7 +37,7 @@ module Vagrant # Find the proper implementation ordered.each do |_, key, impl| - return key if impl.new.usable?(machine, raise_error=false) + return key if impl.new.usable?(machine) end return nil diff --git a/lib/vagrant/action/builtin/synced_folders.rb b/lib/vagrant/action/builtin/synced_folders.rb index b56c212db..f799d2560 100644 --- a/lib/vagrant/action/builtin/synced_folders.rb +++ b/lib/vagrant/action/builtin/synced_folders.rb @@ -128,6 +128,16 @@ module Vagrant # Save the synced folders save_synced_folders(env[:machine], original_folders, **save_opts) end + + # Persist the mounts by adding them to fstab + if env[:machine].guest.capability?(:persist_mount_shared_folder) + if env[:machine].config.vm.allow_fstab_modification + fstab_folders = original_folders + else + fstab_folders = nil + end + env[:machine].guest.capability(:persist_mount_shared_folder, fstab_folders) + end end end end diff --git a/plugins/guests/linux/cap/persist_mount_shared_folder.rb b/plugins/guests/linux/cap/persist_mount_shared_folder.rb index b7a239874..3a0445f97 100644 --- a/plugins/guests/linux/cap/persist_mount_shared_folder.rb +++ b/plugins/guests/linux/cap/persist_mount_shared_folder.rb @@ -14,23 +14,30 @@ module VagrantPlugins # # @param [Machine] machine The machine to run the action on # @param [Map] A map of folders to add to fstab - # @param [String] mount type, ex. vboxfs, cifs, etc - def self.persist_mount_shared_folder(machine, fstab_folders, mount_type) - if fstab_folders.empty? + def self.persist_mount_shared_folder(machine, folders) + if folders.nil? self.remove_vagrant_managed_fstab(machine) return end - export_folders = fstab_folders.map do |name, data| - guest_path = Shellwords.escape(data[:guestpath]) - mount_options, _, _ = machine.synced_folders.types[:virtualbox].capability(:mount_options, name, guest_path, data) - mount_options = "#{mount_options},nofail" - { - name: name, - mount_point: guest_path, - mount_type: mount_type, - mount_options: mount_options, + + ssh_info = machine.ssh_info + export_folders = folders.map { |type, folder| + folder.map { |name, data| + data[:owner] ||= ssh_info[:username] + data[:group] ||= ssh_info[:username] + guest_path = Shellwords.escape(data[:guestpath]) + mount_options, _, _ = machine.synced_folders.types[type].capability( + :mount_options, name, guest_path, data) + mount_options = "#{mount_options},nofail" + { + name: name, + mount_point: guest_path, + mount_type: type, + mount_options: mount_options, + } } - end + }.flatten.compact + fstab_entry = Vagrant::Util::TemplateRenderer.render('guests/linux/etc_fstab', folders: export_folders) self.remove_vagrant_managed_fstab(machine) diff --git a/plugins/hosts/linux/cap/rdp.rb b/plugins/hosts/linux/cap/rdp.rb index a31b92077..3a2229bc6 100644 --- a/plugins/hosts/linux/cap/rdp.rb +++ b/plugins/hosts/linux/cap/rdp.rb @@ -47,7 +47,6 @@ module VagrantPlugins args += rdp_info[:extra_args] if rdp_info[:extra_args] end - # require "pry-byebug"; binding.pry # Finally, run the client. Vagrant::Util::Subprocess.execute(rdp_client, *args, {:detach => true}) end diff --git a/plugins/providers/virtualbox/synced_folder.rb b/plugins/providers/virtualbox/synced_folder.rb index fd0b8f4b8..c983f18ff 100644 --- a/plugins/providers/virtualbox/synced_folder.rb +++ b/plugins/providers/virtualbox/synced_folder.rb @@ -53,20 +53,12 @@ module VagrantPlugins machine.guest.capability( :mount_virtualbox_shared_folder, os_friendly_id(id), data[:guestpath], data) - fstab_folders << [os_friendly_id(id), data] else # If no guest path is specified, then automounting is disabled machine.ui.detail(I18n.t("vagrant.actions.vm.share_folders.nomount_entry", hostpath: data[:hostpath])) end end - if machine.guest.capability?(:persist_mount_shared_folder) - # If Vagrant has been configured to not allow fstab modification, then - # execute the guest capability with an empty list in order to ensure - # there is no Vagrant managed fstab entries. - fstab_folders = [] if !machine.config.vm.allow_fstab_modification - machine.guest.capability(:persist_mount_shared_folder, fstab_folders, "vboxsf") - end end def disable(machine, folders, _opts) From 2a8372a272f98a792074c2657f394ebe9bb5f0c7 Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 12 Aug 2020 16:24:49 -0500 Subject: [PATCH 09/14] Add mount type cap --- .../cap/mount_virtualbox_shared_folder.rb | 7 +++-- .../linux/cap/persist_mount_shared_folder.rb | 3 ++- .../providers/virtualbox/cap/mount_options.rb | 12 +++++++++ plugins/providers/virtualbox/plugin.rb | 5 ++++ .../mount_virtual_box_shared_folder_test.rb | 4 +++ .../cap/persist_mount_shared_folder_test.rb | 16 +++++++++-- .../virtualbox/synced_folder_test.rb | 24 ----------------- .../action/builtin/synced_folders_test.rb | 27 +++++++++++++++++++ 8 files changed, 67 insertions(+), 31 deletions(-) diff --git a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb index 87efb8c82..abbf917d2 100644 --- a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb +++ b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb @@ -6,15 +6,14 @@ module VagrantPlugins class MountVirtualBoxSharedFolder extend SyncedFolder::UnixMountHelpers - VB_MOUNT_TYPE = "vboxsf".freeze - def self.mount_virtualbox_shared_folder(machine, name, guestpath, options) guest_path = Shellwords.escape(guestpath) + mount_type = machine.synced_folders.types[:virtualbox].capability(:mount_type) @@logger.debug("Mounting #{name} (#{options[:hostpath]} to #{guestpath})") - builtin_mount_type = "-cit #{VB_MOUNT_TYPE}" - addon_mount_type = "-t #{VB_MOUNT_TYPE}" + builtin_mount_type = "-cit #{mount_type}" + addon_mount_type = "-t #{mount_type}" mount_options, mount_uid, mount_gid = machine.synced_folders.types[:virtualbox].capability(:mount_options, name, guest_path, options) mount_command = "mount #{addon_mount_type} -o #{mount_options} #{name} #{guest_path}" diff --git a/plugins/guests/linux/cap/persist_mount_shared_folder.rb b/plugins/guests/linux/cap/persist_mount_shared_folder.rb index 3a0445f97..78f187b86 100644 --- a/plugins/guests/linux/cap/persist_mount_shared_folder.rb +++ b/plugins/guests/linux/cap/persist_mount_shared_folder.rb @@ -28,11 +28,12 @@ module VagrantPlugins guest_path = Shellwords.escape(data[:guestpath]) mount_options, _, _ = machine.synced_folders.types[type].capability( :mount_options, name, guest_path, data) + mount_type = machine.synced_folders.types[type].capability(:mount_type) mount_options = "#{mount_options},nofail" { name: name, mount_point: guest_path, - mount_type: type, + mount_type: mount_type, mount_options: mount_options, } } diff --git a/plugins/providers/virtualbox/cap/mount_options.rb b/plugins/providers/virtualbox/cap/mount_options.rb index 1639ba8e9..3f0824ef8 100644 --- a/plugins/providers/virtualbox/cap/mount_options.rb +++ b/plugins/providers/virtualbox/cap/mount_options.rb @@ -6,6 +6,14 @@ module VagrantPlugins module MountOptions extend VagrantPlugins::SyncedFolder::UnixMountHelpers + VB_MOUNT_TYPE = "vboxsf".freeze + + # Returns mount options for a virual box synced folder + # + # @param [Machine] machine + # @param [String] name of mount + # @param [String] path of mount on guest + # @param [Hash] hash of mount options def self.mount_options(machine, name, guest_path, options) mount_options = options.fetch(:mount_options, []) detected_ids = detect_owner_group_ids(machine, guest_path, mount_options, options) @@ -17,6 +25,10 @@ module VagrantPlugins mount_options = mount_options.join(',') return mount_options, mount_uid, mount_gid end + + def self.mount_type(machine) + return VB_MOUNT_TYPE + end end end end diff --git a/plugins/providers/virtualbox/plugin.rb b/plugins/providers/virtualbox/plugin.rb index e193040ca..ad48bd083 100644 --- a/plugins/providers/virtualbox/plugin.rb +++ b/plugins/providers/virtualbox/plugin.rb @@ -73,6 +73,11 @@ module VagrantPlugins require_relative "cap/mount_options" Cap::MountOptions end + + synced_folder_capability(:virtualbox, "mount_type") do + require_relative "cap/mount_options" + Cap::MountOptions + end end autoload :Action, File.expand_path("../action", __FILE__) diff --git a/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb b/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb index dd9e05bc1..3e8662f78 100644 --- a/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb +++ b/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb @@ -43,12 +43,14 @@ describe "VagrantPlugins::GuestLinux::Cap::MountVirtualBoxSharedFolder" do it "generates the expected default mount command" do expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) + expect(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_uid},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) end it "automatically chown's the mounted directory on guest" do expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) + expect(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_uid},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) expect(comm).to receive(:sudo).with("chown #{mount_uid}:#{mount_gid} #{mount_guest_path}") cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) @@ -58,6 +60,7 @@ describe "VagrantPlugins::GuestLinux::Cap::MountVirtualBoxSharedFolder" do it "emits mount event" do expect(comm).to receive(:sudo).with(/initctl emit/) + expect(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) end @@ -72,6 +75,7 @@ EOF } it "should perform guest mount using builtin module" do expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) + expect(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") expect(comm).to receive(:sudo).with(/mount -t vboxsf/, any_args).and_yield(:stderr, vbox_stderr).and_return(1) expect(comm).to receive(:sudo).with(/mount -cit/, any_args) cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) diff --git a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb index b607bc044..030134529 100644 --- a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb +++ b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb @@ -13,11 +13,23 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do let(:options_uid){ '1234' } let(:cap){ caps.get(:persist_mount_shared_folder) } let(:mount_options_cap){ double("opts") } + let(:ssh_info) {{ + :username => "vagrant" + }} + let (:fstab_folders) { { + "test1" => {:guestpath=>"/test1", :hostpath=>"/my/host/path", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=1234", "gid=1234"]}, + "vagrant" => {:guestpath=>"/vagrant", :hostpath=>"/my/host/vagrant", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=1234", "gid=1234"]} + }} + let (:folders) { { + :virtualbox => fstab_folders + } } before do allow(machine).to receive(:communicate).and_return(comm) + allow(machine).to receive(:ssh_info).and_return(ssh_info) allow(machine).to receive_message_chain(:synced_folders, :types).and_return( { :virtualbox => mount_options_cap } ) allow(mount_options_cap).to receive(:capability).with(:mount_options, any_args).and_return(["uid=#{options_uid},gid=#{options_gid}", options_uid, options_gid]) + allow(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") end after do @@ -44,12 +56,12 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do expected_entry_test = "test1 /test1 vboxsf uid=#{options_uid},gid=#{options_gid},nofail 0 0" expect(cap).to receive(:remove_vagrant_managed_fstab) expect(comm).to receive(:sudo).with(/#{expected_entry_test}\n#{expected_entry_vagrant}/) - cap.persist_mount_shared_folder(machine, fstab_folders, "vboxsf") + cap.persist_mount_shared_folder(machine, folders) end it "does not insert an empty set of folders" do expect(cap).to receive(:remove_vagrant_managed_fstab) - cap.persist_mount_shared_folder(machine, [], "type") + cap.persist_mount_shared_folder(machine, nil) end end end diff --git a/test/unit/plugins/providers/virtualbox/synced_folder_test.rb b/test/unit/plugins/providers/virtualbox/synced_folder_test.rb index 9ea631ea9..28e8d9511 100644 --- a/test/unit/plugins/providers/virtualbox/synced_folder_test.rb +++ b/test/unit/plugins/providers/virtualbox/synced_folder_test.rb @@ -78,30 +78,6 @@ describe VagrantPlugins::ProviderVirtualBox::SyncedFolder do allow(machine).to receive(:ssh_info).and_return({:username => "test"}) allow(machine).to receive(:guest).and_return(guest) end - - it "should mount and persist all folders with a guest path" do - expect(guest).to receive(:capability).with(:mount_virtualbox_shared_folder, "folder", any_args) - expect(guest).not_to receive(:capability).with(:mount_virtualbox_shared_folder, "no_guestpath_folder", any_args) - expect(guest).to receive(:capability?).with(:persist_mount_shared_folder).and_return(true) - expect(guest).to receive(:capability).with(:persist_mount_shared_folder, any_args) - test_folders = folders.merge(no_guestpath_folder) - subject.enable(machine, test_folders, nil) - end - - context "fstab modification disabled" do - before do - allow(vm_config).to receive(:allow_fstab_modification).and_return(false) - end - - it "should not persist folders" do - expect(guest).to receive(:capability).with(:mount_virtualbox_shared_folder, "folder", any_args) - expect(guest).not_to receive(:capability).with(:mount_virtualbox_shared_folder, "no_guestpath_folder", any_args) - expect(guest).to receive(:capability?).with(:persist_mount_shared_folder).and_return(true) - expect(guest).to receive(:capability).with(:persist_mount_shared_folder, [], "vboxsf") - test_folders = folders.merge(no_guestpath_folder) - subject.enable(machine, test_folders, nil) - end - end end describe "#prepare" do diff --git a/test/unit/vagrant/action/builtin/synced_folders_test.rb b/test/unit/vagrant/action/builtin/synced_folders_test.rb index d168cd332..7d8357c27 100644 --- a/test/unit/vagrant/action/builtin/synced_folders_test.rb +++ b/test/unit/vagrant/action/builtin/synced_folders_test.rb @@ -45,6 +45,7 @@ describe Vagrant::Action::Builtin::SyncedFolders do allow(subject).to receive(:plugins).and_return(plugins) allow(subject).to receive(:synced_folders).and_return(synced_folders) allow(subject).to receive(:save_synced_folders) + allow(machine).to receive_message_chain(:guest, :capability?).with(:persist_mount_shared_folder).and_return(false) end after do @@ -228,5 +229,31 @@ describe Vagrant::Action::Builtin::SyncedFolders do subject.call(env) end end + + context "with guest capability to persist synced folders" do + it "persists folders" do + synced_folders["default"] = { + "root" => { + hostpath: "foo", + }, + + "other" => { + hostpath: "bar", + create: true, + } + } + allow(machine).to receive_message_chain(:guest, :capability?).with(:persist_mount_shared_folder).and_return(true) + expect(vm_config).to receive(:allow_fstab_modification).and_return(true) + expect(machine).to receive_message_chain(:guest, :capability).with(:persist_mount_shared_folder, synced_folders) + subject.call(env) + end + + it "does not persists folders if configured to not do so" do + allow(machine).to receive_message_chain(:guest, :capability?).with(:persist_mount_shared_folder).and_return(true) + expect(vm_config).to receive(:allow_fstab_modification).and_return(false) + expect(machine).to receive_message_chain(:guest, :capability).with(:persist_mount_shared_folder, nil) + subject.call(env) + end + end end end From cd2d84fda72b703b8e4fa07cb31caab48039b7d7 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 18 Aug 2020 17:00:45 -0700 Subject: [PATCH 10/14] Adjust synced folder collection class --- .../action/builtin/mixin_synced_folders.rb | 50 ++++++++----------- lib/vagrant/plugin/v2/synced_folder.rb | 40 +++++++++++++++ .../cap/mount_virtualbox_shared_folder.rb | 4 +- .../linux/cap/persist_mount_shared_folder.rb | 10 ++-- plugins/kernel_v2/config/vm.rb | 3 +- .../mount_virtual_box_shared_folder_test.rb | 40 +++++++++------ .../cap/persist_mount_shared_folder_test.rb | 28 ++++++----- .../builtin/mixin_synced_folders_test.rb | 49 +++++++++--------- 8 files changed, 135 insertions(+), 89 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_synced_folders.rb b/lib/vagrant/action/builtin/mixin_synced_folders.rb index 81c238fc8..f505d2241 100644 --- a/lib/vagrant/action/builtin/mixin_synced_folders.rb +++ b/lib/vagrant/action/builtin/mixin_synced_folders.rb @@ -98,7 +98,8 @@ module Vagrant end end - folder_data = JSON.dump(folders) + # Remove implementation instances + folder_data = JSON.dump(folders.to_h) # Scrub any register credentials from the synced folders # configuration data to prevent accidental leakage @@ -125,7 +126,7 @@ module Vagrant end config_folders = config.synced_folders - folders = Vagrant::Util::TypedHash.new() + folders = Vagrant::Plugin::V2::SyncedFolder::Collection.new # Determine all the synced folders as well as the implementation # they're going to use. @@ -182,8 +183,10 @@ module Vagrant # Apply the scoped hash overrides to get the options folders.dup.each do |impl_name, fs| + impl = plugins[impl_name].first.new._initialize(machine, impl_name) new_fs = {} fs.each do |id, data| + data[:plugin] = impl id = data[:id] if data[:id] new_fs[id] = scoped_hash_override(data, impl_name) end @@ -191,12 +194,7 @@ module Vagrant folders[impl_name] = new_fs end - folders.types = folders.map { |type, folders| - impl = plugins[type][0].new() - impl._initialize(machine, type) - [type, impl] - }.to_h - return folders + folders end # This finds the difference between two lists of synced folder @@ -245,32 +243,26 @@ module Vagrant protected def cached_synced_folders(machine) - folders = JSON.parse(machine.data_dir.join("synced_folders").read, object_class: Vagrant::Util::TypedHash).tap do |r| - # We have to do all sorts of things to make the proper things - # symbols and - r.keys.each do |k| - r[k].each do |ik, v| - v.keys.each do |vk| - v[vk.to_sym] = v[vk] - v.delete(vk) - end - end - - r[k.to_sym] = r[k] - r.delete(k) - end + import = JSON.parse(machine.data_dir.join("synced_folders").read) + import.each do |type, folders| + impl = plugins[type.to_sym].first.new._initialize(machine, type) + folders.each { |_, v| v[:plugin] = impl } end - folders.types = folders.map { |type, folders| - impl = plugins[type][0].new() - impl._initialize(machine, type) - [type, impl] - }.to_h || {} - folders + # Symbolize the keys we want as symbols + import.keys.dup.each do |k| + import[k].values.each do |item| + item.keys.dup.each do |ik| + item[ik.to_sym] = item.delete(ik) + end + end + import[k.to_sym] = import.delete(k) + end + Vagrant::Plugin::V2::SyncedFolder::Collection[import] rescue Errno::ENOENT # If the file doesn't exist, we probably just have a machine created # by a version of Vagrant that didn't cache shared folders. Report no # shared folders to be safe. - Vagrant::Util::TypedHash.new(types: {}) + Vagrant::Plugin::V2::SyncedFolder::Collection.new end end end diff --git a/lib/vagrant/plugin/v2/synced_folder.rb b/lib/vagrant/plugin/v2/synced_folder.rb index 06b4ec623..851b1ea05 100644 --- a/lib/vagrant/plugin/v2/synced_folder.rb +++ b/lib/vagrant/plugin/v2/synced_folder.rb @@ -3,6 +3,45 @@ module Vagrant module V2 # This is the base class for a synced folder implementation. class SyncedFolder + class Collection < Hash + + # @return [Array] names of synced folder types + def types + keys + end + + # Fetch the synced plugin folder of the given type + # + # @param [Symbol] t Synced folder type + # @return [Vagrant::Plugin::V2::SyncedFolder] + def type(t) + f = detect { |k, _| k.to_sym == t.to_sym }.last + raise KeyError, "Unknown synced folder type" if !f + f.values.first[:plugin] + end + + # Converts to a regular Hash and removes + # plugin instances so the result is ready + # for serialization + # + # @return [Hash] + def to_h + c = lambda do |h| + h.keys.each do |k| + if h[k].is_a?(Hash) + h[k] = c.call(h[k].to_h) + end + end + h + end + h = c.call(super) + h.values.each do |f| + f.delete(:plugin) + end + h + end + end + include CapabilityHost # This is called early when the synced folder is set to determine @@ -61,6 +100,7 @@ module Vagrant plugins = Vagrant.plugin("2").manager.synced_folders capabilities = Vagrant.plugin("2").manager.synced_folder_capabilities initialize_capabilities!(synced_folder_type, plugins, capabilities, machine) + self end end end diff --git a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb index abbf917d2..0d788a306 100644 --- a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb +++ b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb @@ -8,14 +8,14 @@ module VagrantPlugins def self.mount_virtualbox_shared_folder(machine, name, guestpath, options) guest_path = Shellwords.escape(guestpath) - mount_type = machine.synced_folders.types[:virtualbox].capability(:mount_type) + mount_type = options[:plugin].capability(:mount_type) @@logger.debug("Mounting #{name} (#{options[:hostpath]} to #{guestpath})") builtin_mount_type = "-cit #{mount_type}" addon_mount_type = "-t #{mount_type}" - mount_options, mount_uid, mount_gid = machine.synced_folders.types[:virtualbox].capability(:mount_options, name, guest_path, options) + mount_options, mount_uid, mount_gid = options[:plugin].capability(:mount_options, name, guest_path, options) mount_command = "mount #{addon_mount_type} -o #{mount_options} #{name} #{guest_path}" # Create the guest path if it doesn't exist diff --git a/plugins/guests/linux/cap/persist_mount_shared_folder.rb b/plugins/guests/linux/cap/persist_mount_shared_folder.rb index 78f187b86..c2f7c3cc5 100644 --- a/plugins/guests/linux/cap/persist_mount_shared_folder.rb +++ b/plugins/guests/linux/cap/persist_mount_shared_folder.rb @@ -10,8 +10,8 @@ module VagrantPlugins # Inserts fstab entry for a set of synced folders. Will fully replace # the currently managed group of Vagrant managed entries. Note, passing - # empty list of folders will just remove entries - # + # empty list of folders will just remove entries + # # @param [Machine] machine The machine to run the action on # @param [Map] A map of folders to add to fstab def self.persist_mount_shared_folder(machine, folders) @@ -20,15 +20,15 @@ module VagrantPlugins return end - ssh_info = machine.ssh_info + ssh_info = machine.ssh_info export_folders = folders.map { |type, folder| folder.map { |name, data| data[:owner] ||= ssh_info[:username] data[:group] ||= ssh_info[:username] guest_path = Shellwords.escape(data[:guestpath]) - mount_options, _, _ = machine.synced_folders.types[type].capability( + mount_options, _, _ = data[:plugin].capability( :mount_options, name, guest_path, data) - mount_type = machine.synced_folders.types[type].capability(:mount_type) + mount_type = data[:plugin].capability(:mount_type) mount_options = "#{mount_options},nofail" { name: name, diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 6415f58a8..4d64aace6 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -727,7 +727,8 @@ module VagrantPlugins if @allow_fstab_modification == UNSET_VALUE plugins = Vagrant.plugin("2").manager.synced_folders - machine.synced_folders.types.each do |_, inst| + machine.synced_folders.types.each do |impl_name| + inst = machine.synced_folders.type(impl_name) if inst.capability?(:default_fstab_modification) if inst.capability(:default_fstab_modification) == false @allow_fstab_modification = false diff --git a/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb b/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb index 3e8662f78..4ace19225 100644 --- a/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb +++ b/test/unit/plugins/guests/linux/cap/mount_virtual_box_shared_folder_test.rb @@ -16,18 +16,20 @@ describe "VagrantPlugins::GuestLinux::Cap::MountVirtualBoxSharedFolder" do let(:mount_name){ "vagrant" } let(:mount_guest_path){ "/vagrant" } let(:folder_options) do - { - owner: mount_owner, - group: mount_group, - hostpath: "/host/directory/path" - } + Vagrant::Plugin::V2::SyncedFolder::Collection[ + { + owner: mount_owner, + group: mount_group, + hostpath: "/host/directory/path", + plugin: folder_plugin + } + ] end let(:cap){ caps.get(:mount_virtualbox_shared_folder) } - let(:mount_options_cap){ double("opts") } + let(:folder_plugin) { double("folder_plugin") } before do allow(machine).to receive(:communicate).and_return(comm) - allow(machine).to receive_message_chain(:synced_folders, :types).and_return( { :virtualbox => mount_options_cap } ) end after do @@ -42,17 +44,21 @@ describe "VagrantPlugins::GuestLinux::Cap::MountVirtualBoxSharedFolder" do end it "generates the expected default mount command" do - expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) - expect(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") + expect(folder_plugin).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options). + and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) + expect(folder_plugin).to receive(:capability).with(:mount_type).and_return("vboxsf") expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_uid},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) + cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) end it "automatically chown's the mounted directory on guest" do - expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) - expect(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") + expect(folder_plugin).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options). + and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) + expect(folder_plugin).to receive(:capability).with(:mount_type).and_return("vboxsf") expect(comm).to receive(:sudo).with("mount -t vboxsf -o uid=#{mount_uid},gid=#{mount_gid} #{mount_name} #{mount_guest_path}", anything) expect(comm).to receive(:sudo).with("chown #{mount_uid}:#{mount_gid} #{mount_guest_path}") + cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) end @@ -60,8 +66,10 @@ describe "VagrantPlugins::GuestLinux::Cap::MountVirtualBoxSharedFolder" do it "emits mount event" do expect(comm).to receive(:sudo).with(/initctl emit/) - expect(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") - expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) + expect(folder_plugin).to receive(:capability).with(:mount_type).and_return("vboxsf") + expect(folder_plugin).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options). + and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) + cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) end end @@ -74,10 +82,12 @@ mount.vboxsf cannot be used with mainline vboxsf; instead use: EOF } it "should perform guest mount using builtin module" do - expect(mount_options_cap).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options).and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) - expect(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") + expect(folder_plugin).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options). + and_return(["uid=#{mount_uid},gid=#{mount_gid}", mount_uid, mount_gid]) + expect(folder_plugin).to receive(:capability).with(:mount_type).and_return("vboxsf") expect(comm).to receive(:sudo).with(/mount -t vboxsf/, any_args).and_yield(:stderr, vbox_stderr).and_return(1) expect(comm).to receive(:sudo).with(/mount -cit/, any_args) + cap.mount_virtualbox_shared_folder(machine, mount_name, mount_guest_path, folder_options) end end diff --git a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb index 030134529..f7ae6b51b 100644 --- a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb +++ b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb @@ -12,14 +12,20 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do let(:options_gid){ '1234' } let(:options_uid){ '1234' } let(:cap){ caps.get(:persist_mount_shared_folder) } - let(:mount_options_cap){ double("opts") } + let(:folder_plugin){ double("folder_plugin") } let(:ssh_info) {{ :username => "vagrant" }} - let (:fstab_folders) { { - "test1" => {:guestpath=>"/test1", :hostpath=>"/my/host/path", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=1234", "gid=1234"]}, - "vagrant" => {:guestpath=>"/vagrant", :hostpath=>"/my/host/vagrant", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=1234", "gid=1234"]} - }} + let (:fstab_folders) { + Vagrant::Plugin::V2::SyncedFolder::Collection[ + { + "test1" => {guestpath: "/test1", hostpath: "/my/host/path", disabled: false, plugin: folder_plugin, + __vagrantfile: true, owner: "vagrant", group: "vagrant", mount_options: ["uid=#{options_uid}", "gid=#{options_gid}"]}, + "vagrant" => {guestpath: "/vagrant", hostpath: "/my/host/vagrant", disabled: false, __vagrantfile: true, + owner: "vagrant", group: "vagrant", mount_options: ["uid=#{options_uid}", "gid=#{options_gid}}"], plugin: folder_plugin} + } + ] + } let (:folders) { { :virtualbox => fstab_folders } } @@ -27,9 +33,9 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do before do allow(machine).to receive(:communicate).and_return(comm) allow(machine).to receive(:ssh_info).and_return(ssh_info) - allow(machine).to receive_message_chain(:synced_folders, :types).and_return( { :virtualbox => mount_options_cap } ) - allow(mount_options_cap).to receive(:capability).with(:mount_options, any_args).and_return(["uid=#{options_uid},gid=#{options_gid}", options_uid, options_gid]) - allow(mount_options_cap).to receive(:capability).with(:mount_type).and_return("vboxsf") + allow(folder_plugin).to receive(:capability).with(:mount_options, any_args). + and_return(["uid=#{options_uid},gid=#{options_gid}", options_uid, options_gid]) + allow(folder_plugin).to receive(:capability).with(:mount_type).and_return("vboxsf") end after do @@ -38,11 +44,6 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do describe ".persist_mount_shared_folder" do - let (:fstab_folders) { [ - ["test1", {:guestpath=>"/test1", :hostpath=>"/my/host/path", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=#{options_uid}", "gid=#{options_gid}"] }], - ["vagrant", {:guestpath=>"/vagrant", :hostpath=>"/my/host/vagrant", :disabled=>false, :__vagrantfile=>true, :owner=>"vagrant", :group=>"vagrant", :mount_options=>["uid=#{options_uid}", "gid=#{options_gid}"] }] - ]} - let(:ui){ double(:ui) } before do @@ -56,6 +57,7 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do expected_entry_test = "test1 /test1 vboxsf uid=#{options_uid},gid=#{options_gid},nofail 0 0" expect(cap).to receive(:remove_vagrant_managed_fstab) expect(comm).to receive(:sudo).with(/#{expected_entry_test}\n#{expected_entry_vagrant}/) + cap.persist_mount_shared_folder(machine, folders) end diff --git a/test/unit/vagrant/action/builtin/mixin_synced_folders_test.rb b/test/unit/vagrant/action/builtin/mixin_synced_folders_test.rb index adaf6af3a..5d3bc1a90 100644 --- a/test/unit/vagrant/action/builtin/mixin_synced_folders_test.rb +++ b/test/unit/vagrant/action/builtin/mixin_synced_folders_test.rb @@ -14,6 +14,7 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do end let(:data_dir) { Pathname.new(Dir.mktmpdir("vagrant-test-mixin-synced-folders")) } + let(:folders_class) { Vagrant::Plugin::V2::SyncedFolder::Collection } let(:machine) do double("machine").tap do |machine| @@ -91,7 +92,7 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do end describe "synced_folders" do - let(:folders) { {} } + let(:folders) { folders_class.new } let(:plugins) { {} } before do @@ -118,14 +119,14 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do result = subject.synced_folders(machine) expect(result.length).to eq(2) expect(result[:default]).to eq({ - "another" => folders["another"].merge(__vagrantfile: true), - "foo" => folders["foo"].merge(__vagrantfile: true), - "root" => folders["root"].merge(__vagrantfile: true), + "another" => folders["another"].merge(__vagrantfile: true, plugin: true), + "foo" => folders["foo"].merge(__vagrantfile: true, plugin: true), + "root" => folders["root"].merge(__vagrantfile: true, plugin: true), }) expect(result[:nfs]).to eq({ - "nfs" => folders["nfs"].merge(__vagrantfile: true), + "nfs" => folders["nfs"].merge(__vagrantfile: true, plugin: true), }) - expect(result.types.keys).to eq([:default, :nfs]) + expect(result.types).to eq([:default, :nfs]) end it "should return the proper set of folders of a custom config" do @@ -139,9 +140,9 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do result = subject.synced_folders(machine, config: other) expect(result.length).to eq(1) expect(result[:default]).to eq({ - "bar" => other_folders["bar"], + "bar" => other_folders["bar"].merge(plugin: true), }) - expect(result.types.keys).to eq([:default]) + expect(result.types).to eq([:default]) end it "should error if an explicit type is unusable" do @@ -159,7 +160,7 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do result = subject.synced_folders(machine) expect(result.length).to eq(1) expect(result[:default].length).to eq(1) - expect(result.types.keys).to eq([:default]) + expect(result.types).to eq([:default]) end it "should scope hash override the settings" do @@ -171,13 +172,13 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do result = subject.synced_folders(machine) expect(result[:nfs]["root"][:foo]).to eql("bar") - expect(result.types.keys).to eq([:nfs]) + expect(result.types).to eq([:nfs]) end it "returns {} if cached read with no cache" do result = subject.synced_folders(machine, cached: true) expect(result).to eql({}) - expect(result.types).to eq({}) + expect(result.types).to eq([]) end it "should be able to save and retrieve cached versions" do @@ -196,12 +197,12 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do result = subject.synced_folders(machine, cached: true) expect(result.length).to eq(2) expect(result[:default]).to eq({ - "another" => old_folders["another"].merge(__vagrantfile: true), - "foo" => old_folders["foo"].merge(__vagrantfile: true), - "root" => old_folders["root"].merge(__vagrantfile: true), + "another" => old_folders["another"].merge(__vagrantfile: true, plugin: true), + "foo" => old_folders["foo"].merge(__vagrantfile: true, plugin: true), + "root" => old_folders["root"].merge(__vagrantfile: true, plugin: true), }) - expect(result[:nfs]).to eq({ "nfs" => old_folders["nfs"].merge(__vagrantfile: true) }) - expect(result.types.keys).to eq([:default, :nfs]) + expect(result[:nfs]).to eq({ "nfs" => old_folders["nfs"].merge(__vagrantfile: true, plugin: true) }) + expect(result.types).to eq([:default, :nfs]) end it "should be able to save and retrieve cached versions" do @@ -227,13 +228,13 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do result = subject.synced_folders(machine, cached: true) expect(result.length).to eq(2) expect(result[:default]).to eq({ - "foo" => { type: "default" }, - "bar" => { type: "default", __vagrantfile: true}, + "foo" => { type: "default", plugin: true }, + "bar" => { type: "default", __vagrantfile: true, plugin: true }, }) expect(result[:nfs]).to eq({ - "baz" => { type: "nfs", __vagrantfile: true } + "baz" => { type: "nfs", __vagrantfile: true, plugin: true } }) - expect(result.types.keys).to eq([:default, :nfs]) + expect(result.types).to eq([:default, :nfs]) end it "should remove items from the vagrantfile that were removed" do @@ -255,17 +256,17 @@ describe Vagrant::Action::Builtin::MixinSyncedFolders do result = subject.synced_folders(machine, cached: true) expect(result.length).to eq(2) expect(result[:default]).to eq({ - "bar" => { type: "default", __vagrantfile: true}, + "bar" => { type: "default", __vagrantfile: true, plugin: true}, }) expect(result[:nfs]).to eq({ - "baz" => { type: "nfs", __vagrantfile: true } + "baz" => { type: "nfs", __vagrantfile: true, plugin: true } }) - expect(result.types.keys).to eq([:default, :nfs]) + expect(result.types).to eq([:default, :nfs]) end end describe "#save_synced_folders" do - let(:folders) { {} } + let(:folders) { folders_class.new } let(:options) { {} } let(:output_file) { double("output_file") } From 56fa014af87ce074702a71f48c9c3119fb8a1bdf Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 19 Aug 2020 14:59:25 -0500 Subject: [PATCH 11/14] Don't persist shared folders if mount options capability not provided --- lib/vagrant/action/builtin/mixin_synced_folders.rb | 2 +- .../linux/cap/persist_mount_shared_folder.rb | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_synced_folders.rb b/lib/vagrant/action/builtin/mixin_synced_folders.rb index f505d2241..507c5d475 100644 --- a/lib/vagrant/action/builtin/mixin_synced_folders.rb +++ b/lib/vagrant/action/builtin/mixin_synced_folders.rb @@ -245,7 +245,7 @@ module Vagrant def cached_synced_folders(machine) import = JSON.parse(machine.data_dir.join("synced_folders").read) import.each do |type, folders| - impl = plugins[type.to_sym].first.new._initialize(machine, type) + impl = plugins[type.to_sym].first.new._initialize(machine, type.to_sym) folders.each { |_, v| v[:plugin] = impl } end # Symbolize the keys we want as symbols diff --git a/plugins/guests/linux/cap/persist_mount_shared_folder.rb b/plugins/guests/linux/cap/persist_mount_shared_folder.rb index c2f7c3cc5..338d62e07 100644 --- a/plugins/guests/linux/cap/persist_mount_shared_folder.rb +++ b/plugins/guests/linux/cap/persist_mount_shared_folder.rb @@ -23,12 +23,18 @@ module VagrantPlugins ssh_info = machine.ssh_info export_folders = folders.map { |type, folder| folder.map { |name, data| + guest_path = Shellwords.escape(data[:guestpath]) data[:owner] ||= ssh_info[:username] data[:group] ||= ssh_info[:username] - guest_path = Shellwords.escape(data[:guestpath]) - mount_options, _, _ = data[:plugin].capability( - :mount_options, name, guest_path, data) - mount_type = data[:plugin].capability(:mount_type) + + if data[:plugin].capability?(:mount_type) + mount_type = data[:plugin].capability(:mount_type) + mount_options, _, _ = data[:plugin].capability( + :mount_options, name, guest_path, data) + else + next + end + mount_options = "#{mount_options},nofail" { name: name, From 943047d003d9e4552a5c306275cd44e2a4e6464a Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 19 Aug 2020 17:05:39 -0500 Subject: [PATCH 12/14] Add tests for synced folder collections --- lib/vagrant/plugin/v2/synced_folder.rb | 6 ++- .../vagrant/plugin/v2/synced_folder_test.rb | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 test/unit/vagrant/plugin/v2/synced_folder_test.rb diff --git a/lib/vagrant/plugin/v2/synced_folder.rb b/lib/vagrant/plugin/v2/synced_folder.rb index 851b1ea05..bf02ec2f5 100644 --- a/lib/vagrant/plugin/v2/synced_folder.rb +++ b/lib/vagrant/plugin/v2/synced_folder.rb @@ -29,14 +29,16 @@ module Vagrant c = lambda do |h| h.keys.each do |k| if h[k].is_a?(Hash) - h[k] = c.call(h[k].to_h) + h[k] = c.call(h[k].to_h.clone) end end h end h = c.call(super) h.values.each do |f| - f.delete(:plugin) + f.values.each do |g| + g.delete(:plugin) + end end h end diff --git a/test/unit/vagrant/plugin/v2/synced_folder_test.rb b/test/unit/vagrant/plugin/v2/synced_folder_test.rb new file mode 100644 index 000000000..d02f0eb68 --- /dev/null +++ b/test/unit/vagrant/plugin/v2/synced_folder_test.rb @@ -0,0 +1,46 @@ +require File.expand_path("../../../../base", __FILE__) + +describe Vagrant::Plugin::V2::SyncedFolder::Collection do + include_context "unit" + + let(:folders) { described_class[ + :nfs=> + {"/other"=> + {:type=>:nfs, :guestpath=>"/other", :hostpath=>"/other", :disabled=>false, :__vagrantfile=>true, plugin:"someclass"}, + "/tests"=> + {:type=>:nfs, :guestpath=>"/tests", :hostpath=>"/tests", :disabled=>false, :__vagrantfile=>true, plugin:"someclass"}}, + :virtualbox=> + {"/vagrant"=> + {:guestpath=>"/vagrant", :hostpath=>"/vagrant", :disabled=>false, :__vagrantfile=>true, plugin:"someotherclass"}} + ]} + + describe "#types" do + it "gets all the types of synced folders" do + expect(folders.types).to eq([:nfs, :virtualbox]) + end + end + + describe "#type" do + it "returns the plugin for a type" do + expect(folders.type(:nfs)).to eq("someclass") + expect(folders.type(:virtualbox)).to eq("someotherclass") + end + end + + describe "to_h" do + it "removed plugin key" do + original_folders = folders + folders_h = folders.to_h + folders_h.values.each do |v| + v.values.each do |w| + expect(w).not_to include(:plugin) + end + end + original_folders.values.each do |v| + v.values.each do |w| + expect(w).to include(:plugin) + end + end + end + end +end From e96d60c02904294dfe322d0b4c91ce7954929bb3 Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 20 Aug 2020 09:50:00 -0500 Subject: [PATCH 13/14] Add tests for persisting shared folders --- .../action/builtin/mixin_synced_folders.rb | 1 - lib/vagrant/util/typed_hash.rb | 15 --------------- .../linux/cap/persist_mount_shared_folder_test.rb | 13 +++++++++++++ 3 files changed, 13 insertions(+), 16 deletions(-) delete mode 100644 lib/vagrant/util/typed_hash.rb diff --git a/lib/vagrant/action/builtin/mixin_synced_folders.rb b/lib/vagrant/action/builtin/mixin_synced_folders.rb index 507c5d475..870aaee9c 100644 --- a/lib/vagrant/action/builtin/mixin_synced_folders.rb +++ b/lib/vagrant/action/builtin/mixin_synced_folders.rb @@ -2,7 +2,6 @@ require "json" require "set" require 'vagrant/util/scoped_hash_override' -require 'vagrant/util/typed_hash' module Vagrant module Action diff --git a/lib/vagrant/util/typed_hash.rb b/lib/vagrant/util/typed_hash.rb deleted file mode 100644 index 5b8ebc8f4..000000000 --- a/lib/vagrant/util/typed_hash.rb +++ /dev/null @@ -1,15 +0,0 @@ -module Vagrant - module Util - class TypedHash < Hash - - # Types available in the Hash - attr_accessor :types - - def initialize(**opts) - if opts[:types] - @types = opts[:types] - end - end - end - end -end diff --git a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb index f7ae6b51b..aa65b38b7 100644 --- a/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb +++ b/test/unit/plugins/guests/linux/cap/persist_mount_shared_folder_test.rb @@ -33,6 +33,7 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do before do allow(machine).to receive(:communicate).and_return(comm) allow(machine).to receive(:ssh_info).and_return(ssh_info) + allow(folder_plugin).to receive(:capability?).with(:mount_type).and_return(true) allow(folder_plugin).to receive(:capability).with(:mount_options, any_args). and_return(["uid=#{options_uid},gid=#{options_gid}", options_uid, options_gid]) allow(folder_plugin).to receive(:capability).with(:mount_type).and_return("vboxsf") @@ -65,5 +66,17 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do expect(cap).to receive(:remove_vagrant_managed_fstab) cap.persist_mount_shared_folder(machine, nil) end + + context "folders do not support mount_type capability" do + before do + allow(folder_plugin).to receive(:capability?).with(:mount_type).and_return(false) + end + + it "does not inserts folders into /etc/fstab" do + expect(cap).to receive(:remove_vagrant_managed_fstab) + expect(comm).not_to receive(:sudo).with(/echo '' >> \/etc\/fstab/) + cap.persist_mount_shared_folder(machine, folders) + end + end end end From 23901fddf50dc1229a95050450a48a8623fc3084 Mon Sep 17 00:00:00 2001 From: sophia Date: Fri, 21 Aug 2020 11:10:01 -0500 Subject: [PATCH 14/14] Simplify vm config for determining fstab modification setting --- plugins/kernel_v2/config/vm.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 4d64aace6..a016cd3a0 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -726,14 +726,11 @@ module VagrantPlugins errors = _detected_errors if @allow_fstab_modification == UNSET_VALUE - plugins = Vagrant.plugin("2").manager.synced_folders machine.synced_folders.types.each do |impl_name| inst = machine.synced_folders.type(impl_name) - if inst.capability?(:default_fstab_modification) - if inst.capability(:default_fstab_modification) == false - @allow_fstab_modification = false - break - end + if inst.capability?(:default_fstab_modification) && inst.capability(:default_fstab_modification) == false + @allow_fstab_modification = false + break end end @allow_fstab_modification = true if @allow_fstab_modification == UNSET_VALUE