enable_fs tests and target perms fixup

2 files changed, 64 insertions, 49 deletions

diff --git a/lib/chef/provider/mount/solaris.rb b/lib/chef/provider/mount/solaris.rb
index a9e2234664..bb76831559 100644
--- a/lib/chef/provider/mount/solaris.rb
+++ b/lib/chef/provider/mount/solaris.rb
@@ -86,6 +86,7 @@ class Chef
         def enable_fs
           if !mount_options_unchanged?
             # we are enabling because our options have changed, so disable first then re-enable.
+            # XXX: this should be refactored to be the responsibility of the caller
             disable_fs if current_resource.enabled
           end
 
@@ -99,13 +100,15 @@ class Chef
           passstr = pass == 0 ? "-" : pass
           optstr = (actual_options.nil? || actual_options.empty?) ? "-" : actual_options.join(',')
 
-          # FIXME: permissions
           etc_tempfile do |f|
-            f.write(IO.read(VFSTAB))
-            f.puts("#{device}\t-\t#{mount_point}\t#{fstype}\t#{passstr}\t#{autostr}\t#{optstr}")
+            f.write(IO.read(VFSTAB).chomp)
+            f.puts("\n#{device}\t-\t#{mount_point}\t#{fstype}\t#{passstr}\t#{autostr}\t#{optstr}")
             f.close
-            FileUtils.mv f.path, VFSTAB
+            # move, preserving modes of destination file
+            mover = Chef::FileContentManagement::Deploy.strategy(true)
+            mover.deploy(f.path, VFSTAB)
           end
+
         end
 
         def disable_fs
@@ -113,7 +116,7 @@ class Chef
 
           found = false
           ::File.readlines(VFSTAB).reverse_each do |line|
-            if !found && line =~ /^#{device_vfstab_regex}\s+\S+\s+#{Regexp.escape(mount_point)}/
+            if !found && line =~ /^#{device_regex}\s+\S+\s+#{Regexp.escape(mount_point)}/
               found = true
               Chef::Log.debug("#{new_resource} is removed from vfstab")
               next
@@ -122,11 +125,12 @@ class Chef
           end
 
           if found
-            # FIXME: permissions
             etc_tempfile do |f|
               f.write(contents.reverse.join(''))
               f.close
-              FileUtils.mv f.path, VFSTAB
+              # move, preserving modes of destination file
+              mover = Chef::FileContentManagement::Deploy.strategy(true)
+              mover.deploy(f.path, VFSTAB)
             end
           else
             # this is likely some kind of internal error, since we should only call disable_fs when there
@@ -165,7 +169,7 @@ class Chef
             # <device> on <mountpoint> type <fstype> <options> on <date>
             # /dev/dsk/c1t0d0s0 on / type ufs read/write/setuid/devices/intr/largefiles/logging/xattr/onerror=panic/dev=700040 on Tue May  1 11:33:55 2012
             case line
-            when /^#{device_mount_regex}\s+on\s+#{Regexp.escape(mount_point)}\s+/
+            when /^#{device_regex}\s+on\s+#{Regexp.escape(mount_point)}\s+/
               Chef::Log.debug("Special device #{device} is mounted as #{mount_point}")
               mounted = true
             when /^([\/\w]+)\son\s#{Regexp.escape(mount_point)}\s+/
@@ -189,7 +193,7 @@ class Chef
               # solaris /etc/vfstab format:
               # device         device          mount           FS      fsck    mount   mount
               # to mount       to fsck         point           type    pass    at boot options
-            when /^#{device_vfstab_regex}\s+[-\/\w]+\s+#{Regexp.escape(mount_point)}\s+(\S+)\s+(\S+)\s+(\S+)\s+(\S+)/
+            when /^#{device_regex}\s+[-\/\w]+\s+#{Regexp.escape(mount_point)}\s+(\S+)\s+(\S+)\s+(\S+)\s+(\S+)/
               enabled = true
               fstype = $1
               options = $4
@@ -218,7 +222,7 @@ class Chef
           !%w{tmpfs nfs ctfs proc mntfs objfs sharefs fd smbfs}.include?(fstype)
         end
 
-        def device_mount_regex
+        def device_regex
           if ::File.symlink?(device)
             "(?:#{Regexp.escape(device)}|#{Regexp.escape(::File.expand_path(::File.readlink(device),::File.dirname(device)))})"
           else
@@ -226,10 +230,6 @@ class Chef
           end
         end
 
-        def device_vfstab_regex
-          device_mount_regex
-        end
-
       end
     end
   end
diff --git a/spec/unit/provider/mount/solaris_spec.rb b/spec/unit/provider/mount/solaris_spec.rb
index abb0101032..3f680a8d4e 100644
--- a/spec/unit/provider/mount/solaris_spec.rb
+++ b/spec/unit/provider/mount/solaris_spec.rb
@@ -34,11 +34,14 @@ describe Chef::Provider::Mount::Solaris do
 
   let(:mountpoint) { "/mnt/foo" }
 
+  let(:options) { nil }
+
   let(:new_resource) {
     new_resource = Chef::Resource::Mount.new(mountpoint)
     new_resource.device      device
     new_resource.device_type device_type
     new_resource.fstype      fstype
+    new_resource.options     options
 
     new_resource.supports :remount => false
     new_resource
@@ -532,41 +535,53 @@ describe Chef::Provider::Mount::Solaris do
       end
     end
 
-#    describe "when enabling the fs" do
-#      it "should enable if enabled isn't true" do
-#        @current_resource.enabled(false)
-#
-#        @fstab = StringIO.new
-#        File.stub(:open).with("/etc/fstab", "a").and_yield(@fstab)
-#        provider.enable_fs
-#        @fstab.string.should match(%r{^/dev/sdz1\s+/tmp/foo\s+ext3\s+defaults\s+0\s+2\s*$})
-#      end
-#
-#      it "should not enable if enabled is true and resources match" do
-#        @current_resource.enabled(true)
-#        @current_resource.fstype("ext3")
-#        @current_resource.options(["defaults"])
-#        @current_resource.dump(0)
-#        @current_resource.pass(2)
-#        File.should_not_receive(:open).with("/etc/fstab", "a")
-#
-#        provider.enable_fs
-#      end
-#
-#      it "should enable if enabled is true and resources do not match" do
-#        @current_resource.enabled(true)
-#        @current_resource.fstype("auto")
-#        @current_resource.options(["defaults"])
-#        @current_resource.dump(0)
-#        @current_resource.pass(2)
-#        @fstab = StringIO.new
-#        File.stub(:readlines).and_return([])
-#        File.should_receive(:open).once.with("/etc/fstab", "w").and_yield(@fstab)
-#        File.should_receive(:open).once.with("/etc/fstab", "a").and_yield(@fstab)
-#
-#        provider.enable_fs
-#      end
-#    end
+    describe "when enabling the fs" do
+      context "in the typical case" do
+        let(:other_mount) { "/dev/dsk/c0t2d0s0       /dev/rdsk/c0t2d0s0      /            ufs     2       yes     -" }
+
+        let(:this_mount) { "/dev/dsk/c0t2d0s7\t-\t/mnt/foo\tufs\t2\tyes\tdefaults\n" }
+
+        let(:vfstab_file_contents) { [other_mount].join("\n") }
+
+        before do
+          provider.stub(:etc_tempfile).and_yield(Tempfile.open("vfstab"))
+          provider.load_current_resource
+          provider.enable_fs
+        end
+
+        it "should leave the other mountpoint alone" do
+          IO.read(vfstab_file.path).should match(/^#{Regexp.escape(other_mount)}/)
+        end
+
+        it "should enable the mountpoint we care about" do
+          IO.read(vfstab_file.path).should match(/^#{Regexp.escape(this_mount)}/)
+        end
+      end
+
+      context "when the mount has options=noauto" do
+        let(:other_mount) { "/dev/dsk/c0t2d0s0       /dev/rdsk/c0t2d0s0      /            ufs     2       yes     -" }
+
+        let(:this_mount) { "/dev/dsk/c0t2d0s7\t-\t/mnt/foo\tufs\t2\tno\t-\n" }
+
+        let(:options) { [ "noauto" ] }
+
+        let(:vfstab_file_contents) { [other_mount].join("\n") }
+
+        before do
+          provider.stub(:etc_tempfile).and_yield(Tempfile.open("vfstab"))
+          provider.load_current_resource
+          provider.enable_fs
+        end
+
+        it "should leave the other mountpoint alone" do
+          IO.read(vfstab_file.path).should match(/^#{Regexp.escape(other_mount)}/)
+        end
+
+        it "should enable the mountpoint we care about" do
+          IO.read(vfstab_file.path).should match(/^#{Regexp.escape(this_mount)}/)
+        end
+      end
+    end
 
     describe "when disabling the fs" do
       context "in the typical case" do