From 71da422e151fa1c3c9cdd78c450626d095e71b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 1 Nov 2023 14:12:19 -1000 Subject: [PATCH 1/2] Quote and escape special chars in mapping keys Previously, a mapping key could not include spaces because of the data type. Some implementations of autofs do support such keys when they are properly quoted so do not limit their use with the module and properly quote the key when it has special characters. These chars still need to be manualy espaced when used with the fs parameter because it is managed as a single String. This will be addressed in a follow-up commit as it will be a breaking change. --- REFERENCE.md | 4 ++-- manifests/mapping.pp | 13 +++++++++---- spec/defines/mapping_spec.rb | 20 ++++++++++++++++++++ types/fs_mapping.pp | 2 +- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/REFERENCE.md b/REFERENCE.md index 135ccf6..62d221c 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -580,7 +580,7 @@ the remote filesystem to mount ##### `key` -Data type: `Pattern[/\A\S+\z/]` +Data type: `String[1]` the autofs key for this mapping. For indirect maps it is the basename of the mountpoint directory for $fs (not to be confused with @@ -792,7 +792,7 @@ Alias of ```puppet Struct[{ - key => Pattern[/\A\S+\z/], + key => String[1], options => Optional[Autofs::Options], order => Optional[Integer], fs => Pattern[/\S/] # contains at least one non-whitespace character diff --git a/manifests/mapping.pp b/manifests/mapping.pp index e08d7fd..054b8b7 100644 --- a/manifests/mapping.pp +++ b/manifests/mapping.pp @@ -61,13 +61,18 @@ # define autofs::mapping ( Stdlib::Absolutepath $mapfile, - Pattern[/\A\S+\z/] $key, + String[1] $key, Pattern[/\S/] $fs, Enum['present', 'absent'] $ensure = 'present', Optional[Autofs::Options] $options = undef, Integer $order = 10, ) { unless $ensure == 'absent' { + $formatted_key = if $key =~ /[[:blank:]"]/ { + String($key, '%#p') + } else { + $key + } # Format the options string, relying to some extent on the # $options parameter, if specified, to indeed match the # Autofs::Options data type @@ -89,10 +94,10 @@ } # Declare an appropriate fragment of the target map file - if $key == '+' { - $content = "${key}${fs}\n" + if $formatted_key == '+' { + $content = "${formatted_key}${fs}\n" } else { - $content = "${key} ${formatted_options} ${fs}\n" + $content = "${formatted_key}\t${formatted_options}\t${fs}\n" } concat::fragment { "autofs::mapping/${title}": diff --git a/spec/defines/mapping_spec.rb b/spec/defines/mapping_spec.rb index b758956..3d16893 100644 --- a/spec/defines/mapping_spec.rb +++ b/spec/defines/mapping_spec.rb @@ -165,6 +165,26 @@ with(target: '/mnt/auto.data', content: "data -rw storage.host.net:/exports/data backup.host.net:/exports/data\n") end end + + context 'with spaces in path' do + let(:params) do + { + mapfile: '/mnt/auto.data', + key: %(/scary/don't fear "quotes" and spaces), + options: 'rw', + fs: %("storage.host.net:/exports/data/don't fear \\"quotes\\" and spaces") + } + end + + it do + expect(subject).to compile + expect(subject).not_to contain_class('autofs') + expect(subject).to have_concat_resource_count(0) + expect(subject).to have_concat__fragment_resource_count(1) + expect(subject).to contain_concat__fragment('autofs::mapping/data'). + with(target: '/mnt/auto.data', content: %("/scary/don't fear \\"quotes\\" and spaces"\t-rw\t"storage.host.net:/exports/data/don't fear \\"quotes\\" and spaces"\n)) + end + end end end end diff --git a/types/fs_mapping.pp b/types/fs_mapping.pp index 1e66d20..46bb7c2 100644 --- a/types/fs_mapping.pp +++ b/types/fs_mapping.pp @@ -17,7 +17,7 @@ # { 'key' => 'other', 'options' => [ 'ro', 'noexec' ], 'fs' => 'external.net:/the/exported/fs' } # type Autofs::Fs_mapping = Struct[{ - key => Pattern[/\A\S+\z/], + key => String[1], options => Optional[Autofs::Options], order => Optional[Integer], fs => Pattern[/\S/] # contains at least one non-whitespace character From 66cecb532cd8c0e20805457ea1ef3fd1ca9d1135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 1 Nov 2023 14:30:55 -1000 Subject: [PATCH 2/2] Quote and escape special chars in mapping fs Multiple filesystems can be provided for a mapping by separating them with spaces. Some implementations of autofs do support spaces in path if proper quoting is in place. When managing a filesystems with spaces in its path, the end-user of the module was responsible for the correct quoting because the module interface only accept a single string for the "raw" value. Change the data type of the parameter to accept either a `String` (single filesystem) or an `Array[String]` (multiple filesystems) and transfer the responsibility of escaping to the module. This is a breaking change that may affect some users: 1. Users of mapping with multiple filesystems will need to split their `fs` parameter to provide an array of filesystems; 2. Users of mappings with quoted special chars will have to remove the quoting to avoid double-quoting. --- REFERENCE.md | 4 ++-- manifests/mapping.pp | 7 ++++--- spec/defines/mapping_spec.rb | 7 +++++-- types/fs_mapping.pp | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/REFERENCE.md b/REFERENCE.md index 62d221c..5a183b1 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -574,7 +574,7 @@ Default value: `'present'` ##### `fs` -Data type: `Pattern[/\S/]` +Data type: `Variant[String[1], Array[String[1]]]` the remote filesystem to mount @@ -795,7 +795,7 @@ Struct[{ key => String[1], options => Optional[Autofs::Options], order => Optional[Integer], - fs => Pattern[/\S/] # contains at least one non-whitespace character + fs => Variant[String[1], Array[String[1]]], }] ``` diff --git a/manifests/mapping.pp b/manifests/mapping.pp index 054b8b7..a499927 100644 --- a/manifests/mapping.pp +++ b/manifests/mapping.pp @@ -62,7 +62,7 @@ define autofs::mapping ( Stdlib::Absolutepath $mapfile, String[1] $key, - Pattern[/\S/] $fs, + Variant[String[1], Array[String[1]]] $fs, Enum['present', 'absent'] $ensure = 'present', Optional[Autofs::Options] $options = undef, Integer $order = 10, @@ -92,12 +92,13 @@ default => "-${prelim_options}", } } + $formatted_fs = [$fs].flatten.map |$value| { if $value =~ /[[:blank:]"]/ { String($value, '%#p') } else { $value } }.join(' ') # Declare an appropriate fragment of the target map file if $formatted_key == '+' { - $content = "${formatted_key}${fs}\n" + $content = "${formatted_key}${formatted_fs}\n" } else { - $content = "${formatted_key}\t${formatted_options}\t${fs}\n" + $content = "${formatted_key}\t${formatted_options}\t${formatted_fs}\n" } concat::fragment { "autofs::mapping/${title}": diff --git a/spec/defines/mapping_spec.rb b/spec/defines/mapping_spec.rb index 3d16893..82b7908 100644 --- a/spec/defines/mapping_spec.rb +++ b/spec/defines/mapping_spec.rb @@ -152,7 +152,10 @@ mapfile: '/mnt/auto.data', key: 'data', options: 'rw', - fs: 'storage.host.net:/exports/data backup.host.net:/exports/data' + fs: [ + 'storage.host.net:/exports/data', + 'backup.host.net:/exports/data' + ] } end @@ -172,7 +175,7 @@ mapfile: '/mnt/auto.data', key: %(/scary/don't fear "quotes" and spaces), options: 'rw', - fs: %("storage.host.net:/exports/data/don't fear \\"quotes\\" and spaces") + fs: %(storage.host.net:/exports/data/don't fear "quotes" and spaces) } end diff --git a/types/fs_mapping.pp b/types/fs_mapping.pp index 46bb7c2..01e1e26 100644 --- a/types/fs_mapping.pp +++ b/types/fs_mapping.pp @@ -20,5 +20,5 @@ key => String[1], options => Optional[Autofs::Options], order => Optional[Integer], - fs => Pattern[/\S/] # contains at least one non-whitespace character + fs => Variant[String[1], Array[String[1]]], }]