Merge pull request #19 from GabrielNagy/MODULES-8304/whitespace-in-mountpoints

(MODULES-8304) Allow whitespace in mountpoints on Linux

Author: mihaibuzgau

REFERENCE.md

@@ -47,10 +47,11 @@ mount/unmount status.
 
 ##### `device`
 
-The device providing the mount.  This can be whatever
-device is supporting by the mount, including network
-devices or devices specified by UUID rather than device
-path, depending on the operating system.
+The device providing the mount.  This can be whatever device
+is supporting by the mount, including network devices or
+devices specified by UUID rather than device path, depending
+on the operating system. On Linux systems it can contain
+whitespace.
 
 ##### `blockdevice`
 
@@ -100,7 +101,7 @@ The following parameters are available in the `mount` type.
 
 namevar
 
-The mount path for the mount.
+The mount path for the mount. On Linux systems it can contain whitespace.
 
 ##### `remounts`
 

lib/puppet/provider/mount.rb

@@ -60,7 +60,7 @@ def using_explicit_options?
 
   # This only works when the mount point is synced to the fstab.
   def unmount
-    umount(resource[:name])
+    umount resource[:name]
 
     # Update property hash for future queries (e.g. refresh is called)
     case get(:ensure)

lib/puppet/provider/mount/parsed.rb

@@ -186,6 +186,28 @@ def to_line(result)
     end
   else
     record_line name, fields: @fields, separator: %r{\s+}, joiner: "\t", optional: optional_fields, block_eval: :instance do
+      def to_line(record)
+        # convert whitespace to ASCII before writing to fstab
+        # duplicate the record since we don't want our resource to have ASCII whitespaces
+        result = record.dup
+        [:device, :name].each do |param|
+          if record[param].is_a?(String)
+            result[param] = result[param].gsub(' ', '\\\040') if result[param].include?(' ')
+          end
+        end
+        join(result)
+      end
+
+      def post_parse(record)
+        # handle ASCII-encoded whitespaces in fstab
+        [:device, :name].each do |param|
+          if record[param].is_a?(String)
+            record[param].gsub!('\040', ' ') if record[param].include?('\040')
+          end
+        end
+        record
+      end
+
       def pre_gen(record)
         if !record[:options] || record[:options].empty?
           if Facter.value(:kernel) == 'Linux'
@@ -253,16 +275,17 @@ def self.prefetch(resources = nil)
   end
 
   def self.mountinstances
-    # XXX: Will not work for mount points that have spaces in path (does fstab support this anyways?)
     regex = case Facter.value(:osfamily)
             when 'Darwin'
               %r{ on (?:/private/var/automount)?(\S*)}
             when 'Solaris', 'HP-UX'
               %r{^(\S*) on }
             when 'AIX'
               %r{^(?:\S*\s+\S+\s+)(\S+)}
+            when %r{FreeBSD|NetBSD}i
+              %r{ on (.*) \(}
             else
-              %r{ on (\S*)}
+              %r{ on (.*) type }
             end
     instances = []
     mount_output = mountcmd.split("\n")

lib/puppet/type/mount.rb

@@ -124,13 +124,16 @@ def syncothers
   end
 
   newproperty(:device) do
-    desc "The device providing the mount.  This can be whatever
-        device is supporting by the mount, including network
-        devices or devices specified by UUID rather than device
-        path, depending on the operating system."
+    desc "The device providing the mount.  This can be whatever device
+        is supporting by the mount, including network devices or
+        devices specified by UUID rather than device path, depending
+        on the operating system. On Linux systems it can contain
+        whitespace."
 
     validate do |value|
-      raise Puppet::Error, _('device must not contain whitespace: %{value}') % { value: value } if value =~ %r{\s}
+      unless Facter.value(:kernel) == 'Linux'
+        raise Puppet::Error, _('device must not contain whitespace: %{value}') % { value: value } if value =~ %r{\s}
+      end
     end
   end
 
@@ -242,12 +245,14 @@ def munge(value)
   end
 
   newparam(:name) do
-    desc 'The mount path for the mount.'
+    desc 'The mount path for the mount. On Linux systems it can contain whitespace.'
 
     isnamevar
 
     validate do |value|
-      raise Puppet::Error, _('name must not contain whitespace: %{value}') % { value: value } if value =~ %r{\s}
+      unless Facter.value(:kernel) == 'Linux'
+        raise Puppet::Error, _('name must not contain whitespace: %{value}') % { value: value } if value =~ %r{\s}
+      end
     end
 
     munge do |value|

spec/acceptance/lib/mount_utils.rb

@@ -44,7 +44,9 @@ def add_entry_to_filesystem_table(host, mount_name)
       # Note: /dev/hd8 is the default jfs logging device on AIX.
       on(host, "echo '/#{mount_name}:\n  dev = /dev/#{mount_name}\n  vfs = #{fs_type}\n  log = /dev/hd8' >> #{fs_file}")
     when %r{el-|centos|fedora|sles|debian|ubuntu|cumulus}
-      on(host, "echo '/tmp/#{mount_name}  /#{mount_name}  #{fs_type}  loop  0  0' >> #{fs_file}")
+      # Correctly munge whitespaces in mountpoints
+      munged_mount_name = mount_name.gsub(' ', '\\\040')
+      on(host, "echo '/tmp/#{munged_mount_name}  /#{munged_mount_name}  #{fs_type}  loop  0  0' >> #{fs_file}")
     else
       # TODO: Add Solaris and OSX support, as per PUP-5201 and PUP-4823
       fail_test("Adding entries to the filesystem table on #{host['platform']} is not currently supported.")
@@ -63,8 +65,8 @@ def create_filesystem(host, mount_name)
       on(host, "mklv -y #{mount_name} #{volume_group} 1M")
       on(host, "mkfs -V #{fs_type} -l #{mount_name} /dev/#{mount_name}")
     when %r{el-|centos|fedora|sles|debian|ubuntu|cumulus}
-      on(host, "dd if=/dev/zero of=/tmp/#{mount_name} count=10240", acceptable_exit_codes: [0, 1])
-      on(host, "yes | mkfs -t #{fs_type} -q /tmp/#{mount_name}", acceptable_exit_codes: (0..254))
+      on(host, "dd if=/dev/zero of='/tmp/#{mount_name}' count=10240", acceptable_exit_codes: [0, 1])
+      on(host, "yes | mkfs -t #{fs_type} -q '/tmp/#{mount_name}'", acceptable_exit_codes: (0..254))
     else
       # TODO: Add Solaris and OSX support, as per PUP-5201 and PUP-4823
       fail_test("Creating filesystems on #{host['platform']} is not currently supported.")

spec/acceptance/tests/defined_spec.rb

@@ -19,6 +19,20 @@
           fail_test "didn't find the mount #{name}" unless result.stdout.include?(name)
         end
       end
+
+      it 'defines a mount entry with whitespace' do
+        step 'creates a mount'
+        args = ['ensure=defined',
+                "fstype=#{fs_type}",
+                "device='/tmp/#{name_w_whitespace}'"]
+        on(agent, puppet_resource('mount', "'/#{name_w_whitespace}'", args))
+
+        step 'verify entry in filesystem table'
+        on(agent, "cat #{fs_file}") do |result|
+          munged_name = name_w_whitespace.gsub(' ', '\\\040')
+          fail_test "didn't find the mount #{name_w_whitespace}" unless result.stdout.include?(munged_name)
+        end
+      end
     end
   end
 end

spec/acceptance/tests/destroy_spec.rb

@@ -38,6 +38,40 @@
           fail_test "found the mount #{name} mounted" if result.stdout.include?(name)
         end
       end
+
+      it 'deletes an entry with whitespace in filesystem table and unmounts it' do
+        step 'create mount point'
+        on(agent, "mkdir '/#{name_w_whitespace}'", acceptable_exit_codes: [0, 1])
+
+        step 'create new filesystem to be mounted'
+        MountUtils.create_filesystem(agent, name_w_whitespace)
+
+        step 'add entry to the filesystem table'
+        MountUtils.add_entry_to_filesystem_table(agent, name_w_whitespace)
+
+        step 'mount entry'
+        on(agent, "mount '/#{name_w_whitespace}'")
+
+        step 'verify entry exists in filesystem table'
+        on(agent, "cat #{fs_file}") do |result|
+          munged_name = name_w_whitespace.gsub(' ', '\\\040')
+          fail_test "did not find mount '#{name_w_whitespace}'" unless result.stdout.include?(munged_name)
+        end
+
+        step 'destroy a mount with puppet (absent)'
+        on(agent, puppet_resource('mount', "'/#{name_w_whitespace}'", 'ensure=absent'))
+
+        step 'verify entry removed from filesystem table'
+        on(agent, "cat #{fs_file}") do |result|
+          munged_name = name_w_whitespace.gsub(' ', '\\\040')
+          fail_test "found the mount '#{name_w_whitespace}'" if result.stdout.include?(munged_name)
+        end
+
+        step 'verify entry is not mounted'
+        on(agent, 'mount') do |result|
+          fail_test "found the mount '#{name_w_whitespace}' mounted" if result.stdout.include?(name_w_whitespace)
+        end
+      end
     end
   end
 end

spec/acceptance/tests/modify_spec.rb

@@ -30,6 +30,30 @@
           fail_test "attributes not updated for the mount #{name}" unless res.stdout.include? 'bogus'
         end
       end
+
+      it 'modifies an entry with whitespace in the filesystem table' do
+        step '(setup) create mount point'
+        on(agent, "mkdir '/#{name_w_whitespace}'", acceptable_exit_codes: [0, 1])
+
+        step '(setup) create new filesystem to be mounted'
+        MountUtils.create_filesystem(agent, name_w_whitespace)
+
+        step '(setup) add entry to the filesystem table'
+        MountUtils.add_entry_to_filesystem_table(agent, name_w_whitespace)
+
+        step '(setup) mount entry'
+        on(agent, "mount '/#{name_w_whitespace}'")
+
+        step 'modify a mount with puppet (defined)'
+        args = ['ensure=defined',
+                'fstype=bogus']
+        on(agent, puppet_resource('mount', "'/#{name_w_whitespace}'", args))
+
+        step 'verify entry is updated in filesystem table'
+        on(agent, "cat #{fs_file}") do |res|
+          fail_test "attributes not updated for the mount '#{name_w_whitespace}'" unless res.stdout.include? 'bogus'
+        end
+      end
     end
   end
 end

spec/acceptance/tests/mounted_spec.rb

@@ -38,6 +38,32 @@
           fail_test "didn't find the mount #{name} mounted" unless result.stdout.include?(name)
         end
       end
+
+      it 'creates a whitespaced entry in the filesystem table and mounts it' do
+        step '(setup) create mount point'
+        on(agent, "mkdir '/#{name_w_whitespace}'", acceptable_exit_codes: [0, 1])
+
+        step '(setup) create new filesystem to be mounted'
+        MountUtils.create_filesystem(agent, name_w_whitespace)
+
+        step 'create a mount with puppet (mounted)'
+        args = ['ensure=mounted',
+                "fstype=#{fs_type}",
+                'options=loop',
+                "device='/tmp/#{name_w_whitespace}'"]
+        on(agent, puppet_resource('mount', "'/#{name_w_whitespace}'", args))
+
+        step 'verify entry in filesystem table'
+        on(agent, "cat #{fs_file}") do |result|
+          munged_name = name_w_whitespace.gsub(' ', '\\\040')
+          fail_test "didn't find the mount '#{name_w_whitespace}'" unless result.stdout.include?(munged_name)
+        end
+
+        step 'verify entry is mounted'
+        on(agent, 'mount') do |result|
+          fail_test "didn't find the mount '#{name_w_whitespace}' mounted" unless result.stdout.include?(name_w_whitespace)
+        end
+      end
     end
   end
 end

spec/acceptance/tests/query_spec.rb

@@ -17,6 +17,16 @@
         end
       end
 
+      it 'finds an existing filesystem table entry containing whitespace' do
+        step '(setup) add entry to filesystem table'
+        MountUtils.add_entry_to_filesystem_table(agent, name_w_whitespace)
+
+        step 'verify mount with puppet'
+        on(agent, puppet_resource('mount', "'/#{name_w_whitespace}'")) do |result|
+          fail_test "didn't find the mount #{name_w_whitespace}" unless result.stdout =~ %r{'/#{name_w_whitespace}':\s+ensure\s+=>\s+'unmounted'}
+        end
+      end
+
       # There is a discrepancy between how `puppet resource` and `puppet apply` handle this case.
       # With this patch, using a resource title with a trailing slash in `puppet apply` will match a mount resource without a trailing slash.
       # However, `puppet resource mount` with a trailing slash will not match.