Merge pull request #7947 from chrisroberts/update/nfs-exports

Refactor host linux nfs capability

lib/vagrant/errors.rb

@@ -456,6 +456,10 @@ module Vagrant
       error_key(:nfs_bad_exports)
     end
 
+    class NFSExportsFailed < VagrantError
+      error_key(:nfs_exports_failed)
+    end
+
     class NFSCantReadExports < VagrantError
       error_key(:nfs_cant_read_exports)
     end

lib/vagrant/util.rb

@@ -9,6 +9,7 @@ module Vagrant
     autoload :SafeExec,                  'vagrant/util/safe_exec'
     autoload :StackedProcRunner,         'vagrant/util/stacked_proc_runner'
     autoload :TemplateRenderer,          'vagrant/util/template_renderer'
+    autoload :StringBlockEditor,         'vagrant/util/string_block_editor'
     autoload :Subprocess,                'vagrant/util/subprocess'
   end
 end

plugins/hosts/linux/cap/nfs.rb

@@ -6,6 +6,8 @@ module VagrantPlugins
   module HostLinux
     module Cap
       class NFS
+
+        NFS_EXPORTS_PATH = "/etc/exports".freeze
         extend Vagrant::Util::Retryable
 
         def self.nfs_apply_command(env)
@@ -36,16 +38,9 @@ module VagrantPlugins
           ui.info I18n.t("vagrant.hosts.linux.nfs_export")
           sleep 0.5
 
-          nfs_cleanup(id)
+          nfs_cleanup("#{Process.uid} #{id}")
-
+          output = "#{nfs_exports_content}\n#{output}"
-          # Only use "sudo" if we can't write to /etc/exports directly
+          nfs_write_exports(output)
-          sudo_command = ""
-          sudo_command = "sudo " if !File.writable?("/etc/exports")
-
-          output.split("\n").each do |line|
-            line = Vagrant::Util::ShellQuote.escape(line, "'")
-            system(%Q[echo '#{line}' | #{sudo_command}tee -a /etc/exports >/dev/null])
-          end
 
           if nfs_running?(nfs_check_command)
             system("sudo #{nfs_apply_command}")
@@ -62,48 +57,111 @@ module VagrantPlugins
         end
 
         def self.nfs_prune(environment, ui, valid_ids)
-          return if !File.exist?("/etc/exports")
+          return if !File.exist?(NFS_EXPORTS_PATH)
 
           logger = Log4r::Logger.new("vagrant::hosts::linux")
           logger.info("Pruning invalid NFS entries...")
 
-          output = false
           user = Process.uid
 
-          File.read("/etc/exports").lines.each do |line|
+          # Create editor instance for removing invalid IDs
-            if id = line[/^# VAGRANT-BEGIN:( #{user})? ([\.\/A-Za-z0-9\-_:]+?)$/, 2]
+          editor = Vagrant::Util::StringBlockEditor.new(nfs_exports_content)
-              if valid_ids.include?(id)
-                logger.debug("Valid ID: #{id}")
-              else
-                if !output
-                  # We want to warn the user but we only want to output once
-                  ui.info I18n.t("vagrant.hosts.linux.nfs_prune")
-                  output = true
-                end
 
-                logger.info("Invalid ID, pruning: #{id}")
+          # Build composite IDs with UID information and discover invalid entries
-                nfs_cleanup(id)
+          composite_ids = valid_ids.map do |v_id|
-              end
+            "#{user} #{v_id}"
-            end
+          end
+          remove_ids = editor.keys - composite_ids
+
+          logger.debug("Known valid NFS export IDs: #{valid_ids}")
+          logger.debug("Composite valid NFS export IDs with user: #{composite_ids}")
+          logger.debug("NFS export IDs to be removed: #{remove_ids}")
+          if !remove_ids.empty?
+            ui.info I18n.t("vagrant.hosts.linux.nfs_prune")
+            nfs_cleanup(remove_ids)
           end
         end
 
         protected
 
-        def self.nfs_cleanup(id)
+        def self.nfs_cleanup(remove_ids)
-          return if !File.exist?("/etc/exports")
+          return if !File.exist?(NFS_EXPORTS_PATH)
 
-          user = Regexp.escape(Process.uid.to_s)
+          editor = Vagrant::Util::StringBlockEditor.new(nfs_exports_content)
-          id   = Regexp.escape(id.to_s)
+          remove_ids = Array(remove_ids)
 
-          # Only use "sudo" if we can't write to /etc/exports directly
+          # Remove all invalid ID entries
-          sudo_command = ""
+          remove_ids.each do |r_id|
-          sudo_command = "sudo " if !File.writable?("/etc/exports")
+            editor.delete(r_id)
+          end
+          nfs_write_exports(editor.value)
+        end
 
-          # Use sed to just strip out the block of code which was inserted
+        def self.nfs_write_exports(new_exports_content)
-          # by Vagrant
+          if(nfs_exports_content != new_exports_content.strip)
-          tmp = ENV["TMPDIR"] || ENV["TMP"] || "/tmp"
+            begin
-          system("cp /etc/exports '#{tmp}' && #{sudo_command}sed -r -e '\\\x01^# VAGRANT-BEGIN:( #{user})? #{id}\x01,\\\x01^# VAGRANT-END:( #{user})? #{id}\x01 d' -ibak '#{tmp}/exports' ; #{sudo_command}cp '#{tmp}/exports' /etc/exports")
+              # 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)
+              if existing_stat.mode != new_stat.mode
+                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}"
+                result = Vagrant::Util::Subprocess.execute(*Shellwords.split(chown_cmd))
+                if result.exit_code != 0
+                  raise Vagrant::Errors::NFSExportsFailed,
+                    command: chown_cmd,
+                    stderr: result.stderr,
+                    stdout: result.stdout
+                end
+              end
+              # Always force move the file to prevent overwrite prompting
+              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
+                raise Vagrant::Errors::NFSExportsFailed,
+                  command: mv_cmd,
+                  stderr: result.stderr,
+                  stdout: result.stdout
+              end
+            ensure
+              if File.exist?(new_exports_path)
+                File.unlink(new_exports_path)
+              end
+            end
+          end
+        end
+
+        def self.nfs_exports_content
+          if(File.exist?(NFS_EXPORTS_PATH))
+            if(File.readable?(NFS_EXPORTS_PATH))
+              File.read(NFS_EXPORTS_PATH)
+            else
+              cmd = "sudo cat #{NFS_EXPORTS_PATH}"
+              result = Vagrant::Util::Subprocess.execute(*Shellwords.split(cmd))
+              if result.exit_code != 0
+                raise Vagrant::Errors::NFSExportsFailed,
+                  command: cmd,
+                  stderr: result.stderr,
+                  stdout: result.stdout
+              else
+                result.stdout
+              end
+            end
+          else
+            ""
+          end
         end
 
         def self.nfs_opts_setup(folders)

templates/locales/en.yml

@@ -877,6 +877,14 @@ en:
         the issues below and execute "vagrant reload":
 
         %{output}
+      nfs_exports_failed: |-
+        Vagrant failed to install an updated NFS exports file. This may be
+        due to overly restrictive permissions on your NFS exports file. Please
+        validate them and try again.
+
+        command: %{command}
+        stdout: %{stdout}
+        stderr: %{stderr}
       nfs_cant_read_exports: |-
         Vagrant can't read your current NFS exports! The exports file should be
         readable by any user. This is usually caused by invalid permissions

test/unit/plugins/hosts/linux/cap/nfs_test.rb

@@ -0,0 +1,156 @@
+require_relative "../../../../base"
+require_relative "../../../../../../plugins/hosts/linux/cap/nfs"
+require_relative "../../../../../../lib/vagrant/util"
+
+describe VagrantPlugins::HostLinux::Cap::NFS do
+
+  include_context "unit"
+
+  let(:caps) do
+    VagrantPlugins::HostLinux::Plugin
+      .components
+      .host_capabilities[:linux]
+  end
+
+  let(:tmp_exports_path) do
+    @tmp_exports ||= temporary_file
+  end
+  let(:exports_path){ VagrantPlugins::HostLinux::Cap::NFS::NFS_EXPORTS_PATH }
+  let(:env){ double(:env) }
+  let(:ui){ double(:ui) }
+  let(:host){ double(:host) }
+
+  before do
+    @original_exports_path = VagrantPlugins::HostLinux::Cap::NFS::NFS_EXPORTS_PATH
+    VagrantPlugins::HostLinux::Cap::NFS.send(:remove_const, :NFS_EXPORTS_PATH)
+    VagrantPlugins::HostLinux::Cap::NFS.const_set(:NFS_EXPORTS_PATH, tmp_exports_path.to_s)
+  end
+
+  after do
+    VagrantPlugins::HostLinux::Cap::NFS.send(:remove_const, :NFS_EXPORTS_PATH)
+    VagrantPlugins::HostLinux::Cap::NFS.const_set(:NFS_EXPORTS_PATH, @original_exports_path)
+    File.unlink(tmp_exports_path.to_s) if File.exist?(tmp_exports_path.to_s)
+    @tmp_exports = nil
+  end
+
+  describe ".nfs_export" do
+
+    let(:cap){ caps.get(:nfs_export) }
+
+    before do
+      allow(env).to receive(:host).and_return(host)
+      allow(host).to receive(:capability).with(:nfs_apply_command).and_return("/bin/true")
+      allow(host).to receive(:capability).with(:nfs_check_command).and_return("/bin/true")
+      allow(host).to receive(:capability).with(:nfs_start_command).and_return("/bin/true")
+      allow(ui).to receive(:info)
+      allow(cap).to receive(:system).with("sudo /bin/true").and_return(true)
+      allow(cap).to receive(:system).with("/bin/true").and_return(true)
+    end
+
+    it "should export new entries" do
+      cap.nfs_export(env, ui, SecureRandom.uuid, ["127.0.0.1"], "tmp" => {:hostpath => "/tmp"})
+      exports_content = File.read(exports_path)
+      expect(exports_content).to match(/\/tmp.*127\.0\.0\.1/)
+    end
+
+    it "should not remove existing entries" do
+      File.write(exports_path, "/custom/directory hostname1(rw,sync,no_subtree_check)")
+      cap.nfs_export(env, ui, SecureRandom.uuid, ["127.0.0.1"], "tmp" => {:hostpath => "/tmp"})
+      exports_content = File.read(exports_path)
+      expect(exports_content).to match(/\/tmp.*127\.0\.0\.1/)
+      expect(exports_content).to match(/\/custom\/directory.*hostname1/)
+    end
+
+    it "should remove entries no longer valid" do
+      valid_id = SecureRandom.uuid
+      other_id = SecureRandom.uuid
+      content =<<-EOH
+# VAGRANT-BEGIN: #{Process.uid} #{other_id}
+"/tmp" 127.0.0.1(rw,no_subtree_check,all_squash,anonuid=,anongid=,fsid=)
+# VAGRANT-END: #{Process.uid} #{other_id}
+# VAGRANT-BEGIN: #{Process.uid} #{valid_id}
+"/var" 127.0.0.1(rw,no_subtree_check,all_squash,anonuid=,anongid=,fsid=)
+# VAGRANT-END: #{Process.uid} #{valid_id}
+EOH
+      File.write(exports_path, content)
+      cap.nfs_export(env, ui, valid_id, ["127.0.0.1"], "home" => {:hostpath => "/home"})
+      exports_content = File.read(exports_path)
+      expect(exports_content).to include("/home")
+      expect(exports_content).to include("/tmp")
+      expect(exports_content).not_to include("/var")
+    end
+  end
+
+  describe ".nfs_prune" do
+
+    let(:cap){ caps.get(:nfs_prune) }
+
+    before do
+      allow(ui).to receive(:info)
+    end
+
+    it "should remove entries no longer valid" do
+      invalid_id = SecureRandom.uuid
+      valid_id = SecureRandom.uuid
+      content =<<-EOH
+# VAGRANT-BEGIN: #{Process.uid} #{invalid_id}
+"/tmp" 127.0.0.1(rw,no_subtree_check,all_squash,anonuid=,anongid=,fsid=)
+# VAGRANT-END: #{Process.uid} #{invalid_id}
+# VAGRANT-BEGIN: #{Process.uid} #{valid_id}
+"/var" 127.0.0.1(rw,no_subtree_check,all_squash,anonuid=,anongid=,fsid=)
+# VAGRANT-END: #{Process.uid} #{valid_id}
+EOH
+      File.write(exports_path, content)
+      cap.nfs_prune(env, ui, [valid_id])
+      exports_content = File.read(exports_path)
+      expect(exports_content).to include(valid_id)
+      expect(exports_content).not_to include(invalid_id)
+      expect(exports_content).to include("/var")
+      expect(exports_content).not_to include("/tmp")
+    end
+  end
+
+  describe ".nfs_write_exports" do
+
+    before do
+      File.write(tmp_exports_path, "original content")
+    end
+
+    it "should write updated contents to file" do
+      described_class.nfs_write_exports("new content")
+      exports_content = File.read(exports_path)
+      expect(exports_content).to include("new content")
+      expect(exports_content).not_to include("original content")
+    end
+
+    it "should only update contents if different" do
+      original_stat = File.stat(exports_path)
+      described_class.nfs_write_exports("original content")
+      updated_stat = File.stat(exports_path)
+      expect(original_stat).to eq(updated_stat)
+    end
+
+    it "should retain existing file permissions" do
+      File.chmod(0600, exports_path)
+      original_stat = File.stat(exports_path)
+      described_class.nfs_write_exports("original content")
+      updated_stat = File.stat(exports_path)
+      expect(original_stat.mode).to eq(updated_stat.mode)
+    end
+
+    it "should raise exception when failing to move new exports file" do
+      expect(Vagrant::Util::Subprocess).to receive(:execute).and_return(
+        Vagrant::Util::Subprocess::Result.new(1, "Failed to move file", "")
+      )
+      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")
+    end
+
+    it "should raise custom exception when chown fails" do
+      pending("investigate using a simulated FS to test")
+    end
+  end
+end