From 11aeafea5cb1a2ab8d1813a0933d0c9aec76cc4c Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 25 Aug 2020 10:32:18 -0500 Subject: [PATCH] Add tests for smb mount options driven through synced folder capabilities --- .../linux/cap/mount_smb_shared_folder.rb | 21 ++--- .../cap/mount_virtualbox_shared_folder.rb | 6 ++ .../linux/cap/persist_mount_shared_folder.rb | 1 - .../synced_folders/smb/cap/mount_options.rb | 2 + ...der.rb => mount_smb_shared_folder_test.rb} | 91 ++++--------------- .../cap/persist_mount_shared_folder_test.rb | 36 +++++++- .../smb/caps/mount_options_test.rb | 55 +++++++---- .../synced_folders/unix_mount_helpers_test.rb | 39 ++++++++ 8 files changed, 144 insertions(+), 107 deletions(-) rename test/unit/plugins/guests/linux/cap/{mount_smb_shared_folder.rb => mount_smb_shared_folder_test.rb} (59%) create mode 100644 test/unit/plugins/synced_folders/unix_mount_helpers_test.rb diff --git a/plugins/guests/linux/cap/mount_smb_shared_folder.rb b/plugins/guests/linux/cap/mount_smb_shared_folder.rb index d68a4bb6c..a16ce92cd 100644 --- a/plugins/guests/linux/cap/mount_smb_shared_folder.rb +++ b/plugins/guests/linux/cap/mount_smb_shared_folder.rb @@ -9,6 +9,12 @@ module VagrantPlugins extend SyncedFolder::UnixMountHelpers + # Mounts and SMB folder on linux guest + # + # @param [Machine] machine + # @param [String] name of mount + # @param [String] path of mount on guest + # @param [Hash] hash of mount options def self.mount_smb_shared_folder(machine, name, guestpath, options) expanded_guest_path = machine.guest.capability( :shell_expand_guest_path, guestpath) @@ -74,21 +80,6 @@ SCRIPT emit_upstart_notification(machine, expanded_guest_path) end - def self.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 - def self.display_mfsymlinks_warning(env) d_file = env.data_dir.join("mfsymlinks_warning") if !d_file.exist? diff --git a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb index 0d788a306..74a4c9928 100644 --- a/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb +++ b/plugins/guests/linux/cap/mount_virtualbox_shared_folder.rb @@ -6,6 +6,12 @@ module VagrantPlugins class MountVirtualBoxSharedFolder extend SyncedFolder::UnixMountHelpers + # Mounts and virtualbox folder on linux guest + # + # @param [Machine] machine + # @param [String] name of mount + # @param [String] path of mount on guest + # @param [Hash] hash of mount options def self.mount_virtualbox_shared_folder(machine, name, guestpath, options) guest_path = Shellwords.escape(guestpath) mount_type = options[:plugin].capability(:mount_type) diff --git a/plugins/guests/linux/cap/persist_mount_shared_folder.rb b/plugins/guests/linux/cap/persist_mount_shared_folder.rb index 9bc06589a..3f4202906 100644 --- a/plugins/guests/linux/cap/persist_mount_shared_folder.rb +++ b/plugins/guests/linux/cap/persist_mount_shared_folder.rb @@ -36,7 +36,6 @@ module VagrantPlugins data[:smb_host] ||= machine.guest.capability( :choose_addressable_ip_addr, candidate_ips) name = "//#{data[:smb_host]}/#{data[:smb_id]}" - mount_options = "#{mount_options},_netdev" end mount_options, _, _ = data[:plugin].capability( diff --git a/plugins/synced_folders/smb/cap/mount_options.rb b/plugins/synced_folders/smb/cap/mount_options.rb index 055627ed7..2150dbca0 100644 --- a/plugins/synced_folders/smb/cap/mount_options.rb +++ b/plugins/synced_folders/smb/cap/mount_options.rb @@ -16,6 +16,7 @@ module VagrantPlugins # @param [Hash] hash of mount options def self.mount_options(machine, name, guest_path, options) mount_options = options.fetch(:mount_options, []) + options[:smb_id] ||= name detected_ids = detect_owner_group_ids(machine, guest_path, mount_options, options) mount_uid = detected_ids[:uid] mount_gid = detected_ids[:gid] @@ -33,6 +34,7 @@ module VagrantPlugins if !ENV['VAGRANT_DISABLE_SMBMFSYMLINKS'] mnt_opts << "mfsymlinks" end + mnt_opts << "_netdev" mnt_opts = merge_mount_options(mnt_opts, options[:mount_options] || []) mount_options = mnt_opts.join(",") diff --git a/test/unit/plugins/guests/linux/cap/mount_smb_shared_folder.rb b/test/unit/plugins/guests/linux/cap/mount_smb_shared_folder_test.rb similarity index 59% rename from test/unit/plugins/guests/linux/cap/mount_smb_shared_folder.rb rename to test/unit/plugins/guests/linux/cap/mount_smb_shared_folder_test.rb index f20bb86bb..c5e19e6ab 100644 --- a/test/unit/plugins/guests/linux/cap/mount_smb_shared_folder.rb +++ b/test/unit/plugins/guests/linux/cap/mount_smb_shared_folder_test.rb @@ -7,11 +7,13 @@ describe "VagrantPlugins::GuestLinux::Cap::MountSMBSharedFolder" do .guest_capabilities[:linux] end - let(:machine) { double("machine", env: env) } + let(:machine) { double("machine", env: env, config: config) } let(:env) { double("env", host: host, ui: double("ui"), data_dir: double("data_dir")) } let(:host) { double("host") } let(:guest) { double("guest") } let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) } + let(:config) { double("config", vm: vm) } + let(:vm) { double("vm" ) } let(:mount_owner){ "vagrant" } let(:mount_group){ "vagrant" } let(:mount_uid){ "1000" } @@ -19,19 +21,28 @@ describe "VagrantPlugins::GuestLinux::Cap::MountSMBSharedFolder" do let(:mount_name){ "vagrant" } let(:mount_guest_path){ "/vagrant" } let(:folder_options) do - { - owner: mount_owner, - group: mount_group, - smb_host: "localhost", - smb_username: "user", - smb_password: "pass" - } + Vagrant::Plugin::V2::SyncedFolder::Collection[ + { + owner: mount_owner, + group: mount_group, + smb_host: "localhost", + smb_username: "user", + smb_password: "pass", + plugin: folder_plugin + } + ] end + let(:folder_plugin) { double("folder_plugin") } let(:cap){ caps.get(:mount_smb_shared_folder) } before do allow(machine).to receive(:communicate).and_return(comm) allow(host).to receive(:capability?).and_return(false) + allow(vm).to receive(:allow_fstab_modification).and_return(true) + + allow(folder_plugin).to receive(:capability).with(:mount_options, mount_name, mount_guest_path, folder_options). + and_return(["uid=#{mount_uid},gid=#{mount_gid},sec=ntlmssp,credentials=/etc/smb_creds_id", mount_uid, mount_gid]) + allow(folder_plugin).to receive(:capability).with(:mount_type).and_return("cifs") end after do @@ -70,6 +81,7 @@ describe "VagrantPlugins::GuestLinux::Cap::MountSMBSharedFolder" do end it "removes the credentials file before completion" do + allow(vm).to receive(:allow_fstab_modification).and_return(false) expect(comm).to receive(:sudo).with(/rm.+smb_creds_.+/) cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) end @@ -78,69 +90,6 @@ describe "VagrantPlugins::GuestLinux::Cap::MountSMBSharedFolder" do expect(comm).to receive(:sudo).with(/emit/) cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) end - - it "adds mfsymlinks option by default" do - expect(comm).to receive(:sudo).with(/mfsymlinks/, any_args) - cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - - it "does not add mfsymlinks option if env var VAGRANT_DISABLE_SMBMFSYMLINKS exists" do - expect(ENV).to receive(:[]).with("VAGRANT_DISABLE_SMBMFSYMLINKS").and_return(true) - expect(comm).not_to receive(:sudo).with(/mfsymlinks/, any_args) - cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - - context "with custom mount options" do - let(:folder_options) do - { - owner: mount_owner, - group: mount_group, - smb_host: "localhost", - smb_username: "user", - smb_password: "pass", - mount_options: ["ro", "sec=custom"] - } - end - - it "adds given mount options to command" do - expect(comm).to receive(:sudo).with(/ro/, any_args) - cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - - it "replaces defined options" do - expect(comm).to receive(:sudo).with(/sec=custom/, any_args) - cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - - it "does not include replaced options" do - expect(comm).not_to receive(:sudo).with(/sec=ntlm/, any_args) - cap.mount_smb_shared_folder(machine, mount_name, mount_guest_path, folder_options) - end - end - end - - describe ".merge_mount_options" do - let(:base){ ["opt1", "opt2=on", "opt3", "opt4,opt5=off"] } - let(:override){ ["opt8", "opt4=on,opt6,opt7=true"] } - - context "with no override" do - it "should split options into individual options" do - result = cap.merge_mount_options(base, []) - expect(result.size).to eq(5) - end - end - - context "with overrides" do - it "should merge all options" do - result = cap.merge_mount_options(base, override) - expect(result.size).to eq(8) - end - - it "should override options defined in base" do - result = cap.merge_mount_options(base, override) - expect(result).to include("opt4=on") - end - end end describe ".display_mfsymlinks_warning" 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 b6d446e53..40943e5f4 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 @@ -27,8 +27,9 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do ] } let (:folders) { { - :virtualbox => fstab_folders + :folder_type => fstab_folders } } + let(:expected_mount_options) { "uid=#{options_uid},gid=#{options_gid},nofail" } before do allow(machine).to receive(:communicate).and_return(comm) @@ -55,8 +56,8 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do end it "inserts folders into /etc/fstab" do - 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" + expected_entry_vagrant = "vagrant /vagrant vboxsf #{expected_mount_options} 0 0" + expected_entry_test = "test1 /test1 vboxsf #{expected_mount_options} 0 0" expect(cap).to receive(:remove_vagrant_managed_fstab) expect(comm).to receive(:sudo).with(/#{expected_entry_test}\n#{expected_entry_vagrant}/) @@ -99,5 +100,34 @@ describe "VagrantPlugins::GuestLinux::Cap::PersistMountSharedFolder" do cap.persist_mount_shared_folder(machine, nil) end end + + context "smb folder" do + 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", smb_host: "192.168.42.42", smb_id: "vtg-id1" }, + "vagrant" => {guestpath: "/vagrant", hostpath: "/my/host/vagrant", disabled: false, plugin: folder_plugin, + __vagrantfile: true, owner: "vagrant", group: "vagrant", smb_host: "192.168.42.42", smb_id: "vtg-id2"} + } + ] + } + let (:folders) { { + :smb => fstab_folders + } } + + before do + allow(folder_plugin).to receive(:capability).with(:mount_type).and_return("cifs") + end + + it "inserts folders into /etc/fstab" do + expected_entry_vagrant = "//192.168.42.42/vtg-id2 /vagrant cifs #{expected_mount_options} 0 0" + expected_entry_test = "//192.168.42.42/vtg-id1 /test1 cifs #{expected_mount_options} 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 + 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 index 90f6472a5..c62203407 100644 --- a/test/unit/plugins/synced_folders/smb/caps/mount_options_test.rb +++ b/test/unit/plugins/synced_folders/smb/caps/mount_options_test.rb @@ -30,27 +30,48 @@ describe VagrantPlugins::SyncedFolderSMB::Cap::MountOptions do 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" + allow(ENV).to receive(:[]).with("VAGRANT_DISABLE_SMBMFSYMLINKS").and_return(true) + allow(ENV).to receive(:[]).with("GEM_SKIP").and_return(false) 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 + context "with valid existent owner group" do - 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) + before 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}:") + end + + it "generates the expected default mount command" do + 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,_netdev") + expect(out_uid).to eq(mount_uid) + expect(out_gid).to eq(mount_gid) + end + + it "includes provided mount options" do + 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,_netdev,ro") + expect(out_uid).to eq(mount_uid) + expect(out_gid).to eq(mount_gid) + end + + it "overwrites default mount options" do + folder_options[:mount_options] =["ro", "sec=custom"] + out_opts, out_uid, out_gid = cap.mount_options(machine, mount_name, mount_guest_path, folder_options) + expect(out_opts).to eq("sec=custom,credentials=/etc/smb_creds_vagrant,uid=1000,gid=1000,_netdev,ro") + expect(out_uid).to eq(mount_uid) + expect(out_gid).to eq(mount_gid) + end + + it "does not add mfsymlinks option if env var VAGRANT_DISABLE_SMBMFSYMLINKS exists" do + expect(ENV).to receive(:[]).with("VAGRANT_DISABLE_SMBMFSYMLINKS").and_return(false) + 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,mfsymlinks,_netdev") + expect(out_uid).to eq(mount_uid) + expect(out_gid).to eq(mount_gid) + end end context "with non-existent owner group" do diff --git a/test/unit/plugins/synced_folders/unix_mount_helpers_test.rb b/test/unit/plugins/synced_folders/unix_mount_helpers_test.rb new file mode 100644 index 000000000..51c14d1d0 --- /dev/null +++ b/test/unit/plugins/synced_folders/unix_mount_helpers_test.rb @@ -0,0 +1,39 @@ +require_relative "../../base" + +require Vagrant.source_root.join("plugins/synced_folders/unix_mount_helpers") + +describe VagrantPlugins::SyncedFolder::UnixMountHelpers do + include_context "unit" + + subject{ + Class.new do + @@logger = nil + extend VagrantPlugins::SyncedFolder::UnixMountHelpers + end + } + + + describe ".merge_mount_options" do + let(:base){ ["opt1", "opt2=on", "opt3", "opt4,opt5=off"] } + let(:override){ ["opt8", "opt4=on,opt6,opt7=true"] } + + context "with no override" do + it "should split options into individual options" do + result = subject.merge_mount_options(base, []) + expect(result.size).to eq(5) + end + end + + context "with overrides" do + it "should merge all options" do + result = subject.merge_mount_options(base, override) + expect(result.size).to eq(8) + end + + it "should override options defined in base" do + result = subject.merge_mount_options(base, override) + expect(result).to include("opt4=on") + end + end + end +end