From 2a181db993d8a4742598c083d9797e4e9bc05e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 8 Jun 2018 10:30:55 +0200 Subject: [PATCH 1/4] add allow_other to s3ql init class when allow other is set, we ensure that the option "user_allow_other" option is correctly set in `/etc/fuse.conf` --- manifests/init.pp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/manifests/init.pp b/manifests/init.pp index 77a6212..66ffebe 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -7,6 +7,7 @@ $package_name = 's3ql', $package_ensure = 'present', $package_provider = undef, + $allow_other = false, ) { package { 's3ql': @@ -14,4 +15,12 @@ name => $package_name, provider => $package_provider, } + + if $allow_other { + file_line { "user_allow_other ${allow_other}": + path => '/etc/fuse.conf', + line => 'user_allow_other', + match => '^#user_allow_other' + } + } } From 7fbc4c5c93be840ff6543a7542e6f0902366aad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 8 Jun 2018 11:20:41 +0200 Subject: [PATCH 2/4] add allow_other option to type (and provider) with a default breaking change, allow_other is now false we add the parameter to the type and provider an open question is still whether allow_other can be extracted from the mount options, in which case it would need to become a property ;) --- lib/puppet/provider/s3ql_mount/s3ql_mount.rb | 6 +++++- lib/puppet/type/s3ql_mount.rb | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/puppet/provider/s3ql_mount/s3ql_mount.rb b/lib/puppet/provider/s3ql_mount/s3ql_mount.rb index 1847866..f4ffa09 100644 --- a/lib/puppet/provider/s3ql_mount/s3ql_mount.rb +++ b/lib/puppet/provider/s3ql_mount/s3ql_mount.rb @@ -50,8 +50,11 @@ def commands_wrapper(command, *arguments) Puppet::Util::Execution.execute([command, all_args], opts) end + # TODO: This *arguments paramether is kinda pointless def mount_s3ql(*arguments) - mount_args = ['--allow-other', '--metadata-upload-interval', + allow_other = '' + allow_other = '--allow-other' if @resource[:allow_other] + mount_args = [allow_other, '--metadata-upload-interval', @resource[:upload_inverval], arguments].flatten begin commands_wrapper('mount.s3ql', mount_args) @@ -90,6 +93,7 @@ def self.instances owner ||= 0 group = options.sub(%r{.*group(_id)?=([^,)]).*}, '\2') if options =~ %r{group(_id)?} group ||= 0 + # XXX: is allow_others in options? # and initialize @property_hash new(name: mountpoint, diff --git a/lib/puppet/type/s3ql_mount.rb b/lib/puppet/type/s3ql_mount.rb index 530afdb..c44a004 100644 --- a/lib/puppet/type/s3ql_mount.rb +++ b/lib/puppet/type/s3ql_mount.rb @@ -114,6 +114,19 @@ end end + newparam(:allow_other) do + desc <<-EOS + Whether to allow "other" to access this mountpoint + + The default is `false`. To allow it, also make sure to set `allow_other` in `s3ql`. + EOS + defaultto :false + munge do |val| + :false if [false, 'false', :false].include? val + :true if [true, 'true', :true].include? val + end + end + newproperty(:backend) do desc <<-EOS The backend used. From c5066e6f8904dab614de2c58ee0fe278d05911c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 8 Jun 2018 13:42:05 +0200 Subject: [PATCH 3/4] split mount `options` instead of regex parsing this now allows us easily extract and add them as needed. --- lib/puppet/provider/s3ql_mount/s3ql_mount.rb | 21 ++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/puppet/provider/s3ql_mount/s3ql_mount.rb b/lib/puppet/provider/s3ql_mount/s3ql_mount.rb index f4ffa09..1320a1b 100644 --- a/lib/puppet/provider/s3ql_mount/s3ql_mount.rb +++ b/lib/puppet/provider/s3ql_mount/s3ql_mount.rb @@ -89,11 +89,23 @@ def self.instances mounts.map do |mnt| storage_url, _, mountpoint, _, _, options = mnt.split - owner = options.sub(%r{.*user(_id)?=([^,)]+).*}, '\2') if options =~ %r{user(_id)?} + opts_hsh = options.split(',').map { |o| + k = o + v = true + if o.include?('=') + k, v = split('=') + end + [k, v] + }.to_h + + owner = opts_hsh['user'] + owner ||= opts_hsh['user_id'] owner ||= 0 - group = options.sub(%r{.*group(_id)?=([^,)]).*}, '\2') if options =~ %r{group(_id)?} + group = opts_hsh['group'] + group ||= opts_hsh['group_id'] group ||= 0 - # XXX: is allow_others in options? + allow_other = opts_hsh['allow_other'] + allow_other ||= false # and initialize @property_hash new(name: mountpoint, @@ -102,7 +114,8 @@ def self.instances storage_url: storage_url, owner: owner, group: group, - backend: storage_url.split(':')[0]) + backend: storage_url.split(':')[0], + allow_other: allow_other) end end From 2ad4a346763f9319660857a8f80ce41deaf3c40d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Gali=C4=87?= Date: Fri, 8 Jun 2018 13:48:14 +0200 Subject: [PATCH 4/4] allow_other is a property add it to the list of properties in our tests, too --- lib/puppet/type/s3ql_mount.rb | 2 +- spec/unit/provider/s3ql_mount/s3ql_mount.rb | 1 + spec/unit/type/s3ql_mount_spec.rb | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/puppet/type/s3ql_mount.rb b/lib/puppet/type/s3ql_mount.rb index c44a004..fcbbe1e 100644 --- a/lib/puppet/type/s3ql_mount.rb +++ b/lib/puppet/type/s3ql_mount.rb @@ -114,7 +114,7 @@ end end - newparam(:allow_other) do + newproperty(:allow_other) do desc <<-EOS Whether to allow "other" to access this mountpoint diff --git a/spec/unit/provider/s3ql_mount/s3ql_mount.rb b/spec/unit/provider/s3ql_mount/s3ql_mount.rb index cd0fe6d..fb67964 100644 --- a/spec/unit/provider/s3ql_mount/s3ql_mount.rb +++ b/spec/unit/provider/s3ql_mount/s3ql_mount.rb @@ -11,6 +11,7 @@ storage_url: 'gs://bucket/prefix', owner: 'examplewww', group: 'examplewww', + allow_other: true, ) end diff --git a/spec/unit/type/s3ql_mount_spec.rb b/spec/unit/type/s3ql_mount_spec.rb index f476354..243406d 100644 --- a/spec/unit/type/s3ql_mount_spec.rb +++ b/spec/unit/type/s3ql_mount_spec.rb @@ -19,6 +19,7 @@ :owner, :group, :backend, + :allow_other, ] end let :parameters do