Skip to content

Commit c90593b

Browse files
GabrielNagyGabrielNagy
committed
(MODULES-8304) Allow spaces in mounts on Linux
This commit changes the mount type to accept whitespaces in mountpoint names and devices on Linux. This is accomplished by replacing the whitespace character with the ASCII code for space (\040) before writing to /etc/fstab. When reading mountpoints from fstab, names and devices are munged back to the original whitespace character. To make these changes as unintrusive as possible, the whitespace conversion is done by implementing the `to_line` and `post_parse` methods from the ParsedFile provider.
1 parent a532275 commit c90593b

File tree

17 files changed

+276
-44
lines changed

17 files changed

+276
-44
lines changed

‎REFERENCE.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,11 @@ mount/unmount status.
4747

4848
##### `device`
4949

50-
The device providing the mount. This can be whatever
51-
device is supporting by the mount, including network
52-
devices or devices specified by UUID rather than device
53-
path, depending on the operating system.
50+
The device providing the mount. This can be whatever device
51+
is supporting by the mount, including network devices or
52+
devices specified by UUID rather than device path, depending
53+
on the operating system. On Linux systems it can contain
54+
whitespace.
5455

5556
##### `blockdevice`
5657

@@ -100,7 +101,7 @@ The following parameters are available in the `mount` type.
100101

101102
namevar
102103

103-
The mount path for the mount.
104+
The mount path for the mount. On Linux systems it can contain whitespace.
104105

105106
##### `remounts`
106107

‎lib/puppet/provider/mount.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def using_explicit_options?
6060

6161
# This only works when the mount point is synced to the fstab.
6262
def unmount
63-
umount(resource[:name])
63+
umount resource[:name]
6464

6565
# Update property hash for future queries (e.g. refresh is called)
6666
case get(:ensure)

‎lib/puppet/provider/mount/parsed.rb‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,28 @@ def to_line(result)
186186
end
187187
else
188188
record_line name, fields: @fields, separator: %r{\s+}, joiner: "\t", optional: optional_fields, block_eval: :instance do
189+
def to_line(record)
190+
# convert whitespace to ASCII before writing to fstab
191+
# duplicate the record since we don't want our resource to have ASCII whitespaces
192+
result = record.dup
193+
[:device, :name].each do |param|
194+
if record[param].is_a?(String)
195+
result[param] = result[param].gsub(' ', '\\\040') if result[param].include?(' ')
196+
end
197+
end
198+
join(result)
199+
end
200+
201+
def post_parse(record)
202+
# handle ASCII-encoded whitespaces in fstab
203+
[:device, :name].each do |param|
204+
if record[param].is_a?(String)
205+
record[param].gsub!('\040', ' ') if record[param].include?('\040')
206+
end
207+
end
208+
record
209+
end
210+
189211
def pre_gen(record)
190212
if !record[:options] || record[:options].empty?
191213
if Facter.value(:kernel) == 'Linux'
@@ -253,16 +275,17 @@ def self.prefetch(resources = nil)
253275
end
254276

255277
def self.mountinstances
256-
# XXX: Will not work for mount points that have spaces in path (does fstab support this anyways?)
257278
regex = case Facter.value(:osfamily)
258279
when 'Darwin'
259280
%r{ on (?:/private/var/automount)?(\S*)}
260281
when 'Solaris', 'HP-UX'
261282
%r{^(\S*) on }
262283
when 'AIX'
263284
%r{^(?:\S*\s+\S+\s+)(\S+)}
285+
when %r{FreeBSD|NetBSD}i
286+
%r{ on (.*) \(}
264287
else
265-
%r{ on (\S*)}
288+
%r{ on (.*) type }
266289
end
267290
instances = []
268291
mount_output = mountcmd.split("\n")

‎lib/puppet/type/mount.rb‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,13 +124,16 @@ def syncothers
124124
end
125125

126126
newproperty(:device) do
127-
desc "The device providing the mount. This can be whatever
128-
device is supporting by the mount, including network
129-
devices or devices specified by UUID rather than device
130-
path, depending on the operating system."
127+
desc "The device providing the mount. This can be whatever device
128+
is supporting by the mount, including network devices or
129+
devices specified by UUID rather than device path, depending
130+
on the operating system. On Linux systems it can contain
131+
whitespace."
131132

132133
validate do |value|
133-
raise Puppet::Error, _('device must not contain whitespace: %{value}') % { value: value } if value =~ %r{\s}
134+
unless Facter.value(:kernel) == 'Linux'
135+
raise Puppet::Error, _('device must not contain whitespace: %{value}') % { value: value } if value =~ %r{\s}
136+
end
134137
end
135138
end
136139

@@ -242,12 +245,14 @@ def munge(value)
242245
end
243246

244247
newparam(:name) do
245-
desc 'The mount path for the mount.'
248+
desc 'The mount path for the mount. On Linux systems it can contain whitespace.'
246249

247250
isnamevar
248251

249252
validate do |value|
250-
raise Puppet::Error, _('name must not contain whitespace: %{value}') % { value: value } if value =~ %r{\s}
253+
unless Facter.value(:kernel) == 'Linux'
254+
raise Puppet::Error, _('name must not contain whitespace: %{value}') % { value: value } if value =~ %r{\s}
255+
end
251256
end
252257

253258
munge do |value|

‎spec/acceptance/lib/mount_utils.rb‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ def add_entry_to_filesystem_table(host, mount_name)
4444
# Note: /dev/hd8 is the default jfs logging device on AIX.
4545
on(host, "echo '/#{mount_name}:\n dev = /dev/#{mount_name}\n vfs = #{fs_type}\n log = /dev/hd8' >> #{fs_file}")
4646
when %r{el-|centos|fedora|sles|debian|ubuntu|cumulus}
47-
on(host, "echo '/tmp/#{mount_name} /#{mount_name} #{fs_type} loop 0 0' >> #{fs_file}")
47+
# Correctly munge whitespaces in mountpoints
48+
munged_mount_name = mount_name.gsub(' ', '\\\040')
49+
on(host, "echo '/tmp/#{munged_mount_name} /#{munged_mount_name} #{fs_type} loop 0 0' >> #{fs_file}")
4850
else
4951
# TODO: Add Solaris and OSX support, as per PUP-5201 and PUP-4823
5052
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)
6365
on(host, "mklv -y #{mount_name} #{volume_group} 1M")
6466
on(host, "mkfs -V #{fs_type} -l #{mount_name} /dev/#{mount_name}")
6567
when %r{el-|centos|fedora|sles|debian|ubuntu|cumulus}
66-
on(host, "dd if=/dev/zero of=/tmp/#{mount_name} count=10240", acceptable_exit_codes: [0, 1])
67-
on(host, "yes | mkfs -t #{fs_type} -q /tmp/#{mount_name}", acceptable_exit_codes: (0..254))
68+
on(host, "dd if=/dev/zero of='/tmp/#{mount_name}' count=10240", acceptable_exit_codes: [0, 1])
69+
on(host, "yes | mkfs -t #{fs_type} -q '/tmp/#{mount_name}'", acceptable_exit_codes: (0..254))
6870
else
6971
# TODO: Add Solaris and OSX support, as per PUP-5201 and PUP-4823
7072
fail_test("Creating filesystems on #{host['platform']} is not currently supported.")

‎spec/acceptance/tests/defined_spec.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,20 @@
1919
fail_test "didn't find the mount #{name}" unless result.stdout.include?(name)
2020
end
2121
end
22+
23+
it 'defines a mount entry with whitespace' do
24+
step 'creates a mount'
25+
args = ['ensure=defined',
26+
"fstype=#{fs_type}",
27+
"device='/tmp/#{name_w_whitespace}'"]
28+
on(agent, puppet_resource('mount', "'/#{name_w_whitespace}'", args))
29+
30+
step 'verify entry in filesystem table'
31+
on(agent, "cat #{fs_file}") do |result|
32+
munged_name = name_w_whitespace.gsub(' ', '\\\040')
33+
fail_test "didn't find the mount #{name_w_whitespace}" unless result.stdout.include?(munged_name)
34+
end
35+
end
2236
end
2337
end
2438
end

‎spec/acceptance/tests/destroy_spec.rb‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,40 @@
3838
fail_test "found the mount #{name} mounted" if result.stdout.include?(name)
3939
end
4040
end
41+
42+
it 'deletes an entry with whitespace in filesystem table and unmounts it' do
43+
step 'create mount point'
44+
on(agent, "mkdir '/#{name_w_whitespace}'", acceptable_exit_codes: [0, 1])
45+
46+
step 'create new filesystem to be mounted'
47+
MountUtils.create_filesystem(agent, name_w_whitespace)
48+
49+
step 'add entry to the filesystem table'
50+
MountUtils.add_entry_to_filesystem_table(agent, name_w_whitespace)
51+
52+
step 'mount entry'
53+
on(agent, "mount '/#{name_w_whitespace}'")
54+
55+
step 'verify entry exists in filesystem table'
56+
on(agent, "cat #{fs_file}") do |result|
57+
munged_name = name_w_whitespace.gsub(' ', '\\\040')
58+
fail_test "did not find mount '#{name_w_whitespace}'" unless result.stdout.include?(munged_name)
59+
end
60+
61+
step 'destroy a mount with puppet (absent)'
62+
on(agent, puppet_resource('mount', "'/#{name_w_whitespace}'", 'ensure=absent'))
63+
64+
step 'verify entry removed from filesystem table'
65+
on(agent, "cat #{fs_file}") do |result|
66+
munged_name = name_w_whitespace.gsub(' ', '\\\040')
67+
fail_test "found the mount '#{name_w_whitespace}'" if result.stdout.include?(munged_name)
68+
end
69+
70+
step 'verify entry is not mounted'
71+
on(agent, 'mount') do |result|
72+
fail_test "found the mount '#{name_w_whitespace}' mounted" if result.stdout.include?(name_w_whitespace)
73+
end
74+
end
4175
end
4276
end
4377
end

‎spec/acceptance/tests/modify_spec.rb‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,30 @@
3030
fail_test "attributes not updated for the mount #{name}" unless res.stdout.include? 'bogus'
3131
end
3232
end
33+
34+
it 'modifies an entry with whitespace in the filesystem table' do
35+
step '(setup) create mount point'
36+
on(agent, "mkdir '/#{name_w_whitespace}'", acceptable_exit_codes: [0, 1])
37+
38+
step '(setup) create new filesystem to be mounted'
39+
MountUtils.create_filesystem(agent, name_w_whitespace)
40+
41+
step '(setup) add entry to the filesystem table'
42+
MountUtils.add_entry_to_filesystem_table(agent, name_w_whitespace)
43+
44+
step '(setup) mount entry'
45+
on(agent, "mount '/#{name_w_whitespace}'")
46+
47+
step 'modify a mount with puppet (defined)'
48+
args = ['ensure=defined',
49+
'fstype=bogus']
50+
on(agent, puppet_resource('mount', "'/#{name_w_whitespace}'", args))
51+
52+
step 'verify entry is updated in filesystem table'
53+
on(agent, "cat #{fs_file}") do |res|
54+
fail_test "attributes not updated for the mount '#{name_w_whitespace}'" unless res.stdout.include? 'bogus'
55+
end
56+
end
3357
end
3458
end
3559
end

‎spec/acceptance/tests/mounted_spec.rb‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,32 @@
3838
fail_test "didn't find the mount #{name} mounted" unless result.stdout.include?(name)
3939
end
4040
end
41+
42+
it 'creates a whitespaced entry in the filesystem table and mounts it' do
43+
step '(setup) create mount point'
44+
on(agent, "mkdir '/#{name_w_whitespace}'", acceptable_exit_codes: [0, 1])
45+
46+
step '(setup) create new filesystem to be mounted'
47+
MountUtils.create_filesystem(agent, name_w_whitespace)
48+
49+
step 'create a mount with puppet (mounted)'
50+
args = ['ensure=mounted',
51+
"fstype=#{fs_type}",
52+
'options=loop',
53+
"device='/tmp/#{name_w_whitespace}'"]
54+
on(agent, puppet_resource('mount', "'/#{name_w_whitespace}'", args))
55+
56+
step 'verify entry in filesystem table'
57+
on(agent, "cat #{fs_file}") do |result|
58+
munged_name = name_w_whitespace.gsub(' ', '\\\040')
59+
fail_test "didn't find the mount '#{name_w_whitespace}'" unless result.stdout.include?(munged_name)
60+
end
61+
62+
step 'verify entry is mounted'
63+
on(agent, 'mount') do |result|
64+
fail_test "didn't find the mount '#{name_w_whitespace}' mounted" unless result.stdout.include?(name_w_whitespace)
65+
end
66+
end
4167
end
4268
end
4369
end

‎spec/acceptance/tests/query_spec.rb‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@
1717
end
1818
end
1919

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

0 commit comments

Comments
 (0)