diff --git a/plugins/hosts/linux/cap/nfs.rb b/plugins/hosts/linux/cap/nfs.rb index 1d407d52e..630dc0f4e 100644 --- a/plugins/hosts/linux/cap/nfs.rb +++ b/plugins/hosts/linux/cap/nfs.rb @@ -173,16 +173,14 @@ module VagrantPlugins def self.nfs_write_exports(new_exports_content) if(nfs_exports_content != new_exports_content.strip) begin + exports_path = Pathname.new(NFS_EXPORTS_PATH) + # Write contents out to temporary file new_exports_file = Tempfile.create('vagrant') new_exports_file.puts(new_exports_content) new_exports_file.close new_exports_path = new_exports_file.path - # Only use "sudo" if we can't write to /etc/exports directly - sudo_command = "" - sudo_command = "sudo " if !File.writable?(NFS_EXPORTS_PATH) - # Ensure new file mode and uid/gid match existing file to replace existing_stat = File.stat(NFS_EXPORTS_PATH) new_stat = File.stat(new_exports_path) @@ -190,7 +188,7 @@ module VagrantPlugins File.chmod(existing_stat.mode, new_exports_path) end if existing_stat.uid != new_stat.uid || existing_stat.gid != new_stat.gid - chown_cmd = "#{sudo_command}chown #{existing_stat.uid}:#{existing_stat.gid} #{new_exports_path}" + chown_cmd = "sudo chown #{existing_stat.uid}:#{existing_stat.gid} #{new_exports_path}" result = Vagrant::Util::Subprocess.execute(*Shellwords.split(chown_cmd)) if result.exit_code != 0 raise Vagrant::Errors::NFSExportsFailed, @@ -200,6 +198,7 @@ module VagrantPlugins end end # Always force move the file to prevent overwrite prompting + sudo_command = "sudo " if !exports_path.writable? || !exports_path.dirname.writable? mv_cmd = "#{sudo_command}mv -f #{new_exports_path} #{NFS_EXPORTS_PATH}" result = Vagrant::Util::Subprocess.execute(*Shellwords.split(mv_cmd)) if result.exit_code != 0 diff --git a/test/unit/plugins/hosts/linux/cap/nfs_test.rb b/test/unit/plugins/hosts/linux/cap/nfs_test.rb index 144041924..575035ab9 100644 --- a/test/unit/plugins/hosts/linux/cap/nfs_test.rb +++ b/test/unit/plugins/hosts/linux/cap/nfs_test.rb @@ -269,14 +269,72 @@ EOH expect{ described_class.nfs_write_exports("new content") }.to raise_error(Vagrant::Errors::NFSExportsFailed) end - it "should retain existing file owner and group IDs" do - pending("investigate using a simulated FS to test") - test_with_simulated_fs - end + context "exports file modification" do + let(:tmp_stat) { double("tmp_stat", uid: 100, gid: 100, mode: tmp_mode) } + let(:tmp_mode) { 0 } + let(:exports_stat) { double("stat", uid: exports_uid, gid: exports_gid, mode: exports_mode) } + let(:exports_uid) { -1 } + let(:exports_gid) { -1 } + let(:exports_mode) { 0 } + let(:new_exports_file) { double("new_exports_file", path: "/dev/null/exports") } - it "should raise custom exception when chown fails" do - pending("investigate using a simulated FS to test") - test_with_simulated_fs + before do + allow(File).to receive(:stat).with(new_exports_file.path).and_return(tmp_stat) + allow(File).to receive(:stat).with(tmp_exports_path.to_s).and_return(exports_stat) + allow(new_exports_file).to receive(:puts) + allow(new_exports_file).to receive(:close) + allow(Vagrant::Util::Subprocess).to receive(:execute).and_return(Vagrant::Util::Subprocess::Result.new(0, "", "")) + allow(Tempfile).to receive(:create).with("vagrant").and_return(new_exports_file) + end + + it "should retain existing file owner and group IDs" do + expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args| + expect(args).to include("sudo") + expect(args).to include("chown") + }.and_return(Vagrant::Util::Subprocess::Result.new(0, "", "")) + described_class.nfs_write_exports("new content") + end + + it "should raise custom exception when chown fails" do + expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args| + expect(args).to include("sudo") + expect(args).to include("chown") + }.and_return(Vagrant::Util::Subprocess::Result.new(1, "", "")) + expect { described_class.nfs_write_exports("new content") }.to raise_error(Vagrant::Errors::NFSExportsFailed) + end + + context "when user has write access to exports file" do + let(:file_writable?) { true } + let(:dir_writable?) { false } + let(:exports_pathname) { double("exports_pathname", writable?: file_writable?, dirname: exports_dir_pathname) } + let(:exports_dir_pathname) { double("exports_dir_pathname", writable?: dir_writable?) } + + before do + allow(File).to receive(:stat).and_return(exports_stat) + allow(File).to receive(:exist?).and_return(false) + allow(Pathname).to receive(:new).with(tmp_exports_path.to_s).and_return(exports_pathname) + end + + it "should use sudo when moving new file" do + expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args| + expect(args).to include("sudo") + expect(args).to include("mv") + }.and_return(Vagrant::Util::Subprocess::Result.new(0, "", "")) + described_class.nfs_write_exports("new content") + end + + context "and write access to exports parent directory" do + let(:dir_writable?) { true } + + it "should not use sudo when moving new file" do + expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args| + expect(args).not_to include("sudo") + expect(args).to include("mv") + }.and_return(Vagrant::Util::Subprocess::Result.new(0, "", "")) + described_class.nfs_write_exports("new content") + end + end + end end end end