diff --git a/lib/puppet/provider/posix_acl/genericacl.rb b/lib/puppet/provider/posix_acl/genericacl.rb index ce9f25b..3acf1a5 100644 --- a/lib/puppet/provider/posix_acl/genericacl.rb +++ b/lib/puppet/provider/posix_acl/genericacl.rb @@ -1,3 +1,2 @@ -Puppet::Type.type(:posix_acl).provide(:genericacl, :parent => Puppet::Provider) do - +Puppet::Type.type(:posix_acl).provide(:genericacl, parent: Puppet::Provider) do end diff --git a/lib/puppet/provider/posix_acl/posixacl.rb b/lib/puppet/provider/posix_acl/posixacl.rb index b559c06..0ce781f 100644 --- a/lib/puppet/provider/posix_acl/posixacl.rb +++ b/lib/puppet/provider/posix_acl/posixacl.rb @@ -1,11 +1,11 @@ -Puppet::Type.type(:posix_acl).provide(:posixacl, :parent => Puppet::Provider) do - desc "Provide posix 1e acl functions using posix getfacl/setfacl commands" +Puppet::Type.type(:posix_acl).provide(:posixacl, parent: Puppet::Provider) do + desc 'Provide posix 1e acl functions using posix getfacl/setfacl commands' - commands :setfacl => '/usr/bin/setfacl' - commands :getfacl => '/usr/bin/getfacl' + commands setfacl: '/usr/bin/setfacl' + commands getfacl: '/usr/bin/getfacl' - confine :feature => :posix - defaultfor :operatingsystem => [:debian, :ubuntu, :redhat, :centos, :fedora, :sles] + confine feature: :posix + defaultfor operatingsystem: [:debian, :ubuntu, :redhat, :centos, :fedora, :sles] def exists? permission @@ -13,7 +13,7 @@ def exists? def unset_perm(perm, path) # Don't try to unset mode bits, it don't make sense! - if !(perm =~ /^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):/) + unless perm =~ %r{^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):} perm = perm.split(':')[0..-2].join(':') if check_recursive setfacl('-R', '-n', '-x', perm, path) @@ -48,19 +48,17 @@ def purge def permission return [] unless File.exist?(@resource.value(:path)) value = [] - #String#lines would be nice, but we need to support Ruby 1.8.5 + # String#lines would be nice, but we need to support Ruby 1.8.5 getfacl('--absolute-names', '--no-effective', @resource.value(:path)).split("\n").each do |line| # Strip comments and blank lines - if !(line =~ /^#/) and !(line == "") - value << line.gsub('\040', ' ') - end + value << line.gsub('\040', ' ') if line !~ %r{^#} && line != '' end value.sort end def check_recursive # Changed functionality to return boolean true or false - @resource.value(:recursive) == :true and resource.value(:recursemode) == :lazy + @resource.value(:recursive) == :true && resource.value(:recursemode) == :lazy end def check_exact @@ -79,7 +77,7 @@ def check_set @resource.value(:action) == :set end - def permission=(value) + def permission=(_value) # TODO: Investigate why we're not using this parameter Puppet.debug @resource.value(:action) case @resource.value(:action) when :unset @@ -90,15 +88,13 @@ def permission=(value) cur_perm = permission perm_to_set = @resource.value(:permission) - cur_perm perm_to_unset = cur_perm - @resource.value(:permission) - if (perm_to_set.length == 0 && perm_to_unset.length == 0) - return false - end + return false if perm_to_set.empty? && perm_to_unset.empty? # Take supplied perms literally, unset any existing perms which # are absent from ACLs given if check_exact perm_to_unset.each do |perm| # Skip base perms in unset step - if perm =~ /^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):/ + if perm =~ %r{^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):} Puppet.debug "skipping unset of base perm: #{perm}" else unset_perm(perm, @resource.value(:path)) diff --git a/lib/puppet/type/posix_acl.rb b/lib/puppet/type/posix_acl.rb index f1988da..1b42396 100644 --- a/lib/puppet/type/posix_acl.rb +++ b/lib/puppet/type/posix_acl.rb @@ -41,13 +41,13 @@ EOT newparam(:action) do - desc "What do we do with this list of ACLs? Options are set, unset, exact, and purge" + desc 'What do we do with this list of ACLs? Options are set, unset, exact, and purge' newvalues(:set, :unset, :exact, :purge) defaultto :set end newparam(:path) do - desc "The file or directory to which the ACL applies." + desc 'The file or directory to which the ACL applies.' isnamevar validate do |value| path = Pathname.new(value) @@ -74,25 +74,25 @@ def self.is_descendant?(a, b) a_list = File.expand_path(a).split('/') b_list = File.expand_path(b).split('/') - b_list[0..a_list.size-1] == a_list and b_list != a_list + b_list[0..a_list.size - 1] == a_list && b_list != a_list end # Snippet based on upstream Puppet (ASL 2.0) - [:posix_acl, :file].each do | autorequire_type | + [:posix_acl, :file].each do |autorequire_type| autorequire(autorequire_type) do req = [] path = Pathname.new(self[:path]) if autorequire_type != :posix_acl if self[:recursive] == :true - catalog.resources.find_all { |r| - r.is_a?(Puppet::Type.type(autorequire_type)) and self.class.is_descendant?(self[:path], r[:path]) - }.each do | found | + catalog.resources.select do |r| + r.is_a?(Puppet::Type.type(autorequire_type)) && self.class.is_descendant?(self[:path], r[:path]) + end.each do |found| req << found[:path] end end req << self[:path] end - if !path.root? + unless path.root? # Start at our parent, to avoid autorequiring ourself parents = path.parent.enum_for(:ascend) if found = parents.find { |p| catalog.resource(autorequire_type, p.to_s) } @@ -108,11 +108,11 @@ def self.is_descendant?(a, b) ['acl'] end - newproperty(:permission, :array_matching => :all) do - desc "ACL permission(s)." + newproperty(:permission, array_matching: :all) do + desc 'ACL permission(s).' def is_to_s(value) - if value == :absent or value.include?(:absent) + if value == :absent || value.include?(:absent) super else value.sort.inspect @@ -120,7 +120,7 @@ def is_to_s(value) end def should_to_s(value) - if value == :absent or value.include?(:absent) + if value == :absent || value.include?(:absent) super else value.sort.inspect @@ -136,11 +136,11 @@ def strip_perms(pl) user:root:rwx becomes user:root:" - Puppet.debug "permission.strip_perms" + Puppet.debug 'permission.strip_perms' value = [] pl.each do |perm| - unless perm =~ /^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):/ - perm = perm.split(':',-1)[0..-2].join(':') + unless perm =~ %r{^(((u(ser)?)|(g(roup)?)|(m(ask)?)|(o(ther)?)):):} + perm = perm.split(':', -1)[0..-2].join(':') value << perm end end @@ -154,7 +154,7 @@ def strip_perms(pl) def unset_insync(cur_perm) # Puppet.debug "permission.unset_insync" test_should = [] - @should.each { |x| test_should << x.downcase() } + @should.each { |x| test_should << x.downcase } cp = strip_perms(cur_perm) sp = strip_perms(test_should) (sp - cp).sort == sp @@ -162,29 +162,23 @@ def unset_insync(cur_perm) def set_insync(cur_perm) should = @should.uniq.sort - (cur_perm.sort == should) or (provider.check_set and ((should - cur_perm).length == 0)) + (cur_perm.sort == should) || (provider.check_set && (should - cur_perm).empty?) end def purge_insync(cur_perm) # Puppet.debug "permission.purge_insync" cur_perm.each do |perm| # If anything other than the mode bits are set, we're not in sync - if !(perm =~ /^(((u(ser)?)|(g(roup)?)|(o(ther)?)):):/) - return false - end + return false unless perm =~ %r{^(((u(ser)?)|(g(roup)?)|(o(ther)?)):):} end - return true + true end def insync?(is) Puppet.debug "permission.insync? is: #{is.inspect} @should: #{@should.inspect}" - if provider.check_purge - return purge_insync(is) - end - if provider.check_unset - return unset_insync(is) - end - return set_insync(is) + return purge_insync(is) if provider.check_purge + return unset_insync(is) if provider.check_unset + set_insync(is) end # Munge into normalised form @@ -206,20 +200,20 @@ def insync?(is) end t = a.shift # Copy the type. r << case t - when 'u', 'user' - 'user:' - when 'g', 'group' - 'group:' - when 'o', 'other' - 'other:' - when 'm', 'mask' - 'mask:' - else - raise ArgumentError, %(Unknown type "#{t}", expected "user", "group", "other" or "mask".) + when 'u', 'user' + 'user:' + when 'g', 'group' + 'group:' + when 'o', 'other' + 'other:' + when 'm', 'mask' + 'mask:' + else + raise ArgumentError, %(Unknown type "#{t}", expected "user", "group", "other" or "mask".) end r << "#{a.shift}:" # Copy the "who". p = a.shift - if p =~ /[0-7]/ + if p =~ %r{[0-7]} p = p.oct r << (p | 4 ? 'r' : '-') r << (p | 2 ? 'w' : '-') @@ -230,27 +224,25 @@ def insync?(is) r << (s.sub!('r', '') ? 'r' : '-') r << (s.sub!('w', '') ? 'w' : '-') r << (s.sub!('x', '') ? 'x' : '-') - if !s.empty? - raise ArgumentError, %(Invalid permission set "#{p}".) - end + raise ArgumentError, %(Invalid permission set "#{p}".) unless s.empty? end r end end newparam(:recursive) do - desc "Apply ACLs recursively." + desc 'Apply ACLs recursively.' newvalues(:true, :false) defaultto :false end def self.pick_default_perms(acl) - return acl.reject { |a| a.split(':', -1).length == 4 } + acl.reject { |a| a.split(':', -1).length == 4 } end def newchild(path) - options = @original_parameters.merge(:name => path).reject { |param, value| value.nil? } - unless File.directory?(options[:name]) then + options = @original_parameters.merge(name: path).reject { |_param, value| value.nil? } + unless File.directory?(options[:name]) options[:permission] = self.class.pick_default_perms(options[:permission]) if options.include?(:permission) end [:recursive, :recursemode, :path].each do |param| @@ -260,9 +252,9 @@ def newchild(path) end def generate - return [] unless self[:recursive] == :true and self[:recursemode] == :deep + return [] unless self[:recursive] == :true && self[:recursemode] == :deep results = [] - paths = Set.new() + paths = Set.new if File.directory?(self[:path]) Dir.chdir(self[:path]) do Dir['**/*'].each do |path| @@ -273,21 +265,20 @@ def generate # At the time we generate extra resources, all the files might now be present yet. # In prediction to that we also create ACL resources for child file resources that # might not have been applied yet. - catalog.resources.find_all { |r| - r.is_a?(Puppet::Type.type(:file)) and self.class.is_descendant?(self[:path], r[:path]) - }.each do | found | + catalog.resources.select do |r| + r.is_a?(Puppet::Type.type(:file)) && self.class.is_descendant?(self[:path], r[:path]) + end.each do |found| paths << found[:path] end - paths.each { | path | + paths.each do |path| results << newchild(path) - } + end results end validate do unless self[:permission] - raise(Puppet::Error, "permission is a required property.") + raise(Puppet::Error, 'permission is a required property.') end end - end diff --git a/spec/unit/puppet/provider/posixacl_spec.rb b/spec/unit/puppet/provider/posixacl_spec.rb index 3f12a93..b057126 100644 --- a/spec/unit/puppet/provider/posixacl_spec.rb +++ b/spec/unit/puppet/provider/posixacl_spec.rb @@ -4,24 +4,23 @@ provider_class = Puppet::Type.type(:posix_acl).provider(:posixacl) describe provider_class do - it 'declares a getfacl command' do - expect{ + expect do provider_class.command :getfacl - }.not_to raise_error + end.not_to raise_error end it 'declares a setfacl command' do - expect{ + expect do provider_class.command :setfacl - }.not_to raise_error + end.not_to raise_error end it 'encodes spaces in group names' do RSpec::Mocks.with_temporary_scope do Puppet::Type.stubs(:getfacl).returns("group:test group:rwx\n") File.stubs(:exist?).returns(true) - expect{ + expect do provider_class.command :permission - } == ['group:test\040group:rwx'] + end == ['group:test\040group:rwx'] end end end diff --git a/spec/unit/puppet/type/acl_spec.rb b/spec/unit/puppet/type/acl_spec.rb index 54e270f..72d98b1 100644 --- a/spec/unit/puppet/type/acl_spec.rb +++ b/spec/unit/puppet/type/acl_spec.rb @@ -2,130 +2,129 @@ acl_type = Puppet::Type.type(:posix_acl) - describe acl_type do context 'when not setting parameters' do - it 'should fail without permissions' do - expect{ - acl_type.new :name => '/tmp/foo' - }.to raise_error + it 'fails without permissions' do + expect do + acl_type.new name: '/tmp/foo' + end.to raise_error end end context 'when setting parameters' do - it 'should work with a correct permission parameter' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['user:root:rwx'] + it 'works with a correct permission parameter' do + resource = acl_type.new name: '/tmp/foo', permission: ['user:root:rwx'] expect(resource[:name]).to eq('/tmp/foo') expect(resource[:permission]).to eq(['user:root:rwx']) end - it 'should convert a permission string to an array' do - resource = acl_type.new :name => '/tmp/foo', :permission => 'user:root:rwx' + it 'converts a permission string to an array' do + resource = acl_type.new name: '/tmp/foo', permission: 'user:root:rwx' expect(resource[:name]).to eq('/tmp/foo') expect(resource[:permission]).to eq(['user:root:rwx']) end - it 'should convert the u: shorcut to user:' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['u:root:rwx'] + it 'converts the u: shorcut to user:' do + resource = acl_type.new name: '/tmp/foo', permission: ['u:root:rwx'] expect(resource[:name]).to eq('/tmp/foo') expect(resource[:permission]).to eq(['user:root:rwx']) end - it 'should convert the g: shorcut to group:' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['g:root:rwx'] + it 'converts the g: shorcut to group:' do + resource = acl_type.new name: '/tmp/foo', permission: ['g:root:rwx'] expect(resource[:name]).to eq('/tmp/foo') expect(resource[:permission]).to eq(['group:root:rwx']) end - it 'should convert the m: shorcut to mask:' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['m::rwx'] + it 'converts the m: shorcut to mask:' do + resource = acl_type.new name: '/tmp/foo', permission: ['m::rwx'] expect(resource[:name]).to eq('/tmp/foo') expect(resource[:permission]).to eq(['mask::rwx']) end - it 'should convert the o: shorcut to other:' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'] + it 'converts the o: shorcut to other:' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'] expect(resource[:name]).to eq('/tmp/foo') expect(resource[:permission]).to eq(['other::rwx']) end - it 'should have the "set" action by default' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'] + it 'has the "set" action by default' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'] expect(resource[:name]).to eq('/tmp/foo') expect(resource[:action]).to eq(:set) end - it 'should accept an action "set"' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :action => :set + it 'accepts an action "set"' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'], action: :set expect(resource[:name]).to eq('/tmp/foo') expect(resource[:action]).to eq(:set) end - it 'should accept an action "purge"' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :action => :purge + it 'accepts an action "purge"' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'], action: :purge expect(resource[:name]).to eq('/tmp/foo') expect(resource[:action]).to eq(:purge) end - it 'should accept an action "unset"' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :action => :unset + it 'accepts an action "unset"' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'], action: :unset expect(resource[:name]).to eq('/tmp/foo') expect(resource[:action]).to eq(:unset) end - it 'should accept an action "exact"' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :action => :exact + it 'accepts an action "exact"' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'], action: :exact expect(resource[:name]).to eq('/tmp/foo') expect(resource[:action]).to eq(:exact) end - it 'should have path as namevar' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'] + it 'has path as namevar' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'] expect(resource[:name]).to eq('/tmp/foo') expect(resource[:path]).to eq(resource[:name]) end - it 'should accept a path parameter' do - resource = acl_type.new :path => '/tmp/foo', :permission => ['o::rwx'], :action => :exact + it 'accepts a path parameter' do + resource = acl_type.new path: '/tmp/foo', permission: ['o::rwx'], action: :exact expect(resource[:path]).to eq('/tmp/foo') expect(resource[:name]).to eq(resource[:path]) end - it 'should not be recursive by default' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'] + it 'is not recursive by default' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'] expect(resource[:name]).to eq('/tmp/foo') expect(resource[:recursive]).to eq(:false) end - it 'should accept a recursive "true"' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :recursive => true + it 'accepts a recursive "true"' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'], recursive: true expect(resource[:name]).to eq('/tmp/foo') expect(resource[:recursive]).to eq(:true) end - it 'should accept a recurse "false"' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :recursive => false + it 'accepts a recurse "false"' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'], recursive: false expect(resource[:name]).to eq('/tmp/foo') expect(resource[:recursive]).to eq(:false) end - it 'should get recursemode lazy by default' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'] + it 'gets recursemode lazy by default' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'] expect(resource[:name]).to eq('/tmp/foo') expect(resource[:recursemode]).to eq(:lazy) end - it 'should accept a recursemode deep' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :recursemode => 'deep' + it 'accepts a recursemode deep' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'], recursemode: 'deep' expect(resource[:name]).to eq('/tmp/foo') expect(resource[:recursemode]).to eq(:deep) end - it 'should accept a recursemode lazy' do - resource = acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :recursemode => :lazy + it 'accepts a recursemode lazy' do + resource = acl_type.new name: '/tmp/foo', permission: ['o::rwx'], recursemode: :lazy expect(resource[:name]).to eq('/tmp/foo') expect(resource[:recursemode]).to eq(:lazy) end - it 'should fail with a wrong action' do - expect{ - acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :action => :xset - }.to raise_error + it 'fails with a wrong action' do + expect do + acl_type.new name: '/tmp/foo', permission: ['o::rwx'], action: :xset + end.to raise_error end - it 'should fail with a wrong recurselimit' do - expect{ - acl_type.new :name => '/tmp/foo', :permission => ['o::rwx'], :recurselimit => :a - }.to raise_error + it 'fails with a wrong recurselimit' do + expect do + acl_type.new name: '/tmp/foo', permission: ['o::rwx'], recurselimit: :a + end.to raise_error end - it 'should fail with a wrong first argument' do - expect{ - acl_type.new :name => '/tmp/foo', :permission => ['wrong::rwx'] - }.to raise_error + it 'fails with a wrong first argument' do + expect do + acl_type.new name: '/tmp/foo', permission: ['wrong::rwx'] + end.to raise_error end - it 'should fail with a wrong last argument' do - expect{ - acl_type.new :name => '/tmp/foo', :permission => ['user::-_-'] - }.to raise_error + it 'fails with a wrong last argument' do + expect do + acl_type.new name: '/tmp/foo', permission: ['user::-_-'] + end.to raise_error end end @@ -134,23 +133,22 @@ advanced_perms = ['user:foo:rwx', 'group:foo:rwx', 'default:user:foo:---'] advanced_perms_results = ['user:foo:rwx', 'group:foo:rwx'] mysql_perms = [ - "user:mysql:rwx", - "d:user:mysql:rw", - "mask::rwx", + 'user:mysql:rwx', + 'd:user:mysql:rw', + 'mask::rwx' ] mysql_perms_results = [ - "user:mysql:rwx", - "mask::rwx", + 'user:mysql:rwx', + 'mask::rwx' ] - it 'should not do anything with no defaults' do + it 'does not do anything with no defaults' do expect(acl_type.pick_default_perms(basic_perms)).to match_array(basic_perms) end - it 'should remove defaults' do + it 'removes defaults' do expect(acl_type.pick_default_perms(advanced_perms)).to match_array(advanced_perms_results) end - it 'should remove defaults with d:' do + it 'removes defaults with d:' do expect(acl_type.pick_default_perms(mysql_perms)).to match_array(mysql_perms_results) end end - end